-
Notifications
You must be signed in to change notification settings - Fork 12.7k
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
Fixup failing fuchsia tests #127461
Fixup failing fuchsia tests #127461
Conversation
Many tests use stdout and stderr to validate whether the test emitted the correct output. Because fuchsia-test-runner.py was sending all logs, including test output, to stdout, tests could not validate output properly. This change removes the runner logs from stdout and stderr entirely with the exception of output from the test. All runner logs are still available in the "log" file. Fixed: https://fxbug.dev/351356417
Applied formatting suggestions from isort and black via pylsp.
Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @tmandry (or someone else) some time within the next two weeks. Please see the contribution instructions for more information. Namely, in order to ensure the minimum review times lag, PR authors and assigned reviewers should ensure that the review label (
|
See https://ci.chromium.org/ui/p/fuchsia/builders/try/rust_test-x64-linux/b8743058030660555825/overview for verification that these changes fix the failing tests. |
This comment has been minimized.
This comment has been minimized.
bd692bb
to
49d2c81
Compare
The failures were due to adding a line to the source of two tests and the expected output containing line numbers. |
Both test-panic-abort-nocapture.rs and test-panic-abort.rs assert the stderr output of the test. On Fuchsia, if a test fails an assertion, this output will contain a line noting the process returned the code -1028 (ZX_TASK_RETCODE_EXCEPTION_KILL). But the asserted stderr output lacks this note. Presumably this is because other platforms implement -Cpanic=abort by killing the process instead of returned a status code.
d20121b
to
479b0cd
Compare
@bors r+ rollup |
Rollup of 8 pull requests Successful merges: - rust-lang#124211 (Bump `elided_lifetimes_in_associated_constant` to deny) - rust-lang#125627 (migration lint for `expr2024` for the edition 2024) - rust-lang#127091 (impl FusedIterator and a size hint for the error sources iter) - rust-lang#127461 (Fixup failing fuchsia tests) - rust-lang#127484 (`#[doc(alias)]`'s doc: say that ASCII spaces are allowed) - rust-lang#127508 (small search graph refactor) - rust-lang#127521 (Remove spastorino from SMIR) - rust-lang#127532 (documentation: update cmake version) r? `@ghost` `@rustbot` modify labels: rollup
Rollup merge of rust-lang#127461 - c6c7:fixup-failing-fuchsia-tests, r=tmandry Fixup failing fuchsia tests The Fuchsia platform passes all tests with these changes. Two tests are ignored because they rely on Fuchsia not returning a status code upon a process aborting. See rust-lang#102032 and rust-lang#58590 for more details on that topic. Many formatting changes are also included in this PR. r? tmandry r? erickt
This is still in the bors queue as accepted. @bors r- |
The Fuchsia platform passes all tests with these changes. Two tests are ignored because they rely on Fuchsia not returning a status code upon a process aborting. See #102032 and #58590 for more details on that topic.
Many formatting changes are also included in this PR.
r? tmandry
r? erickt