prompt the use of `--nocapture` flag if `cargo test` process is terminated via a signal. #12463

stupendoussuperpowers · 2023-08-07T21:07:07Z

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.

rustbot · 2023-08-07T21:07:12Z

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @ehuss (or someone else) soon.

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 (S-waiting-on-review and S-waiting-on-author) stays updated, invoking these commands when appropriate:

@rustbot author: the review is finished, PR author should check the comments and take action accordingly
@rustbot review: the author is ready for a review, this PR will be queued again in the reviewer's queue

src/cargo/ops/cargo_test.rs

tests/testsuite/test.rs

src/cargo/ops/cargo_test.rs

stupendoussuperpowers · 2023-08-07T22:50:40Z

I've split the changes into 2 commits as you asked, and changed the display string.

The tests are failing right now because it is outputting the --nocapture note even if there's an exec failure. Looking into a fix for this.

ehuss · 2023-08-07T23:02:20Z

r? epage

Hope you don't mind taking this.

tests/testsuite/test.rs

stupendoussuperpowers · 2023-08-08T10:47:10Z

Hi @epage need some help with this.

cargo test -p cargo for linux x86_64 beta is passing, but the test fails for linux x86_64 nightly. Looks like the --no-fail-fast --nocapture are outputting different things for these two. Not sure how to deal with this.

epage · 2023-08-08T18:33:37Z

Hi @epage need some help with this.

cargo test -p cargo for linux x86_64 beta is passing, but the test fails for linux x86_64 nightly. Looks like the --no-fail-fast --nocapture are outputting different things for these two. Not sure how to deal with this.

[..] are a type of wildcard. You can adjust these wildcards to make it still show the relevant information (t1.rs) while hiding the information that is tied to a specific rust version

epage · 2023-08-09T01:27:58Z

@stupendoussuperpowers could you rebase, instead of merge, so I can more easily browse the individual commits?

epage · 2023-08-10T19:58:02Z

btw I'm hosting office hours right now but also available at other times in case you want to do a call. See https://github.com/rust-lang/cargo/wiki/Office-Hours

epage · 2023-08-10T19:59:20Z

Hmm, forgot to look back at this. @stupendoussuperpowers anything left or is this good to go?

EDIT: Wow, missed your marking it as ready

epage · 2023-08-10T19:59:49Z

@bors r+

Thanks for all the work you did on this!

bors · 2023-08-10T19:59:55Z

📌 Commit dd8cd9c has been approved by epage

It is now in the queue for this repository.

bors · 2023-08-10T20:00:06Z

⌛ Testing commit dd8cd9c with merge 4312a90...

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.

bors · 2023-08-10T20:19:56Z

💔 Test failed - checks-actions

weihanglo · 2023-08-10T21:20:20Z

 thread 'git_auth::ssh_something_happens' panicked at '
test failed running `/home/runner/work/cargo/cargo/target/debug/cargo check -v`
error: expected to find:
[..]Connection [..] by [..]

did not find in output:
    Updating git repository `ssh://127.0.0.1:37377/foo/bar`
warning: spurious network error (3 tries remaining): failed to fill whole buffer
error: failed to get `bar` as a dependency of package `foo v0.0.1 (/home/runner/work/cargo/cargo/target/tmp/cit/t99/foo)`

Caused by:
  failed to load source for dependency `bar`

Caused by:
  Unable to update ssh://127.0.0.1:37377/foo/bar

Caused by:
  failed to clone into: /home/runner/work/cargo/cargo/target/tmp/cit/t99/home/.cargo/git/db/bar-d4c725e0d0110fc9

Caused by:
  network failure seems to have happened
  if a proxy or similar is necessary `net.git-fetch-with-cli` may help here
  https://doc.rust-lang.org/cargo/reference/config.html#netgit-fetch-with-cli

Caused by:
  An IO error occurred when talking to the server

Caused by:
  ssh: connect to host 127.0.0.1 port 37377: Connection refused
', tests/testsuite/git_auth.rs:301:10

Not sure what's going on. Could be the TCP connection accidentally closed.

Let's @bors retry and see.

bors · 2023-08-10T21:20:27Z

⌛ Testing commit dd8cd9c with merge 7da1030...

bors · 2023-08-10T22:12:43Z

☀️ Test successful - checks-actions
Approved by: epage
Pushing 7da1030 to master...

Update cargo 21 commits in d78bbf4bde3c6b95caca7512f537c6f9721426ff..7e9de3f4ec3708f500bec142317895b96131e47c 2023-08-03 12:58:25 +0000 to 2023-08-13 00:47:32 +0000 - feat: remove `--keep-going` from `cargo test/bench` (rust-lang/cargo#12478) - chore: window-sys should be a platform-specific dependency (rust-lang/cargo#12483) - docs: make the env var source of rerun-if-env-changed clearer (rust-lang/cargo#12482) - doc: note the backward compatible `.cargo/credential` file exists (rust-lang/cargo#12479) - Fix elided lifetime in associated const (rust-lang/cargo#12475) - prompt the use of `--nocapture` flag if `cargo test` process is terminated via a signal. (rust-lang/cargo#12463) - cargo-credential: reset stdin & stdout to the Console (rust-lang/cargo#12469) - Fix cargo remove incorrectly removing used patches (rust-lang/cargo#12454) - chore(gh): Expand update window (rust-lang/cargo#12466) - Fix panic when enabling http.debug for certain strings (rust-lang/cargo#12468) - fix(cli): Make `--help` easier to browse (rust-lang/cargo#11905) - fix: preserve jobserver file descriptors on rustc invocation to get `TargetInfo` (rust-lang/cargo#12447) - refactor: migrate to `tracing` (rust-lang/cargo#12458) - docs: add example for cargo-credential (rust-lang/cargo#12461) - Bail out an error when using cargo:: in custom build script (rust-lang/cargo#12332) - Fix printing multiple warning messages for unused fields in [registries] table (rust-lang/cargo#12439) - Update windows dependencies (rust-lang/cargo#12453) - Rustfmt a let-else statement (rust-lang/cargo#12451) - Add allow(internal_features) (rust-lang/cargo#12450) - Update pretty_env_logger to 0.5 (rust-lang/cargo#12445) - Remove build metadata from libgit2-sys dependency (rust-lang/cargo#12444) r? `@ghost`

rustbot assigned ehuss Aug 7, 2023

rustbot added Command-test S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Aug 7, 2023

epage reviewed Aug 7, 2023

View reviewed changes

src/cargo/ops/cargo_test.rs Outdated Show resolved Hide resolved

epage reviewed Aug 7, 2023

View reviewed changes

tests/testsuite/test.rs Outdated Show resolved Hide resolved

epage reviewed Aug 7, 2023

View reviewed changes

src/cargo/ops/cargo_test.rs Outdated Show resolved Hide resolved

stupendoussuperpowers force-pushed the nocapture_test_msg branch from 03db36e to 640c4cf Compare August 7, 2023 22:18

rustbot assigned epage and unassigned ehuss Aug 7, 2023

stupendoussuperpowers force-pushed the nocapture_test_msg branch from 640c4cf to 6c0ab39 Compare August 7, 2023 23:05

epage reviewed Aug 7, 2023

View reviewed changes

tests/testsuite/test.rs Show resolved Hide resolved

stupendoussuperpowers force-pushed the nocapture_test_msg branch 2 times, most recently from aa7e623 to 24fb803 Compare August 8, 2023 10:22

stupendoussuperpowers force-pushed the nocapture_test_msg branch 2 times, most recently from 0fb2a72 to 4fffb8b Compare August 8, 2023 14:15

stupendoussuperpowers force-pushed the nocapture_test_msg branch from 0633fa2 to 294a589 Compare August 8, 2023 22:45

stupendoussuperpowers force-pushed the nocapture_test_msg branch from 1509634 to 67a0402 Compare August 9, 2023 19:14

bors added the S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. label Aug 10, 2023

bors added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Aug 10, 2023

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Aug 10, 2023

bors merged commit 7da1030 into rust-lang:master Aug 10, 2023

weihanglo mentioned this pull request Aug 13, 2023

Update cargo rust-lang/rust#114773

Merged

ehuss added this to the 1.73.0 milestone Aug 22, 2023

stupendoussuperpowers deleted the nocapture_test_msg branch September 7, 2024 19:52

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

prompt the use of `--nocapture` flag if `cargo test` process is terminated via a signal. #12463

prompt the use of `--nocapture` flag if `cargo test` process is terminated via a signal. #12463

stupendoussuperpowers commented Aug 7, 2023

rustbot commented Aug 7, 2023

stupendoussuperpowers commented Aug 7, 2023

ehuss commented Aug 7, 2023

stupendoussuperpowers commented Aug 8, 2023

epage commented Aug 8, 2023

epage commented Aug 9, 2023

epage commented Aug 10, 2023

epage commented Aug 10, 2023 •

edited

Loading

epage commented Aug 10, 2023

bors commented Aug 10, 2023

bors commented Aug 10, 2023

bors commented Aug 10, 2023

weihanglo commented Aug 10, 2023

bors commented Aug 10, 2023

bors commented Aug 10, 2023

prompt the use of --nocapture flag if cargo test process is terminated via a signal. #12463

prompt the use of --nocapture flag if cargo test process is terminated via a signal. #12463

Conversation

stupendoussuperpowers commented Aug 7, 2023

rustbot commented Aug 7, 2023

stupendoussuperpowers commented Aug 7, 2023

ehuss commented Aug 7, 2023

stupendoussuperpowers commented Aug 8, 2023

epage commented Aug 8, 2023

epage commented Aug 9, 2023

epage commented Aug 10, 2023

epage commented Aug 10, 2023 • edited Loading

epage commented Aug 10, 2023

bors commented Aug 10, 2023

bors commented Aug 10, 2023

bors commented Aug 10, 2023

weihanglo commented Aug 10, 2023

bors commented Aug 10, 2023

bors commented Aug 10, 2023

prompt the use of `--nocapture` flag if `cargo test` process is terminated via a signal. #12463

prompt the use of `--nocapture` flag if `cargo test` process is terminated via a signal. #12463

epage commented Aug 10, 2023 •

edited

Loading