Skip to content

[Windows] Fix TestURL/test_URLStrings #2506

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jan 7, 2020

Conversation

gmittert
Copy link
Contributor

@gmittert gmittert commented Sep 9, 2019

Since '|' is a valid character in a url on Windows as implemented by
CoreFoundation, on Windows, we should expect that unescaped|pipe
should be properly parsed.

This is hacky, but adds a test case on Windows, and disables it on other
platforms and vice versa.

@gmittert
Copy link
Contributor Author

gmittert commented Sep 9, 2019

@swift-ci please test

@gmittert
Copy link
Contributor Author

gmittert commented Sep 10, 2019

@millenomi Are you okay with doing testing like this? Is there a better way to do it?

@gmittert gmittert requested a review from millenomi September 10, 2019 18:46
@gmittert
Copy link
Contributor Author

@swift-ci please test linux

@gmittert gmittert force-pushed the StillLegalInSanFrancisco branch from abb574c to fefa12a Compare January 7, 2020 21:25
Since '|' is a valid character in a url on Windows as implemented by
CoreFoundation, on Windows, we should expect that `unescaped|pipe`
should be properly parsed.

This is hacky, but adds a test case on Windows, and disables it on other
platforms and vice versa.
@gmittert gmittert force-pushed the StillLegalInSanFrancisco branch from fefa12a to 5ef5dc3 Compare January 7, 2020 21:30
@gmittert
Copy link
Contributor Author

gmittert commented Jan 7, 2020

@swift-ci please test linux

@compnerd
Copy link
Member

compnerd commented Jan 7, 2020

Thank you for cleaning up the logic here! LGTM

@gmittert gmittert merged commit b280852 into swiftlang:master Jan 7, 2020
@gmittert gmittert deleted the StillLegalInSanFrancisco branch January 7, 2020 23:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants