From e37dfa6d91f65f4b97232bea1d587591cb2982e5 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Mon, 30 May 2022 10:27:41 +0200 Subject: [PATCH 1/8] ui_test: support multiple filters --- tests/compiletest.rs | 5 +++-- ui_test/src/lib.rs | 11 ++++++----- ui_test/src/tests.rs | 2 +- 3 files changed, 10 insertions(+), 8 deletions(-) diff --git a/tests/compiletest.rs b/tests/compiletest.rs index 4be658e86cb..30727226427 100644 --- a/tests/compiletest.rs +++ b/tests/compiletest.rs @@ -47,7 +47,8 @@ fn run_tests(mode: Mode, path: &str, target: Option) { (true, true) => panic!("cannot use MIRI_BLESS and MIRI_SKIP_UI_CHECKS at the same time"), }; - let path_filter = std::env::args().skip(1).next(); + // Pass on all arguments as filters. + let path_filter = std::env::args().skip(1); let config = Config { args: flags, @@ -56,7 +57,7 @@ fn run_tests(mode: Mode, path: &str, target: Option) { stdout_filters: STDOUT.clone(), root_dir: PathBuf::from(path), mode, - path_filter, + path_filter: path_filter.collect(), program: miri_path(), output_conflict_handling, }; diff --git a/ui_test/src/lib.rs b/ui_test/src/lib.rs index 81560db6dff..866a9fe46ac 100644 --- a/ui_test/src/lib.rs +++ b/ui_test/src/lib.rs @@ -30,8 +30,8 @@ pub struct Config { pub mode: Mode, pub program: PathBuf, pub output_conflict_handling: OutputConflictHandling, - /// Only run tests with this string in their path/name - pub path_filter: Option, + /// Only run tests with one of these strings in their path/name + pub path_filter: Vec, } #[derive(Debug)] @@ -77,12 +77,13 @@ pub fn run_tests(config: Config) { if !path.extension().map(|ext| ext == "rs").unwrap_or(false) { continue; } - if let Some(path_filter) = &config.path_filter { - if !path.display().to_string().contains(path_filter) { + if !config.path_filter.is_empty() { + let path_display = path.display().to_string(); + if !config.path_filter.iter().any(|filter| path_display.contains(filter)) { ignored.fetch_add(1, Ordering::Relaxed); eprintln!( "{} .. {}", - path.display(), + path_display, "ignored (command line filter)".yellow() ); continue; diff --git a/ui_test/src/tests.rs b/ui_test/src/tests.rs index 5485e6b4f26..b2544e68ada 100644 --- a/ui_test/src/tests.rs +++ b/ui_test/src/tests.rs @@ -10,7 +10,7 @@ fn config() -> Config { stdout_filters: vec![], root_dir: PathBuf::from("."), mode: Mode::Fail, - path_filter: None, + path_filter: vec![], program: PathBuf::from("cake"), output_conflict_handling: OutputConflictHandling::Error, } From 8694e656bee538fb3d1db0bf4fb6625943cd4087 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Mon, 30 May 2022 10:28:07 +0200 Subject: [PATCH 2/8] test mir-opt-level=4 again on run tests --- ci.sh | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/ci.sh b/ci.sh index bdca26fd049..080bd9204db 100755 --- a/ci.sh +++ b/ci.sh @@ -26,8 +26,7 @@ function run_tests { # optimizations up all the way). # Optimizations change diagnostics (mostly backtraces), so we don't check them #FIXME(#2155): we want to only run the pass and panic tests here, not the fail tests. - #MIRIFLAGS="-O -Zmir-opt-level=4" MIRI_SKIP_UI_CHECKS=1 ./miri test --locked - true + MIRIFLAGS="-O -Zmir-opt-level=4" MIRI_SKIP_UI_CHECKS=1 ./miri test --locked -- tests/{run-pass,run-fail} fi # On Windows, there is always "python", not "python3" or "python2". From 4e91c2e368c7e96b85c12cef61363eb9eccac621 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Mon, 30 May 2022 10:29:02 +0200 Subject: [PATCH 3/8] ui_test: printing more consistent with compiletest distinguish "ignored" from "filtered out" --- ui_test/src/lib.rs | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/ui_test/src/lib.rs b/ui_test/src/lib.rs index 866a9fe46ac..b7488d817a6 100644 --- a/ui_test/src/lib.rs +++ b/ui_test/src/lib.rs @@ -61,6 +61,7 @@ pub fn run_tests(config: Config) { let failures = Mutex::new(vec![]); let succeeded = AtomicUsize::default(); let ignored = AtomicUsize::default(); + let filtered = AtomicUsize::default(); crossbeam::scope(|s| { for _ in 0..std::thread::available_parallelism().unwrap().get() { @@ -80,12 +81,7 @@ pub fn run_tests(config: Config) { if !config.path_filter.is_empty() { let path_display = path.display().to_string(); if !config.path_filter.iter().any(|filter| path_display.contains(filter)) { - ignored.fetch_add(1, Ordering::Relaxed); - eprintln!( - "{} .. {}", - path_display, - "ignored (command line filter)".yellow() - ); + filtered.fetch_add(1, Ordering::Relaxed); continue; } } @@ -93,7 +89,7 @@ pub fn run_tests(config: Config) { // Ignore file if only/ignore rules do (not) apply if ignore_file(&comments, &target) { ignored.fetch_add(1, Ordering::Relaxed); - eprintln!("{} .. {}", path.display(), "ignored".yellow()); + eprintln!("{} ... {}", path.display(), "ignored".yellow()); continue; } // Run the test for all revisions @@ -126,6 +122,7 @@ pub fn run_tests(config: Config) { let failures = failures.into_inner().unwrap(); let succeeded = succeeded.load(Ordering::Relaxed); let ignored = ignored.load(Ordering::Relaxed); + let filtered = filtered.load(Ordering::Relaxed); if !failures.is_empty() { for (path, miri, revision, errors) in &failures { eprintln!(); @@ -169,19 +166,22 @@ pub fn run_tests(config: Config) { } } eprintln!( - "{} tests failed, {} tests passed, {} ignored", + "test result: {}. {} tests failed, {} tests passed, {} ignored, {} filtered out", + "FAIL".red(), failures.len().to_string().red().bold(), succeeded.to_string().green(), - ignored.to_string().yellow() + ignored.to_string().yellow(), + filtered.to_string().yellow(), ); std::process::exit(1); } eprintln!(); eprintln!( - "test result: {}. {} tests passed, {} ignored", + "test result: {}. {} tests passed, {} ignored, {} filtered out", "ok".green(), succeeded.to_string().green(), - ignored.to_string().yellow() + ignored.to_string().yellow(), + filtered.to_string().yellow(), ); eprintln!(); } From 86e53f41b099f48e25fe53aa2d13101f17fae049 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Mon, 30 May 2022 10:31:12 +0200 Subject: [PATCH 4/8] print reason for ignoring --- ui_test/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ui_test/src/lib.rs b/ui_test/src/lib.rs index b7488d817a6..305b4721e96 100644 --- a/ui_test/src/lib.rs +++ b/ui_test/src/lib.rs @@ -89,7 +89,7 @@ pub fn run_tests(config: Config) { // Ignore file if only/ignore rules do (not) apply if ignore_file(&comments, &target) { ignored.fetch_add(1, Ordering::Relaxed); - eprintln!("{} ... {}", path.display(), "ignored".yellow()); + eprintln!("{} ... {}", path.display(), "ignored (in-test comment)".yellow()); continue; } // Run the test for all revisions From b9d79a25baa9e2fefa5ee56a93517a7f96904c9c Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Mon, 30 May 2022 12:20:12 +0200 Subject: [PATCH 5/8] also 'check' the test suite --- miri | 2 +- ui_test/src/lib.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/miri b/miri index 352aed530b1..5c9cb81885c 100755 --- a/miri +++ b/miri @@ -115,7 +115,7 @@ install|install-debug) ;; check|check-debug) # Check, and let caller control flags. - cargo check $CARGO_BUILD_FLAGS --manifest-path "$MIRIDIR"/Cargo.toml "$@" + cargo check $CARGO_BUILD_FLAGS --manifest-path "$MIRIDIR"/Cargo.toml --all-targets "$@" cargo check $CARGO_BUILD_FLAGS --manifest-path "$MIRIDIR"/cargo-miri/Cargo.toml "$@" ;; build|build-debug) diff --git a/ui_test/src/lib.rs b/ui_test/src/lib.rs index 305b4721e96..90c475d4bed 100644 --- a/ui_test/src/lib.rs +++ b/ui_test/src/lib.rs @@ -98,7 +98,7 @@ pub fn run_tests(config: Config) { { let (m, errors) = run_test(&path, &config, &target, &revision, &comments); - // Using `format` to prevent messages from threads from getting intermingled. + // Using a single `eprintln!` to prevent messages from threads from getting intermingled. let mut msg = format!("{} ", path.display()); if !revision.is_empty() { write!(msg, "(revision `{revision}`) ").unwrap(); From 80bf204848eb2c97effa717e8e4f876f6545d55f Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Mon, 30 May 2022 12:28:13 +0200 Subject: [PATCH 6/8] don't configure the same regex twice --- tests/compiletest.rs | 2 -- ui_test/src/lib.rs | 20 ++++++++++---------- ui_test/src/tests.rs | 4 ++-- 3 files changed, 12 insertions(+), 14 deletions(-) diff --git a/tests/compiletest.rs b/tests/compiletest.rs index 30727226427..937ae0d9d7d 100644 --- a/tests/compiletest.rs +++ b/tests/compiletest.rs @@ -106,8 +106,6 @@ macro_rules! regexes { r"\\" => "/", // erase platform file paths "sys/[a-z]+/" => "sys/PLATFORM/", - // erase error annotations in tests - r"\s*//~.*" => "", } fn ui(mode: Mode, path: &str) { diff --git a/ui_test/src/lib.rs b/ui_test/src/lib.rs index 90c475d4bed..648efb1f471 100644 --- a/ui_test/src/lib.rs +++ b/ui_test/src/lib.rs @@ -231,6 +231,11 @@ fn run_test( } let output = miri.output().expect("could not execute miri"); let mut errors = config.mode.ok(output.status); + // Always remove annotation comments from stderr. + let annotations = Regex::new(r"\s*//~.*").unwrap(); + let stderr = std::str::from_utf8(&output.stderr).unwrap(); + let stderr = annotations.replace_all(stderr, ""); + let stdout = std::str::from_utf8(&output.stdout).unwrap(); // Check output files (if any) let revised = |extension: &str| { if revision.is_empty() { @@ -241,7 +246,7 @@ fn run_test( }; // Check output files against actual output check_output( - &output.stderr, + &stderr, path, &mut errors, revised("stderr"), @@ -251,7 +256,7 @@ fn run_test( comments, ); check_output( - &output.stdout, + &stdout, path, &mut errors, revised("stdout"), @@ -261,21 +266,17 @@ fn run_test( comments, ); // Check error annotations in the source against output - check_annotations(&output.stderr, &mut errors, config, revision, comments); + check_annotations(&stderr, &mut errors, config, revision, comments); (miri, errors) } fn check_annotations( - unnormalized_stderr: &[u8], + unnormalized_stderr: &str, errors: &mut Errors, config: &Config, revision: &str, comments: &Comments, ) { - let unnormalized_stderr = std::str::from_utf8(unnormalized_stderr).unwrap(); - // erase annotations from the stderr so they don't match themselves - let annotations = Regex::new(r"\s*//~.*").unwrap(); - let unnormalized_stderr = annotations.replace(unnormalized_stderr, ""); let mut found_annotation = false; if let Some((ref error_pattern, definition_line)) = comments.error_pattern { if !unnormalized_stderr.contains(error_pattern) { @@ -313,7 +314,7 @@ fn check_annotations( } fn check_output( - output: &[u8], + output: &str, path: &Path, errors: &mut Errors, kind: String, @@ -322,7 +323,6 @@ fn check_output( config: &Config, comments: &Comments, ) { - let output = std::str::from_utf8(&output).unwrap(); let output = normalize(path, output, filters, comments); let path = output_path(path, comments, kind, target); match config.output_conflict_handling { diff --git a/ui_test/src/tests.rs b/ui_test/src/tests.rs index b2544e68ada..7b25eaeeafe 100644 --- a/ui_test/src/tests.rs +++ b/ui_test/src/tests.rs @@ -42,9 +42,9 @@ fn main() { note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace error: aborting due to previous error "; - check_annotations(unnormalized_stderr.as_bytes(), &mut errors, &config, "", &comments); + check_annotations(unnormalized_stderr, &mut errors, &config, "", &comments); match &errors[..] { [Error::PatternNotFound { .. }] => {} - _ => panic!("{:#?}", errors), + _ => panic!("not the expected error: {:#?}", errors), } } From 4d80880854b2ae832d9e490f0d761bbc5170c3bb Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Mon, 30 May 2022 12:32:49 +0200 Subject: [PATCH 7/8] fmt --- ui_test/src/lib.rs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/ui_test/src/lib.rs b/ui_test/src/lib.rs index 648efb1f471..37af27dcfb1 100644 --- a/ui_test/src/lib.rs +++ b/ui_test/src/lib.rs @@ -89,7 +89,11 @@ pub fn run_tests(config: Config) { // Ignore file if only/ignore rules do (not) apply if ignore_file(&comments, &target) { ignored.fetch_add(1, Ordering::Relaxed); - eprintln!("{} ... {}", path.display(), "ignored (in-test comment)".yellow()); + eprintln!( + "{} ... {}", + path.display(), + "ignored (in-test comment)".yellow() + ); continue; } // Run the test for all revisions From 962957f11f456949395535037b45b98d18bbcf19 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Mon, 30 May 2022 12:56:34 +0200 Subject: [PATCH 8/8] make it possible to test more of ui_test --- ui_test/src/lib.rs | 34 ++++++++++++++++++++++++++++------ ui_test/src/tests.rs | 15 +++++++++------ 2 files changed, 37 insertions(+), 12 deletions(-) diff --git a/ui_test/src/lib.rs b/ui_test/src/lib.rs index 37af27dcfb1..6052efe02e0 100644 --- a/ui_test/src/lib.rs +++ b/ui_test/src/lib.rs @@ -235,11 +235,34 @@ fn run_test( } let output = miri.output().expect("could not execute miri"); let mut errors = config.mode.ok(output.status); + check_test_result( + path, + config, + target, + revision, + comments, + &mut errors, + &output.stdout, + &output.stderr, + ); + (miri, errors) +} + +fn check_test_result( + path: &Path, + config: &Config, + target: &str, + revision: &str, + comments: &Comments, + errors: &mut Errors, + stdout: &[u8], + stderr: &[u8], +) { // Always remove annotation comments from stderr. let annotations = Regex::new(r"\s*//~.*").unwrap(); - let stderr = std::str::from_utf8(&output.stderr).unwrap(); + let stderr = std::str::from_utf8(stderr).unwrap(); let stderr = annotations.replace_all(stderr, ""); - let stdout = std::str::from_utf8(&output.stdout).unwrap(); + let stdout = std::str::from_utf8(stdout).unwrap(); // Check output files (if any) let revised = |extension: &str| { if revision.is_empty() { @@ -252,7 +275,7 @@ fn run_test( check_output( &stderr, path, - &mut errors, + errors, revised("stderr"), target, &config.stderr_filters, @@ -262,7 +285,7 @@ fn run_test( check_output( &stdout, path, - &mut errors, + errors, revised("stdout"), target, &config.stdout_filters, @@ -270,8 +293,7 @@ fn run_test( comments, ); // Check error annotations in the source against output - check_annotations(&stderr, &mut errors, config, revision, comments); - (miri, errors) + check_annotations(&stderr, errors, config, revision, comments); } fn check_annotations( diff --git a/ui_test/src/tests.rs b/ui_test/src/tests.rs index 7b25eaeeafe..d0ef1195d88 100644 --- a/ui_test/src/tests.rs +++ b/ui_test/src/tests.rs @@ -1,6 +1,6 @@ use std::path::{Path, PathBuf}; -use super::{check_annotations, Comments, Config, Error, Mode, OutputConflictHandling}; +use super::*; fn config() -> Config { Config { @@ -8,7 +8,7 @@ fn config() -> Config { target: None, stderr_filters: vec![], stdout_filters: vec![], - root_dir: PathBuf::from("."), + root_dir: PathBuf::from("$RUSTROOT"), mode: Mode::Fail, path_filter: vec![], program: PathBuf::from("cake"), @@ -25,10 +25,12 @@ fn main() { let _x: &i32 = unsafe { mem::transmute(16usize) }; //~ ERROR encountered a dangling reference (address $HEX is unallocated) } "; - let comments = Comments::parse(Path::new(""), s); + let path = Path::new("$DIR/"); + let comments = Comments::parse(&path, s); let mut errors = vec![]; let config = config(); - let unnormalized_stderr = r" + // Crucially, the intended error string *does* appear in this output, as a quote of the comment itself. + let stderr = br" error: Undefined Behavior: type validation failed: encountered a dangling reference (address 0x10 is unallocated) --> tests/compile-fail/validity/dangling_ref1.rs:6:29 | @@ -42,9 +44,10 @@ fn main() { note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace error: aborting due to previous error "; - check_annotations(unnormalized_stderr, &mut errors, &config, "", &comments); + check_test_result(&path, &config, "", "", &comments, &mut errors, /*stdout*/ br"", stderr); + // The "OutputDiffers" is because we cannot open the .rs file match &errors[..] { - [Error::PatternNotFound { .. }] => {} + [Error::OutputDiffers { .. }, Error::PatternNotFound { .. }] => {} _ => panic!("not the expected error: {:#?}", errors), } }