Cleanup unused revision functionality from compiletest (rust-lang#2109)

Author: zhassan-aws

tools/compiletest/src/common.rs

@@ -9,7 +9,6 @@ use std::fmt;
 use std::path::{Path, PathBuf};
 use std::str::FromStr;
 
-use crate::util::PathBufExt;
 use std::time::Duration;
 use test::ColorConfig;
 
@@ -153,22 +152,21 @@ pub fn output_relative_path(config: &Config, relative_dir: &Path) -> PathBuf {
     config.build_base.join(relative_dir)
 }
 
-/// Generates a unique name for the test, such as `testname.revision`.
-pub fn output_testname_unique(testpaths: &TestPaths, revision: Option<&str>) -> PathBuf {
-    PathBuf::from(&testpaths.file.file_stem().unwrap()).with_extra_extension(revision.unwrap_or(""))
+/// Generates a unique name for the test.
+pub fn output_testname_unique(testpaths: &TestPaths) -> PathBuf {
+    PathBuf::from(&testpaths.file.file_stem().unwrap())
 }
 
 /// Absolute path to the directory where all output for the given
-/// test/revision should reside. Example:
-///   /path/to/build/host-triple/test/ui/relative/testname.revision/
-pub fn output_base_dir(config: &Config, testpaths: &TestPaths, revision: Option<&str>) -> PathBuf {
-    output_relative_path(config, &testpaths.relative_dir)
-        .join(output_testname_unique(testpaths, revision))
+/// test should reside. Example:
+///   /path/to/build/host-triple/test/ui/relative/testname/
+pub fn output_base_dir(config: &Config, testpaths: &TestPaths) -> PathBuf {
+    output_relative_path(config, &testpaths.relative_dir).join(output_testname_unique(testpaths))
 }
 
 /// Absolute path to the base filename used as output for the given
-/// test/revision. Example:
-///   /path/to/build/host-triple/test/ui/relative/testname.revision.mode/testname
-pub fn output_base_name(config: &Config, testpaths: &TestPaths, revision: Option<&str>) -> PathBuf {
-    output_base_dir(config, testpaths, revision).join(testpaths.file.file_stem().unwrap())
+/// test. Example:
+///   /path/to/build/host-triple/test/ui/relative/testname.mode/testname
+pub fn output_base_name(config: &Config, testpaths: &TestPaths) -> PathBuf {
+    output_base_dir(config, testpaths).join(testpaths.file.file_stem().unwrap())
 }

tools/compiletest/src/header.rs

@@ -34,27 +34,20 @@ impl TestProps {
         }
     }
 
-    pub fn from_file(testfile: &Path, cfg: Option<&str>, config: &Config) -> Self {
+    pub fn from_file(testfile: &Path, config: &Config) -> Self {
         let mut props = TestProps::new();
-        props.load_from(testfile, cfg, config);
+        props.load_from(testfile, config);
 
         props
     }
 
-    /// Loads properties from `testfile` into `props`. If a property is
-    /// tied to a particular revision `foo` (indicated by writing
-    /// `//[foo]`), then the property is ignored unless `cfg` is
-    /// `Some("foo")`.
-    fn load_from(&mut self, testfile: &Path, cfg: Option<&str>, config: &Config) {
+    /// Loads properties from `testfile` into `props`.
+    fn load_from(&mut self, testfile: &Path, config: &Config) {
         let mut has_edition = false;
         if !testfile.is_dir() {
             let file = File::open(testfile).unwrap();
 
-            iter_header(testfile, file, &mut |revision, ln| {
-                if revision.is_some() && revision != cfg {
-                    return;
-                }
-
+            iter_header(testfile, file, &mut |ln| {
                 if let Some(flags) = config.parse_compile_flags(ln) {
                     self.compile_flags.extend(flags.split_whitespace().map(|s| s.to_owned()));
                 }
@@ -93,7 +86,7 @@ impl TestProps {
     }
 }
 
-fn iter_header<R: Read>(testfile: &Path, rdr: R, it: &mut dyn FnMut(Option<&str>, &str)) {
+fn iter_header<R: Read>(testfile: &Path, rdr: R, it: &mut dyn FnMut(&str)) {
     if testfile.is_dir() {
         return;
     }
@@ -116,7 +109,7 @@ fn iter_header<R: Read>(testfile: &Path, rdr: R, it: &mut dyn FnMut(Option<&str>
         if ln.starts_with("fn") || ln.starts_with("mod") {
             return;
         } else if let Some(rest) = ln.strip_prefix(comment) {
-            it(None, rest.trim_start());
+            it(rest.trim_start());
         }
     }
 }
@@ -186,7 +179,6 @@ pub fn make_test_description<R: Read>(
     name: test::TestName,
     path: &Path,
     src: R,
-    cfg: Option<&str>,
 ) -> test::TestDesc {
     let mut ignore = false;
     let mut should_fail = false;
@@ -223,10 +215,7 @@ pub fn make_test_description<R: Read>(
         should_fail = true;
     }
 
-    iter_header(path, src, &mut |revision, ln| {
-        if revision.is_some() && revision != cfg {
-            return;
-        }
+    iter_header(path, src, &mut |ln| {
         should_fail |= config.parse_name_directive(ln, "should-fail");
     });
 

tools/compiletest/src/main.rs

@@ -369,7 +369,7 @@ fn collect_expected_tests_from_dir(
             fs::create_dir_all(&build_dir.join(file_path.file_stem().unwrap())).unwrap();
             let paths =
                 TestPaths { file: file_path, relative_dir: relative_dir_path.to_path_buf() };
-            tests.extend(make_test(config, &paths, inputs));
+            tests.push(make_test(config, &paths, inputs));
         } else if file_path.is_dir() {
             // recurse on subdirectory
             let relative_file_path = relative_dir_path.join(file.file_name());
@@ -426,7 +426,7 @@ fn collect_exec_tests_from_dir(
         // Create directory for test and add it to the tests to be run
         fs::create_dir_all(&build_dir.join(file_path.file_stem().unwrap())).unwrap();
         let paths = TestPaths { file: file_path, relative_dir: relative_dir_path.to_path_buf() };
-        tests.extend(make_test(config, &paths, inputs));
+        tests.push(make_test(config, &paths, inputs));
     }
     Ok(())
 }
@@ -457,7 +457,7 @@ fn collect_rs_tests_from_dir(
             debug!("found test file: {:?}", file_path.display());
             let paths =
                 TestPaths { file: file_path, relative_dir: relative_dir_path.to_path_buf() };
-            tests.extend(make_test(config, &paths, inputs))
+            tests.push(make_test(config, &paths, inputs))
         } else if file_path.is_dir() {
             let relative_file_path = relative_dir_path.join(file.file_name());
             if &file_name != "auxiliary" {
@@ -484,39 +484,25 @@ pub fn is_test(file_name: &OsString) -> bool {
     !invalid_prefixes.iter().any(|p| file_name.starts_with(p))
 }
 
-fn make_test(config: &Config, testpaths: &TestPaths, inputs: &Stamp) -> Vec<test::TestDescAndFn> {
+fn make_test(config: &Config, testpaths: &TestPaths, inputs: &Stamp) -> test::TestDescAndFn {
     let test_path = PathBuf::from(&testpaths.file);
-    let revisions = vec![None];
-
-    revisions
-        .into_iter()
-        .map(|revision: Option<&String>| {
-            let src_file =
-                std::fs::File::open(&test_path).expect("open test file to parse ignores");
-            let cfg = revision.map(|v| &**v);
-            let test_name = crate::make_test_name(config, testpaths, revision);
-            let mut desc = make_test_description(config, test_name, &test_path, src_file, cfg);
-            // 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, revision.map(|s| s.as_str()), inputs);
-            }
-            test::TestDescAndFn { desc, testfn: make_test_closure(config, testpaths, revision) }
-        })
-        .collect()
+
+    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);
+    let mut desc = make_test_description(config, test_name, &test_path, src_file);
+    // 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, inputs);
+    }
+    test::TestDescAndFn { desc, testfn: make_test_closure(config, testpaths) }
 }
 
-fn stamp(config: &Config, testpaths: &TestPaths, revision: Option<&str>) -> PathBuf {
-    output_base_dir(config, testpaths, revision).join("stamp")
+fn stamp(config: &Config, testpaths: &TestPaths) -> PathBuf {
+    output_base_dir(config, testpaths).join("stamp")
 }
 
-fn is_up_to_date(
-    config: &Config,
-    testpaths: &TestPaths,
-    revision: Option<&str>,
-    inputs: &Stamp,
-) -> bool {
-    let stamp_name = stamp(config, testpaths, revision);
+fn is_up_to_date(config: &Config, testpaths: &TestPaths, inputs: &Stamp) -> bool {
+    let stamp_name = stamp(config, testpaths);
     // Check timestamps.
     let inputs = inputs.clone();
 
@@ -557,36 +543,22 @@ impl Stamp {
     }
 }
 
-fn make_test_name(
-    config: &Config,
-    testpaths: &TestPaths,
-    revision: Option<&String>,
-) -> test::TestName {
+fn make_test_name(config: &Config, testpaths: &TestPaths) -> test::TestName {
     // Convert a complete path to something like
     //
     //    ui/foo/bar/baz.rs
     let path = PathBuf::from(config.src_base.file_name().unwrap())
         .join(&testpaths.relative_dir)
         .join(&testpaths.file.file_name().unwrap());
 
-    test::DynTestName(format!(
-        "[{}] {}{}",
-        config.mode,
-        path.display(),
-        revision.map_or("".to_string(), |rev| format!("#{rev}"))
-    ))
+    test::DynTestName(format!("[{}] {}", config.mode, path.display()))
 }
 
-fn make_test_closure(
-    config: &Config,
-    testpaths: &TestPaths,
-    revision: Option<&String>,
-) -> test::TestFn {
+fn make_test_closure(config: &Config, testpaths: &TestPaths) -> test::TestFn {
     let config = config.clone();
     let testpaths = testpaths.clone();
-    let revision = revision.cloned();
     test::DynTestFn(Box::new(move || {
-        runtest::run(config, &testpaths, revision.as_deref());
+        runtest::run(config, &testpaths);
         Ok(())
     }))
 }

tools/compiletest/src/runtest.rs

@@ -46,17 +46,17 @@ pub fn dylib_env_var() -> &'static str {
     if cfg!(target_os = "macos") { "DYLD_LIBRARY_PATH" } else { "LD_LIBRARY_PATH" }
 }
 
-pub fn run(config: Config, testpaths: &TestPaths, revision: Option<&str>) {
+pub fn run(config: Config, testpaths: &TestPaths) {
     if config.verbose {
         // We're going to be dumping a lot of info. Start on a new line.
         print!("\n\n");
     }
     debug!("running {:?}", testpaths.file.display());
-    let props = TestProps::from_file(&testpaths.file, revision, &config);
+    let props = TestProps::from_file(&testpaths.file, &config);
 
-    let cx = TestCx { config: &config, props: &props, testpaths, revision };
+    let cx = TestCx { config: &config, props: &props, testpaths };
     create_dir_all(&cx.output_base_dir()).unwrap();
-    cx.run_revision();
+    cx.run();
     cx.create_stamp();
 }
 
@@ -65,13 +65,11 @@ struct TestCx<'test> {
     config: &'test Config,
     props: &'test TestProps,
     testpaths: &'test TestPaths,
-    revision: Option<&'test str>,
 }
 
 impl<'test> TestCx<'test> {
-    /// Code executed for each revision in turn (or, if there are no
-    /// revisions, exactly once, with revision == None).
-    fn run_revision(&self) {
+    /// Code executed
+    fn run(&self) {
         match self.config.mode {
             Kani => self.run_kani_test(),
             KaniFixme => self.run_kani_test(),
@@ -128,10 +126,8 @@ impl<'test> TestCx<'test> {
     }
 
     fn dump_output(&self, out: &str, err: &str) {
-        let revision = if let Some(r) = self.revision { format!("{r}.") } else { String::new() };
-
-        self.dump_output_file(out, &format!("{revision}out"));
-        self.dump_output_file(err, &format!("{revision}err"));
+        self.dump_output_file(out, "out");
+        self.dump_output_file(err, "err");
         self.maybe_dump_to_stdout(out, err);
     }
 
@@ -141,29 +137,23 @@ impl<'test> TestCx<'test> {
     }
 
     /// Creates a filename for output with the given extension.
-    /// E.g., `/.../testname.revision.mode/testname.extension`.
+    /// E.g., `/.../testname.mode/testname.extension`.
     fn make_out_name(&self, extension: &str) -> PathBuf {
         self.output_base_name().with_extension(extension)
     }
 
-    /// The revision, ignored for incremental compilation since it wants all revisions in
-    /// the same directory.
-    fn safe_revision(&self) -> Option<&str> {
-        self.revision
-    }
-
     /// Gets the absolute path to the directory where all output for the given
-    /// test/revision should reside.
-    /// E.g., `/path/to/build/host-triple/test/ui/relative/testname.revision.mode/`.
+    /// test should reside.
+    /// E.g., `/path/to/build/host-triple/test/ui/relative/testname.mode/`.
     fn output_base_dir(&self) -> PathBuf {
-        output_base_dir(self.config, self.testpaths, self.safe_revision())
+        output_base_dir(self.config, self.testpaths)
     }
 
     /// Gets the absolute path to the base filename used as output for the given
-    /// test/revision.
-    /// E.g., `/.../relative/testname.revision.mode/testname`.
+    /// test.
+    /// E.g., `/.../relative/testname.mode/testname`.
     fn output_base_name(&self) -> PathBuf {
-        output_base_name(self.config, self.testpaths, self.safe_revision())
+        output_base_name(self.config, self.testpaths)
     }
 
     fn maybe_dump_to_stdout(&self, out: &str, err: &str) {
@@ -177,10 +167,7 @@ impl<'test> TestCx<'test> {
     }
 
     fn error(&self, err: &str) {
-        match self.revision {
-            Some(rev) => println!("\nerror in revision `{rev}`: {err}"),
-            None => println!("\nerror: {err}"),
-        }
+        println!("\nerror: {err}");
     }
 
     fn fatal_proc_rec(&self, err: &str, proc_res: &ProcRes) -> ! {
@@ -480,7 +467,7 @@ impl<'test> TestCx<'test> {
     }
 
     fn create_stamp(&self) {
-        let stamp = crate::stamp(self.config, self.testpaths, self.revision);
+        let stamp = crate::stamp(self.config, self.testpaths);
         fs::write(&stamp, "we only support one configuration").unwrap();
     }
 }