Move the bless command to a field in Config

Author: Alexendoo

src/config.rs

@@ -31,6 +31,8 @@ pub struct Config {
     pub cfgs: CommandBuilder,
     /// What to do in case the stdout/stderr output differs from the expected one.
     pub output_conflict_handling: OutputConflictHandling,
+    /// The recommended command to bless failing tests.
+    pub bless_command: Option<String>,
     /// 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
@@ -85,6 +87,7 @@ impl Config {
             program: CommandBuilder::rustc(),
             cfgs: CommandBuilder::cfgs(),
             output_conflict_handling: OutputConflictHandling::Bless,
+            bless_command: None,
             dependencies_crate_manifest_path: None,
             dependency_builder: CommandBuilder::cargo(),
             out_dir: std::env::var_os("CARGO_TARGET_DIR")
@@ -118,11 +121,7 @@ impl Config {
     }
 
     /// Populate the config with the values from parsed command line arguments.
-    /// If neither `--bless` or `--check` are provided `default_bless` is used.
-    ///
-    /// The default output conflict handling command suggests adding `--bless`
-    /// to the end of the current command.
-    pub fn with_args(&mut self, args: &Args, default_bless: bool) {
+    pub fn with_args(&mut self, args: &Args) {
         let Args {
             ref filters,
             check,
@@ -144,22 +143,11 @@ impl Config {
 
         self.list = list;
 
-        let bless = match (bless, check) {
-            (_, true) => false,
-            (true, _) => true,
-            _ => default_bless,
-        };
-        self.output_conflict_handling = if bless {
-            OutputConflictHandling::Bless
-        } else {
-            OutputConflictHandling::Error(format!(
-                "{} --bless",
-                std::env::args()
-                    .map(|s| format!("{s:?}"))
-                    .collect::<Vec<_>>()
-                    .join(" ")
-            ))
-        };
+        if check {
+            self.output_conflict_handling = OutputConflictHandling::Error;
+        } else if bless {
+            self.output_conflict_handling = OutputConflictHandling::Bless;
+        }
     }
 
     /// Replace all occurrences of a path in stderr/stdout with a byte string.
@@ -289,8 +277,10 @@ impl Config {
 #[derive(Debug, Clone)]
 /// The different options for what to do when stdout/stderr files differ from the actual output.
 pub enum OutputConflictHandling {
-    /// The string should be a command that can be executed to bless all tests.
-    Error(String),
+    /// Fail the test when mismatches are found, if provided the command string
+    /// in [`Config::bless_command`] will be suggested as a way to bless the
+    /// test.
+    Error,
     /// Ignore mismatches in the stderr/stdout files.
     Ignore,
     /// Instead of erroring if the stderr/stdout differs from the expected

src/dependencies.rs

@@ -69,7 +69,7 @@ pub(crate) fn build_dependencies(config: &Config) -> Result<Dependencies> {
             .unwrap(),
     ) {
         (_, Mode::Yolo { .. }) => {}
-        (OutputConflictHandling::Error(_), _) => {
+        (OutputConflictHandling::Error, _) => {
             cmd.arg("--locked");
         }
         _ => {}

src/error.rs

@@ -43,7 +43,7 @@ pub enum Error {
         /// 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,
+        bless_command: Option<String>,
     },
     /// There were errors that don't have a pattern.
     ErrorsWithoutPattern {

src/lib.rs

@@ -127,7 +127,7 @@ pub fn run_tests(mut config: Config) -> Result<()> {
         Format::Terse => status_emitter::Text::quiet(),
         Format::Pretty => status_emitter::Text::verbose(),
     };
-    config.with_args(&args, true);
+    config.with_args(&args);
 
     run_tests_generic(
         vec![config],
@@ -1183,14 +1183,14 @@ fn check_output(
     let output = normalize(output, comments, revision, kind);
     let path = output_path(path, comments, revised(revision, kind), target, revision);
     match &config.output_conflict_handling {
-        OutputConflictHandling::Error(bless_command) => {
+        OutputConflictHandling::Error => {
             let expected_output = std::fs::read(&path).unwrap_or_default();
             if output != expected_output {
                 errors.push(Error::OutputDiffers {
                     path: path.clone(),
                     actual: output.clone(),
                     expected: expected_output,
-                    bless_command: bless_command.clone(),
+                    bless_command: config.bless_command.clone(),
                 });
             }
         }

src/status_emitter.rs

@@ -466,11 +466,13 @@ fn print_error(error: &Error, path: &Path) {
             bless_command,
         } => {
             print_error_header("actual output differed from expected");
-            println!(
-                "Execute `{}` to update `{}` to the actual output",
-                bless_command,
-                output_path.display()
-            );
+            if let Some(bless_command) = bless_command {
+                println!(
+                    "Execute `{}` to update `{}` to the actual output",
+                    bless_command,
+                    output_path.display()
+                );
+            }
             println!("{}", format!("--- {}", output_path.display()).red());
             println!(
                 "{}",
@@ -670,11 +672,13 @@ fn gha_error(error: &Error, test_path: &str, revision: &str) {
                     test_path,
                     "test generated output, but there was no output file",
                 );
-                writeln!(
-                    err,
-                    "you likely need to bless the tests with `{bless_command}`"
-                )
-                .unwrap();
+                if let Some(bless_command) = bless_command {
+                    writeln!(
+                        err,
+                        "you likely need to bless the tests with `{bless_command}`"
+                    )
+                    .unwrap();
+                }
                 return;
             }
 

tests/integration.rs

@@ -6,9 +6,12 @@ use ui_test::{spanned::Spanned, *};
 fn main() -> Result<()> {
     let path = Path::new(file!()).parent().unwrap();
     let root_dir = path.join("integrations");
-    let mut config = Config::cargo(root_dir.clone());
+    let mut config = Config {
+        bless_command: Some("cargo test".to_string()),
+        ..Config::cargo(root_dir.clone())
+    };
     let args = Args::test()?;
-    config.with_args(&args, true);
+    config.with_args(&args);
 
     config.program.args = vec![
         "test".into(),

tests/integrations/basic-bin/tests/ui_tests.rs

@@ -7,8 +7,9 @@ fn main() -> ui_test::color_eyre::Result<()> {
         output_conflict_handling: if std::env::var_os("BLESS").is_some() {
             OutputConflictHandling::Bless
         } else {
-            OutputConflictHandling::Error("cargo test".to_string())
+            OutputConflictHandling::Error
         },
+        bless_command: Some("cargo test".to_string()),
         ..Config::rustc("tests/actual_tests")
     };
     config.stderr_filter("in ([0-9]m )?[0-9\\.]+s", "");

tests/integrations/basic-fail-mode/tests/ui_tests.rs

@@ -8,8 +8,9 @@ fn main() -> ui_test::color_eyre::Result<()> {
         output_conflict_handling: if std::env::var_os("BLESS").is_some() {
             OutputConflictHandling::Bless
         } else {
-            OutputConflictHandling::Error("cargo test".to_string())
+            OutputConflictHandling::Error
         },
+        bless_command: Some("cargo test".to_string()),
         ..Config::rustc("tests/actual_tests")
     };
     config.comment_defaults.base().mode = Spanned::dummy(Mode::Fail {

tests/integrations/basic-fail/Cargo.stderr

@@ -6,7 +6,7 @@ error: test failed, to rerun pass `--test ui_tests`
 
 Caused by:
   process didn't exit successfully: `$DIR/target/ui/tests/integrations/basic-fail/debug/deps/ui_tests-HASH` (exit status: 1)
-thread 'main' panicked at tests/ui_tests_bless.rs:52:18:
+thread 'main' panicked at tests/ui_tests_bless.rs:53:18:
 invalid mode/result combo: yolo: Err(tests failed
 
 Location:

tests/integrations/basic-fail/tests/ui_tests.rs

@@ -5,10 +5,8 @@ fn main() -> ui_test::color_eyre::Result<()> {
     let mut config = Config {
         dependencies_crate_manifest_path: Some("Cargo.toml".into()),
         // Never bless integrations-fail tests, we want to see stderr mismatches
-        output_conflict_handling: OutputConflictHandling::Error(
-            "DO NOT BLESS. These are meant to fail".into(),
-        ),
-
+        output_conflict_handling: OutputConflictHandling::Error,
+        bless_command: Some("DO NOT BLESS. These are meant to fail".to_string()),
         ..Config::rustc("tests/actual_tests")
     };