Skip to content

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

Merged
merged 2 commits into from
Mar 1, 2019

Conversation

compnerd
Copy link
Member

@compnerd compnerd commented Mar 1, 2019

Replace this paragraph with a description of your changes and rationale. Provide links to external references/discussions if appropriate.

Resolves SR-NNNN.

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.
@compnerd
Copy link
Member Author

compnerd commented Mar 1, 2019

CC: @moiseev

@compnerd
Copy link
Member Author

compnerd commented Mar 1, 2019

@swift-ci please test

@swift-ci
Copy link
Contributor

swift-ci commented Mar 1, 2019

Build failed
Swift Test Linux Platform
Git Sha - 14d7218

@compnerd
Copy link
Member Author

compnerd commented Mar 1, 2019

@swift-ci please test Linux platform

@swift-ci
Copy link
Contributor

swift-ci commented Mar 1, 2019

Build failed
Swift Test Linux Platform
Git Sha - 14d7218

@compnerd
Copy link
Member Author

compnerd commented Mar 1, 2019

@swift-ci please test Linux platform

Copy link
Member

@lorentey lorentey left a 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)
Copy link
Member

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?

Copy link
Member Author

@compnerd compnerd Mar 1, 2019

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.

@compnerd compnerd merged commit 3f849d4 into swiftlang:master Mar 1, 2019
@compnerd compnerd deleted the file-the-line-with-the-registry branch March 1, 2019 23:01
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.

3 participants