Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Make it more clear which module is being tested when running cargo test #9195

Merged
merged 2 commits into from
Feb 24, 2021
Merged

Make it more clear which module is being tested when running cargo test #9195

merged 2 commits into from
Feb 24, 2021

Conversation

guswynn
Copy link
Contributor

@guswynn guswynn commented Feb 21, 2021 •
edited
Loading

I recently asked in zulip if this is a good idea, as I find it hard to find the module thats being tested from the complex path with the hash.

Output of cargo test:

2021-02-21 13:29:33 I > ~/repos/cargo/target/debug/cargo test
    Finished test [unoptimized + debuginfo] target(s) in 0.42s
     Running unittests (target/debug/deps/test_tests-759130ea61f71571)

running 1 test
test tests::unit_test ... ok

test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in
0.00s

     Running tests2/lib.rs (target/debug/deps/integration_tests-6b7f9fcd1721f083)

running 2 tests
test tests2_lib ... ok
test second_test::tests2_second ... ok

test result: ok. 2 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in
0.00s

     Running tests/lib.rs (target/debug/deps/lib-d2be6d29597c2790)

running 2 tests
test second_test::tests_i_am_run_twice ... ok
test tests_lib ... ok

test result: ok. 2 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in
0.00s
...

@rust-highfive
Copy link

r? @alexcrichton

(rust-highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Feb 21, 2021
guswynn
guswynn commented Feb 21, 2021
View reviewed changes
src/cargo/ops/cargo_test.rs
test_path
.strip_prefix(unit.pkg.root())
.unwrap_or(&test_path)
.display(),
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this returns an opaque struct that implements Display, which I think makes it harder to collapse these 2 format!'s into 1 unless I to_string() this call and the "unittests" &has str, let me know what people would prefer

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nah this seems fine I think, no need to worry too much about perf here!

@alexcrichton alexcrichton added the T-cargo Team: Cargo label Feb 23, 2021
@alexcrichton
Copy link
Member

@rfcbot fcp merge

Thanks for the PR! This looks good to me and I think it's pretty reasonable!

I've cc'd other folks on the Cargo team to get their thoughts too since this is changing the output of Cargo by default.

@rfcbot
Copy link
Collaborator

rfcbot commented Feb 23, 2021 •
edited by ehuss
Loading

Team member @alexcrichton 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.

@rfcbot rfcbot added proposed-final-comment-period An FCP proposal has started, but not yet signed off. disposition-merge FCP with intent to merge labels Feb 23, 2021
@joshtriplett
Copy link
Member

This looks great, thank you! I'm glad to see test output being made more helpful.

@rfcbot reviewed

@rfcbot rfcbot added final-comment-period FCP — a period for last comments before action is taken and removed proposed-final-comment-period An FCP proposal has started, but not yet signed off. labels Feb 23, 2021
@rfcbot
Copy link
Collaborator

rfcbot commented Feb 23, 2021

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

@ehuss
Copy link
Contributor

ehuss commented Feb 24, 2021

@bors r+

@bors
Copy link
Contributor

bors commented Feb 24, 2021

📌 Commit 04c8372 has been approved by ehuss

@bors 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 Feb 24, 2021
@bors
Copy link
Contributor

bors commented Feb 24, 2021

⌛ Testing commit 04c8372 with merge 9a7fe2c...

@bors
Copy link
Contributor

bors commented Feb 24, 2021

☀️ Test successful - checks-actions
Approved by: ehuss
Pushing 9a7fe2c to master...

@bors bors merged commit 9a7fe2c into rust-lang:master Feb 24, 2021
@ehuss ehuss mentioned this pull request Feb 24, 2021
Merged
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this pull request Feb 27, 2021
@Dylan-DPC
Rollup merge of rust-lang#82490 - ehuss:update-cargo, r=ehuss
a1f7540 
Update cargo

11 commits in bf5a5d5e5d3ae842a63bfce6d070dfd438cf6070..572e201536dc2e4920346e28037b63c0f4d88b3c
2021-02-18 15:49:14 +0000 to 2021-02-24 16:51:20 +0000
- Pass the error message format to rustdoc (rust-lang/cargo#9128)
- Fix test target_in_environment_contains_lower_case (rust-lang/cargo#9203)
- Fix hang on broken stderr. (rust-lang/cargo#9201)
- Make it more clear which module is being tested when running cargo test (rust-lang/cargo#9195)
- Updates to edition handling. (rust-lang/cargo#9184)
- Add --cfg and --rustc-cfg flags to output compiler configuration (rust-lang/cargo#9002)
- Run rustdoc doctests relative to the workspace (rust-lang/cargo#9105)
- Add support for [env] section in .cargo/config.toml (rust-lang/cargo#9175)
- Add schema field and `features2` to the index. (rust-lang/cargo#9161)
- Document the default location where cargo install emitting build artifacts (rust-lang/cargo#9189)
- Do not exit prematurely if anything failed installing. (rust-lang/cargo#9185)
@rfcbot 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 Mar 5, 2021
@ehuss ehuss added this to the 1.52.0 milestone Feb 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@alexcrichton alexcrichton alexcrichton left review comments

Assignees

@alexcrichton alexcrichton

Labels
disposition-merge FCP with intent to merge finished-final-comment-period FCP complete S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-cargo Team: Cargo to-announce
Projects
None yet
Milestone
1.52.0
Development

Successfully merging this pull request may close these issues.
7 participants
@guswynn @rust-highfive @alexcrichton @rfcbot @joshtriplett @ehuss @bors