diff --git a/src/tools/compiletest/src/cli.rs b/src/tools/compiletest/src/cli.rs index 8a69360575d7a..1325afc2adaa3 100644 --- a/src/tools/compiletest/src/cli.rs +++ b/src/tools/compiletest/src/cli.rs @@ -8,9 +8,7 @@ use std::sync::{Arc, OnceLock}; use camino::{Utf8Path, Utf8PathBuf}; use clap::Parser; -use crate::common::{ - CodegenBackend, CompareMode, Config, Debugger, ForcePassMode, TestMode, TestSuite, -}; +use crate::common::{CodegenBackend, CompareMode, Config, ForcePassMode, TestMode, TestSuite}; use crate::edition::Edition; use crate::{debuggers, directives, early_config_check, run_tests}; @@ -200,9 +198,6 @@ struct Args { /// Default Rust edition. #[arg(long)] edition: Option, - /// Only test a specific debugger in debuginfo tests. - #[arg(long)] - debugger: Option, /// The codegen backend currently used. #[arg(long)] default_codegen_backend: Option, @@ -422,11 +417,6 @@ pub(crate) fn parse_config(args: Vec) -> Config { mode, suite: args.suite, - debugger: args.debugger.map(|debugger| { - debugger - .parse::() - .unwrap_or_else(|_| panic!("unknown `--debugger` option `{debugger}` given")) - }), run_ignored: args.ignored, with_rustc_debug_assertions: args.with_rustc_debug_assertions, with_std_debug_assertions: args.with_std_debug_assertions, diff --git a/src/tools/compiletest/src/common.rs b/src/tools/compiletest/src/common.rs index 755f6d1446366..21bfa8e40e9e1 100644 --- a/src/tools/compiletest/src/common.rs +++ b/src/tools/compiletest/src/common.rs @@ -10,6 +10,7 @@ use camino::{Utf8Path, Utf8PathBuf}; use semver::Version; use crate::edition::Edition; +use crate::executor::TestVariant; use crate::fatal; use crate::util::{Utf8PathBufExt, add_dylib_path, string_enum}; @@ -452,18 +453,6 @@ pub(crate) struct Config { /// [`TestMode::CoverageMap`]. pub(crate) suite: TestSuite, - /// When specified, **only** the specified [`Debugger`] will be used to run against the - /// `tests/debuginfo` test suite. When unspecified, `compiletest` will attempt to find all three - /// of {`lldb`, `cdb`, `gdb`} implicitly, and then try to run the `debuginfo` test suite against - /// all three debuggers. - /// - /// FIXME: this implicit behavior is really nasty, in that it makes it hard for the user to - /// control *which* debugger(s) are available and used to run the debuginfo test suite. We - /// should have `bootstrap` allow the user to *explicitly* configure the debuggers, and *not* - /// try to implicitly discover some random debugger from the user environment. This makes the - /// debuginfo test suite particularly hard to work with. - pub(crate) debugger: Option, - /// Run ignored tests *unconditionally*, overriding their ignore reason. /// /// FIXME: this is wired up through the test execution logic, but **not** accessible from @@ -1306,13 +1295,13 @@ pub(crate) fn output_relative_path(config: &Config, relative_dir: &Utf8Path) -> pub(crate) fn output_testname_unique( config: &Config, testpaths: &TestPaths, - revision: Option<&str>, + variant: &TestVariant, ) -> Utf8PathBuf { let mode = config.compare_mode.as_ref().map_or("", |m| m.to_str()); - let debugger = config.debugger.as_ref().map_or("", |m| m.to_str()); + let debugger = variant.debugger.as_ref().map_or("", |m| m.to_str()); Utf8PathBuf::from(&testpaths.file.file_stem().unwrap()) .with_extra_extension(config.mode.output_dir_disambiguator()) - .with_extra_extension(revision.unwrap_or("")) + .with_extra_extension(variant.revision().unwrap_or("")) .with_extra_extension(mode) .with_extra_extension(debugger) } @@ -1323,10 +1312,10 @@ pub(crate) fn output_testname_unique( pub(crate) fn output_base_dir( config: &Config, testpaths: &TestPaths, - revision: Option<&str>, + variant: &TestVariant, ) -> Utf8PathBuf { output_relative_path(config, &testpaths.relative_dir) - .join(output_testname_unique(config, testpaths, revision)) + .join(output_testname_unique(config, testpaths, variant)) } /// Absolute path to the base filename used as output for the given @@ -1335,9 +1324,9 @@ pub(crate) fn output_base_dir( pub(crate) fn output_base_name( config: &Config, testpaths: &TestPaths, - revision: Option<&str>, + variant: &TestVariant, ) -> Utf8PathBuf { - output_base_dir(config, testpaths, revision).join(testpaths.file.file_stem().unwrap()) + output_base_dir(config, testpaths, variant).join(testpaths.file.file_stem().unwrap()) } /// Absolute path to the directory to use for incremental compilation. Example: @@ -1345,7 +1334,7 @@ pub(crate) fn output_base_name( pub(crate) fn incremental_dir( config: &Config, testpaths: &TestPaths, - revision: Option<&str>, + variant: &TestVariant, ) -> Utf8PathBuf { - output_base_name(config, testpaths, revision).with_extension("inc") + output_base_name(config, testpaths, variant).with_extension("inc") } diff --git a/src/tools/compiletest/src/directives.rs b/src/tools/compiletest/src/directives.rs index bb4a595a568c5..7bc9117f29833 100644 --- a/src/tools/compiletest/src/directives.rs +++ b/src/tools/compiletest/src/directives.rs @@ -20,7 +20,7 @@ use crate::directives::line::DirectiveLine; use crate::directives::needs::PreparedNeedsConditions; use crate::edition::{Edition, parse_edition}; use crate::errors::ErrorKind; -use crate::executor::{CollectedTestDesc, ShouldFail}; +use crate::executor::{CollectedTestDesc, ShouldFail, TestVariant}; use crate::util::static_regex; use crate::{fatal, help}; @@ -892,7 +892,7 @@ pub(crate) fn make_test_description( path: &Utf8Path, filterable_path: &Utf8Path, file_directives: &FileDirectives<'_>, - test_revision: Option<&str>, + variant: &TestVariant, poisoned: &mut bool, aux_props: &mut AuxProps, ) -> CollectedTestDesc { @@ -901,25 +901,25 @@ pub(crate) fn make_test_description( // Perform a per-file (rather than per-line) ignore decision to skip running debuginfo tests // if we don't have a debugger for them available. - // This is needed because we duplicate the Config once for each debugger. - if config.mode == TestMode::DebugInfo { - match &config.debugger { - Some(Debugger::Cdb) => { + // We do this to materialize debuginfo tests for each debugger and explicitly ignore + // the variants that are not supported in our environment. + if let Some(debugger) = variant.debugger.as_ref() { + match debugger { + Debugger::Cdb => { if let Some(msg) = check_cdb_support(config) { ignore_message = Some(Cow::Owned(msg)); } } - Some(Debugger::Gdb) => { + Debugger::Gdb => { if let Some(msg) = check_gdb_support(config) { ignore_message = Some(Cow::Owned(msg)); } } - Some(Debugger::Lldb) => { + Debugger::Lldb => { if let Some(msg) = check_lldb_support(config) { ignore_message = Some(Cow::Owned(msg)); } } - None => {} } } @@ -929,7 +929,7 @@ pub(crate) fn make_test_description( config, file_directives, &mut |ln @ &DirectiveLine { line_number, .. }| { - if !ln.applies_to_test_revision(test_revision) { + if !ln.applies_to_test_revision(variant.revision()) { return; } @@ -958,9 +958,9 @@ pub(crate) fn make_test_description( decision!(ignore_llvm(config, ln)); decision!(ignore_backends(config, ln)); decision!(needs_backends(config, ln)); - decision!(ignore_cdb(config, ln)); - decision!(ignore_gdb(config, ln)); - decision!(ignore_lldb(config, ln)); + decision!(ignore_cdb(config, variant, ln)); + decision!(ignore_gdb(config, variant, ln)); + decision!(ignore_lldb(config, variant, ln)); decision!(ignore_parallel_frontend(config, ln)); if config.target == "wasm32-unknown-unknown" @@ -1019,9 +1019,17 @@ fn check_lldb_support(config: &Config) -> Option { if config.lldb.is_none() { Some("lldb is not available".to_string()) } else { None } } -fn ignore_cdb(config: &Config, line: &DirectiveLine<'_>) -> IgnoreDecision { - if config.debugger != Some(Debugger::Cdb) { - return IgnoreDecision::Continue; +fn ignore_cdb(config: &Config, variant: &TestVariant, line: &DirectiveLine<'_>) -> IgnoreDecision { + if variant.debugger != Some(Debugger::Cdb) { + return if line.name == "only-cdb" { + IgnoreDecision::Ignore { reason: "debugger is not cdb".to_string() } + } else { + IgnoreDecision::Continue + }; + } + + if line.name == "ignore-cdb" { + return IgnoreDecision::Ignore { reason: "debugger is cdb".to_string() }; } if let Some(actual_version) = config.cdb_version { @@ -1044,9 +1052,17 @@ fn ignore_cdb(config: &Config, line: &DirectiveLine<'_>) -> IgnoreDecision { IgnoreDecision::Continue } -fn ignore_gdb(config: &Config, line: &DirectiveLine<'_>) -> IgnoreDecision { - if config.debugger != Some(Debugger::Gdb) { - return IgnoreDecision::Continue; +fn ignore_gdb(config: &Config, variant: &TestVariant, line: &DirectiveLine<'_>) -> IgnoreDecision { + if variant.debugger != Some(Debugger::Gdb) { + return if line.name == "only-gdb" { + IgnoreDecision::Ignore { reason: "debugger is not gdb".to_string() } + } else { + IgnoreDecision::Continue + }; + } + + if line.name == "ignore-gdb" { + return IgnoreDecision::Ignore { reason: "debugger is gdb".to_string() }; } if let Some(actual_version) = config.gdb_version { @@ -1096,9 +1112,17 @@ fn ignore_gdb(config: &Config, line: &DirectiveLine<'_>) -> IgnoreDecision { IgnoreDecision::Continue } -fn ignore_lldb(config: &Config, line: &DirectiveLine<'_>) -> IgnoreDecision { - if config.debugger != Some(Debugger::Lldb) { - return IgnoreDecision::Continue; +fn ignore_lldb(config: &Config, variant: &TestVariant, line: &DirectiveLine<'_>) -> IgnoreDecision { + if variant.debugger != Some(Debugger::Lldb) { + return if line.name == "only-lldb" { + IgnoreDecision::Ignore { reason: "debugger is not lldb".to_string() } + } else { + IgnoreDecision::Continue + }; + } + + if line.name == "ignore-lldb" { + return IgnoreDecision::Ignore { reason: "debugger is lldb".to_string() }; } if let Some(actual_version) = config.lldb_version { diff --git a/src/tools/compiletest/src/directives/cfg.rs b/src/tools/compiletest/src/directives/cfg.rs index 25ebb86ad28d3..b9036462f3447 100644 --- a/src/tools/compiletest/src/directives/cfg.rs +++ b/src/tools/compiletest/src/directives/cfg.rs @@ -1,7 +1,7 @@ use std::collections::{HashMap, HashSet}; use std::sync::{Arc, LazyLock}; -use crate::common::{CompareMode, Config, Debugger}; +use crate::common::{CompareMode, Config}; use crate::directives::{DirectiveLine, IgnoreDecision}; const EXTRA_ARCHS: &[&str] = &["spirv"]; @@ -9,17 +9,33 @@ const EXTRA_ARCHS: &[&str] = &["spirv"]; const EXTERNAL_IGNORES_LIST: &[&str] = &[ // tidy-alphabetical-start "ignore-backends", + "ignore-cdb", + "ignore-gdb", "ignore-gdb-version", + "ignore-lldb", "ignore-llvm-version", "ignore-parallel-frontend", // tidy-alphabetical-end ]; +const EXTERNAL_ONLY_LIST: &[&str] = &[ + // tidy-alphabetical-start + "only-cdb", + "only-gdb", + "only-lldb", + // tidy-alphabetical-end +]; + /// Directive names that begin with `ignore-`, but are disregarded by this /// module because they are handled elsewhere. pub(crate) static EXTERNAL_IGNORES_SET: LazyLock> = LazyLock::new(|| EXTERNAL_IGNORES_LIST.iter().copied().collect()); +/// Directive names that begin with `only-`, but are disregarded by this +/// module because they are handled elsewhere. +pub(crate) static EXTERNAL_ONLY_SET: LazyLock> = + LazyLock::new(|| EXTERNAL_ONLY_LIST.iter().copied().collect()); + pub(super) fn handle_ignore( conditions: &PreparedConditions, line: &DirectiveLine<'_>, @@ -75,6 +91,8 @@ fn parse_cfg_name_directive<'a>( if prefix == "ignore-" && EXTERNAL_IGNORES_SET.contains(line.name) { return ParsedNameDirective::not_handled_here(); + } else if prefix == "only-" && EXTERNAL_ONLY_SET.contains(line.name) { + return ParsedNameDirective::not_handled_here(); } // FIXME(Zalathar): This currently allows either a space or a colon, and @@ -209,14 +227,6 @@ pub(crate) fn prepare_conditions(config: &Config) -> PreparedConditions { "when std is built with remapping of debuginfo", ); - for &debugger in Debugger::STR_VARIANTS { - builder.cond( - debugger, - Some(debugger) == config.debugger.as_ref().map(Debugger::to_str), - &format!("when the debugger is {debugger}"), - ); - } - for &compare_mode in CompareMode::STR_VARIANTS { builder.cond( &format!("compare-mode-{compare_mode}"), diff --git a/src/tools/compiletest/src/directives/tests.rs b/src/tools/compiletest/src/directives/tests.rs index 8ef03d1ac1468..992ace208a42f 100644 --- a/src/tools/compiletest/src/directives/tests.rs +++ b/src/tools/compiletest/src/directives/tests.rs @@ -9,7 +9,7 @@ use crate::directives::{ FileDirectives, KNOWN_DIRECTIVE_NAMES_SET, LineNumber, extract_llvm_version, extract_version_range, line_directive, parse_edition, parse_normalize_rule, }; -use crate::executor::{CollectedTestDesc, ShouldFail}; +use crate::executor::{CollectedTestDesc, ShouldFail, TestVariant}; /// All directive handlers should have a name that is also in `KNOWN_DIRECTIVE_NAMES_SET`. #[test] @@ -46,11 +46,14 @@ fn make_test_description( filterable_path: &Utf8Path, file_contents: &str, revision: Option<&str>, + debugger: Option, ) -> CollectedTestDesc { let cache = DirectivesCache::load(config); let mut poisoned = false; let file_directives = FileDirectives::from_file_contents(path, file_contents); + let variant = TestVariant { revision: revision.map(str::to_owned), debugger }; + let mut aux_props = AuxProps::default(); let test = crate::directives::make_test_description( config, @@ -59,7 +62,7 @@ fn make_test_description( path, filterable_path, &file_directives, - revision, + &variant, &mut poisoned, &mut aux_props, ); @@ -283,7 +286,14 @@ fn parse_early_props(config: &Config, contents: &str) -> EarlyProps { fn check_ignore(config: &Config, contents: &str) -> bool { let tn = String::new(); let p = Utf8Path::new("a.rs"); - let d = make_test_description(&config, tn, p, p, contents, None); + let d = make_test_description(&config, tn, p, p, contents, None, None); + d.is_ignored() +} + +fn check_ignore_debugger(config: &Config, contents: &str, debugger: Option) -> bool { + let tn = String::new(); + let p = Utf8Path::new("a.rs"); + let d = make_test_description(&config, tn, p, p, contents, None, debugger); d.is_ignored() } @@ -293,9 +303,9 @@ fn should_fail() { let tn = String::new(); let p = Utf8Path::new("a.rs"); - let d = make_test_description(&config, tn.clone(), p, p, "", None); + let d = make_test_description(&config, tn.clone(), p, p, "", None, None); assert_eq!(d.should_fail, ShouldFail::No); - let d = make_test_description(&config, tn, p, p, "//@ should-fail", None); + let d = make_test_description(&config, tn, p, p, "//@ should-fail", None, None); assert_eq!(d.should_fail, ShouldFail::Yes); } @@ -449,18 +459,11 @@ fn cross_compile() { #[test] fn debugger() { - let mut config = cfg().build(); - config.debugger = None; - assert!(!check_ignore(&config, "//@ ignore-cdb")); - - config.debugger = Some(Debugger::Cdb); - assert!(check_ignore(&config, "//@ ignore-cdb")); - - config.debugger = Some(Debugger::Gdb); - assert!(check_ignore(&config, "//@ ignore-gdb")); - - config.debugger = Some(Debugger::Lldb); - assert!(check_ignore(&config, "//@ ignore-lldb")); + let config = cfg().build(); + assert!(!check_ignore_debugger(&config, "//@ ignore-cdb", None)); + assert!(check_ignore_debugger(&config, "//@ ignore-cdb", Some(Debugger::Cdb))); + assert!(check_ignore_debugger(&config, "//@ ignore-gdb", Some(Debugger::Gdb))); + assert!(check_ignore_debugger(&config, "//@ ignore-lldb", Some(Debugger::Lldb))); } #[test] diff --git a/src/tools/compiletest/src/executor.rs b/src/tools/compiletest/src/executor.rs index 78303438b3a37..d094363d5e498 100644 --- a/src/tools/compiletest/src/executor.rs +++ b/src/tools/compiletest/src/executor.rs @@ -14,7 +14,7 @@ use std::{env, hint, mem, panic, thread}; use camino::Utf8PathBuf; -use crate::common::{Config, TestPaths}; +use crate::common::{Config, Debugger, TestPaths}; use crate::output_capture::{self, ConsoleOut}; use crate::panic_hook; @@ -111,7 +111,7 @@ fn spawn_test_thread( id, config: Arc::clone(&test.config), testpaths: test.testpaths.clone(), - revision: test.revision.clone(), + variant: test.variant.clone(), should_fail: test.desc.should_fail, completion_sender, }; @@ -126,7 +126,7 @@ struct TestThreadArgs { config: Arc, testpaths: TestPaths, - revision: Option, + variant: TestVariant, should_fail: ShouldFail, completion_sender: mpsc::Sender, @@ -152,13 +152,7 @@ fn test_thread_main(args: TestThreadArgs) { // require a major overhaul of error handling in the test runners. let panic_payload = panic::catch_unwind(|| { __rust_begin_short_backtrace(|| { - crate::runtest::run( - &args.config, - stdout, - stderr, - &args.testpaths, - args.revision.as_deref(), - ); + crate::runtest::run(&args.config, stdout, stderr, &args.testpaths, &args.variant); }); }) .err(); @@ -324,12 +318,25 @@ fn get_concurrency() -> usize { } } +/// Data related to a specific variant of a test. +#[derive(Clone, Debug)] +pub(crate) struct TestVariant { + pub(crate) revision: Option, + pub(crate) debugger: Option, +} + +impl TestVariant { + pub(crate) fn revision(&self) -> Option<&str> { + self.revision.as_deref() + } +} + /// Information that was historically needed to create a libtest `TestDescAndFn`. pub(crate) struct CollectedTest { pub(crate) desc: CollectedTestDesc, pub(crate) config: Arc, pub(crate) testpaths: TestPaths, - pub(crate) revision: Option, + pub(crate) variant: TestVariant, } /// Information that was historically needed to create a libtest `TestDesc`. diff --git a/src/tools/compiletest/src/lib.rs b/src/tools/compiletest/src/lib.rs index b7beaa6f18c2c..70408f13a5cf8 100644 --- a/src/tools/compiletest/src/lib.rs +++ b/src/tools/compiletest/src/lib.rs @@ -43,7 +43,7 @@ use crate::common::{ output_base_dir, output_relative_path, }; use crate::directives::{AuxProps, DirectivesCache, FileDirectives}; -use crate::executor::CollectedTest; +use crate::executor::{CollectedTest, TestVariant}; /// Called by `main` after the config has been parsed. fn run_tests(config: Arc) { @@ -76,43 +76,31 @@ fn run_tests(config: Arc) { // SAFETY: at this point we're still single-threaded. unsafe { env::set_var("__COMPAT_LAYER", "RunAsInvoker") }; - let mut configs = Vec::new(); + // Debugging emscripten code doesn't make sense today + let ignore_tests = config.mode == TestMode::DebugInfo && config.target.contains("emscripten"); + if let TestMode::DebugInfo = config.mode { - // Debugging emscripten code doesn't make sense today - if !config.target.contains("emscripten") { - // FIXME: ideally, we would just have one config, and then have some mechanism of - // generating multiple variants of a test, one for each debugger (something like - // debuginfo revisions). But for now, we just create three configs. - configs.extend([ - Arc::new(Config { debugger: Some(Debugger::Cdb), ..config.as_ref().clone() }), - Arc::new(Config { debugger: Some(Debugger::Gdb), ..config.as_ref().clone() }), - Arc::new(Config { debugger: Some(Debugger::Lldb), ..config.as_ref().clone() }), - ]); - - // FIXME: this should ideally happen somewhere else.. - if config.target.contains("android") { - println!("{} debug-info test uses tcp 5039 port. please reserve it", config.target); - - // android debug-info test uses remote debugger so, we test 1 thread - // at once as they're all sharing the same TCP port to communicate - // over. - // - // we should figure out how to lift this restriction! (run them all - // on different ports allocated dynamically). - // - // SAFETY: at this point we are still single-threaded. - unsafe { env::set_var("RUST_TEST_THREADS", "1") }; - } + // FIXME: this should ideally happen somewhere else.. + if config.target.contains("android") { + println!("{} debug-info test uses tcp 5039 port. please reserve it", config.target); + + // android debug-info test uses remote debugger so, we test 1 thread + // at once as they're all sharing the same TCP port to communicate + // over. + // + // we should figure out how to lift this restriction! (run them all + // on different ports allocated dynamically). + // + // SAFETY: at this point we are still single-threaded. + unsafe { env::set_var("RUST_TEST_THREADS", "1") }; } - } else { - configs.push(config.clone()); }; // Discover all of the tests in the test suite directory, and build a `CollectedTest` // structure for each test (or each revision of a multi-revision test). let mut tests = Vec::new(); - for c in configs { - tests.extend(collect_and_make_tests(c)); + if !ignore_tests { + tests.extend(collect_and_make_tests(config.clone())); } tests.sort_by(|a, b| Ord::cmp(&a.desc.name, &b.desc.name)); @@ -446,56 +434,68 @@ fn make_test(cx: &TestCollectorCx, collector: &mut TestCollector, testpaths: &Te early_props.revisions.iter().map(|r| Some(r.as_str())).collect() }; - // For each revision (or the sole dummy revision), create and append a - // `CollectedTest` that can be handed over to the test executor. - collector.tests.extend(revisions.into_iter().map(|revision| { - // Create a test name and description to hand over to the executor. - let (test_name, filterable_path) = - make_test_name_and_filterable_path(&cx.config, testpaths, revision); - - // While scanning for ignore/only/needs directives, also collect aux - // paths for up-to-date checking. - let mut aux_props = AuxProps::default(); - - // Create a description struct for the test/revision. - // This is where `ignore-*`/`only-*`/`needs-*` directives are handled, - // because they historically needed to set the libtest ignored flag. - let mut desc = make_test_description( - &cx.config, - &cx.cache, - test_name, - &test_path, - &filterable_path, - &file_directives, - revision, - &mut collector.poisoned, - &mut aux_props, - ); + // For debuginfo tests, we have to run them once for each debugger. + // We thus create a cartesian product of each revision and each supported debugger here. + let debuggers = if cx.config.mode == TestMode::DebugInfo { + vec![Some(Debugger::Cdb), Some(Debugger::Gdb), Some(Debugger::Lldb)] + } else { + vec![None] + }; - // If a test's inputs haven't changed since the last time it ran, - // mark it as ignored so that the executor will skip it. - if !desc.is_ignored() - && !cx.config.force_rerun - && is_up_to_date(cx, testpaths, &aux_props, revision) - { - // Keep this in sync with the "up-to-date" message detected by bootstrap. - // FIXME(Zalathar): Now that we are no longer tied to libtest, we could - // find a less fragile way to communicate this status to bootstrap. - desc.ignore_message = Some("up-to-date".into()); - } + // For each revision (or the sole dummy revision) and each debugger, create and append a + // `CollectedTest` that can be handed over to the test executor. + for debugger in debuggers { + collector.tests.extend(revisions.iter().map(|&revision| { + let revision = revision.map(str::to_owned); + let variant = TestVariant { revision, debugger }; + + // Create a test name and description to hand over to the executor. + let (test_name, filterable_path) = + make_test_name_and_filterable_path(&cx.config, testpaths, &variant); + + // While scanning for ignore/only/needs directives, also collect aux + // paths for up-to-date checking. + let mut aux_props = AuxProps::default(); + + // Create a description struct for the test/revision. + // This is where `ignore-*`/`only-*`/`needs-*` directives are handled, + // because they historically needed to set the libtest ignored flag. + let mut desc = make_test_description( + &cx.config, + &cx.cache, + test_name, + &test_path, + &filterable_path, + &file_directives, + &variant, + &mut collector.poisoned, + &mut aux_props, + ); + + // If a test's inputs haven't changed since the last time it ran, + // mark it as ignored so that the executor will skip it. + if !desc.is_ignored() + && !cx.config.force_rerun + && is_up_to_date(cx, testpaths, &aux_props, &variant) + { + // Keep this in sync with the "up-to-date" message detected by bootstrap. + // FIXME(Zalathar): Now that we are no longer tied to libtest, we could + // find a less fragile way to communicate this status to bootstrap. + desc.ignore_message = Some("up-to-date".into()); + } - let config = Arc::clone(&cx.config); - let testpaths = testpaths.clone(); - let revision = revision.map(str::to_owned); + let config = Arc::clone(&cx.config); + let testpaths = testpaths.clone(); - CollectedTest { desc, config, testpaths, revision } - })); + CollectedTest { desc, config, testpaths, variant } + })); + } } /// The path of the `stamp` file that gets created or updated whenever a /// particular test completes successfully. -fn stamp_file_path(config: &Config, testpaths: &TestPaths, revision: Option<&str>) -> Utf8PathBuf { - output_base_dir(config, testpaths, revision).join("stamp") +fn stamp_file_path(config: &Config, testpaths: &TestPaths, variant: &TestVariant) -> Utf8PathBuf { + output_base_dir(config, testpaths, variant).join("stamp") } /// Returns a list of files that, if modified, would cause this test to no @@ -552,9 +552,9 @@ fn is_up_to_date( cx: &TestCollectorCx, testpaths: &TestPaths, aux_props: &AuxProps, - revision: Option<&str>, + variant: &TestVariant, ) -> bool { - let stamp_file_path = stamp_file_path(&cx.config, testpaths, revision); + let stamp_file_path = stamp_file_path(&cx.config, testpaths, variant); // Check the config hash inside the stamp file. let contents = match fs::read_to_string(&stamp_file_path) { Ok(f) => f, @@ -562,7 +562,7 @@ fn is_up_to_date( // The test hasn't succeeded yet, so it is not up-to-date. Err(_) => return false, }; - let expected_hash = runtest::compute_stamp_hash(&cx.config); + let expected_hash = runtest::compute_stamp_hash(&cx.config, variant); if contents != expected_hash { // Some part of compiletest configuration has changed since the test // last succeeded, so it is not up-to-date. @@ -572,7 +572,7 @@ fn is_up_to_date( // Check the timestamp of the stamp file against the last modified time // of all files known to be relevant to the test. let mut inputs_stamp = cx.common_inputs_stamp.clone(); - for path in files_related_to_test(&cx.config, testpaths, aux_props, revision) { + for path in files_related_to_test(&cx.config, testpaths, aux_props, variant.revision()) { inputs_stamp.add_path(&path); } @@ -627,12 +627,12 @@ impl Stamp { fn make_test_name_and_filterable_path( config: &Config, testpaths: &TestPaths, - revision: Option<&str>, + variant: &TestVariant, ) -> (String, Utf8PathBuf) { // Print the name of the file, relative to the sources root. let path = testpaths.file.strip_prefix(&config.src_root).unwrap(); - let debugger = match config.debugger { - Some(d) => format!("-{}", d), + let debugger = match variant.debugger.as_ref() { + Some(d) => format!("-{d}"), None => String::new(), }; let mode_suffix = match config.compare_mode { @@ -646,7 +646,7 @@ fn make_test_name_and_filterable_path( debugger, mode_suffix, path, - revision.map_or("".to_string(), |rev| format!("#{}", rev)) + variant.revision().map_or("".to_string(), |rev| format!("#{}", rev)) ); // `path` is the full path from the repo root like, `tests/ui/foo/bar.rs`. diff --git a/src/tools/compiletest/src/runtest.rs b/src/tools/compiletest/src/runtest.rs index 4243c60001244..a4fa28e13b911 100644 --- a/src/tools/compiletest/src/runtest.rs +++ b/src/tools/compiletest/src/runtest.rs @@ -20,6 +20,7 @@ use crate::common::{ }; use crate::directives::{AuxCrate, TestProps}; use crate::errors::{Error, ErrorKind, load_errors}; +use crate::executor::TestVariant; use crate::output_capture::ConsoleOut; use crate::read2::{Truncated, read2_abbreviated}; use crate::runtest::compute_diff::{DiffLine, diff_by_lines, make_diff, write_diff}; @@ -111,7 +112,7 @@ pub(crate) fn run( stdout: &dyn ConsoleOut, stderr: &dyn ConsoleOut, testpaths: &TestPaths, - revision: Option<&str>, + variant: &TestVariant, ) { match &*config.target { "arm-linux-androideabi" @@ -122,15 +123,7 @@ pub(crate) fn run( panic!("android device not available"); } } - - _ => { - // FIXME: this logic seems strange as well. - - // android has its own gdb handling - if config.debugger == Some(Debugger::Gdb) && config.gdb.is_none() { - panic!("gdb not available but debuginfo gdb debuginfo test requested"); - } - } + _ => {} } if config.verbose { @@ -138,16 +131,16 @@ pub(crate) fn run( write!(stdout, "\n\n"); } debug!("running {}", testpaths.file); - let mut props = TestProps::from_file(&testpaths.file, revision, &config); + let mut props = TestProps::from_file(&testpaths.file, variant.revision(), &config); // For non-incremental (i.e. regular UI) tests, the incremental directory // takes into account the revision name, since the revisions are independent // of each other and can race. if props.incremental { - props.incremental_dir = Some(incremental_dir(&config, testpaths, revision)); + props.incremental_dir = Some(incremental_dir(&config, testpaths, variant)); } - let cx = TestCx { config: &config, stdout, stderr, props: &props, testpaths, revision }; + let cx = TestCx { config: &config, stdout, stderr, props: &props, testpaths, variant }; if let Err(e) = create_dir_all(&cx.output_base_dir()) { panic!("failed to create output base directory {}: {e}", cx.output_base_dir()); @@ -170,7 +163,10 @@ pub(crate) fn run( stderr, props: &revision_props, testpaths, - revision: Some(revision), + variant: &TestVariant { + revision: Some(revision.clone()), + debugger: variant.debugger, + }, }; rev_cx.run_revision(); } @@ -181,13 +177,13 @@ pub(crate) fn run( cx.create_stamp(); } -pub(crate) fn compute_stamp_hash(config: &Config) -> String { +pub(crate) fn compute_stamp_hash(config: &Config, variant: &TestVariant) -> String { let mut hash = DefaultHasher::new(); config.stage_id.hash(&mut hash); config.run.hash(&mut hash); config.edition.hash(&mut hash); - match config.debugger { + match variant.debugger { Some(Debugger::Cdb) => { config.cdb.hash(&mut hash); } @@ -223,7 +219,7 @@ struct TestCx<'test> { stderr: &'test dyn ConsoleOut, props: &'test TestProps, testpaths: &'test TestPaths, - revision: Option<&'test str>, + variant: &'test TestVariant, } enum ReadFrom { @@ -480,7 +476,7 @@ impl<'test> TestCx<'test> { // Otherwise the `--cfg` flag is not valid. let normalize_revision = |revision: &str| revision.to_lowercase().replace("-", "_"); - if let Some(revision) = self.revision { + if let Some(revision) = self.variant.revision() { let normalized_revision = normalize_revision(revision); let cfg_arg = ["--cfg", &normalized_revision]; let arg = format!("--cfg={normalized_revision}"); @@ -660,7 +656,7 @@ impl<'test> TestCx<'test> { /// Check `//~ KIND message` annotations. fn check_expected_errors(&self, proc_res: &ProcRes) { - let expected_errors = load_errors(&self.testpaths.file, self.revision); + let expected_errors = load_errors(&self.testpaths.file, self.variant.revision()); debug!( "check_expected_errors: expected_errors={:?} proc_res.status={:?}", expected_errors, proc_res.status @@ -988,14 +984,15 @@ impl<'test> TestCx<'test> { for rel_ab in &self.props.aux.builds { let aux_path = self.resolve_aux_path(rel_ab); - let props_for_aux = self.props.from_aux_file(&aux_path, self.revision, self.config); + let props_for_aux = + self.props.from_aux_file(&aux_path, self.variant.revision(), self.config); let aux_cx = TestCx { config: self.config, stdout: self.stdout, stderr: self.stderr, props: &props_for_aux, testpaths: self.testpaths, - revision: self.revision, + variant: self.variant, }; // Create the directory for the stdout/stderr files. create_dir_all(aux_cx.output_base_dir()).unwrap(); @@ -1351,7 +1348,8 @@ impl<'test> TestCx<'test> { aux_type: Option, ) -> AuxType { let aux_path = self.resolve_aux_path(source_path); - let mut aux_props = self.props.from_aux_file(&aux_path, self.revision, self.config); + let mut aux_props = + self.props.from_aux_file(&aux_path, self.variant.revision(), self.config); if aux_type == Some(AuxType::ProcMacro) { aux_props.force_host = true; } @@ -1369,7 +1367,7 @@ impl<'test> TestCx<'test> { stderr: self.stderr, props: &aux_props, testpaths: self.testpaths, - revision: self.revision, + variant: self.variant, }; // Create the directory for the stdout/stderr files. create_dir_all(aux_cx.output_base_dir()).unwrap(); @@ -2009,7 +2007,8 @@ impl<'test> TestCx<'test> { } fn dump_output(&self, print_output: bool, proc_name: &str, out: &str, err: &str) { - let revision = if let Some(r) = self.revision { format!("{}.", r) } else { String::new() }; + let revision = + if let Some(r) = self.variant.revision() { format!("{}.", r) } else { String::new() }; self.dump_output_file(out, &format!("{}out", revision)); self.dump_output_file(err, &format!("{}err", revision)); @@ -2066,22 +2065,26 @@ impl<'test> TestCx<'test> { /// The revision, ignored for incremental compilation since it wants all revisions in /// the same directory. - fn safe_revision(&self) -> Option<&str> { - if self.config.mode == TestMode::Incremental { None } else { self.revision } + fn variant_with_safe_revision(&self) -> TestVariant { + if self.config.mode == TestMode::Incremental { + TestVariant { revision: None, debugger: self.variant.debugger } + } else { + self.variant.clone() + } } /// Gets the absolute path to the directory where all output for the given /// test/revision should reside. /// E.g., `/path/to/build/host-tuple/test/ui/relative/testname.revision.mode/`. fn output_base_dir(&self) -> Utf8PathBuf { - output_base_dir(self.config, self.testpaths, self.safe_revision()) + output_base_dir(self.config, self.testpaths, &self.variant_with_safe_revision()) } /// Gets the absolute path to the base filename used as output for the given /// test/revision. /// E.g., `/.../relative/testname.revision.mode/testname`. fn output_base_name(&self) -> Utf8PathBuf { - output_base_name(self.config, self.testpaths, self.safe_revision()) + output_base_name(self.config, self.testpaths, &self.variant_with_safe_revision()) } /// Prints a message to (captured) stdout if `config.verbose` is true. @@ -2100,7 +2103,7 @@ impl<'test> TestCx<'test> { /// includes the revision name for tests that use revisions. #[must_use] fn error_prefix(&self) -> String { - match self.revision { + match self.variant.revision() { Some(rev) => format!("error in revision `{rev}`"), None => format!("error"), } @@ -2177,7 +2180,7 @@ impl<'test> TestCx<'test> { // TL;DR We may not want to conflate `compiletest` revisions and `FileCheck` prefixes. // HACK: tests are allowed to use a revision name as a check prefix. - if let Some(rev) = self.revision { + if let Some(rev) = self.variant.revision() { filecheck.arg("--check-prefix").arg(rev); } @@ -2663,17 +2666,21 @@ impl<'test> TestCx<'test> { } fn expected_output_path(&self, kind: &str) -> Utf8PathBuf { - let mut path = - expected_output_path(&self.testpaths, self.revision, &self.config.compare_mode, kind); + let mut path = expected_output_path( + &self.testpaths, + self.variant.revision(), + &self.config.compare_mode, + kind, + ); if !path.exists() { if let Some(CompareMode::Polonius) = self.config.compare_mode { - path = expected_output_path(&self.testpaths, self.revision, &None, kind); + path = expected_output_path(&self.testpaths, self.variant.revision(), &None, kind); } } if !path.exists() { - path = expected_output_path(&self.testpaths, self.revision, &None, kind); + path = expected_output_path(&self.testpaths, self.variant.revision(), &None, kind); } path @@ -2712,8 +2719,12 @@ impl<'test> TestCx<'test> { actual_unnormalized: &str, expected: &str, ) -> CompareOutcome { - let expected_path = - expected_output_path(self.testpaths, self.revision, &self.config.compare_mode, stream); + let expected_path = expected_output_path( + self.testpaths, + self.variant.revision(), + &self.config.compare_mode, + stream, + ); if self.config.bless && actual.is_empty() && expected_path.exists() { self.delete_file(&expected_path); @@ -2774,7 +2785,7 @@ impl<'test> TestCx<'test> { // Write the actual output to a file in build directory. let actual_path = self .output_base_name() - .with_extra_extension(self.revision.unwrap_or("")) + .with_extra_extension(self.variant.revision().unwrap_or("")) .with_extra_extension( self.config.compare_mode.as_ref().map(|cm| cm.to_str()).unwrap_or(""), ) @@ -2802,7 +2813,7 @@ impl<'test> TestCx<'test> { } else { // Delete non-revision .stderr/.stdout file if revisions are used. // Without this, we'd just generate the new files and leave the old files around. - if self.revision.is_some() { + if self.variant.revision().is_some() { let old = expected_output_path(self.testpaths, None, &self.config.compare_mode, stream); self.delete_file(&old); @@ -2917,7 +2928,7 @@ impl<'test> TestCx<'test> { ) { for kind in UI_EXTENSIONS { let canon_comparison_path = - expected_output_path(&self.testpaths, self.revision, &None, kind); + expected_output_path(&self.testpaths, self.variant.revision(), &None, kind); let canon = match self.load_expected_output_from_path(&canon_comparison_path) { Ok(canon) => canon, @@ -2925,8 +2936,12 @@ impl<'test> TestCx<'test> { }; let bless = self.config.bless; let check_and_prune_duplicate_outputs = |mode: &CompareMode, require_same: bool| { - let examined_path = - expected_output_path(&self.testpaths, self.revision, &Some(mode.clone()), kind); + let examined_path = expected_output_path( + &self.testpaths, + self.variant.revision(), + &Some(mode.clone()), + kind, + ); // If there is no output, there is nothing to do let examined_content = match self.load_expected_output_from_path(&examined_path) { @@ -2962,8 +2977,8 @@ impl<'test> TestCx<'test> { } fn create_stamp(&self) { - let stamp_file_path = stamp_file_path(&self.config, self.testpaths, self.revision); - fs::write(&stamp_file_path, compute_stamp_hash(&self.config)).unwrap(); + let stamp_file_path = stamp_file_path(&self.config, self.testpaths, self.variant); + fs::write(&stamp_file_path, compute_stamp_hash(&self.config, self.variant)).unwrap(); } fn init_incremental_test(&self) { diff --git a/src/tools/compiletest/src/runtest/codegen_units.rs b/src/tools/compiletest/src/runtest/codegen_units.rs index e4c924ed18a31..902835da78c5c 100644 --- a/src/tools/compiletest/src/runtest/codegen_units.rs +++ b/src/tools/compiletest/src/runtest/codegen_units.rs @@ -6,7 +6,7 @@ use crate::{errors, fatal}; impl TestCx<'_> { pub(super) fn run_codegen_units_test(&self) { - assert!(self.revision.is_none(), "revisions not relevant here"); + assert!(self.variant.revision.is_none(), "revisions not relevant here"); let proc_res = self.compile_test(WillExecute::No, Emit::None); diff --git a/src/tools/compiletest/src/runtest/debuginfo.rs b/src/tools/compiletest/src/runtest/debuginfo.rs index 867331bf7d13c..4ac8215371f1b 100644 --- a/src/tools/compiletest/src/runtest/debuginfo.rs +++ b/src/tools/compiletest/src/runtest/debuginfo.rs @@ -12,7 +12,7 @@ use crate::util::ArgFileCommand; impl TestCx<'_> { pub(super) fn run_debuginfo_test(&self) { - match self.config.debugger.unwrap() { + match self.variant.debugger.as_ref().unwrap() { Debugger::Cdb => self.run_debuginfo_cdb_test(), Debugger::Gdb => self.run_debuginfo_gdb_test(), Debugger::Lldb => self.run_debuginfo_lldb_test(), @@ -46,8 +46,9 @@ impl TestCx<'_> { } // Parse debugger commands etc from test files - let dbg_cmds = DebuggerCommands::parse_from(&self.testpaths.file, "cdb", self.revision) - .unwrap_or_else(|e| self.fatal(&e)); + let dbg_cmds = + DebuggerCommands::parse_from(&self.testpaths.file, "cdb", self.variant.revision()) + .unwrap_or_else(|e| self.fatal(&e)); // https://docs.microsoft.com/en-us/windows-hardware/drivers/debugger/debugger-commands let mut script_str = String::with_capacity(2048); @@ -105,8 +106,9 @@ impl TestCx<'_> { } fn run_debuginfo_gdb_test(&self) { - let dbg_cmds = DebuggerCommands::parse_from(&self.testpaths.file, "gdb", self.revision) - .unwrap_or_else(|e| self.fatal(&e)); + let dbg_cmds = + DebuggerCommands::parse_from(&self.testpaths.file, "gdb", self.variant.revision()) + .unwrap_or_else(|e| self.fatal(&e)); let mut cmds = dbg_cmds.commands.join("\n"); // compile test file (it should have 'compile-flags:-g' in the directive) @@ -374,8 +376,9 @@ impl TestCx<'_> { } // Parse debugger commands etc from test files - let dbg_cmds = DebuggerCommands::parse_from(&self.testpaths.file, "lldb", self.revision) - .unwrap_or_else(|e| self.fatal(&e)); + let dbg_cmds = + DebuggerCommands::parse_from(&self.testpaths.file, "lldb", self.variant.revision()) + .unwrap_or_else(|e| self.fatal(&e)); // Write debugger script: // We don't want to hang when calling `quit` while the process is still running diff --git a/src/tools/compiletest/src/runtest/incremental.rs b/src/tools/compiletest/src/runtest/incremental.rs index 6529882715a90..8e573d8cfe308 100644 --- a/src/tools/compiletest/src/runtest/incremental.rs +++ b/src/tools/compiletest/src/runtest/incremental.rs @@ -38,7 +38,8 @@ impl IncrRevKind { impl TestCx<'_> { /// Runs a single revision of an incremental test. pub(super) fn run_incremental_test(&self) { - let revision = self.revision.expect("incremental tests require a list of revisions"); + let revision = + self.variant.revision().expect("incremental tests require a list of revisions"); // Incremental workproduct directory should have already been created. let incremental_dir = self.props.incremental_dir.as_ref().unwrap(); diff --git a/src/tools/compiletest/src/runtest/js_doc.rs b/src/tools/compiletest/src/runtest/js_doc.rs index 1b5d4608f2728..badad93759c26 100644 --- a/src/tools/compiletest/src/runtest/js_doc.rs +++ b/src/tools/compiletest/src/runtest/js_doc.rs @@ -19,7 +19,7 @@ impl TestCx<'_> { .arg("--test-file") .arg(self.testpaths.file.with_extension("js")) .arg("--revision") - .arg(self.revision.unwrap_or_default()); + .arg(self.variant.revision().unwrap_or_default()); let res = self.run_command_to_procres(cmd); if !res.status.success() { self.fatal_proc_rec("rustdoc-js test failed!", &res); diff --git a/src/tools/compiletest/src/runtest/pretty.rs b/src/tools/compiletest/src/runtest/pretty.rs index 2655772723352..9a2ef66e89729 100644 --- a/src/tools/compiletest/src/runtest/pretty.rs +++ b/src/tools/compiletest/src/runtest/pretty.rs @@ -20,7 +20,10 @@ impl TestCx<'_> { let mut round = 0; while round < rounds { - self.logv(format_args!("pretty-printing round {round} revision {:?}", self.revision)); + self.logv(format_args!( + "pretty-printing round {round} revision {:?}", + self.variant.revision + )); let read_from = if round == 0 { ReadFrom::Path } else { ReadFrom::Stdin(srcs[round].to_owned()) }; @@ -29,7 +32,7 @@ impl TestCx<'_> { self.fatal_proc_rec( &format!( "pretty-printing failed in round {} revision {:?}", - round, self.revision + round, self.variant.revision ), &proc_res, ); diff --git a/src/tools/compiletest/src/runtest/rustdoc.rs b/src/tools/compiletest/src/runtest/rustdoc.rs index a6a826a377301..bee5c86ce62ed 100644 --- a/src/tools/compiletest/src/runtest/rustdoc.rs +++ b/src/tools/compiletest/src/runtest/rustdoc.rs @@ -3,7 +3,7 @@ use crate::util::ArgFileCommand; impl TestCx<'_> { pub(super) fn run_rustdoc_html_test(&self) { - assert!(self.revision.is_none(), "revisions not supported in this test suite"); + assert!(self.variant.revision.is_none(), "revisions not supported in this test suite"); let out_dir = self.output_base_dir(); remove_and_create_dir_all(&out_dir).unwrap_or_else(|e| { diff --git a/src/tools/compiletest/src/runtest/rustdoc_json.rs b/src/tools/compiletest/src/runtest/rustdoc_json.rs index 29626b1f40c04..311c2c67f2dfa 100644 --- a/src/tools/compiletest/src/runtest/rustdoc_json.rs +++ b/src/tools/compiletest/src/runtest/rustdoc_json.rs @@ -5,7 +5,7 @@ impl TestCx<'_> { pub(super) fn run_rustdoc_json_test(&self) { //FIXME: Add bless option. - assert!(self.revision.is_none(), "revisions not supported in this test suite"); + assert!(self.variant.revision.is_none(), "revisions not supported in this test suite"); let out_dir = self.output_base_dir(); remove_and_create_dir_all(&out_dir).unwrap_or_else(|e| { diff --git a/src/tools/compiletest/src/runtest/ui.rs b/src/tools/compiletest/src/runtest/ui.rs index f8a0c00083aec..a0936e578c6c1 100644 --- a/src/tools/compiletest/src/runtest/ui.rs +++ b/src/tools/compiletest/src/runtest/ui.rs @@ -259,7 +259,7 @@ impl TestCx<'_> { // (including the revision) here to avoid the test writer having to manually specify a // `#![crate_name = "..."]` as a workaround. This is okay since we're only checking if // the fixed code is compilable. - if self.revision.is_some() { + if self.variant.revision.is_some() { let crate_name = self.testpaths.file.file_stem().expect("test must have a file stem"); // crate name must be alphanumeric or `_`. diff --git a/src/tools/compiletest/src/rustdoc_gui_test.rs b/src/tools/compiletest/src/rustdoc_gui_test.rs index 57ce0c5a6d5f7..215fee768f254 100644 --- a/src/tools/compiletest/src/rustdoc_gui_test.rs +++ b/src/tools/compiletest/src/rustdoc_gui_test.rs @@ -79,7 +79,6 @@ fn incomplete_config_for_rustdoc_gui_test() -> Config { sysroot_base: Utf8PathBuf::default(), stage: Default::default(), stage_id: String::default(), - debugger: Default::default(), run_ignored: Default::default(), with_rustc_debug_assertions: Default::default(), with_std_debug_assertions: Default::default(),