Skip to content

[test] diff --strip-trailing-cr is non-standard. #33931

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
Sep 14, 2020

Conversation

3405691582
Copy link
Member

Since we're saving command output to file and then comparing, we can use
sed to convert the inputs to the same line ending convention when
comparing. This has the same effect as the --strip-trailing-cr flag.
Since this flag is a nonstandard extension, it won't be available on
other platforms, and the absence of this flag will cause misleading test
failures.

@3405691582
Copy link
Member Author

cc @compnerd: since this affects Windows line endings, could you do a CI run for this for me?

@compnerd
Copy link
Member

This increases the FS traffic which is noticeable on Windows, so this is going to also slow down running tests. What do you think of hoisting the SourceKit %diff substitution to the top level lit.cfg, and conditionalizing it there fore OpenBSD vs other targets?

This flag is a GNU extension, and would cause misleading test failures
on other platforms where this extension is not available. However, the
necessity to switch line endings is only required on Windows when
testing. We could use sed to canonicalize line endings before comparing,
but that may cause higher amounts of filesystem traffic on Windows which
would slow down testing.

Instead, move the substitution for diff in SourceKit's lit.local.cfg up
to the top level, and conditionalize the substitution which has the flag
on Windows, but not on other platforms (where it should not be required).
@3405691582
Copy link
Member Author

Yeah, that's a nice idea. Done.

(I made the necessary move of the substitution but could not check if the SourceKit tests function properly locally.)

@compnerd
Copy link
Member

@swift-ci please test

@swift-ci
Copy link
Contributor

Build failed
Swift Test OS X Platform
Git Sha - 7ebea3bd62cbe4d63d747b5ca55c98b36bab5918

@compnerd compnerd merged commit 76a7160 into swiftlang:master Sep 14, 2020
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