Pr 280 support windows #282
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Based on Robert McGibbon's patch
Based on Robert McGibbon's patch
Based on Robert McGibbon's patch
Closed
The place where this easily goes over is in graph data files, which are stored in a deep directory tree.
…ne character early
…on spawn/forkserver mode (on Windows)
Traceback parsing has bugs on some pytest versions, cf. https://bitbucket.org/pytest-dev/py/issue/55/broken-comment-parsin
pv
force-pushed
the
pr-280-windows
branch
6 times, most recently
from
a38d41a
to
5fbbc81
Compare
It appears the Python DLLs in created (Conda) environments stay in use when run on Windows/Appveyor, so that removing the environments fails and their re-creation cannot be tested. Does not occur on all Windows platoforms, however, and adding sleeps does not seem to resolve it.
|
Tests pass currently; discussion on the one additional xfail is in #280 (comment) |
…not handle long ints
…ew server on windows doesn't seem to serve them right)
| return exe | ||
| exe = os.path.join(self._path, executable) | ||
| if os.path.isfile(exe): | ||
| return exe |
There was a problem hiding this comment.
Maybe move this to util or a base class, since it's repeated in virtualenv.py?
|
Ok, refactored a bit. |
Mark the tests as skipped rather than xfail, since we expect them to pass if the environment is set up.
|
Ok to merge? |
| raise | ||
|
|
||
| self.save_info_file(self._path) | ||
|
|
||
| self._is_setup = True | ||
|
|
||
| def _setup(self): | ||
| def setup(self): |
There was a problem hiding this comment.
Why the change to public? The original intention here was that this should only be called from create...
There was a problem hiding this comment.
Ok, reverted setup back to _setup.
|
Great to see this working on Windows! Other than my few minor comments, I think this is ready to merge. Thanks for this! |
| # This loop cannot use xrange, because xrange on Python2 on | ||
| # 32-bit systems can only deal with 32-bit integers, and | ||
| # Javascript timestamps (1000*unix_timestamp) handled here | ||
| # overflow this range |
There was a problem hiding this comment.
Thanks. The comment addresses this nicely.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Rebased #280 plus some additional fixes for Windows.
New PR so that Appveyor tests run.