Skip to content

[test] Remove redirection stderr->out that corrupted output. #26916

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

Conversation

drodriguez
Copy link
Contributor

The RUN line was redirecting stderr to stdout, but nothing in the test
was checking for the stderr output.

However, the last line of stderr was an empty newline, which was printed
(in my machine) in the middle of the dump from the AST, breaking the
test.

If we ever need to check the stderr, it should be redirected to a file,
and the file should be passed to FileCheck independently, to avoid
concurrent uses of the output buffer.

A similar change was performed in 39d3963 to the second line of the test.

/cc @akyrtzi

The RUN line was redirecting stderr to stdout, but nothing in the test
was checking for the stderr output.

However, the last line of stderr was an empty newline, which was printed
(in my machine) in the middle of the dump from the AST, breaking the
test.

If we ever need to check the stderr, it should be redirected to a file,
and the file should be passed to FileCheck independently, to avoid
concurrent uses of the output buffer.
@drodriguez drodriguez requested a review from pschuh August 28, 2019 21:21
@drodriguez
Copy link
Contributor Author

@swift-ci please smoke test

@akyrtzi
Copy link
Contributor

akyrtzi commented Aug 28, 2019

Thanks for fixing! This was added back when -dump* was outputting to stderr.

@drodriguez
Copy link
Contributor Author

Makes sense. Thanks for explaining the reason!

@drodriguez drodriguez merged commit 3a78945 into swiftlang:master Aug 29, 2019
@drodriguez drodriguez deleted the test-dump-parse-without-stderr branch August 29, 2019 18:15
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