Merge pull request #157 from Alexendoo/args-to-config

Populate `Config` with `Args` instead of passing it to `run_tests_generic`

Author: oli-obk

Cargo.lock

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

Cargo.toml

@@ -48,7 +48,7 @@ impl CommandBuilder {
         }
     }
 
-    /// Same as [`rustc`], but with arguments for obtaining the cfgs.
+    /// Same as [`CommandBuilder::rustc`], but with arguments for obtaining the cfgs.
     pub fn cfgs() -> Self {
         Self {
             args: vec!["--print".into(), "cfg".into()],

src/cmd.rs

@@ -5,6 +5,7 @@ pub use color_eyre;
 use color_eyre::eyre::Result;
 use std::{
     ffi::OsString,
+    num::NonZeroUsize,
     path::{Path, PathBuf},
 };
 
@@ -34,16 +35,24 @@ 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.
+    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
-    /// dependencies in `dependencies_crate_manifest_path`
+    /// dependencies in `dependencies_crate_manifest_path`.
     pub dependency_builder: CommandBuilder,
     /// Where to dump files like the binaries compiled from tests.
     /// Defaults to `target/ui` in the current directory.
     pub out_dir: PathBuf,
-    /// The default edition to use on all tests
+    /// The default edition to use on all tests.
     pub edition: Option<String>,
+    /// Skip test files whose names contain any of these entries.
+    pub skip_files: Vec<String>,
+    /// Only test files whose names contain any of these entries.
+    pub filter_files: Vec<String>,
+    /// Override the number of threads to use.
+    pub threads: Option<NonZeroUsize>,
 }
 
 impl Config {
@@ -74,13 +83,17 @@ impl Config {
             },
             program: CommandBuilder::rustc(),
             cfgs: CommandBuilder::cfgs(),
+            output_conflict_handling: OutputConflictHandling::Bless,
             dependencies_crate_manifest_path: None,
             dependency_builder: CommandBuilder::cargo(),
             out_dir: std::env::var_os("CARGO_TARGET_DIR")
                 .map(PathBuf::from)
                 .unwrap_or_else(|| std::env::current_dir().unwrap().join("target"))
                 .join("ui"),
             edition: Some("2021".into()),
+            skip_files: Vec::new(),
+            filter_files: Vec::new(),
+            threads: None,
         }
     }
 
@@ -98,6 +111,44 @@ 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) {
+        let Args {
+            ref filters,
+            quiet: _,
+            check,
+            bless,
+            threads,
+            ref skip,
+        } = *args;
+
+        self.threads = threads.or(self.threads);
+
+        self.filter_files.extend_from_slice(filters);
+        self.skip_files.extend_from_slice(skip);
+
+        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(" ")
+            ))
+        };
+    }
+
     /// Replace all occurrences of a path in stderr/stdout with a byte string.
     pub fn path_filter(&mut self, path: &Path, replacement: &'static (impl AsRef<[u8]> + ?Sized)) {
         self.path_stderr_filter(path, replacement);
@@ -154,8 +205,8 @@ impl Config {
 
     /// Compile dependencies and return the right flags
     /// to find the dependencies.
-    pub fn build_dependencies(&self, args: &Args) -> Result<Vec<OsString>> {
-        let dependencies = build_dependencies(args, self)?;
+    pub fn build_dependencies(&self) -> Result<Vec<OsString>> {
+        let dependencies = build_dependencies(self)?;
         let mut args = vec![];
         for (name, artifacts) in dependencies.dependencies {
             for dependency in artifacts {
@@ -207,3 +258,15 @@ 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 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.rs

@@ -3,10 +3,10 @@
 
 use std::{borrow::Cow, num::NonZeroUsize};
 
-use color_eyre::eyre::{bail, ensure, eyre, Result};
+use color_eyre::eyre::{bail, ensure, Result};
 
 /// Plain arguments if `ui_test` is used as a binary.
-#[derive(Debug)]
+#[derive(Debug, Default)]
 pub struct Args {
     /// Filters that will be used to match on individual tests
     pub filters: Vec<String>,
@@ -23,37 +23,17 @@ pub struct Args {
     pub bless: bool,
 
     /// The number of threads to use
-    pub threads: NonZeroUsize,
+    pub threads: Option<NonZeroUsize>,
 
     /// Skip tests whose names contain any of these entries.
     pub skip: Vec<String>,
 }
 
 impl 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,
-            bless,
-            check: !bless,
-            skip: vec![],
-            threads: match std::env::var_os("RUST_TEST_THREADS") {
-                None => std::thread::available_parallelism()?,
-                Some(n) => n
-                    .to_str()
-                    .ok_or_else(|| eyre!("could not parse RUST_TEST_THREADS env var"))?
-                    .parse()?,
-            },
-        })
-    }
-
     /// 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))
+    pub fn test() -> Result<Self> {
+        Self::default().parse_args(std::env::args().skip(1))
     }
 
     /// Parse arguments into an existing `Args` struct.
@@ -73,7 +53,7 @@ impl Args {
             } else if arg == "--help" {
                 bail!("available flags: --quiet, --check, --bless, --test-threads=n, --skip")
             } else if let Some(n) = parse_value("--test-threads", &arg, &mut iter)? {
-                self.threads = n.parse()?;
+                self.threads = Some(n.parse()?);
             } else if arg.starts_with("--") {
                 bail!(
                     "unknown command line flag `{arg}`: {:?}",

src/config/args.rs

@@ -10,7 +10,9 @@ use std::{
     sync::{Arc, OnceLock, RwLock},
 };
 
-use crate::{build_aux, status_emitter::StatusEmitter, Args, Config, Errored, Mode};
+use crate::{
+    build_aux, status_emitter::StatusEmitter, Config, Errored, Mode, OutputConflictHandling,
+};
 
 #[derive(Default, Debug)]
 pub struct Dependencies {
@@ -43,7 +45,7 @@ fn cfgs(config: &Config) -> Result<Vec<Cfg>> {
 }
 
 /// Compiles dependencies and returns the crate names and corresponding rmeta files.
-pub(crate) fn build_dependencies(args: &Args, config: &Config) -> Result<Dependencies> {
+pub(crate) fn build_dependencies(config: &Config) -> Result<Dependencies> {
     let manifest_path = match &config.dependencies_crate_manifest_path {
         Some(path) => path.to_owned(),
         None => return Ok(Default::default()),
@@ -57,10 +59,12 @@ pub(crate) fn build_dependencies(args: &Args, config: &Config) -> Result<Depende
     }
 
     // Reusable closure for setting up the environment both for artifact generation and `cargo_metadata`
-    let set_locking = |cmd: &mut Command| {
-        if !matches!(config.mode, Mode::Yolo { .. }) && args.check {
+    let set_locking = |cmd: &mut Command| match (&config.output_conflict_handling, &config.mode) {
+        (_, Mode::Yolo { .. }) => {}
+        (OutputConflictHandling::Error(_), _) => {
             cmd.arg("--locked");
         }
+        _ => {}
     };
 
     set_locking(&mut build);
@@ -233,12 +237,7 @@ 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,
-        args: &Args,
-    ) -> Result<Vec<OsString>, Errored> {
+    pub fn build(&self, what: Build, config: &Config) -> 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 {
@@ -276,7 +275,7 @@ impl<'a> BuildManager<'a> {
                 .register_test(what.description().into())
                 .for_revision("");
             let res = match &what {
-                Build::Dependencies => match config.build_dependencies(args) {
+                Build::Dependencies => match config.build_dependencies() {
                     Ok(args) => Ok(args),
                     Err(e) => {
                         err = Some(Errored {
@@ -288,7 +287,7 @@ impl<'a> BuildManager<'a> {
                         Err(())
                     }
                 },
-                Build::Aux { aux_file } => match build_aux(aux_file, config, args, self) {
+                Build::Aux { aux_file } => match build_aux(aux_file, config, self) {
                     Ok(args) => Ok(args.iter().map(Into::into).collect()),
                     Err(e) => {
                         err = Some(e);

src/dependencies.rs

@@ -32,6 +32,8 @@ 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 {