prompt the use of `--nocapture` flag if `cargo test` process is terminated via a signal. by stupendoussuperpowers · Pull Request #12463 · rust-lang/cargo

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?

stupendoussuperpowers · 2023-08-09T19:17:09Z

give me a day or so to work on this, really embarrassing but i'm running into multiple git issues on my local machine. will mark it ready once it's done.

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...

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...

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 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

rustbot added Command-new Command-owner Command-package Command-pkgid Command-publish Command-read-manifest Command-remove Command-run Command-rustc Command-rustdoc Command-search Command-tree Command-uninstall Command-update Command-vendor Command-yank labels Aug 9, 2023

stupendoussuperpowers marked this pull request as draft August 9, 2023 19:15

stupendoussuperpowers added 2 commits August 10, 2023 01:16

add --nocapture tests

dde17cf

add nocapture message to report_test_err

dd8cd9c

weihanglo mentioned this pull request Aug 13, 2023

Update cargo rust-lang/rust#114773

Merged

Conversation

stupendoussuperpowers commented Aug 7, 2023

Uh oh!

rustbot commented Aug 7, 2023

Uh oh!

Uh oh!

Uh oh!

Uh oh!

stupendoussuperpowers commented Aug 7, 2023

Uh oh!

ehuss commented Aug 7, 2023

Uh oh!

Uh oh!

stupendoussuperpowers commented Aug 8, 2023

Uh oh!

epage commented Aug 8, 2023

Uh oh!

epage commented Aug 9, 2023

Uh oh!

stupendoussuperpowers commented Aug 9, 2023

Uh oh!

epage commented Aug 10, 2023

Uh oh!

epage commented Aug 10, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

epage commented Aug 10, 2023

Uh oh!

bors commented Aug 10, 2023

Uh oh!

bors commented Aug 10, 2023

Uh oh!

bors commented Aug 10, 2023

Uh oh!

weihanglo commented Aug 10, 2023

Uh oh!

bors commented Aug 10, 2023

Uh oh!

bors commented Aug 10, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

epage commented Aug 10, 2023 •

edited

Loading