Print executable name on `cargo test --no-run`. #9959

vrongmeal · 2021-10-04T21:53:39Z

rust-highfive · 2021-10-04T21:53:41Z

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

Please see the contribution instructions for more information.

Closes rust-lang#9957 Signed-off-by: Vaibhav <vrongmeal@gmail.com>

ehuss · 2021-10-07T20:25:07Z

tests/testsuite/test.rs

@@ -1346,6 +1346,7 @@ fn test_no_run() {
            "\
 [COMPILING] foo v0.0.1 ([CWD])
 [FINISHED] test [unoptimized + debuginfo] target(s) in [..]
+[EXECUTABLE] [..] (target/debug/deps/foo-[..][EXE])


Is there a particular reason to use [..] here to ignore part of the output?

I actually looked at other tests such as test_run_simple where the [RUNNING] part was tested like this.

Hm, yea I'm not sure why it does that since in that case it is just the word "unittests". I think it would be good to include the actual output here so that it is clearer what is being emitted and checked.

If you have a chance, I would appreciate if you can remove the [..] here and the other tests just so that the tests are verifying the actual output.

ehuss · 2021-10-07T20:25:12Z

Thanks, looks good. Since this is a somewhat prominent change in cargo's output, I just wanted to double check with the rest of the team:

@rfcbot fcp merge

Also, just a few minor observations. You don't have to make these changes if you don't want to, since the old output had the same issues, but it might be nice to fix it while we are changing it:

If you have a lib and bin, the output can be confusing:

   Compiling foo v0.1.0 (/Users/eric/Temp/foo)
    Finished test [unoptimized + debuginfo] target(s) in 0.41s
  Executable unittests (target/debug/deps/foo-4f58aab180f74d2e)
  Executable unittests (target/debug/deps/foo-688670071dc25e8f)

Which of those two executables is for the library and which is for the binary? I might suggest making the first term be a little more descriptive. Maybe something like Executable unittests src/lib.rs and Executable unittests src/main.rs.

cargo bench indicates unittests for everything, including things in the benches directory:

   Compiling foo v0.1.0 (/Users/eric/Temp/foo)
    Finished bench [optimized] target(s) in 0.28s
  Executable unittests (target/release/deps/foo-6302f637f80147b5)
  Executable unittests (target/release/deps/foo-17cf3e5ac006e4ef)
  Executable unittests (target/release/deps/b1-5d74936019546851)

I might suggest changing the first term to differentiate depending on if this is cargo bench or cargo test. Maybe something like Executable benches/b1.rs for things in benches, and then Executable bench src/lib.rs and Executable bench src/main.rs for example.

rfcbot · 2021-10-07T20:25:13Z

Team member @ehuss has proposed to merge this. The next step is review by the rest of the tagged team members:

No concerns currently listed.

Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

See this document for info about what commands tagged team members can give me.

alexcrichton · 2021-10-11T15:13:18Z

I think @ehuss's suggestion for tweaking the output is pretty reasonable, but having this as a feature also seems good to me and doesn't necessarily need to be blocked on output changes.

rfcbot · 2021-10-26T19:17:02Z

🔔 This is now entering its final comment period, as per the review above. 🔔

ehuss · 2021-11-04T14:18:41Z

src/cargo/ops/cargo_test.rs

+        let config = ws.config();
+        let cwd = config.cwd();
+        for UnitOutput {
+            unit,
+            path,
+            script_meta,
+        } in compilation.tests.iter()
+        {
+            let (exe_display, cmd) = cmd_builds(
+                config,
+                cwd,
+                unit,
+                path,
+                script_meta,
+                test_args,
+                &compilation,
+            )?;
+            config
+                .shell()
+                .concise(|shell| shell.status("Executable", &exe_display))?;
+            config
+                .shell()
+                .verbose(|shell| shell.status("Executable", &cmd))?;
+        }


Since this is quite a lot of duplicated code with run_benches, can this be extracted out to a function to deduplicate it?

ehuss · 2021-11-04T14:21:00Z

tests/testsuite/test.rs

@@ -1346,6 +1346,7 @@ fn test_no_run() {
            "\
 [COMPILING] foo v0.0.1 ([CWD])
 [FINISHED] test [unoptimized + debuginfo] target(s) in [..]
+[EXECUTABLE] [..] (target/debug/deps/foo-[..][EXE])


If you have a chance, I would appreciate if you can remove the [..] here and the other tests just so that the tests are verifying the actual output.

rfcbot · 2021-11-05T19:19:01Z

The final comment period, with a disposition to merge, as per the review above, is now complete.

As the automated representative of the governance process, I would like to thank the author for their work and everyone else who contributed.

This will be merged soon.

bors · 2021-11-07T02:36:43Z

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

ehuss · 2021-12-14T17:45:57Z

Ping @vrongmeal Just checking in to see if you are still interested in working on this, or if you had any questions.

Print executable name on cargo test --no-run #2 Closes #9957 This is a continuation of #9959.

rust-highfive assigned Eh2406 Oct 4, 2021

rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Oct 4, 2021

vrongmeal mentioned this pull request Oct 4, 2021

cargo test --no-run should print the resulting binary name #9957

Closed

vrongmeal force-pushed the no-run-test-executable-path branch 2 times, most recently from 1ae10a6 to fc44b2d Compare October 5, 2021 17:54

Print executable name on cargo test --no-run.

bef4d79

Closes rust-lang#9957 Signed-off-by: Vaibhav <vrongmeal@gmail.com>

vrongmeal force-pushed the no-run-test-executable-path branch from fc44b2d to bef4d79 Compare October 5, 2021 18:17

vrongmeal marked this pull request as ready for review October 6, 2021 20:06

ehuss added the T-cargo Team: Cargo label Oct 7, 2021

ehuss reviewed Oct 7, 2021

View reviewed changes

rfcbot added proposed-final-comment-period An FCP proposal has started, but not yet signed off. disposition-merge FCP with intent to merge labels Oct 7, 2021

rfcbot added the final-comment-period FCP — a period for last comments before action is taken label Oct 26, 2021

rfcbot removed the proposed-final-comment-period An FCP proposal has started, but not yet signed off. label Oct 26, 2021

ehuss reviewed Nov 4, 2021

View reviewed changes

rfcbot added finished-final-comment-period FCP complete to-announce and removed final-comment-period FCP — a period for last comments before action is taken labels Nov 5, 2021

ehuss added S-waiting-on-author Status: The marked PR is awaiting some action (such as code changes) from the PR author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Nov 12, 2021

yerke mentioned this pull request Jan 31, 2022

Print executable name on cargo test --no-run #2 #10346

Merged

bors added a commit that referenced this pull request Feb 20, 2022

Auto merge of #10346 - yerke:yerke/no-run-test-executable-path, r=ehuss

46c9b51

Print executable name on cargo test --no-run #2 Closes #9957 This is a continuation of #9959.

bors merged commit eca63cf into rust-lang:master Feb 20, 2022

ehuss added this to the 1.61.0 milestone Nov 6, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Print executable name on `cargo test --no-run`. #9959

Print executable name on `cargo test --no-run`. #9959

vrongmeal commented Oct 4, 2021

rust-highfive commented Oct 4, 2021

ehuss Oct 7, 2021

vrongmeal Oct 7, 2021

ehuss Oct 11, 2021

ehuss Nov 4, 2021

ehuss commented Oct 7, 2021

rfcbot commented Oct 7, 2021 •

edited by Eh2406

Loading

alexcrichton commented Oct 11, 2021

rfcbot commented Oct 26, 2021

ehuss Nov 4, 2021

ehuss Nov 4, 2021

rfcbot commented Nov 5, 2021

bors commented Nov 7, 2021

ehuss commented Dec 14, 2021

Print executable name on cargo test --no-run. #9959

Print executable name on cargo test --no-run. #9959

Conversation

vrongmeal commented Oct 4, 2021

rust-highfive commented Oct 4, 2021

ehuss Oct 7, 2021

Choose a reason for hiding this comment

vrongmeal Oct 7, 2021

Choose a reason for hiding this comment

ehuss Oct 11, 2021

Choose a reason for hiding this comment

ehuss Nov 4, 2021

Choose a reason for hiding this comment

ehuss commented Oct 7, 2021

rfcbot commented Oct 7, 2021 • edited by Eh2406 Loading

alexcrichton commented Oct 11, 2021

rfcbot commented Oct 26, 2021

ehuss Nov 4, 2021

Choose a reason for hiding this comment

ehuss Nov 4, 2021

Choose a reason for hiding this comment

rfcbot commented Nov 5, 2021

bors commented Nov 7, 2021

ehuss commented Dec 14, 2021

Print executable name on `cargo test --no-run`. #9959

Print executable name on `cargo test --no-run`. #9959

rfcbot commented Oct 7, 2021 •

edited by Eh2406

Loading