Skip to content
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.

Sign up for GitHub

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

Jump to bottom

cargo test does not report failures when a test exits the process #87323

Closed
EverlastingBugstopper opened this issue Jul 20, 2021 · 6 comments
Closed

cargo test does not report failures when a test exits the process #87323

EverlastingBugstopper opened this issue Jul 20, 2021 · 6 comments
Labels
A-libtest Area: `#[test]` / the `test` library

Comments

@EverlastingBugstopper
Copy link

Problem
I'm running cargo test with tests that fail, and it exits with code 0 and does not gather and report errors.

Steps

  1. git clone https://github.com/apollographql/rover
  2. git checkout avery/repro-tests
  3. Notice there is a test at the root crate that has a single call to panic!("It fails")
  4. Run cargo test, you should notice the FAILED test only if you scroll up to look for it, the message at the end makes you think the tests pass.
  5. Run echo $? and see output 0

Possible Solution(s)
No idea why this is happening and... it's quite alarming!

If I run the same tests through my IDE, it makes a call to cargo test --package rover --lib -- test command::supergraph::config::tests::it_can_fail_a_test --exact --nocapture < which does fail properly!

Notes

$ cargo version
cargo 1.53.0 (4369396ce 2021-04-27)
$ rustc -V      
rustc 1.53.0 (53cb7b09b 2021-06-17)
$ rustup version
rustup 1.24.3 (ce5817a94 2021-05-31)
info: This is the version for the rustup toolchain manager, not the rustc compiler.
info: The currently active `rustc` version is `rustc 1.53.0 (53cb7b09b 2021-06-17)`

This seems to be a cross-platform bug, as our CI runner does not detect issues with any of the following targets:

x86_64-unknown-linux-musl,
x86_64-unknown-linux-gnu,
x86_64-pc-windows-msvc,
x86_64-apple-darwin,

This is a workspace using the 2018 edition, and seems to happen on test runs with both v1 and v2 of the resolver. It also happens whether you're testing a single package with -p, the whole workspace with --workspace, or just cargo test by itself.
@ehuss ehuss transferred this issue from rust-lang/cargo Jul 20, 2021
@ehuss ehuss changed the title cargo test does not report failures cargo test does not report failures when a test exits the process Jul 20, 2021
@ehuss
Copy link
Contributor

ehuss commented Jul 20, 2021

Unfortunately this is pretty fundamentally difficult to fix. The issue is that it_can_serialize_commands_with_arguments exits the process. The test harness is unable to prevent that from happening, and thus the process exits with a 0 status.

There are some potential solutions, such as running each test in a separate process, but that would likely have substantial performance penalties. It might be possible to have something like an atexit bomb that would explode if the process tries to exit without completing the tests, but that sounds like it would have its own can of worms.

I've transferred this to rust-lang/rust. This is where libtest harness issues are tracked. I thought there was already an issue about this, but I couldn't find it.

@ehuss ehuss added the A-libtest Area: `#[test]` / the `test` library label Jul 20, 2021
@ehuss
Copy link
Contributor

ehuss commented Jul 20, 2021 •
edited
Loading

Hm, another thought I just had was that if the tests used JSON, Cargo could take over the responsibility of handling success/failed. It wouldn't be a complete solution, because a test exiting the process would still prevent any other test from running, but just an idea. Maybe there could be a "completed" JSON message that is required for the complete run to be considered a success?

EDIT: Link to #49359 for JSON test support.

@jyn514
Copy link
Member

jyn514 commented Jul 21, 2021

I've transferred this to rust-lang/rust. This is where libtest harness issues are tracked. I thought there was already an issue about this, but I couldn't find it.

You might be thinking of #67650.

@jyn514
Copy link
Member

jyn514 commented Jul 21, 2021

Also this is a really terrible idea, but theoretically libtest could add a SIGABRT handler which doesn't actually exit the process and fails the test instead. Not sure how that could work with parallel tests though.

@ghost
Copy link

ghost commented Jul 21, 2021

This seems similar to #84137.

Merged
@jyn514 jyn514 mentioned this issue Sep 21, 2021
Closed
@Enselic
Copy link
Member

Enselic commented Dec 19, 2023

Triage: let's close as duplicate to #67650 which is for this use case

@Enselic Enselic closed this as not planned Won't fix, can't repro, duplicate, stale Dec 19, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
A-libtest Area: `#[test]` / the `test` library
Projects
testing-devex backlog
Status: Done
Milestone
No milestone
Development

No branches or pull requests
4 participants
@ehuss @Enselic @EverlastingBugstopper @jyn514