Skip to content

Commit 333478d

Browse files
committed
Auto merge of rust-lang#10929 - ehuss:ignore-reason, r=weihanglo
Add reasons to all ignored tests. 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 ```
2 parents 9803862 + 7eb007d commit 333478d

10 files changed

+73
-30
lines changed

crates/cargo-test-macro/src/lib.rs

+48-14
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,17 @@ pub fn cargo_test(attr: TokenStream, item: TokenStream) -> TokenStream {
2020
// but they don't really handle the absence of files well.
2121
let mut ignore = false;
2222
let mut requires_reason = false;
23-
let mut found_reason = false;
23+
let mut explicit_reason = None;
24+
let mut implicit_reasons = Vec::new();
25+
macro_rules! set_ignore {
26+
($predicate:expr, $($arg:tt)*) => {
27+
let p = $predicate;
28+
ignore |= p;
29+
if p {
30+
implicit_reasons.push(std::fmt::format(format_args!($($arg)*)));
31+
}
32+
};
33+
}
2434
let is_not_nightly = !version().1;
2535
for rule in split_rules(attr) {
2636
match rule.as_str() {
@@ -29,57 +39,81 @@ pub fn cargo_test(attr: TokenStream, item: TokenStream) -> TokenStream {
2939
// explicit opt-in (these generally only work on linux, and
3040
// have some extra requirements, and are slow, and can pollute
3141
// the environment since it downloads dependencies).
32-
ignore |= is_not_nightly;
33-
ignore |= option_env!("CARGO_RUN_BUILD_STD_TESTS").is_none();
42+
set_ignore!(is_not_nightly, "requires nightly");
43+
set_ignore!(
44+
option_env!("CARGO_RUN_BUILD_STD_TESTS").is_none(),
45+
"CARGO_RUN_BUILD_STD_TESTS must be set"
46+
);
3447
}
3548
"build_std_mock" => {
3649
// Only run the "mock" build-std tests on nightly and disable
3750
// for windows-gnu which is missing object files (see
3851
// https://github.com/rust-lang/wg-cargo-std-aware/issues/46).
39-
ignore |= is_not_nightly;
40-
ignore |= cfg!(all(target_os = "windows", target_env = "gnu"));
52+
set_ignore!(is_not_nightly, "requires nightly");
53+
set_ignore!(
54+
cfg!(all(target_os = "windows", target_env = "gnu")),
55+
"does not work on windows-gnu"
56+
);
4157
}
4258
"nightly" => {
4359
requires_reason = true;
44-
ignore |= is_not_nightly;
60+
set_ignore!(is_not_nightly, "requires nightly");
4561
}
4662
s if s.starts_with("requires_") => {
4763
let command = &s[9..];
48-
ignore |= !has_command(command);
64+
set_ignore!(!has_command(command), "{command} not installed");
4965
}
5066
s if s.starts_with(">=1.") => {
5167
requires_reason = true;
5268
let min_minor = s[4..].parse().unwrap();
53-
ignore |= version().0 < min_minor;
69+
let minor = version().0;
70+
set_ignore!(minor < min_minor, "requires rustc 1.{minor} or newer");
5471
}
5572
s if s.starts_with("reason=") => {
56-
found_reason = true;
73+
explicit_reason = Some(s[7..].parse().unwrap());
5774
}
5875
_ => panic!("unknown rule {:?}", rule),
5976
}
6077
}
61-
if requires_reason && !found_reason {
78+
if requires_reason && explicit_reason.is_none() {
6279
panic!(
6380
"#[cargo_test] with a rule also requires a reason, \
6481
such as #[cargo_test(nightly, reason = \"needs -Z unstable-thing\")]"
6582
);
6683
}
6784

85+
// Construct the appropriate attributes.
6886
let span = Span::call_site();
6987
let mut ret = TokenStream::new();
70-
let add_attr = |ret: &mut TokenStream, attr_name| {
88+
let add_attr = |ret: &mut TokenStream, attr_name, attr_input| {
7189
ret.extend(Some(TokenTree::from(Punct::new('#', Spacing::Alone))));
7290
let attr = TokenTree::from(Ident::new(attr_name, span));
91+
let mut attr_stream: TokenStream = attr.into();
92+
if let Some(input) = attr_input {
93+
attr_stream.extend(input);
94+
}
7395
ret.extend(Some(TokenTree::from(Group::new(
7496
Delimiter::Bracket,
75-
attr.into(),
97+
attr_stream,
7698
))));
7799
};
78-
add_attr(&mut ret, "test");
100+
add_attr(&mut ret, "test", None);
79101
if ignore {
80-
add_attr(&mut ret, "ignore");
102+
let reason = explicit_reason
103+
.or_else(|| {
104+
(!implicit_reasons.is_empty())
105+
.then(|| TokenTree::from(Literal::string(&implicit_reasons.join(", "))).into())
106+
})
107+
.map(|reason: TokenStream| {
108+
let mut stream = TokenStream::new();
109+
stream.extend(Some(TokenTree::from(Punct::new('=', Spacing::Alone))));
110+
stream.extend(Some(reason));
111+
stream
112+
});
113+
add_attr(&mut ret, "ignore", reason);
81114
}
82115

116+
// Find where the function body starts, and add the boilerplate at the start.
83117
for token in item {
84118
let group = match token {
85119
TokenTree::Group(g) => {

tests/testsuite/advanced_env.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ use cargo_test_support::{paths, project, registry::Package};
55
#[cargo_test]
66
// I don't know why, but `Command` forces all env keys to be upper case on
77
// Windows. Seems questionable, since I think Windows is case-preserving.
8-
#[cfg_attr(windows, ignore)]
8+
#[cfg_attr(windows, ignore = "broken due to not preserving case on Windows")]
99
fn source_config_env() {
1010
// Try to define [source] with environment variables.
1111
let p = project()

tests/testsuite/artifact_dep.rs

+3-2
Original file line numberDiff line numberDiff line change
@@ -1331,7 +1331,8 @@ fn build_script_deps_adopts_target_platform_if_target_equals_target() {
13311331
}
13321332

13331333
#[cargo_test]
1334-
#[cfg_attr(target_env = "msvc", ignore)] // TODO(ST): rename bar (dependency) to something else and un-ignore this with RFC-3176
1334+
// TODO(ST): rename bar (dependency) to something else and un-ignore this with RFC-3176
1335+
#[cfg_attr(target_env = "msvc", ignore = "msvc not working")]
13351336
fn profile_override_basic() {
13361337
let p = project()
13371338
.file(
@@ -1450,7 +1451,7 @@ foo v0.0.0 ([CWD])
14501451
// For reference, see comments by ehuss https://github.com/rust-lang/cargo/pull/9992#discussion_r801086315 and
14511452
// joshtriplett https://github.com/rust-lang/cargo/pull/9992#issuecomment-1033394197 .
14521453
#[cargo_test]
1453-
#[ignore]
1454+
#[ignore = "broken, need artifact info in index"]
14541455
fn targets_are_picked_up_from_non_workspace_artifact_deps() {
14551456
if cross_compile::disabled() {
14561457
return;

tests/testsuite/build_script.rs

+4-2
Original file line numberDiff line numberDiff line change
@@ -4236,8 +4236,10 @@ fn rename_with_link_search_path() {
42364236
}
42374237

42384238
#[cargo_test]
4239-
// Don't have a cdylib cross target on macos.
4240-
#[cfg_attr(target_os = "macos", ignore)]
4239+
#[cfg_attr(
4240+
target_os = "macos",
4241+
ignore = "don't have a cdylib cross target on macos"
4242+
)]
42414243
fn rename_with_link_search_path_cross() {
42424244
if cross_compile::disabled() {
42434245
return;

tests/testsuite/collisions.rs

+4-2
Original file line numberDiff line numberDiff line change
@@ -91,9 +91,11 @@ This may become a hard error in the future; see <https://github.com/rust-lang/ca
9191
}
9292

9393
#[cargo_test]
94-
// --out-dir and examples are currently broken on MSVC and apple.
9594
// See https://github.com/rust-lang/cargo/issues/7493
96-
#[cfg_attr(any(target_env = "msvc", target_vendor = "apple"), ignore)]
95+
#[cfg_attr(
96+
any(target_env = "msvc", target_vendor = "apple"),
97+
ignore = "--out-dir and examples are currently broken on MSVC and apple"
98+
)]
9799
fn collision_export() {
98100
// `--out-dir` combines some things which can cause conflicts.
99101
let p = project()

tests/testsuite/cross_compile.rs

+4-2
Original file line numberDiff line numberDiff line change
@@ -1206,8 +1206,10 @@ fn platform_specific_variables_reflected_in_build_scripts() {
12061206
}
12071207

12081208
#[cargo_test]
1209-
// Don't have a dylib cross target on macos.
1210-
#[cfg_attr(target_os = "macos", ignore)]
1209+
#[cfg_attr(
1210+
target_os = "macos",
1211+
ignore = "don't have a dylib cross target on macos"
1212+
)]
12111213
fn cross_test_dylib() {
12121214
if cross_compile::disabled() {
12131215
return;

tests/testsuite/doc.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -2424,7 +2424,7 @@ fn scrape_examples_avoid_build_script_cycle() {
24242424
// The example is calling a function from a proc-macro, but proc-macros don't
24252425
// export functions. It is not clear what this test is trying to exercise.
24262426
// #[cargo_test(nightly, reason = "rustdoc scrape examples flags are unstable")]
2427-
#[ignore]
2427+
#[ignore = "broken, needs fixing"]
24282428
#[cargo_test]
24292429
fn scrape_examples_complex_reverse_dependencies() {
24302430
let p = project()

tests/testsuite/old_cargos.rs

+3-3
Original file line numberDiff line numberDiff line change
@@ -110,7 +110,7 @@ fn default_toolchain_is_stable() -> bool {
110110
// The optional dependency `new-baz-dep` should not be activated.
111111
// * `bar` 1.0.2 has a dependency on `baz` that *requires* the new feature
112112
// syntax.
113-
#[ignore]
113+
#[ignore = "must be run manually, requires old cargo installations"]
114114
#[cargo_test]
115115
fn new_features() {
116116
let registry = registry::init();
@@ -534,7 +534,7 @@ fn new_features() {
534534
}
535535

536536
#[cargo_test]
537-
#[ignore]
537+
#[ignore = "must be run manually, requires old cargo installations"]
538538
fn index_cache_rebuild() {
539539
// Checks that the index cache gets rebuilt.
540540
//
@@ -618,7 +618,7 @@ foo v0.1.0 [..]
618618
}
619619

620620
#[cargo_test]
621-
#[ignore]
621+
#[ignore = "must be run manually, requires old cargo installations"]
622622
fn avoids_split_debuginfo_collision() {
623623
// Test needs two different toolchains.
624624
// If the default toolchain is stable, then it won't work.

tests/testsuite/profile_custom.rs

+4-1
Original file line numberDiff line numberDiff line change
@@ -94,7 +94,10 @@ Caused by:
9494
}
9595

9696
#[cargo_test]
97-
#[ignore] // dir-name is currently disabled
97+
// We are currently uncertain if dir-name will ever be exposed to the user.
98+
// The code for it still roughly exists, but only for the internal profiles.
99+
// This test was kept in case we ever want to enable support for it again.
100+
#[ignore = "dir-name is disabled"]
98101
fn invalid_dir_name() {
99102
let p = project()
100103
.file(

tests/testsuite/rustdoc_extern_html.rs

+1-2
Original file line numberDiff line numberDiff line change
@@ -55,8 +55,7 @@ fn simple() {
5555
assert!(myfun.contains(r#"href="https://docs.rs/bar/1.0.0/bar/struct.Straw.html""#));
5656
}
5757

58-
// Broken, temporarily disable until https://github.com/rust-lang/rust/pull/82776 is resolved.
59-
#[ignore]
58+
#[ignore = "Broken, temporarily disabled until https://github.com/rust-lang/rust/pull/82776 is resolved."]
6059
#[cargo_test]
6160
// #[cargo_test(nightly, reason = "--extern-html-root-url is unstable")]
6261
fn std_docs() {

0 commit comments

Comments
 (0)