Skip to content

Commit

Permalink
Auto merge of #10929 - ehuss:ignore-reason, r=weihanglo
Browse files Browse the repository at this point in the history
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
```
  • Loading branch information
bors committed Aug 3, 2022
2 parents 9803862 + 7eb007d commit 333478d
Show file tree
Hide file tree
Showing 10 changed files with 73 additions and 30 deletions.
62 changes: 48 additions & 14 deletions crates/cargo-test-macro/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,17 @@ pub fn cargo_test(attr: TokenStream, item: TokenStream) -> TokenStream {
// but they don't really handle the absence of files well.
let mut ignore = false;
let mut requires_reason = false;
let mut found_reason = false;
let mut explicit_reason = None;
let mut implicit_reasons = Vec::new();
macro_rules! set_ignore {
($predicate:expr, $($arg:tt)*) => {
let p = $predicate;
ignore |= p;
if p {
implicit_reasons.push(std::fmt::format(format_args!($($arg)*)));
}
};
}
let is_not_nightly = !version().1;
for rule in split_rules(attr) {
match rule.as_str() {
Expand All @@ -29,57 +39,81 @@ pub fn cargo_test(attr: TokenStream, item: TokenStream) -> TokenStream {
// explicit opt-in (these generally only work on linux, and
// have some extra requirements, and are slow, and can pollute
// the environment since it downloads dependencies).
ignore |= is_not_nightly;
ignore |= option_env!("CARGO_RUN_BUILD_STD_TESTS").is_none();
set_ignore!(is_not_nightly, "requires nightly");
set_ignore!(
option_env!("CARGO_RUN_BUILD_STD_TESTS").is_none(),
"CARGO_RUN_BUILD_STD_TESTS must be set"
);
}
"build_std_mock" => {
// Only run the "mock" build-std tests on nightly and disable
// for windows-gnu which is missing object files (see
// https://github.com/rust-lang/wg-cargo-std-aware/issues/46).
ignore |= is_not_nightly;
ignore |= cfg!(all(target_os = "windows", target_env = "gnu"));
set_ignore!(is_not_nightly, "requires nightly");
set_ignore!(
cfg!(all(target_os = "windows", target_env = "gnu")),
"does not work on windows-gnu"
);
}
"nightly" => {
requires_reason = true;
ignore |= is_not_nightly;
set_ignore!(is_not_nightly, "requires nightly");
}
s if s.starts_with("requires_") => {
let command = &s[9..];
ignore |= !has_command(command);
set_ignore!(!has_command(command), "{command} not installed");
}
s if s.starts_with(">=1.") => {
requires_reason = true;
let min_minor = s[4..].parse().unwrap();
ignore |= version().0 < min_minor;
let minor = version().0;
set_ignore!(minor < min_minor, "requires rustc 1.{minor} or newer");
}
s if s.starts_with("reason=") => {
found_reason = true;
explicit_reason = Some(s[7..].parse().unwrap());
}
_ => panic!("unknown rule {:?}", rule),
}
}
if requires_reason && !found_reason {
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\")]"
);
}

// Construct the appropriate attributes.
let span = Span::call_site();
let mut ret = TokenStream::new();
let add_attr = |ret: &mut TokenStream, attr_name| {
let add_attr = |ret: &mut TokenStream, attr_name, attr_input| {
ret.extend(Some(TokenTree::from(Punct::new('#', Spacing::Alone))));
let attr = TokenTree::from(Ident::new(attr_name, span));
let mut attr_stream: TokenStream = attr.into();
if let Some(input) = attr_input {
attr_stream.extend(input);
}
ret.extend(Some(TokenTree::from(Group::new(
Delimiter::Bracket,
attr.into(),
attr_stream,
))));
};
add_attr(&mut ret, "test");
add_attr(&mut ret, "test", None);
if ignore {
add_attr(&mut ret, "ignore");
let reason = explicit_reason
.or_else(|| {
(!implicit_reasons.is_empty())
.then(|| TokenTree::from(Literal::string(&implicit_reasons.join(", "))).into())
})
.map(|reason: TokenStream| {
let mut stream = TokenStream::new();
stream.extend(Some(TokenTree::from(Punct::new('=', Spacing::Alone))));
stream.extend(Some(reason));
stream
});
add_attr(&mut ret, "ignore", reason);
}

// Find where the function body starts, and add the boilerplate at the start.
for token in item {
let group = match token {
TokenTree::Group(g) => {
Expand Down
2 changes: 1 addition & 1 deletion tests/testsuite/advanced_env.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ use cargo_test_support::{paths, project, registry::Package};
#[cargo_test]
// I don't know why, but `Command` forces all env keys to be upper case on
// Windows. Seems questionable, since I think Windows is case-preserving.
#[cfg_attr(windows, ignore)]
#[cfg_attr(windows, ignore = "broken due to not preserving case on Windows")]
fn source_config_env() {
// Try to define [source] with environment variables.
let p = project()
Expand Down
5 changes: 3 additions & 2 deletions tests/testsuite/artifact_dep.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1331,7 +1331,8 @@ fn build_script_deps_adopts_target_platform_if_target_equals_target() {
}

#[cargo_test]
#[cfg_attr(target_env = "msvc", ignore)] // TODO(ST): rename bar (dependency) to something else and un-ignore this with RFC-3176
// TODO(ST): rename bar (dependency) to something else and un-ignore this with RFC-3176
#[cfg_attr(target_env = "msvc", ignore = "msvc not working")]
fn profile_override_basic() {
let p = project()
.file(
Expand Down Expand Up @@ -1450,7 +1451,7 @@ foo v0.0.0 ([CWD])
// For reference, see comments by ehuss https://github.com/rust-lang/cargo/pull/9992#discussion_r801086315 and
// joshtriplett https://github.com/rust-lang/cargo/pull/9992#issuecomment-1033394197 .
#[cargo_test]
#[ignore]
#[ignore = "broken, need artifact info in index"]
fn targets_are_picked_up_from_non_workspace_artifact_deps() {
if cross_compile::disabled() {
return;
Expand Down
6 changes: 4 additions & 2 deletions tests/testsuite/build_script.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4236,8 +4236,10 @@ fn rename_with_link_search_path() {
}

#[cargo_test]
// Don't have a cdylib cross target on macos.
#[cfg_attr(target_os = "macos", ignore)]
#[cfg_attr(
target_os = "macos",
ignore = "don't have a cdylib cross target on macos"
)]
fn rename_with_link_search_path_cross() {
if cross_compile::disabled() {
return;
Expand Down
6 changes: 4 additions & 2 deletions tests/testsuite/collisions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -91,9 +91,11 @@ This may become a hard error in the future; see <https://github.com/rust-lang/ca
}

#[cargo_test]
// --out-dir and examples are currently broken on MSVC and apple.
// See https://github.com/rust-lang/cargo/issues/7493
#[cfg_attr(any(target_env = "msvc", target_vendor = "apple"), ignore)]
#[cfg_attr(
any(target_env = "msvc", target_vendor = "apple"),
ignore = "--out-dir and examples are currently broken on MSVC and apple"
)]
fn collision_export() {
// `--out-dir` combines some things which can cause conflicts.
let p = project()
Expand Down
6 changes: 4 additions & 2 deletions tests/testsuite/cross_compile.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1206,8 +1206,10 @@ fn platform_specific_variables_reflected_in_build_scripts() {
}

#[cargo_test]
// Don't have a dylib cross target on macos.
#[cfg_attr(target_os = "macos", ignore)]
#[cfg_attr(
target_os = "macos",
ignore = "don't have a dylib cross target on macos"
)]
fn cross_test_dylib() {
if cross_compile::disabled() {
return;
Expand Down
2 changes: 1 addition & 1 deletion tests/testsuite/doc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2424,7 +2424,7 @@ fn scrape_examples_avoid_build_script_cycle() {
// The example is calling a function from a proc-macro, but proc-macros don't
// export functions. It is not clear what this test is trying to exercise.
// #[cargo_test(nightly, reason = "rustdoc scrape examples flags are unstable")]
#[ignore]
#[ignore = "broken, needs fixing"]
#[cargo_test]
fn scrape_examples_complex_reverse_dependencies() {
let p = project()
Expand Down
6 changes: 3 additions & 3 deletions tests/testsuite/old_cargos.rs
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,7 @@ fn default_toolchain_is_stable() -> bool {
// The optional dependency `new-baz-dep` should not be activated.
// * `bar` 1.0.2 has a dependency on `baz` that *requires* the new feature
// syntax.
#[ignore]
#[ignore = "must be run manually, requires old cargo installations"]
#[cargo_test]
fn new_features() {
let registry = registry::init();
Expand Down Expand Up @@ -534,7 +534,7 @@ fn new_features() {
}

#[cargo_test]
#[ignore]
#[ignore = "must be run manually, requires old cargo installations"]
fn index_cache_rebuild() {
// Checks that the index cache gets rebuilt.
//
Expand Down Expand Up @@ -618,7 +618,7 @@ foo v0.1.0 [..]
}

#[cargo_test]
#[ignore]
#[ignore = "must be run manually, requires old cargo installations"]
fn avoids_split_debuginfo_collision() {
// Test needs two different toolchains.
// If the default toolchain is stable, then it won't work.
Expand Down
5 changes: 4 additions & 1 deletion tests/testsuite/profile_custom.rs
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,10 @@ Caused by:
}

#[cargo_test]
#[ignore] // dir-name is currently disabled
// We are currently uncertain if dir-name will ever be exposed to the user.
// The code for it still roughly exists, but only for the internal profiles.
// This test was kept in case we ever want to enable support for it again.
#[ignore = "dir-name is disabled"]
fn invalid_dir_name() {
let p = project()
.file(
Expand Down
3 changes: 1 addition & 2 deletions tests/testsuite/rustdoc_extern_html.rs
Original file line number Diff line number Diff line change
Expand Up @@ -55,8 +55,7 @@ fn simple() {
assert!(myfun.contains(r#"href="https://docs.rs/bar/1.0.0/bar/struct.Straw.html""#));
}

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

0 comments on commit 333478d

Please sign in to comment.