Add --quiet option for `cargo test` #6358

collin5 · 2018-11-28T18:56:41Z

dwijnand · 2018-11-28T19:54:42Z

tests/testsuite/test.rs

+        ).build();
+
+        p.cargo("test -q hello")
+            .with_stdout_contains("running 1 test")


Shouldn't it not contain this? In fact, should the two assertions be the following?

.with_stdout("") .with_stderr("")

The output only contains no of tests being run plus the test result. It only doesn't show the actual tests being run. This is the same as doing

$ rustc --test unit-test.rs $ ./unit-test --quiet

cc @dwijnand

Right, that's what I thought. libtest's help says:

-q, --quiet Display one character per test instead of one line. Alias to --format=terse

So I think if we incorporate that information into cargo test --help and expand this test to test the full output of out and err, for me, we can make this change.

collin5 · 2018-11-29T00:33:07Z

Updated. Thank you!

dwijnand · 2018-11-29T06:56:43Z

Hmm that's the global -q for all commands. We'd have to find a way to do it just for the test command...

collin5 · 2018-11-29T13:57:24Z

🤦‍♂️ Oops! my bad. Hadn't noticed this. Going to fix this in a few.

collin5 · 2018-11-29T19:05:39Z

Fixed. test command --quiet description should now override global option. Thanks!

alexcrichton

Thanks for this! FWIW this is not quite as trivial as it may seem I think because the libtest-based test harness isn't the only one out there in use by Cargo. Through harness = false we'd be passing a --quiet option to test harnesses which may not expect this, so this runs the risk of breaking existing workflows.

In general though this is definitely a papercut and one we've wanted to fix for quite some time. We've never really sat down though and thought through the consequences. I'd personally prefer if we established a defined set of arguments that we may passed to test harnesses to define the Cargo conventions they should follow. I'm somewhat wary to land piecemeal adjustments before that though due to the breakage it could cause.

alexcrichton · 2018-12-07T21:27:29Z

src/bin/cargo/cli.rs

+        )
+}
+
+// add --quiet option to command


I'm a little confused about why this function is necessary, should the quiet argument be added to the below subcommands if it's not already present?

Added this because quiet was already present (set to global) for all cargo subcommands

cargo/src/bin/cargo/cli.rs

Line 223 in c4b5a63

.global(true),

. I'm adding this to all subcommands except those that already have this option defined to allow these to override this option (as currently couldn't find a way to override the description for a global option for clap.rs). One other option would be to explicitly add --quiet for all other subcommands OR for only cargo test if this is not required for others.

Hm ok, if we end up landing this I think we'll want to take the route of declaratively adding --quiet to each subcommand, either through a shared implementation or manually, I don't think we'll want to sneak in --quiet as an argument right at the end

Thanks, makes sense. Also I noticed --quiet does nothing for some subcommands i.e cargo search, generate-lockfile, etc. Is it ok to not add the --quiet for these? The option was present for all before so my worry is removing it may cause breakage.

Nah I think it's fine to add this for those commands to, they'll likely just pick up the existing behavior for --quiet naturally!

collin5 · 2018-12-16T05:40:04Z

Updated the PR to explicitly add --quiet to each subcommand. Any ideas on how to go about defining arguments to pass to the test harnesses. Thanks

alexcrichton · 2018-12-17T16:55:48Z

In terms of definition, I think in theory --quiet should be defined next to --verbose? In terms of how to make progress on this though with tests harnesses I'm not entirely sure :(

ehuss · 2018-12-17T18:14:23Z

Maybe a crazy idea, but related to custom test frameworks, the [testing.framework] section could have some sort of table which describes what capabilities a harness has. I can imagine other things like JSON output, color control, setting --test-threads, could be useful.

In the short term, maybe it can just skip the flag when harness=false?

alexcrichton · 2018-12-17T18:31:09Z

That seems plausible to me! In any case not passing it through automatically with harness = false sounds reasonable to me

bors · 2019-02-05T16:06:28Z

☔ The latest upstream changes (presumably #6631) made this pull request unmergeable. Please resolve the merge conflicts.

ehuss · 2019-02-14T00:19:29Z

Ping @collin5 do you think you'd be able to change it so that it doesn't forward flags if harness is false?

collin5 · 2019-02-14T14:51:14Z

@ehuss Sorry for the delay on this, been a bit busy lately. Will be working on it this weekend. Thanks for the reminder.

collin5 · 2019-02-21T17:21:47Z

Changed to forwarding the --quiet, -q flag only if harness is true. Thank you. cc @alexcrichton @ehuss

ehuss · 2019-02-25T20:54:13Z

src/cargo/ops/cargo_test.rs

@@ -83,7 +84,12 @@ fn run_unit_tests(
    for &(ref pkg, ref kind, ref test, ref exe) in &compilation.tests {
        let exe_display = exe.strip_prefix(cwd).unwrap_or(exe).display();
        let mut cmd = compilation.target_process(exe, pkg)?;
+        let targets = pkg.manifest().targets();
+        let harness = targets.iter().any(|t| t.kind() == &TargetKind::Test && t.harness());


I don't think this is the correct way to check for a harness. For example, this won't work if the target is a lib or bin. It also won't work properly if some targets of a harness and some don't. I think maybe the entire Target struct will need to be included in Compilation::tests?

Makes sense. Will do. Thank you!

collin5 · 2019-03-09T00:53:45Z

Updated! Thank you. @ehuss

ehuss · 2019-03-10T23:38:07Z

src/cargo/ops/cargo_test.rs

        let exe_display = exe.strip_prefix(cwd).unwrap_or(exe).display();
        let mut cmd = compilation.target_process(exe, pkg)?;
        cmd.args(test_args);
+        if target.harness() && options.quiet_run {


Sorry, just one more minor question. Is there a reason quiet_run was added to TestOptions instead of just checking the verbosity level? Something like: config.shell().verbosity() == Verbosity::Quiet. The reason I ask is because "cargo -q test" behaves differently from "cargo test -q".

Oops! Had missed this. I've updated changes. Thanks again. @ehuss

ehuss · 2019-03-11T17:00:04Z

Thanks @collin5!

@bors r+

bors · 2019-03-11T17:00:06Z

📌 Commit d93b2d7 has been approved by ehuss

bors · 2019-03-11T17:00:18Z

⌛ Testing commit d93b2d7 with merge d74d879...

Add --quiet option for `cargo test` Fixes #4325

bors · 2019-03-11T17:33:11Z

☀️ Test successful - checks-travis, status-appveyor
Approved by: ehuss
Pushing d74d879 to master...

Update cargo 5 commits in 95b45eca19ac785263fed98ecefe540bb47337ac..dd761226d944712a363ea515cb294f4e2b6bdbe5 2019-03-06 19:24:30 +0000 to 2019-03-11 18:51:14 +0000 - Fingerprint build script deps only for path packages. (rust-lang/cargo#6734) - Add --quiet option for `cargo test` (rust-lang/cargo#6358) - .gitignore should end with a newline. (rust-lang/cargo#6732) - Emit warning on misspelled environment variables (rust-lang/cargo#6694) - Update glob requirement from 0.2.11 to 0.3.0 (rust-lang/cargo#6724)

Update cargo 6 commits in 95b45eca19ac785263fed98ecefe540bb47337ac..0e35bd8af0ec72d3225c4819b330b94628f0e9d0 2019-03-06 19:24:30 +0000 to 2019-03-13 06:52:51 +0000 - Make `hg` optional for tests. (rust-lang/cargo#6739) - Fingerprint build script deps only for path packages. (rust-lang/cargo#6734) - Add --quiet option for `cargo test` (rust-lang/cargo#6358) - .gitignore should end with a newline. (rust-lang/cargo#6732) - Emit warning on misspelled environment variables (rust-lang/cargo#6694) - Update glob requirement from 0.2.11 to 0.3.0 (rust-lang/cargo#6724)

This fixes an issue where `--quiet` doesn't work with commands that have subcommands. This is because `config_configure` only looks at the global and top-level subcommand, and not deeper subcommands. The issue was that `--quiet` was not defined as a global flag. This was changed in rust-lang#6358 in order to give a better help message for `cargo test --quiet`. I don't remember if clap just didn't support overriding at the time, or if we just didn't know how it worked. Anyways, it seems to work to override it now, so I think it should be fine to mark it as global. This should bring in `--quiet` more in-line with how `--verbose` works. This means that `--quiet` is now accepted with `cargo report`, `cargo help`, and `cargo config`. This also fixes `--quiet` with `cargo clean gc`. This should also help with supporting `--quiet` with the new `cargo owner` subcommands being added in rust-lang#11879. Fixes rust-lang#12957

Fix --quiet being used with nested subcommands. This fixes an issue where `--quiet` doesn't work with commands that have subcommands. This is because `config_configure` only looks at the global and top-level subcommand, and not deeper subcommands. The issue was that `--quiet` was not defined as a global flag. This was changed in #6358 in order to give a better help message for `cargo test --quiet`. I don't remember if clap just didn't support overriding at the time, or if we just didn't know how it worked. Anyways, it seems to work to override it now, so I think it should be fine to mark it as global. This should bring in `--quiet` more in-line with how `--verbose` works. This means that `--quiet` is now accepted with `cargo report`, `cargo help`, and `cargo config`. This also fixes `--quiet` with `cargo clean gc`. This should also help with supporting `--quiet` with the new `cargo owner` subcommands being added in #11879. As for this diff, the help text changes because the `--quiet` flag is changing position (except for the 3 commands mentioned above where it is now added). Fixes #12957

dwijnand reviewed Nov 28, 2018

View reviewed changes

collin5 force-pushed the b4325 branch from 9eec8c0 to 2b4071d Compare November 28, 2018 23:55

collin5 force-pushed the b4325 branch 2 times, most recently from 03db201 to 9e76402 Compare November 29, 2018 18:33

alexcrichton reviewed Dec 7, 2018

View reviewed changes

collin5 force-pushed the b4325 branch from 9e76402 to c99a5e9 Compare December 13, 2018 16:07

ehuss mentioned this pull request Dec 18, 2018

Disabling colored output requires convoluted cargo test --color=never -- --color=never incantation #1983

Open

pass -q flag to test harness

c891441

collin5 force-pushed the b4325 branch from c99a5e9 to bbab0cf Compare February 21, 2019 15:17

collin5 added 2 commits February 21, 2019 18:35

update --quiet description for cargo test

57d5048

explicitly add --quiet option to all subcommands

e873e4e

collin5 force-pushed the b4325 branch from bbab0cf to 9b344bb Compare February 21, 2019 15:37

ehuss reviewed Feb 25, 2019

View reviewed changes

nrc assigned ehuss Mar 6, 2019

ehuss added the S-waiting-on-author Status: The marked PR is awaiting some action (such as code changes) from the PR author. label Mar 7, 2019

collin5 force-pushed the b4325 branch from 9b344bb to 649bb56 Compare March 9, 2019 00:20

ehuss reviewed Mar 10, 2019

View reviewed changes

collin5 added 2 commits March 11, 2019 03:01

forward --quiet flag only if harness=true

805acff

test --quiet option with and without harness

d93b2d7

collin5 force-pushed the b4325 branch from 649bb56 to d93b2d7 Compare March 11, 2019 00:04

bors added a commit that referenced this pull request Mar 11, 2019

Auto merge of #6358 - collin5:b4325, r=ehuss

d74d879

Add --quiet option for `cargo test` Fixes #4325

bors merged commit d93b2d7 into rust-lang:master Mar 11, 2019

collin5 deleted the b4325 branch March 11, 2019 19:29

ehuss mentioned this pull request Mar 12, 2019

Update cargo rust-lang/rust#59143

Merged

ehuss added this to the 1.35.0 milestone Feb 6, 2022

ehuss mentioned this pull request Nov 12, 2023

Fix --quiet being used with nested subcommands. #12959

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add --quiet option for `cargo test` #6358

Add --quiet option for `cargo test` #6358

collin5 commented Nov 28, 2018

dwijnand Nov 28, 2018

collin5 Nov 28, 2018

dwijnand Nov 28, 2018

collin5 commented Nov 29, 2018

dwijnand commented Nov 29, 2018

collin5 commented Nov 29, 2018

collin5 commented Nov 29, 2018 •

edited

Loading

alexcrichton left a comment

alexcrichton Dec 7, 2018

collin5 Dec 8, 2018 •

edited

Loading

alexcrichton Dec 11, 2018

collin5 Dec 11, 2018

alexcrichton Dec 12, 2018

collin5 commented Dec 16, 2018

alexcrichton commented Dec 17, 2018

ehuss commented Dec 17, 2018

alexcrichton commented Dec 17, 2018

bors commented Feb 5, 2019

ehuss commented Feb 14, 2019

collin5 commented Feb 14, 2019

collin5 commented Feb 21, 2019

ehuss Feb 25, 2019

collin5 Mar 4, 2019

collin5 commented Mar 9, 2019

ehuss Mar 10, 2019

collin5 Mar 11, 2019 •

edited

Loading

ehuss commented Mar 11, 2019

bors commented Mar 11, 2019

bors commented Mar 11, 2019

bors commented Mar 11, 2019

Add --quiet option for cargo test #6358

Add --quiet option for cargo test #6358

Conversation

collin5 commented Nov 28, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

collin5 commented Nov 29, 2018

dwijnand commented Nov 29, 2018

collin5 commented Nov 29, 2018

collin5 commented Nov 29, 2018 • edited Loading

alexcrichton left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

collin5 Dec 8, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

collin5 commented Dec 16, 2018

alexcrichton commented Dec 17, 2018

ehuss commented Dec 17, 2018

alexcrichton commented Dec 17, 2018

bors commented Feb 5, 2019

ehuss commented Feb 14, 2019

collin5 commented Feb 14, 2019

collin5 commented Feb 21, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

collin5 commented Mar 9, 2019

Choose a reason for hiding this comment

collin5 Mar 11, 2019 • edited Loading

Choose a reason for hiding this comment

ehuss commented Mar 11, 2019

bors commented Mar 11, 2019

bors commented Mar 11, 2019

bors commented Mar 11, 2019

Add --quiet option for `cargo test` #6358

Add --quiet option for `cargo test` #6358

collin5 commented Nov 29, 2018 •

edited

Loading

collin5 Dec 8, 2018 •

edited

Loading

collin5 Mar 11, 2019 •

edited

Loading