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

Add reasons to all ignored tests. #10929

Merged
merged 1 commit into from
Aug 3, 2022
Merged

Add reasons to all ignored tests. #10929

merged 1 commit into from
Aug 3, 2022

Conversation

ehuss
Copy link
Contributor

@ehuss ehuss commented Aug 2, 2022

This adds a reason string to all #[ignore] attributes. This will be displayed when running the test (since 1.61), which can help quickly see and identify why tests are being ignored. It looks roughly like:

test basic ... ignored, requires nightly, CARGO_RUN_BUILD_STD_TESTS must be set
test build::simple_terminal_width ... ignored, --diagnostic-width is stabilized in 1.64
test check_cfg::features_with_cargo_check ... ignored, --check-cfg is unstable
test plugins::panic_abort_plugins ... ignored, requires rustc_private

@rust-highfive
Copy link

@ehuss: no appropriate reviewer found, 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 Aug 2, 2022
Muscraft
Muscraft reviewed Aug 2, 2022
View reviewed changes
crates/cargo-test-macro/src/lib.rs
Comment on lines +78 to 83
if requires_reason && explicit_reason.is_none() {
panic!(
"#[cargo_test] with a rule also requires a reason, \
such as #[cargo_test(nightly, reason = \"needs -Z unstable-thing\")]"
);
}
Copy link
Member

Choose a reason for hiding this comment

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

Would it make sense to add ignore to #[cargo_test] so that it would require a reason?

#[cargo_test(ignore, reason = "broken, need artifact info in index")]

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It could, but it wouldn't prevent someone from adding #[ignore] and would also make it tricky to write conditional ignores, like:

#[cfg_attr(
    any(target_env = "msvc", target_vendor = "apple"),
    ignore = "--out-dir and examples are currently broken on MSVC and apple"
)]

I wouldn't want to complicate the macro too much, or to have multiple ways of ignoring (#[cargo_test(ignore)] and #[ignore]).

For the most part, these ignore reasons are just a convenience. I don't see it critical that they get printed 100% of the time.

I think in my ideal world, tests could be ignored from within the test using code instead of using attributes, which are not well suited for complex conditions.

weihanglo
weihanglo approved these changes Aug 3, 2022
View reviewed changes
Copy link
Member

@weihanglo weihanglo left a comment

Choose a reason for hiding this comment

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

Looks nice. Thanks for this enhancement!

@weihanglo
Copy link
Member

@bors r+

@bors
Copy link
Collaborator

bors commented Aug 3, 2022

📌 Commit 7eb007d has been approved by weihanglo

It is now in the queue for this repository.

@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 Aug 3, 2022
@bors
Copy link
Collaborator

bors commented Aug 3, 2022

⌛ Testing commit 7eb007d with merge 333478d...

@bors
Copy link
Collaborator

bors commented Aug 3, 2022

☀️ Test successful - checks-actions
Approved by: weihanglo
Pushing 333478d to master...

@bors bors merged commit 333478d into rust-lang:master Aug 3, 2022
@bors bors mentioned this pull request Aug 3, 2022
Merged
@ehuss ehuss mentioned this pull request Aug 3, 2022
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request Aug 3, 2022
@bors
Auto merge of rust-lang#100065 - ehuss:update-cargo, r=ehuss
59d99f5 
Update cargo, rls

14 commits in 85b500ccad8cd0b63995fd94a03ddd4b83f7905b..4fd148c47e733770c537efac5220744945d572ef
2022-07-24 21:10:46 +0000 to 2022-08-03 15:03:52 +0000
- Revert "Drop check for mingw32-make." (rust-lang/cargo#10934)
- Add reasons to all ignored tests. (rust-lang/cargo#10929)
- Grammar fixup unused patch message (rust-lang/cargo#10933)
- Always allow hg to be missing on CI. (rust-lang/cargo#10931)
- Fix formats_source test requiring rustfmt. (rust-lang/cargo#10918)
- Disable scrape_examples_complex_reverse_dependencies (rust-lang/cargo#10921)
- Contrib: Add docs on the rustbot ready command (rust-lang/cargo#10916)
- Support for negative --jobs parameter, counting backwards from max CPUs (rust-lang/cargo#10844)
- Add requirements to cargo_test. (rust-lang/cargo#9892)
- Contrib: Document submodule update process (rust-lang/cargo#10913)
- Contrib: Add docs on how to use crater (rust-lang/cargo#10912)
- Contrib: Document new-release process (rust-lang/cargo#10914)
- Override to resolver=1 in published package (rust-lang/cargo#10911)
- fix(add): Update the lock file (rust-lang/cargo#10902)

1 commits in fcf1f94c9ab2acc18cfd4368a4aeb38e77da9649..4d8b0a19986a4daab37287a5b5fe2da0775d1873
2022-07-14 17:19:11 +0200 to 2022-08-02 22:34:34 -0400
- Update cargo (rust-lang/rls#1782)
@ehuss ehuss added this to the 1.64.0 milestone Aug 8, 2022
RalfJung pushed a commit to RalfJung/rust-analyzer that referenced this pull request Apr 20, 2024
@bors
Auto merge of #100065 - ehuss:update-cargo, r=ehuss
d6f30b7 
Update cargo, rls

14 commits in 85b500ccad8cd0b63995fd94a03ddd4b83f7905b..4fd148c47e733770c537efac5220744945d572ef
2022-07-24 21:10:46 +0000 to 2022-08-03 15:03:52 +0000
- Revert "Drop check for mingw32-make." (rust-lang/cargo#10934)
- Add reasons to all ignored tests. (rust-lang/cargo#10929)
- Grammar fixup unused patch message (rust-lang/cargo#10933)
- Always allow hg to be missing on CI. (rust-lang/cargo#10931)
- Fix formats_source test requiring rustfmt. (rust-lang/cargo#10918)
- Disable scrape_examples_complex_reverse_dependencies (rust-lang/cargo#10921)
- Contrib: Add docs on the rustbot ready command (rust-lang/cargo#10916)
- Support for negative --jobs parameter, counting backwards from max CPUs (rust-lang/cargo#10844)
- Add requirements to cargo_test. (rust-lang/cargo#9892)
- Contrib: Document submodule update process (rust-lang/cargo#10913)
- Contrib: Add docs on how to use crater (rust-lang/cargo#10912)
- Contrib: Document new-release process (rust-lang/cargo#10914)
- Override to resolver=1 in published package (rust-lang/cargo#10911)
- fix(add): Update the lock file (rust-lang/cargo#10902)

1 commits in fcf1f94c9ab2acc18cfd4368a4aeb38e77da9649..4d8b0a19986a4daab37287a5b5fe2da0775d1873
2022-07-14 17:19:11 +0200 to 2022-08-02 22:34:34 -0400
- Update cargo (rust-lang/rls#1782)
RalfJung pushed a commit to RalfJung/rust-analyzer that referenced this pull request Apr 27, 2024
@bors
Auto merge of #100065 - ehuss:update-cargo, r=ehuss
4c00837 
Update cargo, rls

14 commits in 85b500ccad8cd0b63995fd94a03ddd4b83f7905b..4fd148c47e733770c537efac5220744945d572ef
2022-07-24 21:10:46 +0000 to 2022-08-03 15:03:52 +0000
- Revert "Drop check for mingw32-make." (rust-lang/cargo#10934)
- Add reasons to all ignored tests. (rust-lang/cargo#10929)
- Grammar fixup unused patch message (rust-lang/cargo#10933)
- Always allow hg to be missing on CI. (rust-lang/cargo#10931)
- Fix formats_source test requiring rustfmt. (rust-lang/cargo#10918)
- Disable scrape_examples_complex_reverse_dependencies (rust-lang/cargo#10921)
- Contrib: Add docs on the rustbot ready command (rust-lang/cargo#10916)
- Support for negative --jobs parameter, counting backwards from max CPUs (rust-lang/cargo#10844)
- Add requirements to cargo_test. (rust-lang/cargo#9892)
- Contrib: Document submodule update process (rust-lang/cargo#10913)
- Contrib: Add docs on how to use crater (rust-lang/cargo#10912)
- Contrib: Document new-release process (rust-lang/cargo#10914)
- Override to resolver=1 in published package (rust-lang/cargo#10911)
- fix(add): Update the lock file (rust-lang/cargo#10902)

1 commits in fcf1f94c9ab2acc18cfd4368a4aeb38e77da9649..4d8b0a19986a4daab37287a5b5fe2da0775d1873
2022-07-14 17:19:11 +0200 to 2022-08-02 22:34:34 -0400
- Update cargo (rust-lang/rls#1782)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Muscraft Muscraft Muscraft left review comments

@weihanglo weihanglo weihanglo approved these changes

Assignees
No one assigned
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Milestone
1.64.0
Development

Successfully merging this pull request may close these issues.
5 participants
@ehuss @rust-highfive @weihanglo @bors @Muscraft