Auto merge of rust-lang#8374 - Alexendoo:bless-revisions, r=camsteffen

Support `cargo dev bless` for tests with revisions

changelog: internal: Support `cargo dev bless` for tests with revisions

Previously bless wouldn't pick up the saved stderr from `target/debug/tests/manual_assert.stage-id.edition2021.stderr` or `target/debug/tests/manual_assert.stage-id.edition2018.stderr` due to there being multiple revisions of the test output

This tweaks compile-test so the built files end up in e.g. `target/debug/tests/ui`, `target/debug/tests/ui-cargo` rather than share the `tests` dir. `cargo dev bless` then uses that to update all the `.stdout/stdout/fixed` files it can find

Also removes an empty file I found, and the logic to remove empty outputs as compiletest doesn't produce empty `.stdout/stderr` files

Author: bors

clippy_dev/src/bless.rs

@@ -5,9 +5,7 @@ use std::ffi::OsStr;
 use std::fs;
 use std::lazy::SyncLazy;
 use std::path::{Path, PathBuf};
-use walkdir::WalkDir;
-
-use crate::clippy_project_root;
+use walkdir::{DirEntry, WalkDir};
 
 #[cfg(not(windows))]
 static CARGO_CLIPPY_EXE: &str = "cargo-clippy";
@@ -24,43 +22,25 @@ static CLIPPY_BUILD_TIME: SyncLazy<Option<std::time::SystemTime>> = SyncLazy::ne
 ///
 /// Panics if the path to a test file is broken
 pub fn bless(ignore_timestamp: bool) {
-    let test_suite_dirs = [
-        clippy_project_root().join("tests").join("ui"),
-        clippy_project_root().join("tests").join("ui-internal"),
-        clippy_project_root().join("tests").join("ui-toml"),
-        clippy_project_root().join("tests").join("ui-cargo"),
-    ];
-    for test_suite_dir in &test_suite_dirs {
-        WalkDir::new(test_suite_dir)
-            .into_iter()
-            .filter_map(Result::ok)
-            .filter(|f| f.path().extension() == Some(OsStr::new("rs")))
-            .for_each(|f| {
-                let test_name = f.path().strip_prefix(test_suite_dir).unwrap();
-                for &ext in &["stdout", "stderr", "fixed"] {
-                    let test_name_ext = format!("stage-id.{}", ext);
-                    update_reference_file(
-                        f.path().with_extension(ext),
-                        test_name.with_extension(test_name_ext),
-                        ignore_timestamp,
-                    );
-                }
-            });
-    }
+    let extensions = ["stdout", "stderr", "fixed"].map(OsStr::new);
+
+    WalkDir::new(build_dir())
+        .into_iter()
+        .map(Result::unwrap)
+        .filter(|entry| entry.path().extension().map_or(false, |ext| extensions.contains(&ext)))
+        .for_each(|entry| update_reference_file(&entry, ignore_timestamp));
 }
 
-fn update_reference_file(reference_file_path: PathBuf, test_name: PathBuf, ignore_timestamp: bool) {
-    let test_output_path = build_dir().join(test_name);
-    let relative_reference_file_path = reference_file_path.strip_prefix(clippy_project_root()).unwrap();
+fn update_reference_file(test_output_entry: &DirEntry, ignore_timestamp: bool) {
+    let test_output_path = test_output_entry.path();
 
-    // If compiletest did not write any changes during the test run,
-    // we don't have to update anything
-    if !test_output_path.exists() {
-        return;
-    }
+    let reference_file_name = test_output_entry.file_name().to_str().unwrap().replace(".stage-id", "");
+    let reference_file_path = Path::new("tests")
+        .join(test_output_path.strip_prefix(build_dir()).unwrap())
+        .with_file_name(reference_file_name);
 
     // If the test output was not updated since the last clippy build, it may be outdated
-    if !ignore_timestamp && !updated_since_clippy_build(&test_output_path).unwrap_or(true) {
+    if !ignore_timestamp && !updated_since_clippy_build(test_output_entry).unwrap_or(true) {
         return;
     }
 
@@ -69,23 +49,14 @@ fn update_reference_file(reference_file_path: PathBuf, test_name: PathBuf, ignor
 
     if test_output_file != reference_file {
         // If a test run caused an output file to change, update the reference file
-        println!("updating {}", &relative_reference_file_path.display());
+        println!("updating {}", reference_file_path.display());
         fs::copy(test_output_path, &reference_file_path).expect("Could not update reference file");
-
-        // We need to re-read the file now because it was potentially updated from copying
-        let reference_file = fs::read(&reference_file_path).unwrap_or_default();
-
-        if reference_file.is_empty() {
-            // If we copied over an empty output file, we remove the now empty reference file
-            println!("removing {}", &relative_reference_file_path.display());
-            fs::remove_file(reference_file_path).expect("Could not remove reference file");
-        }
     }
 }
 
-fn updated_since_clippy_build(path: &Path) -> Option<bool> {
+fn updated_since_clippy_build(entry: &DirEntry) -> Option<bool> {
     let clippy_build_time = (*CLIPPY_BUILD_TIME)?;
-    let modified = fs::metadata(path).ok()?.modified().ok()?;
+    let modified = entry.metadata().ok()?.modified().ok()?;
     Some(modified >= clippy_build_time)
 }
 

tests/compile-test.rs

@@ -11,6 +11,7 @@ use std::env::{self, remove_var, set_var, var_os};
 use std::ffi::{OsStr, OsString};
 use std::fs;
 use std::io;
+use std::lazy::SyncLazy;
 use std::path::{Path, PathBuf};
 use test_utils::IS_RUSTC_TEST_SUITE;
 
@@ -64,11 +65,11 @@ extern crate tokio;
 /// dependencies must be added to Cargo.toml at the project root. Test
 /// dependencies that are not *directly* used by this test module require an
 /// `extern crate` declaration.
-fn extern_flags() -> String {
+static EXTERN_FLAGS: SyncLazy<String> = SyncLazy::new(|| {
     let current_exe_depinfo = {
         let mut path = env::current_exe().unwrap();
         path.set_extension("d");
-        std::fs::read_to_string(path).unwrap()
+        fs::read_to_string(path).unwrap()
     };
     let mut crates: HashMap<&str, &str> = HashMap::with_capacity(TEST_DEPENDENCIES.len());
     for line in current_exe_depinfo.lines() {
@@ -112,24 +113,25 @@ fn extern_flags() -> String {
         .into_iter()
         .map(|(name, path)| format!(" --extern {}={}", name, path))
         .collect()
-}
+});
 
-fn default_config() -> compiletest::Config {
+fn base_config(test_dir: &str) -> compiletest::Config {
     let mut config = compiletest::Config {
         edition: Some("2021".into()),
+        mode: TestMode::Ui,
         ..compiletest::Config::default()
     };
 
     if let Ok(filters) = env::var("TESTNAME") {
-        config.filters = filters.split(',').map(std::string::ToString::to_string).collect();
+        config.filters = filters.split(',').map(ToString::to_string).collect();
     }
 
     if let Some(path) = option_env!("RUSTC_LIB_PATH") {
         let path = PathBuf::from(path);
         config.run_lib_path = path.clone();
         config.compile_lib_path = path;
     }
-    let current_exe_path = std::env::current_exe().unwrap();
+    let current_exe_path = env::current_exe().unwrap();
     let deps_path = current_exe_path.parent().unwrap();
     let profile_path = deps_path.parent().unwrap();
 
@@ -143,10 +145,11 @@ fn default_config() -> compiletest::Config {
         "--emit=metadata -Dwarnings -Zui-testing -L dependency={}{}{}",
         deps_path.display(),
         host_libs,
-        extern_flags(),
+        &*EXTERN_FLAGS,
     ));
 
-    config.build_base = profile_path.join("test");
+    config.src_base = Path::new("tests").join(test_dir);
+    config.build_base = profile_path.join("test").join(test_dir);
     config.rustc_path = profile_path.join(if cfg!(windows) {
         "clippy-driver.exe"
     } else {
@@ -155,38 +158,31 @@ fn default_config() -> compiletest::Config {
     config
 }
 
-fn run_ui(cfg: &mut compiletest::Config) {
-    cfg.mode = TestMode::Ui;
-    cfg.src_base = Path::new("tests").join("ui");
+fn run_ui() {
+    let config = base_config("ui");
     // use tests/clippy.toml
-    let _g = VarGuard::set("CARGO_MANIFEST_DIR", std::fs::canonicalize("tests").unwrap());
-    compiletest::run_tests(cfg);
+    let _g = VarGuard::set("CARGO_MANIFEST_DIR", fs::canonicalize("tests").unwrap());
+    compiletest::run_tests(&config);
 }
 
-fn run_ui_test(cfg: &mut compiletest::Config) {
-    cfg.mode = TestMode::Ui;
-    cfg.src_base = Path::new("tests").join("ui_test");
-    let _g = VarGuard::set("CARGO_MANIFEST_DIR", std::fs::canonicalize("tests").unwrap());
-    let rustcflags = cfg.target_rustcflags.get_or_insert_with(Default::default);
-    let len = rustcflags.len();
+fn run_ui_test() {
+    let mut config = base_config("ui_test");
+    let _g = VarGuard::set("CARGO_MANIFEST_DIR", fs::canonicalize("tests").unwrap());
+    let rustcflags = config.target_rustcflags.get_or_insert_with(Default::default);
     rustcflags.push_str(" --test");
-    compiletest::run_tests(cfg);
-    if let Some(ref mut flags) = &mut cfg.target_rustcflags {
-        flags.truncate(len);
-    }
+    compiletest::run_tests(&config);
 }
 
-fn run_internal_tests(cfg: &mut compiletest::Config) {
+fn run_internal_tests() {
     // only run internal tests with the internal-tests feature
     if !RUN_INTERNAL_TESTS {
         return;
     }
-    cfg.mode = TestMode::Ui;
-    cfg.src_base = Path::new("tests").join("ui-internal");
-    compiletest::run_tests(cfg);
+    let config = base_config("ui-internal");
+    compiletest::run_tests(&config);
 }
 
-fn run_ui_toml(config: &mut compiletest::Config) {
+fn run_ui_toml() {
     fn run_tests(config: &compiletest::Config, mut tests: Vec<tester::TestDescAndFn>) -> Result<bool, io::Error> {
         let mut result = true;
         let opts = compiletest::test_opts(config);
@@ -222,12 +218,12 @@ fn run_ui_toml(config: &mut compiletest::Config) {
         Ok(result)
     }
 
-    config.mode = TestMode::Ui;
-    config.src_base = Path::new("tests").join("ui-toml").canonicalize().unwrap();
+    let mut config = base_config("ui-toml");
+    config.src_base = config.src_base.canonicalize().unwrap();
 
-    let tests = compiletest::make_tests(config);
+    let tests = compiletest::make_tests(&config);
 
-    let res = run_tests(config, tests);
+    let res = run_tests(&config, tests);
     match res {
         Ok(true) => {},
         Ok(false) => panic!("Some tests failed"),
@@ -237,7 +233,7 @@ fn run_ui_toml(config: &mut compiletest::Config) {
     }
 }
 
-fn run_ui_cargo(config: &mut compiletest::Config) {
+fn run_ui_cargo() {
     fn run_tests(
         config: &compiletest::Config,
         filters: &[String],
@@ -310,13 +306,13 @@ fn run_ui_cargo(config: &mut compiletest::Config) {
         return;
     }
 
-    config.mode = TestMode::Ui;
-    config.src_base = Path::new("tests").join("ui-cargo").canonicalize().unwrap();
+    let mut config = base_config("ui-cargo");
+    config.src_base = config.src_base.canonicalize().unwrap();
 
-    let tests = compiletest::make_tests(config);
+    let tests = compiletest::make_tests(&config);
 
     let current_dir = env::current_dir().unwrap();
-    let res = run_tests(config, &config.filters, tests);
+    let res = run_tests(&config, &config.filters, tests);
     env::set_current_dir(current_dir).unwrap();
 
     match res {
@@ -331,12 +327,11 @@ fn run_ui_cargo(config: &mut compiletest::Config) {
 #[test]
 fn compile_test() {
     set_var("CLIPPY_DISABLE_DOCS_LINKS", "true");
-    let mut config = default_config();
-    run_ui(&mut config);
-    run_ui_test(&mut config);
-    run_ui_toml(&mut config);
-    run_ui_cargo(&mut config);
-    run_internal_tests(&mut config);
+    run_ui();
+    run_ui_test();
+    run_ui_toml();
+    run_ui_cargo();
+    run_internal_tests();
 }
 
 /// Restores an env var on drop