-
Notifications
You must be signed in to change notification settings - Fork 10.5k
Windows test accomodations #23011
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
Windows test accomodations #23011
Conversation
Windows uses carriage returns and newlines. The string matching is sensitive to this. Simply erase carriage returns from the standard error stream to compensate.
The path separator is different on Windows. Match just the last path component to allow the test to be portable.
CC: @moiseev |
@swift-ci please test |
Build failed |
@swift-ci please test Linux platform |
Build failed |
@swift-ci please test Linux platform |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
(Blushing for not realizing those slashes will be an issue on Windows...)
while let line = self._childStderr.getline() { | ||
while var line = self._childStderr.getline() { | ||
if let cr = line.firstIndex(of: "\r") { | ||
line.remove(at: cr) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hm, we should stop adding more workarounds for getline()
, and fix its behavior instead. Is there anything that depends on getline()
splitting on LF only?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, that would be great to fix, I just have not been able to do that properly for some reason. Its not the splitting on LF that things are dependent on, its the string matching not expecting the CR to be there that causes the problems.
Replace this paragraph with a description of your changes and rationale. Provide links to external references/discussions if appropriate.
Resolves SR-NNNN.