diff --git a/src/tools/compiletest/src/debuggers.rs b/src/tools/compiletest/src/debuggers.rs index 4a1a74ddfe1bd..7b57c4c11052b 100644 --- a/src/tools/compiletest/src/debuggers.rs +++ b/src/tools/compiletest/src/debuggers.rs @@ -1,59 +1,7 @@ -use std::env; use std::process::Command; -use std::sync::Arc; use camino::Utf8Path; -use crate::common::{Config, Debugger}; - -pub(crate) fn configure_cdb(config: &Config) -> Option> { - config.cdb.as_ref()?; - - Some(Arc::new(Config { debugger: Some(Debugger::Cdb), ..config.clone() })) -} - -pub(crate) fn configure_gdb(config: &Config) -> Option> { - config.gdb_version?; - - if config.matches_env("msvc") { - return None; - } - - if config.remote_test_client.is_some() && !config.target.contains("android") { - println!( - "WARNING: debuginfo tests are not available when \ - testing with remote" - ); - return None; - } - - 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") }; - } - - Some(Arc::new(Config { debugger: Some(Debugger::Gdb), ..config.clone() })) -} - -pub(crate) fn configure_lldb(config: &Config) -> Option> { - config.lldb.as_ref()?; - - Some(Arc::new(Config { debugger: Some(Debugger::Lldb), ..config.clone() })) -} - pub(crate) fn query_cdb_version(cdb: &Utf8Path) -> Option<[u16; 4]> { let mut version = None; if let Ok(output) = Command::new(cdb).arg("/version").output() { diff --git a/src/tools/compiletest/src/directives.rs b/src/tools/compiletest/src/directives.rs index 49b860d9cd4cf..19e966b2f1abe 100644 --- a/src/tools/compiletest/src/directives.rs +++ b/src/tools/compiletest/src/directives.rs @@ -1,3 +1,4 @@ +use std::borrow::Cow; use std::collections::HashSet; use std::process::Command; use std::{env, fs}; @@ -895,57 +896,85 @@ pub(crate) fn make_test_description( poisoned: &mut bool, aux_props: &mut AuxProps, ) -> CollectedTestDesc { - let mut ignore = false; - let mut ignore_message = None; + let mut ignore_message: Option> = None; let mut should_fail = false; - // Scan through the test file to handle `ignore-*`, `only-*`, and `needs-*` directives. - iter_directives(config, file_directives, &mut |ln @ &DirectiveLine { line_number, .. }| { - if !ln.applies_to_test_revision(test_revision) { - return; + // 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) => { + if let Some(msg) = check_cdb_support(config) { + ignore_message = Some(Cow::Owned(msg)); + } + } + Some(Debugger::Gdb) => { + if let Some(msg) = check_gdb_support(config) { + ignore_message = Some(Cow::Owned(msg)); + } + } + Some(Debugger::Lldb) => { + if let Some(msg) = check_lldb_support(config) { + ignore_message = Some(Cow::Owned(msg)); + } + } + None => {} } + } - // Parse `aux-*` directives, for use by up-to-date checks. - parse_and_update_aux(config, ln, aux_props); + if ignore_message.is_none() { + // Scan through the test file to handle `ignore-*`, `only-*`, and `needs-*` directives. + iter_directives( + config, + file_directives, + &mut |ln @ &DirectiveLine { line_number, .. }| { + if !ln.applies_to_test_revision(test_revision) { + return; + } - macro_rules! decision { - ($e:expr) => { - match $e { - IgnoreDecision::Ignore { reason } => { - ignore = true; - ignore_message = Some(reason.into()); - } - IgnoreDecision::Error { message } => { - error!("{path}:{line_number}: {message}"); - *poisoned = true; - return; - } - IgnoreDecision::Continue => {} + // Parse `aux-*` directives, for use by up-to-date checks. + parse_and_update_aux(config, ln, aux_props); + + macro_rules! decision { + ($e:expr) => { + match $e { + IgnoreDecision::Ignore { reason } => { + ignore_message = Some(reason.into()); + } + IgnoreDecision::Error { message } => { + error!("{path}:{line_number}: {message}"); + *poisoned = true; + return; + } + IgnoreDecision::Continue => {} + } + }; } - }; - } - decision!(cfg::handle_ignore(&cache.cfg_conditions, ln)); - decision!(cfg::handle_only(&cache.cfg_conditions, ln)); - decision!(needs::handle_needs(&cache.needs, config, ln)); - 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_parallel_frontend(config, ln)); - - if config.target == "wasm32-unknown-unknown" - && config.parse_name_directive(ln, directives::CHECK_RUN_RESULTS) - { - decision!(IgnoreDecision::Ignore { - reason: "ignored on WASM as the run results cannot be checked there".into(), - }); - } + decision!(cfg::handle_ignore(&cache.cfg_conditions, ln)); + decision!(cfg::handle_only(&cache.cfg_conditions, ln)); + decision!(needs::handle_needs(&cache.needs, config, ln)); + 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_parallel_frontend(config, ln)); + + if config.target == "wasm32-unknown-unknown" + && config.parse_name_directive(ln, directives::CHECK_RUN_RESULTS) + { + decision!(IgnoreDecision::Ignore { + reason: "ignored on WASM as the run results cannot be checked there".into(), + }); + } - should_fail |= config.parse_name_directive(ln, "should-fail"); - }); + should_fail |= config.parse_name_directive(ln, "should-fail"); + }, + ); + } // The `should-fail` annotation doesn't apply to pretty tests, // since we run the pretty printer across all tests by default. @@ -959,12 +988,37 @@ pub(crate) fn make_test_description( CollectedTestDesc { name, filterable_path: filterable_path.to_owned(), - ignore, ignore_message, should_fail, } } +/// Returns `None` if CDB is available, otherwise returns an ignore message. +fn check_cdb_support(config: &Config) -> Option { + if config.cdb.is_none() { Some("cdb is not available".to_string()) } else { None } +} + +/// Returns `None` if GDB is available, otherwise returns an ignore message. +fn check_gdb_support(config: &Config) -> Option { + if config.gdb_version.is_none() { + return Some("gdb is not available".to_string()); + } + + if config.matches_env("msvc") { + return Some("gdb tests do not run on msvc".to_string()); + } + + if config.remote_test_client.is_some() && !config.target.contains("android") { + return Some("gdb tests are not available when testing with remote".to_string()); + } + None +} + +/// Returns `None` if LLDB is available, otherwise returns an ignore message. +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; diff --git a/src/tools/compiletest/src/directives/tests.rs b/src/tools/compiletest/src/directives/tests.rs index 65d7b5360ae0d..bdbe4578b3f58 100644 --- a/src/tools/compiletest/src/directives/tests.rs +++ b/src/tools/compiletest/src/directives/tests.rs @@ -284,7 +284,7 @@ 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); - d.ignore + d.is_ignored() } #[test] diff --git a/src/tools/compiletest/src/executor.rs b/src/tools/compiletest/src/executor.rs index c800d11d6b2fd..78303438b3a37 100644 --- a/src/tools/compiletest/src/executor.rs +++ b/src/tools/compiletest/src/executor.rs @@ -100,7 +100,7 @@ fn spawn_test_thread( test: &CollectedTest, completion_sender: mpsc::Sender, ) -> Option> { - if test.desc.ignore && !test.config.run_ignored { + if test.desc.is_ignored() && !test.config.run_ignored { completion_sender .send(TestCompletion { id, outcome: TestOutcome::Ignored, stdout: None }) .unwrap(); @@ -336,11 +336,16 @@ pub(crate) struct CollectedTest { pub(crate) struct CollectedTestDesc { pub(crate) name: String, pub(crate) filterable_path: Utf8PathBuf, - pub(crate) ignore: bool, pub(crate) ignore_message: Option>, pub(crate) should_fail: ShouldFail, } +impl CollectedTestDesc { + pub(crate) fn is_ignored(&self) -> bool { + self.ignore_message.is_some() + } +} + /// Tests with `//@ should-fail` are tests of compiletest itself, and should /// be reported as successful if and only if they would have _failed_. #[derive(Copy, Clone, Debug, PartialEq, Eq, Hash)] diff --git a/src/tools/compiletest/src/lib.rs b/src/tools/compiletest/src/lib.rs index 0836caa590f81..95132e9c53dc7 100644 --- a/src/tools/compiletest/src/lib.rs +++ b/src/tools/compiletest/src/lib.rs @@ -561,19 +561,28 @@ fn run_tests(config: Arc) { if let TestMode::DebugInfo = config.mode { // Debugging emscripten code doesn't make sense today if !config.target.contains("emscripten") { - match config.debugger { - Some(Debugger::Cdb) => configs.extend(debuggers::configure_cdb(&config)), - Some(Debugger::Gdb) => configs.extend(debuggers::configure_gdb(&config)), - Some(Debugger::Lldb) => configs.extend(debuggers::configure_lldb(&config)), - // FIXME: the *implicit* debugger discovery makes it really difficult to control - // which {`cdb`, `gdb`, `lldb`} are used. These should **not** be implicitly - // discovered by `compiletest`; these should be explicit `bootstrap` configuration - // options that are passed to `compiletest`! - None => { - configs.extend(debuggers::configure_cdb(&config)); - configs.extend(debuggers::configure_gdb(&config)); - configs.extend(debuggers::configure_lldb(&config)); - } + // 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") }; } } } else { @@ -946,11 +955,10 @@ fn make_test(cx: &TestCollectorCx, collector: &mut TestCollector, testpaths: &Te // 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.ignore + if !desc.is_ignored() && !cx.config.force_rerun && is_up_to_date(cx, testpaths, &aux_props, revision) { - desc.ignore = true; // 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.