remote-run: make tests partially pass on Windows #20961
Merged
Conversation
This file contains hidden or 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
Not all the tests are possible to run on Windows since they expect a Unix-like shell environment. However, the rest of the tests can be accommodated. This actually found an issue in the implementation. Since the implementation assumes the target is POSIX/Unix-y, we should use posixpath rather than os.path which uses the host's path style.
|
@swift-ci please test |
|
Build failed |
|
@swift-ci please test macOS platform |
jrose-apple
approved these changes
Dec 3, 2018
jrose-apple
reviewed
Dec 3, 2018
| if source_prefix.endswith(os.path.sep): | ||
| source_prefix = source_prefix[:-len(os.path.sep)] | ||
| if source_prefix.endswith(posixpath.sep): | ||
| source_prefix = source_prefix[:-len(posixpath.sep)] |
There was a problem hiding this comment.
Arguably these should be using host-style paths, but I guess the tests aren't going to do that any time soon.
There was a problem hiding this comment.
I think that some of it needs to be host style and some of it target style. However, since we can always use / even on Windows, I think we can get away with this. The target style paths will be needed at some point, and we can deal with it then.
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.
Not all the tests are possible to run on Windows since they expect a
Unix-like shell environment. However, the rest of the tests can be
accommodated. This actually found an issue in the implementation.
Since the implementation assumes the target is POSIX/Unix-y, we should
use posixpath rather than os.path which uses the host's path style.
Replace this paragraph with a description of your changes and rationale. Provide links to external references/discussions if appropriate.
Resolves SR-NNNN.