From 0e53d66c4f3374ccb5fffb8fca3df68fda120848 Mon Sep 17 00:00:00 2001 From: Oli Scherer Date: Wed, 30 Aug 2023 16:07:42 +0000 Subject: [PATCH 1/3] Refactor `default_filter_by_arg` to be a list of checks that could filter the test. This makes adding more things easier --- src/lib.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/lib.rs b/src/lib.rs index ba051be8..601db3d0 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -155,7 +155,10 @@ pub fn default_any_file_filter(path: &Path, config: &Config) -> bool { return false; } - config.filter_files.is_empty() || contains_path(&config.filter_files) + if !config.filter_files.is_empty() && !contains_path(&config.filter_files) { + return false; + } + true } /// The default per-file config used by `run_tests`. From 04690e384b0bfa39585d857cee4fb76f3d48a6a9 Mon Sep 17 00:00:00 2001 From: Oli Scherer Date: Wed, 30 Aug 2023 16:20:05 +0000 Subject: [PATCH 2/3] Cache when a test passed successfully and skip it when run again --- src/config.rs | 5 +++ src/config/args.rs | 6 ++++ src/lib.rs | 35 +++++++++++++++++-- .../integrations/basic-bin/tests/ui_tests.rs | 1 + .../basic-fail-mode/tests/ui_tests.rs | 1 + tests/integrations/basic-fail/Cargo.stderr | 2 +- .../integrations/basic-fail/tests/ui_tests.rs | 2 +- .../basic-fail/tests/ui_tests_bless.rs | 1 + .../tests/ui_tests_invalid_program.rs | 1 + .../tests/ui_tests_invalid_program2.rs | 1 + tests/integrations/basic/tests/ui_tests.rs | 1 + 11 files changed, 51 insertions(+), 5 deletions(-) diff --git a/src/config.rs b/src/config.rs index 6779bab7..eb8abcff 100644 --- a/src/config.rs +++ b/src/config.rs @@ -53,6 +53,8 @@ pub struct Config { pub filter_files: Vec, /// Override the number of threads to use. pub threads: Option, + /// Rerun tests even if they passed previously + pub force_rerun: bool, } impl Config { @@ -94,6 +96,7 @@ impl Config { skip_files: Vec::new(), filter_files: Vec::new(), threads: None, + force_rerun: false, } } @@ -120,6 +123,7 @@ impl Config { let Args { ref filters, quiet: _, + force_rerun, check, bless, threads, @@ -130,6 +134,7 @@ impl Config { self.filter_files.extend_from_slice(filters); self.skip_files.extend_from_slice(skip); + self.force_rerun |= force_rerun; let bless = match (bless, check) { (_, true) => false, diff --git a/src/config/args.rs b/src/config/args.rs index 2d65dc7d..de959d39 100644 --- a/src/config/args.rs +++ b/src/config/args.rs @@ -18,6 +18,10 @@ pub struct Args { /// output. pub check: bool, + /// Rerun all tests, even if their test files are unchanged and they passed + /// in a previous run. + pub force_rerun: bool, + /// Whether to overwrite `.stderr` files on mismtach with the actual /// output. pub bless: bool, @@ -48,6 +52,8 @@ impl Args { self.check = true; } else if arg == "--bless" { self.bless = true; + } else if arg == "--force-rerun" { + self.force_rerun = true; } else if let Some(skip) = parse_value("--skip", &arg, &mut iter)? { self.skip.push(skip.into_owned()); } else if arg == "--help" { diff --git a/src/lib.rs b/src/lib.rs index 601db3d0..ef8f6976 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -148,8 +148,8 @@ pub fn default_file_filter(path: &Path, config: &Config) -> bool { /// /// To only include rust files see [`default_file_filter`]. pub fn default_any_file_filter(path: &Path, config: &Config) -> bool { - let path = path.display().to_string(); - let contains_path = |files: &[String]| files.iter().any(|f| path.contains(f)); + let path_str = path.display().to_string(); + let contains_path = |files: &[String]| files.iter().any(|f| path_str.contains(f)); if contains_path(&config.skip_files) { return false; @@ -158,7 +158,25 @@ pub fn default_any_file_filter(path: &Path, config: &Config) -> bool { if !config.filter_files.is_empty() && !contains_path(&config.filter_files) { return false; } - true + config.force_rerun || modified_since_last_successful_run(path, config) +} + +/// Returns `true` if the file was modified since the last time it was tested and passed successfully. +pub fn modified_since_last_successful_run(path: &Path, config: &Config) -> bool { + macro_rules! modified { + ($path:expr) => { + if let Ok(time) = std::fs::metadata(&$path).and_then(|m| m.modified()) { + time + } else { + // If we can't get a time, assume the file has been modified. + return true; + } + }; + } + let file_modified = modified!(path); + let test_suite = modified!(std::env::current_exe().unwrap()); + let check_file = modified!(config.out_dir.join(path.with_extension("timestamp"))); + (check_file < test_suite) || (check_file < file_modified) } /// The default per-file config used by `run_tests`. @@ -274,6 +292,10 @@ pub fn run_tests_generic( todo.push_back((entry, config)); } } else if file_filter(&path, config) { + // If we start a test, remove the marker that it was cached. + let check_file = config.out_dir.join(path.with_extension("timestamp")); + // The file may not exist, in this case we don't need to do anything. + let _ = std::fs::remove_file(check_file); let status = status_emitter.register_test(path); // Forward .rs files to the test workers. submit.send((status, config)).unwrap(); @@ -639,6 +661,13 @@ impl dyn TestStatus { run_rustfix( &stderr, &stdout, path, comments, revision, config, *mode, extra_args, )?; + + // Make sure the `default_file_filter` recognizes that the test has already been run successfully + // and avoid running it again. + let check_file = config.out_dir.join(path.with_extension("timestamp")); + std::fs::create_dir_all(check_file.parent().unwrap()).unwrap(); + std::fs::File::create(check_file).unwrap(); + Ok(TestOk::Ok) } diff --git a/tests/integrations/basic-bin/tests/ui_tests.rs b/tests/integrations/basic-bin/tests/ui_tests.rs index 2a030db9..f33423ed 100644 --- a/tests/integrations/basic-bin/tests/ui_tests.rs +++ b/tests/integrations/basic-bin/tests/ui_tests.rs @@ -9,6 +9,7 @@ fn main() -> ui_test::color_eyre::Result<()> { } else { OutputConflictHandling::Error("cargo test".to_string()) }, + force_rerun: true, ..Config::rustc("tests/actual_tests") }; config.stderr_filter("in ([0-9]m )?[0-9\\.]+s", ""); diff --git a/tests/integrations/basic-fail-mode/tests/ui_tests.rs b/tests/integrations/basic-fail-mode/tests/ui_tests.rs index 30b47abc..9f3fcbb4 100644 --- a/tests/integrations/basic-fail-mode/tests/ui_tests.rs +++ b/tests/integrations/basic-fail-mode/tests/ui_tests.rs @@ -13,6 +13,7 @@ fn main() -> ui_test::color_eyre::Result<()> { } else { OutputConflictHandling::Error("cargo test".to_string()) }, + force_rerun: true, ..Config::rustc("tests/actual_tests") }; config.stderr_filter("in ([0-9]m )?[0-9\\.]+s", ""); diff --git a/tests/integrations/basic-fail/Cargo.stderr b/tests/integrations/basic-fail/Cargo.stderr index ddb541a3..96e3ac0e 100644 --- a/tests/integrations/basic-fail/Cargo.stderr +++ b/tests/integrations/basic-fail/Cargo.stderr @@ -9,7 +9,7 @@ Caused by: thread 'main' panicked at 'invalid mode/result combo: yolo: Err(tests failed Location: - $DIR/src/lib.rs:LL:CC)', tests/ui_tests_bless.rs:52:18 + $DIR/src/lib.rs:LL:CC)', tests/ui_tests_bless.rs:53:18 note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace error: test failed, to rerun pass `--test ui_tests_bless` Error: failed to parse rustc version info: invalid_foobarlaksdfalsdfj diff --git a/tests/integrations/basic-fail/tests/ui_tests.rs b/tests/integrations/basic-fail/tests/ui_tests.rs index b51861cd..d289cfb3 100644 --- a/tests/integrations/basic-fail/tests/ui_tests.rs +++ b/tests/integrations/basic-fail/tests/ui_tests.rs @@ -8,7 +8,7 @@ fn main() -> ui_test::color_eyre::Result<()> { output_conflict_handling: OutputConflictHandling::Error( "DO NOT BLESS. These are meant to fail".into(), ), - + force_rerun: true, ..Config::rustc("tests/actual_tests") }; diff --git a/tests/integrations/basic-fail/tests/ui_tests_bless.rs b/tests/integrations/basic-fail/tests/ui_tests_bless.rs index 20f481ec..d0917f8a 100644 --- a/tests/integrations/basic-fail/tests/ui_tests_bless.rs +++ b/tests/integrations/basic-fail/tests/ui_tests_bless.rs @@ -26,6 +26,7 @@ fn main() -> ui_test::color_eyre::Result<()> { OutputConflictHandling::Error("cargo test".to_string()) }, mode, + force_rerun: true, ..Config::rustc(root_dir) }; diff --git a/tests/integrations/basic-fail/tests/ui_tests_invalid_program.rs b/tests/integrations/basic-fail/tests/ui_tests_invalid_program.rs index 4c2493a5..6bb8d366 100644 --- a/tests/integrations/basic-fail/tests/ui_tests_invalid_program.rs +++ b/tests/integrations/basic-fail/tests/ui_tests_invalid_program.rs @@ -7,6 +7,7 @@ fn main() -> ui_test::color_eyre::Result<()> { output_conflict_handling: OutputConflictHandling::Error( "DO NOT BLESS. These are meant to fail".into(), ), + force_rerun: true, ..Config::rustc("tests/actual_tests") }; diff --git a/tests/integrations/basic-fail/tests/ui_tests_invalid_program2.rs b/tests/integrations/basic-fail/tests/ui_tests_invalid_program2.rs index 97a3101e..fbed2884 100644 --- a/tests/integrations/basic-fail/tests/ui_tests_invalid_program2.rs +++ b/tests/integrations/basic-fail/tests/ui_tests_invalid_program2.rs @@ -8,6 +8,7 @@ fn main() -> ui_test::color_eyre::Result<()> { "DO NOT BLESS. These are meant to fail".into(), ), host: Some("foo".into()), + force_rerun: true, ..Config::rustc("tests/actual_tests") }; diff --git a/tests/integrations/basic/tests/ui_tests.rs b/tests/integrations/basic/tests/ui_tests.rs index 2d972ae0..26a4a3ce 100644 --- a/tests/integrations/basic/tests/ui_tests.rs +++ b/tests/integrations/basic/tests/ui_tests.rs @@ -9,6 +9,7 @@ fn main() -> ui_test::color_eyre::Result<()> { } else { OutputConflictHandling::Error("cargo test".to_string()) }, + force_rerun: true, ..Config::rustc("tests/actual_tests") }; config.stderr_filter("in ([0-9]m )?[0-9\\.]+s", ""); From fb9e54537ded15d3b4aa40d4cc879e5f2cdce184 Mon Sep 17 00:00:00 2001 From: Oli Scherer Date: Mon, 4 Sep 2023 10:14:47 +0000 Subject: [PATCH 3/3] Catch all possible cases of passing tests --- src/lib.rs | 22 ++++++++++++---------- 1 file changed, 12 insertions(+), 10 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index ef8f6976..6d4df502 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -295,15 +295,15 @@ pub fn run_tests_generic( // If we start a test, remove the marker that it was cached. let check_file = config.out_dir.join(path.with_extension("timestamp")); // The file may not exist, in this case we don't need to do anything. - let _ = std::fs::remove_file(check_file); + let _ = std::fs::remove_file(&check_file); let status = status_emitter.register_test(path); // Forward .rs files to the test workers. - submit.send((status, config)).unwrap(); + submit.send((status, config, check_file)).unwrap(); } } }, |receive, finished_files_sender| -> Result<()> { - for (status, config) in receive { + for (status, config, check_file) in receive { let path = status.path(); let file_contents = std::fs::read(path).unwrap(); let mut config = config.clone(); @@ -311,7 +311,15 @@ pub fn run_tests_generic( let result = match std::panic::catch_unwind(|| { parse_and_test_file(&build_manager, &status, config, file_contents) }) { - Ok(Ok(res)) => res, + Ok(Ok(res)) => { + if res.iter().all(|res| matches!(res.result, Ok(TestOk::Ok))) { + // Make sure the `default_file_filter` recognizes that the test has already been run successfully + // and avoid running it again. + std::fs::create_dir_all(check_file.parent().unwrap()).unwrap(); + std::fs::File::create(check_file).unwrap(); + } + res + } Ok(Err(err)) => { finished_files_sender.send(TestRun { result: Err(err), @@ -662,12 +670,6 @@ impl dyn TestStatus { &stderr, &stdout, path, comments, revision, config, *mode, extra_args, )?; - // Make sure the `default_file_filter` recognizes that the test has already been run successfully - // and avoid running it again. - let check_file = config.out_dir.join(path.with_extension("timestamp")); - std::fs::create_dir_all(check_file.parent().unwrap()).unwrap(); - std::fs::File::create(check_file).unwrap(); - Ok(TestOk::Ok) }