From 84861fa7653cfd11c850bf06cc01daa9d5f45124 Mon Sep 17 00:00:00 2001 From: Zalathar Date: Mon, 20 Oct 2025 14:12:23 +1100 Subject: [PATCH 1/2] Skip up-to-date checking for tests that are already ignored --- src/tools/compiletest/src/lib.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/tools/compiletest/src/lib.rs b/src/tools/compiletest/src/lib.rs index 4cfb1e20f9ae9..2c68a771169e2 100644 --- a/src/tools/compiletest/src/lib.rs +++ b/src/tools/compiletest/src/lib.rs @@ -907,7 +907,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 !cx.config.force_rerun && is_up_to_date(cx, testpaths, &early_props, revision) { + if !desc.ignore + && !cx.config.force_rerun + && is_up_to_date(cx, testpaths, &early_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 From d828c11f96df3bce24433238a867f64f1fe6b28b Mon Sep 17 00:00:00 2001 From: Zalathar Date: Sun, 19 Oct 2025 18:33:12 +1100 Subject: [PATCH 2/2] Move `AuxProps` out of `EarlyProps` The primary purpose of `EarlyProps` is to discover revisions, so that we can create a separate test structure for each revision. Revisions can (and do) have different auxiliaries, and up-to-date checking is already done per-revision, so it makes more sense to perform up-to-date checks based on the current revisions's auxiliaries only. --- src/tools/compiletest/src/directives.rs | 14 +++---- src/tools/compiletest/src/directives/tests.rs | 41 ++++++------------- src/tools/compiletest/src/lib.rs | 21 +++++++--- 3 files changed, 35 insertions(+), 41 deletions(-) diff --git a/src/tools/compiletest/src/directives.rs b/src/tools/compiletest/src/directives.rs index e8b6b377bf40d..fe352b4fa3f21 100644 --- a/src/tools/compiletest/src/directives.rs +++ b/src/tools/compiletest/src/directives.rs @@ -8,7 +8,8 @@ use tracing::*; use crate::common::{CodegenBackend, Config, Debugger, FailMode, PassMode, RunFailMode, TestMode}; use crate::debuggers::{extract_cdb_version, extract_gdb_version}; -use crate::directives::auxiliary::{AuxProps, parse_and_update_aux}; +pub(crate) use crate::directives::auxiliary::AuxProps; +use crate::directives::auxiliary::parse_and_update_aux; use crate::directives::directive_names::{ KNOWN_DIRECTIVE_NAMES, KNOWN_HTMLDOCCK_DIRECTIVE_NAMES, KNOWN_JSONDOCCK_DIRECTIVE_NAMES, }; @@ -21,7 +22,7 @@ use crate::executor::{CollectedTestDesc, ShouldPanic}; use crate::util::static_regex; use crate::{fatal, help}; -pub(crate) mod auxiliary; +mod auxiliary; mod cfg; mod directive_names; mod file; @@ -44,10 +45,6 @@ impl DirectivesCache { /// the test. #[derive(Default)] pub(crate) struct EarlyProps { - /// Auxiliary crates that should be built and made available to this test. - /// Included in [`EarlyProps`] so that the indicated files can participate - /// in up-to-date checking. Building happens via [`TestProps::aux`] instead. - pub(crate) aux: AuxProps, pub(crate) revisions: Vec, } @@ -66,7 +63,6 @@ impl EarlyProps { file_directives, // (dummy comment to force args into vertical layout) &mut |ln: &DirectiveLine<'_>| { - parse_and_update_aux(config, ln, &mut props.aux); config.parse_and_update_revisions(ln, &mut props.revisions); }, ); @@ -1310,6 +1306,7 @@ pub(crate) fn make_test_description( file_directives: &FileDirectives<'_>, test_revision: Option<&str>, poisoned: &mut bool, + aux_props: &mut AuxProps, ) -> CollectedTestDesc { let mut ignore = false; let mut ignore_message = None; @@ -1327,6 +1324,9 @@ pub(crate) fn make_test_description( return; } + // 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 { diff --git a/src/tools/compiletest/src/directives/tests.rs b/src/tools/compiletest/src/directives/tests.rs index b683c8317e49b..b204b57403ace 100644 --- a/src/tools/compiletest/src/directives/tests.rs +++ b/src/tools/compiletest/src/directives/tests.rs @@ -3,8 +3,9 @@ use semver::Version; use crate::common::{Config, Debugger, TestMode}; use crate::directives::{ - DirectivesCache, EarlyProps, Edition, EditionRange, FileDirectives, extract_llvm_version, - extract_version_range, iter_directives, line_directive, parse_edition, parse_normalize_rule, + AuxProps, DirectivesCache, EarlyProps, Edition, EditionRange, FileDirectives, + extract_llvm_version, extract_version_range, iter_directives, line_directive, parse_edition, + parse_normalize_rule, }; use crate::executor::{CollectedTestDesc, ShouldPanic}; @@ -20,6 +21,7 @@ fn make_test_description( let mut poisoned = false; let file_directives = FileDirectives::from_file_contents(path, file_contents); + let mut aux_props = AuxProps::default(); let test = crate::directives::make_test_description( config, &cache, @@ -29,6 +31,7 @@ fn make_test_description( &file_directives, revision, &mut poisoned, + &mut aux_props, ); if poisoned { panic!("poisoned!"); @@ -225,7 +228,7 @@ fn cfg() -> ConfigBuilder { ConfigBuilder::default() } -fn parse_rs(config: &Config, contents: &str) -> EarlyProps { +fn parse_early_props(config: &Config, contents: &str) -> EarlyProps { let file_directives = FileDirectives::from_file_contents(Utf8Path::new("a.rs"), contents); EarlyProps::from_file_directives(config, &file_directives) } @@ -253,25 +256,7 @@ fn should_fail() { fn revisions() { let config: Config = cfg().build(); - assert_eq!(parse_rs(&config, "//@ revisions: a b c").revisions, vec!["a", "b", "c"],); -} - -#[test] -fn aux_build() { - let config: Config = cfg().build(); - - assert_eq!( - parse_rs( - &config, - r" - //@ aux-build: a.rs - //@ aux-build: b.rs - " - ) - .aux - .builds, - vec!["a.rs", "b.rs"], - ); + assert_eq!(parse_early_props(&config, "//@ revisions: a b c").revisions, vec!["a", "b", "c"],); } #[test] @@ -550,7 +535,7 @@ fn test_extract_version_range() { #[should_panic(expected = "duplicate revision: `rpass1` in line ` rpass1 rpass1`")] fn test_duplicate_revisions() { let config: Config = cfg().build(); - parse_rs(&config, "//@ revisions: rpass1 rpass1"); + parse_early_props(&config, "//@ revisions: rpass1 rpass1"); } #[test] @@ -559,14 +544,14 @@ fn test_duplicate_revisions() { )] fn test_assembly_mode_forbidden_revisions() { let config = cfg().mode("assembly").build(); - parse_rs(&config, "//@ revisions: CHECK"); + parse_early_props(&config, "//@ revisions: CHECK"); } #[test] #[should_panic(expected = "revision name `true` is not permitted")] fn test_forbidden_revisions() { let config = cfg().mode("ui").build(); - parse_rs(&config, "//@ revisions: true"); + parse_early_props(&config, "//@ revisions: true"); } #[test] @@ -575,7 +560,7 @@ fn test_forbidden_revisions() { )] fn test_codegen_mode_forbidden_revisions() { let config = cfg().mode("codegen").build(); - parse_rs(&config, "//@ revisions: CHECK"); + parse_early_props(&config, "//@ revisions: CHECK"); } #[test] @@ -584,7 +569,7 @@ fn test_codegen_mode_forbidden_revisions() { )] fn test_miropt_mode_forbidden_revisions() { let config = cfg().mode("mir-opt").build(); - parse_rs(&config, "//@ revisions: CHECK"); + parse_early_props(&config, "//@ revisions: CHECK"); } #[test] @@ -608,7 +593,7 @@ fn test_forbidden_revisions_allowed_in_non_filecheck_dir() { let content = format!("//@ revisions: {rev}"); for mode in modes { let config = cfg().mode(mode).build(); - parse_rs(&config, &content); + parse_early_props(&config, &content); } } } diff --git a/src/tools/compiletest/src/lib.rs b/src/tools/compiletest/src/lib.rs index 2c68a771169e2..798008ab432ed 100644 --- a/src/tools/compiletest/src/lib.rs +++ b/src/tools/compiletest/src/lib.rs @@ -41,7 +41,7 @@ use crate::common::{ CodegenBackend, CompareMode, Config, Debugger, PassMode, TestMode, TestPaths, UI_EXTENSIONS, expected_output_path, output_base_dir, output_relative_path, }; -use crate::directives::{DirectivesCache, FileDirectives}; +use crate::directives::{AuxProps, DirectivesCache, FileDirectives}; use crate::edition::parse_edition; use crate::executor::{CollectedTest, ColorConfig}; @@ -891,6 +891,11 @@ fn make_test(cx: &TestCollectorCx, collector: &mut TestCollector, testpaths: &Te // 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. @@ -903,13 +908,14 @@ fn make_test(cx: &TestCollectorCx, collector: &mut TestCollector, testpaths: &Te &file_directives, revision, &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.ignore && !cx.config.force_rerun - && is_up_to_date(cx, testpaths, &early_props, revision) + && 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. @@ -939,7 +945,7 @@ fn stamp_file_path(config: &Config, testpaths: &TestPaths, revision: Option<&str fn files_related_to_test( config: &Config, testpaths: &TestPaths, - props: &EarlyProps, + aux_props: &AuxProps, revision: Option<&str>, ) -> Vec { let mut related = vec![]; @@ -956,8 +962,11 @@ fn files_related_to_test( related.push(testpaths.file.clone()); } - for aux in props.aux.all_aux_path_strings() { + for aux in aux_props.all_aux_path_strings() { // FIXME(Zalathar): Perform all `auxiliary` path resolution in one place. + // FIXME(Zalathar): This only finds auxiliary files used _directly_ by + // the test file; if a transitive auxiliary is modified, the test might + // be treated as "up-to-date" even though it should run. let path = testpaths.file.parent().unwrap().join("auxiliary").join(aux); related.push(path); } @@ -982,7 +991,7 @@ fn files_related_to_test( fn is_up_to_date( cx: &TestCollectorCx, testpaths: &TestPaths, - props: &EarlyProps, + aux_props: &AuxProps, revision: Option<&str>, ) -> bool { let stamp_file_path = stamp_file_path(&cx.config, testpaths, revision); @@ -1003,7 +1012,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, props, revision) { + for path in files_related_to_test(&cx.config, testpaths, aux_props, revision) { inputs_stamp.add_path(&path); }