Fix installing in 'venv' and datetime tests on Windows

[lycantropos]

Resolves #468.

• Run cargo fmt (This is checked by travis ci).
• Run cargo clippy and check there are no hard errors (There are a bunch of existing warnings; This is also checked by travis).
• If applicable, add an entry in the changelog.
• If applicable, add documentation to all new items and extend the guide.
• If applicable, add tests for all new or fixed functions

[codecov]

Codecov Report

Merging #472 into master will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master     #472   +/-   ##
=======================================
  Coverage   87.87%   87.87%           
=======================================
  Files          65       65           
  Lines        3398     3398           
=======================================
  Hits         2986     2986           
  Misses        412      412

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update be10800...bc37f96. Read the comment docs.

[codecov]

Codecov Report

Merging #472 into master will decrease coverage by 0.01%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master     #472      +/-   ##
==========================================
- Coverage   87.87%   87.85%   -0.02%     
==========================================
  Files          65       65              
  Lines        3398     3393       -5     
==========================================
- Hits         2986     2981       -5     
  Misses        412      412

Impacted Files	Coverage Δ
src/instance.rs	96.73% <0%> (-0.11%)	⬇️
src/objectprotocol.rs	95.83% <0%> (ø)	⬆️
src/types/datetime.rs	100% <0%> (ø)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update be10800...4cbc428. Read the comment docs.

[pganssle]

In addition to the issue I mentioned about pip or pep517.build vs setup.py, I think this is the wrong way to do this test. These are a bunch of system commands, so there's no need to write this in Rust, or to use conditional compilation here.

I think maybe start by just adding a script to the CI that you invoke on Appveyor that executes one of these builds.

I also don't see the fix here, is that coming in a later commit?

[lycantropos]

In addition to the issue I mentioned about pip or pep517.build vs setup.py, I think this is the wrong way to do this test. These are a bunch of system commands, so there's no need to write this in Rust, or to use conditional compilation here.

I think maybe start by just adding a script to the CI that you invoke on Appveyor that executes one of these builds.

I also don't see the fix here, is that coming in a later commit?

There should be different script for Travis CI as well? To be sure that I'm not breaking something. If so, where to place scripts? Directly in YAML configurations?

[kngwyu]

I think maybe start by just adding a script to the CI that you invoke on Appveyor that executes one of these builds.

👍

There should be different script for Travis CI as well? To be sure that I'm not breaking something. If so, where to place scripts? Directly in YAML configurations?

In ci/travis/test.sh or so.

[pganssle]

@lycantropos @kngwyu I think Appveyor is the important one, but having it on Travis doesn't hurt.

I think maybe ci/travis is the wrong place, maybe just ci/test_venv.sh or something like that.

[kngwyu]

How about using tox to test venv?
Then we don't need to add additional scripts.

[lycantropos]

what should we do with this kind of message:

./ci/travis/test.sh: line 6: cargo: command not found
The command "./ci/travis/test.sh" exited with 127.

?
it seems like modifying PATH in setup.sh is not working

[lycantropos]

How about using tox to test venv?
Then we don't need to add additional scripts.

@kngwyu: I'm not so familiar with tox, can you please provide an example of activating venv in it?

[konstin]

I'm not so familiar with tox, can you please provide an example of activating venv in it?

Not kngwyu, but anyways: The cool thing about tox is that it creates the venv itself. If the correct python version is active, you should only need to call tox -e py (and maybe pip install tox beforehand). You can easily try this out by just running tox on your local machine

[lycantropos]

I've added tox-venv plugin to use stdlib venv instead of virtualenv because the latter seems to create full copy of Python and override sys.base_prefix as well, but didn't make a copy of Python/libs directory which is needed for projects to compile on Windows.
I've also added pip & setuptools in examples' requirements because it seems like they need to be upgraded manually until this PR is merged and test-venv is updated (AFAIK virtualenv updates them automatically).

Finally, we have errors related to hypothesis strategies used for datetime.timestamp method testing, which min/max limits are platform-dependent.

[lycantropos]

I've manually found edge-cases for datetime.timestamp, for Linux environment I've used Docker containers. If someone has access to Python on "real Linux" it will be great to check them by:

• calling method and making sure it does not throw

  MIN_DATETIME_FROM_TIMESTAMP.timestamp()
  MAX_DATETIME_FROM_TIMESTAMP.timestamp()

• moving out of bounds and making sure it throws

  datetime.fromtimestamp(MIN_DATETIME_FROM_TIMESTAMP.timestamp() - 1).timestamp()
  datetime.fromtimestamp(MAX_DATETIME_FROM_TIMESTAMP.timestamp() + 1).timestamp()

I can also add this as a separate test.

[kngwyu]

@lycantropos
Good catch, thanks.
But probably the fix for the datetime example should be in a separate PR.

[lycantropos]

@lycantropos
Good catch, thanks.
But probably the fix for the datetime example should be in a separate PR.

I thought that there is no way of merging if tests fail. Also I do not know what is left to do or I can add entry to changelog and we are done?

[pganssle]

If you could, please at least justify the formatting changes you've made, they seem somewhat arbitrary. If we're going to be doing formatting changes, we should adopt black for the Python portions of the code, and do that in a separate PR.

I think the per-platform bounds tweaking is probably fine, at least for now.

[kngwyu]

I changed the title instead of separating the PR

[kngwyu]

It's merged at last, thanks.