Merge pull request #231 from phansch/bless

Add basic `bless` support

Cargo.toml

@@ -25,7 +25,8 @@ serde = "1.0"
 serde_json = "1.0"
 serde_derive = "1.0"
 rustfix = "0.5"
-tester = "0.8"
+tester = "0.9"
+lazy_static = "1.4"
 
 [target."cfg(unix)".dependencies]
 libc = "0.2"

src/common.rs

@@ -102,6 +102,9 @@ impl fmt::Display for Mode {
 
 #[derive(Clone)]
 pub struct Config {
+    /// `true` to overwrite stderr/stdout/fixed files instead of complaining about changes in output.
+    pub bless: bool,
+
     /// The library paths required for running the compiler
     pub compile_lib_path: PathBuf,
 
@@ -145,8 +148,8 @@ pub struct Config {
     /// Run ignored tests
     pub run_ignored: bool,
 
-    /// Only run tests that match this filter
-    pub filter: Option<String>,
+    /// Only run tests that match these filters
+    pub filters: Vec<String>,
 
     /// Exactly match the filter, rather than a substring
     pub filter_exact: bool,
@@ -239,6 +242,25 @@ pub struct TestPaths {
     pub relative_dir: PathBuf, // e.g., foo/bar
 }
 
+/// Used by `ui` tests to generate things like `foo.stderr` from `foo.rs`.
+pub fn expected_output_path(
+    testpaths: &TestPaths,
+    revision: Option<&str>,
+    kind: &str,
+) -> PathBuf {
+    assert!(UI_EXTENSIONS.contains(&kind));
+    let mut parts = Vec::new();
+
+    if let Some(x) = revision {
+        parts.push(x);
+    }
+    parts.push(kind);
+
+    let extension = parts.join(".");
+    testpaths.file.with_extension(extension)
+}
+
+pub const UI_EXTENSIONS: &[&str] = &[UI_STDERR, UI_STDOUT, UI_FIXED];
 pub const UI_STDERR: &str = "stderr";
 pub const UI_STDOUT: &str = "stdout";
 pub const UI_FIXED: &str = "fixed";
@@ -335,6 +357,7 @@ impl Default for Config {
         let platform = rustc_session::config::host_triple().to_string();
 
         Config {
+            bless: false,
             compile_lib_path: PathBuf::from(""),
             run_lib_path: PathBuf::from(""),
             rustc_path: PathBuf::from("rustc"),
@@ -349,7 +372,7 @@ impl Default for Config {
             stage_id: "stage-id".to_owned(),
             mode: Mode::RunPass,
             run_ignored: false,
-            filter: None,
+            filters: vec![],
             filter_exact: false,
             logfile: None,
             runtool: None,

src/lib.rs

@@ -111,7 +111,7 @@ pub fn run_tests(config: &Config) {
 
 pub fn test_opts(config: &Config) -> test::TestOpts {
     test::TestOpts {
-        filter: config.filter.clone(),
+        filters: config.filters.clone(),
         filter_exact: config.filter_exact,
         exclude_should_panic: false,
         force_run_in_process: false,

src/runtest.rs

@@ -9,7 +9,7 @@
 // except according to those terms.
 
 use common::{Config, TestPaths};
-use common::{UI_FIXED, UI_STDERR, UI_STDOUT};
+use common::{expected_output_path, UI_FIXED, UI_STDERR, UI_STDOUT};
 use common::{CompileFail, ParseFail, Pretty, RunFail, RunPass, RunPassValgrind};
 use common::{Codegen, DebugInfoLldb, DebugInfoGdb, Rustdoc, CodegenUnits};
 use common::{Incremental, RunMake, Ui, MirOpt};
@@ -20,7 +20,7 @@ use json;
 use regex::Regex;
 use rustfix::{apply_suggestions, get_suggestions_from_json, Filter};
 use header::TestProps;
-use util::logv;
+use crate::util::{logv, PathBufExt};
 
 use std::collections::HashMap;
 use std::collections::HashSet;
@@ -2167,6 +2167,18 @@ actual:\n\
         // compiler flags set in the test cases:
         cmd.env_remove("RUSTFLAGS");
 
+        if self.config.bless {
+            cmd.env("RUSTC_BLESS_TEST", "--bless");
+            // Assume this option is active if the environment variable is "defined", with _any_ value.
+            // As an example, a `Makefile` can use this option by:
+            //
+            //   ifdef RUSTC_BLESS_TEST
+            //       cp "$(TMPDIR)"/actual_something.ext expected_something.ext
+            //   else
+            //       $(DIFF) expected_something.ext "$(TMPDIR)"/actual_something.ext
+            //   endif
+        }
+
         if self.config.target.contains("msvc") {
             // We need to pass a path to `lib.exe`, so assume that `cc` is `cl.exe`
             // and that `lib.exe` lives next to it.
@@ -2323,16 +2335,17 @@ actual:\n\
         }
 
         if errors > 0 {
-            println!("To update references, run this command from build directory:");
+            println!("To update references, rerun the tests and pass the `--bless` flag");
             let relative_path_to_file =
-                self.testpaths.relative_dir
-                              .join(self.testpaths.file.file_name().unwrap());
-            println!("{}/update-references.sh '{}' '{}'",
-                     self.config.src_base.display(),
-                     self.config.build_base.display(),
-                     relative_path_to_file.display());
-            self.fatal_proc_rec(&format!("{} errors occurred comparing output.", errors),
-                                &proc_res);
+                self.testpaths.relative_dir.join(self.testpaths.file.file_name().unwrap());
+            println!(
+                "To only update this specific test, also pass `--test-args {}`",
+                relative_path_to_file.display(),
+            );
+            self.fatal_proc_rec(
+                &format!("{} errors occurred comparing output.", errors),
+                &proc_res,
+            );
         }
 
         if self.props.run_pass {
@@ -2566,11 +2579,14 @@ actual:\n\
     }
 
     fn expected_output_path(&self, kind: &str) -> PathBuf {
-        let extension = match self.revision {
-            Some(r) => format!("{}.{}", r, kind),
-            None => kind.to_string(),
-        };
-        self.testpaths.file.with_extension(extension)
+        let mut path =
+            expected_output_path(&self.testpaths, self.revision, kind);
+
+        if !path.exists() {
+            path = expected_output_path(&self.testpaths, self.revision, kind);
+        }
+
+        path
     }
 
     fn load_expected_output(&self, path: &Path) -> String {
@@ -2594,35 +2610,68 @@ actual:\n\
         })
     }
 
+    fn delete_file(&self, file: &PathBuf) {
+        if !file.exists() {
+            // Deleting a nonexistant file would error.
+            return;
+        }
+        if let Err(e) = fs::remove_file(file) {
+            self.fatal(&format!("failed to delete `{}`: {}", file.display(), e,));
+        }
+    }
+
     fn compare_output(&self, kind: &str, actual: &str, expected: &str) -> usize {
         if actual == expected {
             return 0;
         }
 
-        println!("normalized {}:\n{}\n", kind, actual);
-        println!("expected {}:\n{}\n", kind, expected);
-        println!("diff of {}:\n", kind);
-
-        for diff in diff::lines(expected, actual) {
-            match diff {
-                diff::Result::Left(l)    => println!("-{}", l),
-                diff::Result::Both(l, _) => println!(" {}", l),
-                diff::Result::Right(r)   => println!("+{}", r),
+        if !self.config.bless {
+            if expected.is_empty() {
+                println!("normalized {}:\n{}\n", kind, actual);
+            } else {
+                println!("diff of {}:\n", kind);
+                for diff in diff::lines(expected, actual) {
+                    match diff {
+                        diff::Result::Left(l)    => println!("-{}", l),
+                        diff::Result::Both(l, _) => println!(" {}", l),
+                        diff::Result::Right(r)   => println!("+{}", r),
+                    }
+                }
             }
         }
 
-        let output_file = self.output_base_name().with_extension(kind);
-        match File::create(&output_file).and_then(|mut f| f.write_all(actual.as_bytes())) {
-            Ok(()) => { }
-            Err(e) => {
-                self.fatal(&format!("failed to write {} to `{}`: {}",
-                                    kind, output_file.display(), e))
+        let output_file = self
+            .output_base_name()
+            .with_extra_extension(self.revision.unwrap_or(""))
+            .with_extra_extension(kind);
+
+        let mut files = vec![output_file];
+        if self.config.bless {
+            files.push(expected_output_path(
+                self.testpaths,
+                self.revision,
+                kind,
+            ));
+        }
+
+        for output_file in &files {
+            if actual.is_empty() {
+                self.delete_file(output_file);
+            } else if let Err(err) = fs::write(&output_file, &actual) {
+                self.fatal(&format!(
+                    "failed to write {} to `{}`: {}",
+                    kind,
+                    output_file.display(),
+                    err,
+                ));
             }
         }
 
         println!("\nThe actual {0} differed from the expected {0}.", kind);
-        println!("Actual {} saved to {}", kind, output_file.display());
-        1
+        for output_file in files {
+            println!("Actual {} saved to {}", kind, output_file.display());
+        }
+        if self.config.bless { 0 } else { 1 }
     }
 }
 

src/util.rs

@@ -10,6 +10,8 @@
 
 use std::env;
 use common::Config;
+use std::ffi::OsStr;
+use std::path::PathBuf;
 
 /// Conversion table from triple OS name to Rust SYSNAME
 const OS_TABLE: &'static [(&'static str, &'static str)] = &[
@@ -108,3 +110,23 @@ pub fn logv(config: &Config, s: String) {
         println!("{}", s);
     }
 }
+
+pub trait PathBufExt {
+    /// Append an extension to the path, even if it already has one.
+    fn with_extra_extension<S: AsRef<OsStr>>(&self, extension: S) -> PathBuf;
+}
+
+impl PathBufExt for PathBuf {
+    fn with_extra_extension<S: AsRef<OsStr>>(&self, extension: S) -> PathBuf {
+        if extension.as_ref().is_empty() {
+            self.clone()
+        } else {
+            let mut fname = self.file_name().unwrap().to_os_string();
+            if !extension.as_ref().to_str().unwrap().starts_with('.') {
+                fname.push(".");
+            }
+            fname.push(extension);
+            self.with_file_name(fname)
+        }
+    }
+}

tests/bless.rs

@@ -0,0 +1,85 @@
+//! Tests for the `bless` option
+
+extern crate compiletest_rs as compiletest;
+
+mod test_support;
+use test_support::{testsuite, TestsuiteBuilder, GLOBAL_ROOT};
+use compiletest::Config;
+
+fn setup(mode: &str) -> (Config, TestsuiteBuilder) {
+    let builder = testsuite(mode);
+    let mut config = Config::default();
+    let cfg_mode = mode.parse().expect("Invalid mode");
+    config.mode = cfg_mode;
+    config.src_base = builder.root.clone();
+    config.build_base = GLOBAL_ROOT.join("build_base");
+
+    (config, builder)
+}
+
+#[test]
+fn test_bless_new_file() {
+    let (mut config, builder) = setup("ui");
+    config.bless = true;
+
+    builder.mk_file(
+              "foobar.rs",
+              r#"
+                  #[warn(unused_variables)]
+                  fn main() {
+                      let abc = "foobar";
+                  }
+              "#,
+          );
+    compiletest::run_tests(&config);
+
+    // Blessing should cause the stderr to be created directly
+    assert!(builder.file_contents("foobar.stderr").contains("unused variable"));
+
+    // And a second run of the tests, with blessing disabled should work just fine
+    config.bless = false;
+    compiletest::run_tests(&config);
+}
+
+#[test]
+fn test_bless_update_file() {
+    let (mut config, builder) = setup("ui");
+    config.bless = true;
+
+    builder.mk_file(
+        "foobar2.rs",
+        r#"
+            #[warn(unused_variables)]
+            fn main() {
+                let abc = "foobar_update";
+            }
+        "#,
+    );
+    builder.mk_file(
+        "foobar2.stderr",
+        r#"
+            warning: unused variable: `abc`
+             --> $DIR/foobar2.rs:4:27
+              |
+            4 |                       let abc = "foobar";
+              |                           ^^^ help: if this is intentional, prefix it with an underscore: `_abc`
+              |
+            note: the lint level is defined here
+             --> $DIR/foobar2.rs:2:26
+              |
+            2 |                   #[warn(unused_variables)]
+              |                          ^^^^^^^^^^^^^^^^
+
+            warning: 1 warning emitted
+        "#,
+    );
+    compiletest::run_tests(&config);
+
+    // Blessing should cause the stderr to be created directly
+    assert!(builder.file_contents("foobar2.stderr").contains("unused variable"));
+    assert!(builder.file_contents("foobar2.stderr").contains("foobar_update"));
+
+    // And a second run of the tests, with blessing disabled should work just fine
+    config.bless = false;
+    compiletest::run_tests(&config);
+}