Merge pull request #154 from oli-obk/bless

Avoid blessing by default, let the users choose instead

Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "ui_test"
-version = "0.18.1"
+version = "0.19.0"
 edition = "2021"
 license = "MIT OR Apache-2.0"
 description = "A test framework for testing rustc diagnostics output"

src/config.rs

@@ -34,11 +34,6 @@ pub struct Config {
     pub program: CommandBuilder,
     /// The command to run to obtain the cfgs that the output is supposed to
     pub cfgs: CommandBuilder,
-    /// What to do in case the stdout/stderr output differs from the expected one.
-    /// By default, errors in case of conflict, but emits a message informing the user
-    /// that running `cargo test -- -- --bless` will automatically overwrite the
-    /// `.stdout` and `.stderr` files with the latest output.
-    pub output_conflict_handling: OutputConflictHandling,
     /// Path to a `Cargo.toml` that describes which dependencies the tests can access.
     pub dependencies_crate_manifest_path: Option<PathBuf>,
     /// The command to run can be changed from `cargo` to any custom command to build the
@@ -79,13 +74,6 @@ impl Config {
             },
             program: CommandBuilder::rustc(),
             cfgs: CommandBuilder::cfgs(),
-            output_conflict_handling: OutputConflictHandling::Error(format!(
-                "{} --bless",
-                std::env::args()
-                    .map(|s| format!("{s:?}"))
-                    .collect::<Vec<_>>()
-                    .join(" ")
-            )),
             dependencies_crate_manifest_path: None,
             dependency_builder: CommandBuilder::cargo(),
             out_dir: std::env::var_os("CARGO_TARGET_DIR")
@@ -166,8 +154,8 @@ impl Config {
 
     /// Compile dependencies and return the right flags
     /// to find the dependencies.
-    pub fn build_dependencies(&self) -> Result<Vec<OsString>> {
-        let dependencies = build_dependencies(self)?;
+    pub fn build_dependencies(&self, args: &Args) -> Result<Vec<OsString>> {
+        let dependencies = build_dependencies(args, self)?;
         let mut args = vec![];
         for (name, artifacts) in dependencies.dependencies {
             for dependency in artifacts {
@@ -219,17 +207,3 @@ impl Config {
             .any(|arch| self.target.as_ref().unwrap().contains(arch))
     }
 }
-
-#[derive(Debug, Clone)]
-/// The different options for what to do when stdout/stderr files differ from the actual output.
-pub enum OutputConflictHandling {
-    /// The default: emit a diff of the expected/actual output.
-    ///
-    /// The string should be a command that can be executed to bless all tests.
-    Error(String),
-    /// Ignore mismatches in the stderr/stdout files.
-    Ignore,
-    /// Instead of erroring if the stderr/stdout differs from the expected
-    /// automatically replace it with the found output (after applying filters).
-    Bless,
-}

src/config/args.rs

@@ -14,10 +14,14 @@ pub struct Args {
     /// Whether to minimize output given to the user.
     pub quiet: bool,
 
-    /// Whether error on mismatches between `.stderr` files and actual
-    /// output. Will update the files otherwise.
+    /// Whether to error on mismatches between `.stderr` files and actual
+    /// output.
     pub check: bool,
 
+    /// Whether to overwrite `.stderr` files on mismtach with the actual
+    /// output.
+    pub bless: bool,
+
     /// The number of threads to use
     pub threads: NonZeroUsize,
 
@@ -26,12 +30,14 @@ pub struct Args {
 }
 
 impl Args {
-    /// Arguments if `ui_test` is used as a `harness=false` test, called from `cargo test`.
-    pub fn test() -> Result<Self> {
-        let mut args = Args {
+    /// Dummy arguments, but with the number of threads loaded from the environment.
+    /// The boolearn argument decides whether to bless (`true`) or whether to error (`false`)
+    pub fn default(bless: bool) -> Result<Self> {
+        Ok(Args {
             filters: vec![],
             quiet: false,
-            check: false,
+            bless,
+            check: !bless,
             skip: vec![],
             threads: match std::env::var_os("RUST_TEST_THREADS") {
                 None => std::thread::available_parallelism()?,
@@ -40,32 +46,42 @@ impl Args {
                     .ok_or_else(|| eyre!("could not parse RUST_TEST_THREADS env var"))?
                     .parse()?,
             },
-        };
-        let mut iter = std::env::args().skip(1);
+        })
+    }
+
+    /// Parse the program arguments.
+    /// This is meant to be used if `ui_test` is used as a `harness=false` test, called from `cargo test`.
+    /// The boolearn argument decides whether to bless (`true`) or whether to error (`false`)
+    pub fn test(bless: bool) -> Result<Self> {
+        Self::default(bless)?.parse_args(std::env::args().skip(1))
+    }
+
+    /// Parse arguments into an existing `Args` struct.
+    pub fn parse_args(mut self, mut iter: impl Iterator<Item = String>) -> Result<Self> {
         while let Some(arg) = iter.next() {
             if arg == "--" {
                 continue;
             }
             if arg == "--quiet" {
-                args.quiet = true;
+                self.quiet = true;
             } else if arg == "--check" {
-                args.check = true;
+                self.check = true;
             } else if let Some(skip) = parse_value("--skip", &arg, &mut iter)? {
-                args.skip.push(skip.into_owned());
+                self.skip.push(skip.into_owned());
             } else if arg == "--help" {
                 bail!("available flags: --quiet, --check, --test-threads=n, --skip")
             } else if let Some(n) = parse_value("--test-threads", &arg, &mut iter)? {
-                args.threads = n.parse()?;
+                self.threads = n.parse()?;
             } else if arg.starts_with("--") {
                 bail!(
                     "unknown command line flag `{arg}`: {:?}",
-                    std::env::args().collect::<Vec<_>>()
+                    iter.collect::<Vec<_>>()
                 );
             } else {
-                args.filters.push(arg);
+                self.filters.push(arg);
             }
         }
-        Ok(args)
+        Ok(self)
     }
 }
 

src/dependencies.rs

@@ -10,9 +10,7 @@ use std::{
     sync::{Arc, OnceLock, RwLock},
 };
 
-use crate::{
-    build_aux, status_emitter::StatusEmitter, Config, Errored, Mode, OutputConflictHandling,
-};
+use crate::{build_aux, status_emitter::StatusEmitter, Args, Config, Errored, Mode};
 
 #[derive(Default, Debug)]
 pub struct Dependencies {
@@ -45,7 +43,7 @@ fn cfgs(config: &Config) -> Result<Vec<Cfg>> {
 }
 
 /// Compiles dependencies and returns the crate names and corresponding rmeta files.
-pub(crate) fn build_dependencies(config: &Config) -> Result<Dependencies> {
+pub(crate) fn build_dependencies(args: &Args, config: &Config) -> Result<Dependencies> {
     let manifest_path = match &config.dependencies_crate_manifest_path {
         Some(path) => path.to_owned(),
         None => return Ok(Default::default()),
@@ -59,12 +57,10 @@ pub(crate) fn build_dependencies(config: &Config) -> Result<Dependencies> {
     }
 
     // Reusable closure for setting up the environment both for artifact generation and `cargo_metadata`
-    let set_locking = |cmd: &mut Command| match (&config.output_conflict_handling, &config.mode) {
-        (_, Mode::Yolo { .. }) => {}
-        (OutputConflictHandling::Error(_), _) => {
+    let set_locking = |cmd: &mut Command| {
+        if !matches!(config.mode, Mode::Yolo { .. }) && args.check {
             cmd.arg("--locked");
         }
-        _ => {}
     };
 
     set_locking(&mut build);
@@ -237,7 +233,12 @@ impl<'a> BuildManager<'a> {
     /// that need to be passed in order to build the dependencies.
     /// The error is only reported once, all follow up invocations of the same build will
     /// have a generic error about a previous build failing.
-    pub fn build(&self, what: Build, config: &Config) -> Result<Vec<OsString>, Errored> {
+    pub fn build(
+        &self,
+        what: Build,
+        config: &Config,
+        args: &Args,
+    ) -> Result<Vec<OsString>, Errored> {
         // Fast path without much contention.
         if let Some(res) = self.cache.read().unwrap().get(&what).and_then(|o| o.get()) {
             return res.clone().map_err(|()| Errored {
@@ -275,7 +276,7 @@ impl<'a> BuildManager<'a> {
                 .register_test(what.description().into())
                 .for_revision("");
             let res = match &what {
-                Build::Dependencies => match config.build_dependencies() {
+                Build::Dependencies => match config.build_dependencies(args) {
                     Ok(args) => Ok(args),
                     Err(e) => {
                         err = Some(Errored {
@@ -287,7 +288,7 @@ impl<'a> BuildManager<'a> {
                         Err(())
                     }
                 },
-                Build::Aux { aux_file } => match build_aux(aux_file, config, self) {
+                Build::Aux { aux_file } => match build_aux(aux_file, config, args, self) {
                     Ok(args) => Ok(args.iter().map(Into::into).collect()),
                     Err(e) => {
                         err = Some(e);

src/error.rs

@@ -32,8 +32,6 @@ pub enum Error {
         actual: Vec<u8>,
         /// The contents of the file.
         expected: Vec<u8>,
-        /// A command, that when run, causes the output to get blessed instead of erroring.
-        bless_command: String,
     },
     /// There were errors that don't have a pattern.
     ErrorsWithoutPattern {