[nextest-runner] add a --tool-config-file option

Add an option for tools that integrate with nextest to specify configs
that are lower than .config/nextest.toml in priority, but higher than
the default config.

Author: sunshowers

cargo-nextest/src/dispatch.rs

@@ -15,7 +15,7 @@ use nextest_filtering::FilteringExpr;
 use nextest_metadata::{BinaryListSummary, BuildPlatform};
 use nextest_runner::{
     cargo_config::{CargoConfigs, TargetTriple},
-    config::{NextestConfig, NextestProfile, TestThreads},
+    config::{NextestConfig, NextestProfile, TestThreads, ToolConfigFile},
     errors::WriteTestListError,
     list::{BinaryList, OutputFormat, RustTestArtifact, SerializableFormat, TestList},
     partition::PartitionerBuilder,
@@ -164,10 +164,27 @@ impl AppOpts {
 }
 
 #[derive(Debug, Args)]
+#[clap(next_help_heading = "CONFIG OPTIONS")]
 struct ConfigOpts {
     /// Config file [default: workspace-root/.config/nextest.toml]
     #[clap(long, global = true, value_name = "PATH")]
     pub config_file: Option<Utf8PathBuf>,
+
+    /// Tool-specific config files
+    ///
+    /// Some tools on top of nextest may want to set up their own default configuration but
+    /// prioritize user configuration on top. Use this argument to insert configuration
+    /// that's lower than --config-file in priority but above the default config shipped with
+    /// nextest.
+    ///
+    /// Arguments are specified in the format "tool:abs_path", for example
+    /// "my-tool:/path/to/nextest.toml" (or "my-tool:C:\\path\\to\\nextest.toml" on Windows).
+    /// Paths must be absolute.
+    ///
+    /// This argument may be specified multiple times. Files that come later are lower priority
+    /// than those that come earlier.
+    #[clap(long = "tool-config-file", global = true, value_name = "TOOL:ABS_PATH")]
+    pub tool_config_files: Vec<ToolConfigFile>,
 }
 
 impl ConfigOpts {
@@ -177,8 +194,13 @@ impl ConfigOpts {
         workspace_root: &Utf8Path,
         graph: &PackageGraph,
     ) -> Result<NextestConfig> {
-        NextestConfig::from_sources(workspace_root, graph, self.config_file.as_deref())
-            .map_err(ExpectedError::config_parse_error)
+        NextestConfig::from_sources(
+            workspace_root,
+            graph,
+            self.config_file.as_deref(),
+            &self.tool_config_files,
+        )
+        .map_err(ExpectedError::config_parse_error)
     }
 }
 

nextest-runner/src/config.rs

@@ -6,7 +6,7 @@
 use crate::{
     errors::{
         ConfigParseError, ConfigParseErrorKind, ConfigParseOverrideError, ProfileNotFound,
-        TestThreadsParseError,
+        TestThreadsParseError, ToolConfigFileParseError,
     },
     reporter::{FinalStatusLevel, StatusLevel, TestOutputDisplay},
 };
@@ -54,17 +54,29 @@ impl NextestConfig {
     /// Reads the nextest config from the given file, or if not specified from `.config/nextest.toml`
     /// in the workspace root.
     ///
-    /// If the file isn't specified and the directory doesn't have `.config/nextest.toml`, uses the
+    /// `tool_config_files` are lower priority than `config_file` but higher priority than the
+    /// default config. Files in `tool_config_files` that come earlier are higher priority than those
+    /// that come later.
+    ///
+    /// If no config files are specified and this file doesn't have `.config/nextest.toml`, uses the
     /// default config options.
-    pub fn from_sources(
+    pub fn from_sources<'a, I>(
         workspace_root: impl Into<Utf8PathBuf>,
         graph: &PackageGraph,
         config_file: Option<&Utf8Path>,
-    ) -> Result<Self, ConfigParseError> {
+        tool_config_files: impl IntoIterator<IntoIter = I>,
+    ) -> Result<Self, ConfigParseError>
+    where
+        I: Iterator<Item = &'a ToolConfigFile> + DoubleEndedIterator,
+    {
         let workspace_root = workspace_root.into();
-        let (config_file, config) = Self::read_from_sources(&workspace_root, config_file)?;
+        let tool_config_files_rev = tool_config_files.into_iter().rev();
+        let (config_file, config) =
+            Self::read_from_sources(&workspace_root, config_file, tool_config_files_rev)?;
         let inner: NextestConfigImpl =
             serde_path_to_error::deserialize(config).map_err(|error| {
+                // TODO: now that lowpri configs exist, we need better attribution for the exact path at which
+                // an error occurred.
                 ConfigParseError::new(
                     config_file.clone(),
                     ConfigParseErrorKind::DeserializeError(error),
@@ -109,12 +121,22 @@ impl NextestConfig {
     // Helper methods
     // ---
 
-    fn read_from_sources(
+    fn read_from_sources<'a>(
         workspace_root: &Utf8Path,
         file: Option<&Utf8Path>,
+        tool_config_files_rev: impl Iterator<Item = &'a ToolConfigFile>,
     ) -> Result<(Utf8PathBuf, Config), ConfigParseError> {
         // First, get the default config.
-        let builder = Self::make_default_config();
+        let mut builder = Self::make_default_config();
+
+        // Next, merge in tool configs.
+        for ToolConfigFile {
+            config_file,
+            tool: _,
+        } in tool_config_files_rev
+        {
+            builder = builder.add_source(File::new(config_file.as_str(), FileFormat::Toml));
+        }
 
         // Next, merge in the config from the given file.
         let (builder, config_path) = match file {
@@ -169,6 +191,54 @@ impl NextestConfig {
     }
 }
 
+/// A tool-specific config file.
+///
+/// Tool-specific config files are lower priority than repository configs, but higher priority than
+/// the default config shipped with nextest.
+#[derive(Clone, Debug, Eq, PartialEq)]
+pub struct ToolConfigFile {
+    /// The name of the tool.
+    pub tool: String,
+
+    /// The path to the config file.
+    pub config_file: Utf8PathBuf,
+}
+
+impl FromStr for ToolConfigFile {
+    type Err = ToolConfigFileParseError;
+
+    fn from_str(input: &str) -> Result<Self, Self::Err> {
+        match input.split_once(':') {
+            Some((tool, config_file)) => {
+                if tool.is_empty() {
+                    Err(ToolConfigFileParseError::EmptyToolName {
+                        input: input.to_owned(),
+                    })
+                } else if config_file.is_empty() {
+                    Err(ToolConfigFileParseError::EmptyConfigFile {
+                        input: input.to_owned(),
+                    })
+                } else {
+                    let config_file = Utf8Path::new(config_file);
+                    if config_file.is_absolute() {
+                        Ok(Self {
+                            tool: tool.to_owned(),
+                            config_file: Utf8PathBuf::from(config_file),
+                        })
+                    } else {
+                        Err(ToolConfigFileParseError::ConfigFileNotAbsolute {
+                            config_file: config_file.to_owned(),
+                        })
+                    }
+                }
+            }
+            None => Err(ToolConfigFileParseError::InvalidFormat {
+                input: input.to_owned(),
+            }),
+        }
+    }
+}
+
 /// A configuration profile for nextest. Contains most configuration used by the nextest runner.
 ///
 /// Returned by [`NextestConfig::profile`].
@@ -790,7 +860,7 @@ mod tests {
         let graph = temp_workspace(workspace_path, config_contents);
 
         let nextest_config_result =
-            NextestConfig::from_sources(graph.workspace().root(), &graph, None);
+            NextestConfig::from_sources(graph.workspace().root(), &graph, None, []);
 
         match expected_default {
             Ok(expected_default) => {
@@ -918,7 +988,8 @@ mod tests {
         let graph = temp_workspace(workspace_path, config_contents);
         let package_id = graph.workspace().iter().next().unwrap().id();
 
-        let config = NextestConfig::from_sources(graph.workspace().root(), &graph, None).unwrap();
+        let config =
+            NextestConfig::from_sources(graph.workspace().root(), &graph, None, []).unwrap();
         let query = TestQuery {
             binary_query: BinaryQuery {
                 package_id,
@@ -939,6 +1010,99 @@ mod tests {
         );
     }
 
+    #[test]
+    fn parse_tool_config_file() {
+        cfg_if::cfg_if! {
+            if #[cfg(windows)] {
+                let valid = ["tool:C:\\foo\\bar", "tool:\\\\?\\C:\\foo\\bar"];
+                let invalid = ["C:\\foo\\bar", "tool:\\foo\\bar", "tool:", ":/foo/bar"];
+            } else {
+                let valid = ["tool:/foo/bar"];
+                let invalid = ["/foo/bar", "tool:", ":/foo/bar", "tool:foo/bar"];
+            }
+        }
+
+        for valid_input in valid {
+            valid_input.parse::<ToolConfigFile>().unwrap_or_else(|err| {
+                panic!("valid input {valid_input} should parse correctly: {err}")
+            });
+        }
+
+        for invalid_input in invalid {
+            invalid_input
+                .parse::<ToolConfigFile>()
+                .expect_err(&format!("invalid input {invalid_input} should error out"));
+        }
+    }
+
+    #[test]
+    fn lowpri_config() {
+        let config_contents = r#"
+        [profile.default]
+        retries = 3
+        "#;
+
+        let lowpri1_config_contents = r#"
+        [profile.default]
+        retries = 4
+
+        [profile.lowpri]
+        retries = 12
+        "#;
+
+        let lowpri2_config_contents = r#"
+        [profile.default]
+        retries = 5
+
+        [profile.lowpri]
+        retries = 16
+
+        [profile.lowpri2]
+        retries = 18
+        "#;
+
+        let workspace_dir = tempdir().unwrap();
+        let workspace_path: &Utf8Path = workspace_dir.path().try_into().unwrap();
+
+        let graph = temp_workspace(workspace_path, config_contents);
+        let workspace_root = graph.workspace().root();
+        let lowpri1_path = workspace_root.join(".config/lowpri1.toml");
+        let lowpri2_path = workspace_root.join(".config/lowpri2.toml");
+        std::fs::write(&lowpri1_path, lowpri1_config_contents).unwrap();
+        std::fs::write(&lowpri2_path, lowpri2_config_contents).unwrap();
+
+        let config = NextestConfig::from_sources(
+            workspace_root,
+            &graph,
+            None,
+            &[
+                ToolConfigFile {
+                    tool: "lowpri1".to_owned(),
+                    config_file: lowpri1_path,
+                },
+                ToolConfigFile {
+                    tool: "lowpri2".to_owned(),
+                    config_file: lowpri2_path,
+                },
+            ],
+        )
+        .expect("parsing config failed");
+
+        let default_profile = config
+            .profile(NextestConfig::DEFAULT_PROFILE)
+            .expect("default profile is present");
+        // This is present in .config/nextest.toml and is the highest priority
+        assert_eq!(default_profile.retries(), 3);
+
+        let lowpri_profile = config.profile("lowpri").expect("lowpri profile is present");
+        assert_eq!(lowpri_profile.retries(), 12);
+
+        let lowpri2_profile = config
+            .profile("lowpri2")
+            .expect("lowpri2 profile is present");
+        assert_eq!(lowpri2_profile.retries(), 18);
+    }
+
     fn temp_workspace(temp_dir: &Utf8Path, config_contents: &str) -> PackageGraph {
         Command::new(cargo_path())
             .args(["init", "--lib", "--name=test-package"])

nextest-runner/src/errors.rs

@@ -136,13 +136,48 @@ impl StatusLevelParseError {
     }
 }
 
+/// Error returned while parsing a [`ToolConfigFile`](crate::config::ToolConfigFile) value.
+#[derive(Clone, Debug, Error)]
+pub enum ToolConfigFileParseError {
+    #[error(
+        "tool-config-file has invalid format: {input}\n(hint: tool configs must be in the format <tool-name>:<path>)"
+    )]
+    /// The input was not in the format "tool:path".
+    InvalidFormat {
+        /// The input that failed to parse.
+        input: String,
+    },
+
+    /// The tool name was empty.
+    #[error("tool-config-file has empty tool name: {input}")]
+    EmptyToolName {
+        /// The input that failed to parse.
+        input: String,
+    },
+
+    /// The config file path was empty.
+    #[error("tool-config-file has empty config file path: {input}")]
+    EmptyConfigFile {
+        /// The input that failed to parse.
+        input: String,
+    },
+
+    /// The config file was not an absolute path.
+    #[error("tool-config-file is not an absolute path: {config_file}")]
+    ConfigFileNotAbsolute {
+        /// The file name that wasn't absolute.
+        config_file: Utf8PathBuf,
+    },
+}
+
 /// Error returned while parsing a [`TestThreads`](crate::config::TestThreads) value.
 #[derive(Clone, Debug, Error)]
 #[error(
-    "unrecognized value for test-threads: {input}\n(expected either an integer or \"num-cpus\")"
+    "unrecognized value for test-threads: {input}\n(hint: expected either an integer or \"num-cpus\")"
 )]
 pub struct TestThreadsParseError {
-    input: String,
+    /// The input that failed to parse.
+    pub input: String,
 }
 
 impl TestThreadsParseError {

nextest-runner/tests/integration/basic.rs

@@ -88,8 +88,7 @@ fn test_run() -> Result<()> {
 
     let test_filter = TestFilterBuilder::any(RunIgnored::Default);
     let test_list = FIXTURE_TARGETS.make_test_list(&test_filter, &TargetRunner::empty());
-    let config = NextestConfig::from_sources(workspace_root(), &*PACKAGE_GRAPH, None)
-        .expect("loaded fixture config");
+    let config = load_config();
     let profile = config
         .profile(NextestConfig::DEFAULT_PROFILE)
         .expect("default config is valid");
@@ -180,8 +179,7 @@ fn test_run_ignored() -> Result<()> {
 
     let test_filter = TestFilterBuilder::any(RunIgnored::IgnoredOnly);
     let test_list = FIXTURE_TARGETS.make_test_list(&test_filter, &TargetRunner::empty());
-    let config = NextestConfig::from_sources(workspace_root(), &*PACKAGE_GRAPH, None)
-        .expect("loaded fixture config");
+    let config = load_config();
     let profile = config
         .profile(NextestConfig::DEFAULT_PROFILE)
         .expect("default config is valid");
@@ -368,8 +366,7 @@ fn test_retries(retries: Option<usize>) -> Result<()> {
 
     let test_filter = TestFilterBuilder::any(RunIgnored::Default);
     let test_list = FIXTURE_TARGETS.make_test_list(&test_filter, &TargetRunner::empty());
-    let config = NextestConfig::from_sources(workspace_root(), &*PACKAGE_GRAPH, None)
-        .expect("loaded fixture config");
+    let config = load_config();
     let profile = config
         .profile("with-retries")
         .expect("with-retries config is valid");
@@ -507,8 +504,7 @@ fn test_termination() -> Result<()> {
     );
 
     let test_list = FIXTURE_TARGETS.make_test_list(&test_filter, &TargetRunner::empty());
-    let config = NextestConfig::from_sources(workspace_root(), &*PACKAGE_GRAPH, None)
-        .expect("loaded fixture config");
+    let config = load_config();
     let profile = config
         .profile("with-termination")
         .expect("with-termination config is valid");