From 7c77d4a4fb734564d3d3f3146baffbef9469f084 Mon Sep 17 00:00:00 2001 From: Trevor Gross Date: Fri, 12 Apr 2024 23:22:28 -0400 Subject: [PATCH] Stop UI tests if an unknown revision name is specified --- src/tools/compiletest/src/errors.rs | 42 ++++++++++++++++++++-------- src/tools/compiletest/src/lib.rs | 16 ++++++----- src/tools/compiletest/src/runtest.rs | 30 +++++++++++++++----- 3 files changed, 63 insertions(+), 25 deletions(-) diff --git a/src/tools/compiletest/src/errors.rs b/src/tools/compiletest/src/errors.rs index 140c3aa0a64ec..7545098eb3c6b 100644 --- a/src/tools/compiletest/src/errors.rs +++ b/src/tools/compiletest/src/errors.rs @@ -74,7 +74,11 @@ enum WhichLine { /// /// If revision is not None, then we look /// for `//[X]~` instead, where `X` is the current revision. -pub fn load_errors(testfile: &Path, revision: Option<&str>) -> Vec { +pub fn load_errors( + testfile: &Path, + revision: Option<&str>, + all_revisions: &[Option<&str>], +) -> Vec { let rdr = BufReader::new(File::open(testfile).unwrap()); // `last_nonfollow_error` tracks the most recently seen @@ -90,16 +94,20 @@ pub fn load_errors(testfile: &Path, revision: Option<&str>) -> Vec { rdr.lines() .enumerate() .filter_map(|(line_num, line)| { - parse_expected(last_nonfollow_error, line_num + 1, &line.unwrap(), revision).map( - |(which, error)| { - match which { - FollowPrevious(_) => {} - _ => last_nonfollow_error = Some(error.line_num), - } - - error - }, + parse_expected( + last_nonfollow_error, + line_num + 1, + &line.unwrap(), + revision, + all_revisions, ) + .map(|(which, error)| { + if !matches!(which, FollowPrevious(_)) { + last_nonfollow_error = Some(error.line_num); + } + + error + }) }) .collect() } @@ -109,6 +117,7 @@ fn parse_expected( line_num: usize, line: &str, test_revision: Option<&str>, + all_revisions: &[Option<&str>], ) -> Option<(WhichLine, Error)> { // Matches comments like: // //~ @@ -125,7 +134,18 @@ fn parse_expected( match (test_revision, captures.name("revs")) { // Only error messages that contain our revision between the square brackets apply to us. (Some(test_revision), Some(revision_filters)) => { - if !revision_filters.as_str().split(',').any(|r| r == test_revision) { + let mut error_revisions = revision_filters.as_str().split(','); + // Make sure all revisions are valid + for rev in error_revisions.clone() { + if !all_revisions.contains(&Some(rev)) { + panic!( + "found unexpected revision '{rev}' at line {line_num}. \ + Expected one of {all_revisions:?}" + ); + } + } + + if !error_revisions.any(|r| r == test_revision) { return None; } } diff --git a/src/tools/compiletest/src/lib.rs b/src/tools/compiletest/src/lib.rs index 1624e2a60843a..2dbd89146fdc9 100644 --- a/src/tools/compiletest/src/lib.rs +++ b/src/tools/compiletest/src/lib.rs @@ -770,21 +770,21 @@ fn make_test( }; revisions - .into_iter() + .iter() .map(|revision| { let src_file = std::fs::File::open(&test_path).expect("open test file to parse ignores"); - let test_name = crate::make_test_name(&config, testpaths, revision); + let test_name = crate::make_test_name(&config, testpaths, *revision); let mut desc = make_test_description( - &config, cache, test_name, &test_path, src_file, revision, poisoned, + &config, cache, test_name, &test_path, src_file, *revision, poisoned, ); // Ignore tests that already run and are up to date with respect to inputs. if !config.force_rerun { - desc.ignore |= is_up_to_date(&config, testpaths, &early_props, revision, inputs); + desc.ignore |= is_up_to_date(&config, testpaths, &early_props, *revision, inputs); } test::TestDescAndFn { desc, - testfn: make_test_closure(config.clone(), testpaths, revision), + testfn: make_test_closure(config.clone(), testpaths, *revision, &revisions), } }) .collect() @@ -922,12 +922,14 @@ fn make_test_closure( config: Arc, testpaths: &TestPaths, revision: Option<&str>, + all_revisions: &[Option<&str>], ) -> test::TestFn { let config = config.clone(); let testpaths = testpaths.clone(); - let revision = revision.map(str::to_owned); + let revision = revision.map(ToOwned::to_owned); + let all_revisions: Vec<_> = all_revisions.iter().map(|v| (*v).map(ToOwned::to_owned)).collect(); test::DynTestFn(Box::new(move || { - runtest::run(config, &testpaths, revision.as_deref()); + runtest::run(config, &testpaths, revision.as_deref(), &all_revisions); Ok(()) })) } diff --git a/src/tools/compiletest/src/runtest.rs b/src/tools/compiletest/src/runtest.rs index 5212f1430d831..cb8984b23506d 100644 --- a/src/tools/compiletest/src/runtest.rs +++ b/src/tools/compiletest/src/runtest.rs @@ -100,7 +100,14 @@ fn get_lib_name(lib: &str, aux_type: AuxType) -> Option { } } -pub fn run(config: Arc, testpaths: &TestPaths, revision: Option<&str>) { +pub fn run( + config: Arc, + testpaths: &TestPaths, + revision: Option<&str>, + all_revisions: &[Option], +) { + let all_revisions_refs: Vec<_> = all_revisions.iter().map(Option::as_deref).collect(); + let all_revisions = &all_revisions_refs; match &*config.target { "arm-linux-androideabi" | "armv7-linux-androideabi" @@ -133,7 +140,7 @@ pub fn run(config: Arc, testpaths: &TestPaths, revision: Option<&str>) { props.incremental_dir = Some(incremental_dir(&config, testpaths, revision)); } - let cx = TestCx { config: &config, props: &props, testpaths, revision }; + let cx = TestCx { config: &config, props: &props, testpaths, revision, all_revisions }; create_dir_all(&cx.output_base_dir()) .with_context(|| { format!("failed to create output base directory {}", cx.output_base_dir().display()) @@ -155,6 +162,7 @@ pub fn run(config: Arc, testpaths: &TestPaths, revision: Option<&str>) { props: &revision_props, testpaths, revision: Some(revision), + all_revisions, }; rev_cx.run_revision(); } @@ -209,6 +217,8 @@ struct TestCx<'test> { props: &'test TestProps, testpaths: &'test TestPaths, revision: Option<&'test str>, + /// List of all revisions for the test, so we can reject any that are unexpected + all_revisions: &'test [Option<&'test str>], } enum ReadFrom { @@ -339,7 +349,8 @@ impl<'test> TestCx<'test> { self.check_no_compiler_crash(&proc_res, self.props.should_ice); let output_to_check = self.get_output(&proc_res); - let expected_errors = errors::load_errors(&self.testpaths.file, self.revision); + let expected_errors = + errors::load_errors(&self.testpaths.file, self.revision, self.all_revisions); if !expected_errors.is_empty() { if !self.props.error_patterns.is_empty() || !self.props.regex_error_patterns.is_empty() { @@ -417,7 +428,8 @@ impl<'test> TestCx<'test> { } // FIXME(#41968): Move this check to tidy? - if !errors::load_errors(&self.testpaths.file, self.revision).is_empty() { + if !errors::load_errors(&self.testpaths.file, self.revision, self.all_revisions).is_empty() + { self.fatal("compile-pass tests with expected warnings should be moved to ui/"); } } @@ -432,7 +444,8 @@ impl<'test> TestCx<'test> { } // FIXME(#41968): Move this check to tidy? - if !errors::load_errors(&self.testpaths.file, self.revision).is_empty() { + if !errors::load_errors(&self.testpaths.file, self.revision, self.all_revisions).is_empty() + { self.fatal("run-pass tests with expected warnings should be moved to ui/"); } @@ -1922,6 +1935,7 @@ impl<'test> TestCx<'test> { props: &aux_props, testpaths: &aux_testpaths, revision: self.revision, + all_revisions: self.all_revisions, }; // Create the directory for the stdout/stderr files. create_dir_all(aux_cx.output_base_dir()).unwrap(); @@ -2183,6 +2197,7 @@ impl<'test> TestCx<'test> { props: &aux_props, testpaths: &aux_testpaths, revision: self.revision, + all_revisions: self.all_revisions, }; // Create the directory for the stdout/stderr files. create_dir_all(aux_cx.output_base_dir()).unwrap(); @@ -3318,7 +3333,7 @@ impl<'test> TestCx<'test> { .map(|line| str_to_mono_item(line, true)) .collect(); - let expected: Vec = errors::load_errors(&self.testpaths.file, None) + let expected: Vec = errors::load_errors(&self.testpaths.file, None, &[]) .iter() .map(|e| str_to_mono_item(&e.msg[..], false)) .collect(); @@ -4207,7 +4222,8 @@ impl<'test> TestCx<'test> { ); } - let expected_errors = errors::load_errors(&self.testpaths.file, self.revision); + let expected_errors = + errors::load_errors(&self.testpaths.file, self.revision, self.all_revisions); if let WillExecute::Yes = should_run { let proc_res = self.exec_compiled_test();