Stop UI tests if an unknown revision name is specified

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<Error> {
+pub fn load_errors(
+    testfile: &Path,
+    revision: Option<&str>,
+    all_revisions: &[Option<&str>],
+) -> Vec<Error> {
     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<Error> {
     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;
             }
         }

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<Config>,
     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(())
     }))
 }

src/tools/compiletest/src/runtest.rs

@@ -100,7 +100,14 @@ fn get_lib_name(lib: &str, aux_type: AuxType) -> Option<String> {
     }
 }
 
-pub fn run(config: Arc<Config>, testpaths: &TestPaths, revision: Option<&str>) {
+pub fn run(
+    config: Arc<Config>,
+    testpaths: &TestPaths,
+    revision: Option<&str>,
+    all_revisions: &[Option<String>],
+) {
+    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<Config>, 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<Config>, 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<MonoItem> = errors::load_errors(&self.testpaths.file, None)
+        let expected: Vec<MonoItem> = 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();