Skip to content

Commit

Permalink
Auto merge of #12463 - stupendoussuperpowers:nocapture_test_msg, r=epage
Browse files Browse the repository at this point in the history
prompt the use of `--nocapture` flag if `cargo test` process is terminated via a signal.

Fixes #10855

As per the discussion on this issue, we want to prompt the user to use `--nocapture` if a test is terminated abnormally. The motivation for this change is described in the issue.

We check for 3 things before we display this flag. -
- `!is_simple` (if the test ended with a non 101 status code)
- `harness` (if the standard test harness was used), and
- `!nocapture` (whether or not the `--nocapture` flag was already passed to the test)

There's further tests added to `test::nonzero_exit_status` that check that the `stderr` is correct for the various combinations possible when a test ends with a non-101 status code.

The new expected behavior is -
- Display `--nocapture` note for only non-zero exit statuses, when the `--nocapture` flag is not passed.
- Only display the note if we use a standard test harness since custom test harnesses do not implement the `--nocapture` flag.

To implement the check for the `--nocapture` flag, the function definition for `report_test_errors` was changed to add the `test_args: &[&str]` parameter. This parameter is passed from the immediate calling function. This private function is only called twice change and is not causing regression after making the appropriate changes to both the places it's called in.
  • Loading branch information
bors committed Aug 10, 2023
2 parents 54550ef + dd8cd9c commit 7da1030
Show file tree
Hide file tree
Showing 2 changed files with 39 additions and 4 deletions.
19 changes: 15 additions & 4 deletions src/cargo/ops/cargo_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,7 @@ fn run_unit_tests(
unit: unit.clone(),
kind: test_kind,
};
report_test_error(ws, &options.compile_opts, &unit_err, e);
report_test_error(ws, test_args, &options.compile_opts, &unit_err, e);
errors.push(unit_err);
if !options.no_fail_fast {
return Err(CliError::code(code));
Expand Down Expand Up @@ -275,7 +275,7 @@ fn run_doc_tests(
unit: unit.clone(),
kind: TestKind::Doctest,
};
report_test_error(ws, &options.compile_opts, &unit_err, e);
report_test_error(ws, test_args, &options.compile_opts, &unit_err, e);
errors.push(unit_err);
if !options.no_fail_fast {
return Err(CliError::code(code));
Expand Down Expand Up @@ -407,6 +407,7 @@ fn no_fail_fast_err(
/// Displays an error on the console about a test failure.
fn report_test_error(
ws: &Workspace<'_>,
test_args: &[&str],
opts: &ops::CompileOptions,
unit_err: &UnitTestError,
test_error: anyhow::Error,
Expand All @@ -420,13 +421,23 @@ fn report_test_error(
let mut err = format_err!("{}, to rerun pass `{}`", which, unit_err.cli_args(ws, opts));
// Don't show "process didn't exit successfully" for simple errors.
// libtest exits with 101 for normal errors.
let is_simple = test_error
let (is_simple, executed) = test_error
.downcast_ref::<ProcessError>()
.and_then(|proc_err| proc_err.code)
.map_or(false, |code| code == 101);
.map_or((false, false), |code| (code == 101, true));

if !is_simple {
err = test_error.context(err);
}

crate::display_error(&err, &mut ws.config().shell());

let harness: bool = unit_err.unit.target.harness();
let nocapture: bool = test_args.contains(&"--nocapture");

if !is_simple && executed && harness && !nocapture {
drop(ws.config().shell().note(
"test exited abnormally; to see the full output pass --nocapture to the harness.",
));
}
}
24 changes: 24 additions & 0 deletions tests/testsuite/test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4792,6 +4792,21 @@ error: test failed, to rerun pass `--test t1`
[RUNNING] tests/t2.rs (target/debug/deps/t2[..])
error: test failed, to rerun pass `--test t2`
Caused by:
process didn't exit successfully: `[ROOT]/foo/target/debug/deps/t2[..]` (exit [..]: 4)
note: test exited abnormally; to see the full output pass --nocapture to the harness.
",
)
.with_status(4)
.run();

p.cargo("test --test t2 -- --nocapture")
.with_stderr(
"\
[FINISHED] test [..]
[RUNNING] tests/t2.rs (target/debug/deps/t2[..])
error: test failed, to rerun pass `--test t2`
Caused by:
process didn't exit successfully: `[ROOT]/foo/target/debug/deps/t2[..]` (exit [..]: 4)
",
Expand All @@ -4811,11 +4826,20 @@ error: test failed, to rerun pass `--test t2`
Caused by:
process didn't exit successfully: `[ROOT]/foo/target/debug/deps/t2[..]` (exit [..]: 4)
note: test exited abnormally; to see the full output pass --nocapture to the harness.
error: 2 targets failed:
`--test t1`
`--test t2`
",
)
.with_status(101)
.run();

p.cargo("test --no-fail-fast -- --nocapture")
.with_stderr_does_not_contain("test exited abnormally; to see the full output pass --nocapture to the harness.")
.with_stderr_contains("[..]thread 't' panicked [..] tests/t1[..]")
.with_stderr_contains("note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace")
.with_stderr_contains("[..]process didn't exit successfully: `[ROOT]/foo/target/debug/deps/t2[..]` (exit [..]: 4)")
.with_status(101)
.run();
}

0 comments on commit 7da1030

Please sign in to comment.