Respect env defined in cargo.toml

Author: arxanas

cargo-nextest/src/dispatch.rs

@@ -12,7 +12,7 @@ use clap::{ArgAction, Args, Parser, Subcommand, ValueEnum};
 use guppy::graph::PackageGraph;
 use itertools::Itertools;
 use nextest_filtering::FilteringExpr;
-use nextest_metadata::{BinaryListSummary, BuildPlatform};
+use nextest_metadata::{BinaryListSummary, BuildPlatform, EnvironmentMap};
 use nextest_runner::{
     cargo_config::{CargoConfigs, TargetTriple},
     config::{NextestConfig, NextestProfile, RetryPolicy, TestThreads, ToolConfigFile},
@@ -442,6 +442,7 @@ impl TestBuildFilter {
         binary_list: Arc<BinaryList>,
         test_filter_builder: TestFilterBuilder,
         runner: &TargetRunner,
+        env: EnvironmentMap,
         reuse_build: &ReuseBuildInfo,
     ) -> Result<TestList<'g>> {
         let path_mapper = make_path_mapper(
@@ -463,6 +464,7 @@ impl TestBuildFilter {
             rust_build_meta,
             &test_filter_builder,
             runner,
+            env,
             // TODO: do we need to allow customizing this?
             num_cpus::get(),
         )
@@ -1037,11 +1039,13 @@ impl App {
         test_filter_builder: TestFilterBuilder,
         target_runner: &TargetRunner,
     ) -> Result<TestList> {
+        let env = self.base.cargo_configs.env()?;
         self.build_filter.compute_test_list(
             self.base.graph(),
             binary_list,
             test_filter_builder,
             target_runner,
+            env,
             &self.base.reuse_build,
         )
     }

cargo-nextest/src/tests_integration/fixtures.rs

@@ -138,6 +138,10 @@ pub(super) fn set_env_vars() {
     // This environment variable is required to test the #[bench] fixture. Note that THIS IS FOR
     // TEST CODE ONLY. NEVER USE THIS IN PRODUCTION.
     std::env::set_var("RUSTC_BOOTSTRAP", "1");
+
+    // Disable the tests which check for environment variables being set in `config.toml`, as they
+    // won't be in the search path when running integration tests.
+    std::env::set_var("__NEXTEST_NO_CHECK_CARGO_ENV_VARS", "1");
 }
 
 #[track_caller]

fixtures/nextest-tests/.cargo/config

@@ -12,3 +12,10 @@ replace-with = "vendored-sources"
 
 [source.vendored-sources]
 directory = "vendor"
+
+[env]
+__NEXTEST_ENV_VAR_FOR_TESTING_NOT_IN_PARENT_ENV = "if-this-value-is-present-then-test-passed"
+__NEXTEST_ENV_VAR_FOR_TESTING_IN_PARENT_ENV_NO_OVERRIDE = "if-this-value-is-present-then-test-failed"
+__NEXTEST_ENV_VAR_FOR_TESTING_IN_PARENT_ENV_OVERRIDDEN = { value = "if-this-value-is-present-then-test-passed", force = true }
+__NEXTEST_ENV_VAR_FOR_TESTING_IN_PARENT_ENV_RELATIVE_NO_OVERRIDE = { value = "if-this-value-is-present-then-test-failed", relative = true }
+__NEXTEST_ENV_VAR_FOR_TESTING_IN_PARENT_ENV_RELATIVE_OVERRIDDEN = { value = "if-this-value-is-present-then-test-passed", force = true, relative = true }

fixtures/nextest-tests/tests/basic.rs

@@ -156,6 +156,32 @@ fn test_cargo_env_vars() {
     // CARGO_PRIMARY_PACKAGE is missing at runtime
     // CARGO_TARGET_TMPDIR is missing at runtime
     // Dynamic library paths are tested by actually executing the tests -- they depend on the dynamic library.
+
+    if std::env::var("__NEXTEST_NO_CHECK_CARGO_ENV_VARS").is_err() {
+        assert_eq!(
+            std::env::var("__NEXTEST_ENV_VAR_FOR_TESTING_NOT_IN_PARENT_ENV").as_deref(),
+            Ok("if-this-value-is-present-then-test-passed")
+        );
+        assert_eq!(
+            std::env::var("__NEXTEST_ENV_VAR_FOR_TESTING_IN_PARENT_ENV_NO_OVERRIDE").as_deref(),
+            Ok("if-this-value-is-present-then-test-passed")
+        );
+        assert_eq!(
+            std::env::var("__NEXTEST_ENV_VAR_FOR_TESTING_IN_PARENT_ENV_OVERRIDDEN").as_deref(),
+            Ok("if-this-value-is-present-then-test-passed")
+        );
+        assert_eq!(
+            std::env::var("__NEXTEST_ENV_VAR_FOR_TESTING_IN_PARENT_ENV_RELATIVE_NO_OVERRIDE")
+                .as_deref(),
+            Ok("if-this-value-is-present-then-test-passed")
+        );
+        let overridden_path =
+            std::env::var("__NEXTEST_ENV_VAR_FOR_TESTING_IN_PARENT_ENV_RELATIVE_OVERRIDDEN").unwrap();
+        assert!(overridden_path.ends_with(&format!("{SEP}.cargo{SEP}if-this-value-is-present-then-test-passed", SEP = std::path::MAIN_SEPARATOR)),
+                "Path {:?} should start end with the relative file path",
+                overridden_path,
+        );
+    }
 }
 
 #[test]

nextest-metadata/src/test_list.rs

@@ -12,6 +12,36 @@ use std::{
     process::Command,
 };
 
+/// An environment variable set in `config.toml`. See https://doc.rust-lang.org/cargo/reference/config.html#env
+#[derive(Clone, Debug, Serialize, Deserialize, PartialEq, Eq)]
+pub struct CargoEnvironmentVariable {
+    /// The source `config.toml` file. See
+    /// https://doc.rust-lang.org/cargo/reference/config.html#hierarchical-structure for the lookup
+    /// order.
+    pub source: Option<Utf8PathBuf>,
+
+    /// The name of the environment variable to set.
+    pub name: String,
+
+    /// The value of the environment variable to set.
+    pub value: String,
+
+    /// If the environment variable is already set in the environment, it is not reassigned unless
+    /// `force` is set to `true`.
+    pub force: bool,
+
+    /// Interpret the environment variable as a path relative to the directory containing the source
+    /// `config.toml` file.
+    pub relative: bool,
+}
+
+/// A list of environment variables to set when running tests.
+///
+/// This is a `Vec` instead of a map because, on Windows, environment variables are case-insensitive
+/// but case-preserving. We produce the environment as-is and let the caller handle the case of
+/// duplicates.
+pub type EnvironmentMap = Vec<CargoEnvironmentVariable>;
+
 /// Command builder for `cargo nextest list`.
 #[derive(Clone, Debug, Default)]
 pub struct ListCommand {

nextest-runner/src/cargo_config.rs

@@ -8,6 +8,7 @@
 
 use crate::errors::{CargoConfigError, InvalidCargoCliConfigReason, TargetTripleError};
 use camino::{Utf8Path, Utf8PathBuf};
+use nextest_metadata::{CargoEnvironmentVariable, EnvironmentMap};
 use once_cell::sync::OnceCell;
 use serde::Deserialize;
 use std::{collections::BTreeMap, fmt};
@@ -241,6 +242,50 @@ impl CargoConfigs {
         &self.cwd
     }
 
+    /// The environment variables to set when running Cargo commands.
+    pub fn env(&self) -> Result<EnvironmentMap, CargoConfigError> {
+        let env = self
+            .discovered_configs()?
+            .filter_map(|config| match config {
+                DiscoveredConfig::CliOption { config, source }
+                | DiscoveredConfig::File { config, source } => Some((config, source)),
+                DiscoveredConfig::Env => None,
+            })
+            .flat_map(|(config, source)| {
+                let source = match source {
+                    CargoConfigSource::CliOption => None,
+                    CargoConfigSource::File(path) => Some(path.clone()),
+                };
+                config
+                    .env
+                    .clone()
+                    .into_iter()
+                    .map(move |(name, value)| (source.clone(), name, value))
+            })
+            .map(|(source, name, value)| match value {
+                CargoConfigEnv::Value(value) => CargoEnvironmentVariable {
+                    source,
+                    name,
+                    value,
+                    force: false,
+                    relative: false,
+                },
+                CargoConfigEnv::Fields {
+                    value,
+                    force,
+                    relative,
+                } => CargoEnvironmentVariable {
+                    source,
+                    name,
+                    value,
+                    force,
+                    relative,
+                },
+            })
+            .collect();
+        Ok(env)
+    }
+
     pub(crate) fn discovered_configs(
         &self,
     ) -> Result<
@@ -493,11 +538,26 @@ fn load_file(
     Ok((CargoConfigSource::File(path), config))
 }
 
+#[derive(Clone, Deserialize, Debug)]
+#[serde(untagged)]
+pub(crate) enum CargoConfigEnv {
+    Value(String),
+    Fields {
+        value: String,
+        #[serde(default)]
+        force: bool,
+        #[serde(default)]
+        relative: bool,
+    },
+}
+
 #[derive(Deserialize, Debug)]
 pub(crate) struct CargoConfig {
     #[serde(default)]
     pub(crate) build: CargoConfigBuild,
     pub(crate) target: Option<BTreeMap<String, CargoConfigRunner>>,
+    #[serde(default)]
+    pub(crate) env: BTreeMap<String, CargoConfigEnv>,
 }
 
 #[derive(Deserialize, Default, Debug)]
@@ -745,6 +805,20 @@ mod tests {
         assert_eq!(find_target_triple(&[], None, &dir_path, &dir_path), None);
     }
 
+    #[test]
+    fn test_env_var_precedence() {
+        let dir = setup_temp_dir().unwrap();
+        let dir_path = Utf8PathBuf::try_from(dir.path().canonicalize().unwrap()).unwrap();
+        let dir_foo_path = dir_path.join("foo");
+        let dir_foo_bar_path = dir_foo_path.join("bar");
+
+        let configs =
+            CargoConfigs::new_with_isolation(&[] as &[&str], &dir_foo_bar_path, &dir_path).unwrap();
+        let env = configs.env().unwrap();
+        let env_values: Vec<&str> = env.iter().map(|elem| elem.value.as_str()).collect();
+        assert_eq!(env_values, vec!["foo-bar-config", "foo-config"]);
+    }
+
     fn setup_temp_dir() -> Result<TempDir> {
         let dir = tempfile::Builder::new()
             .tempdir()
@@ -794,11 +868,17 @@ mod tests {
     static FOO_CARGO_CONFIG_CONTENTS: &str = r#"
     [build]
     target = "x86_64-pc-windows-msvc"
+
+    [env]
+    SOME_VAR = { value = "foo-config", force = true }
     "#;
 
     static FOO_BAR_CARGO_CONFIG_CONTENTS: &str = r#"
     [build]
     target = "x86_64-unknown-linux-gnu"
+
+    [env]
+    SOME_VAR = { value = "foo-bar-config", force = true }
     "#;
 
     static FOO_EXTRA_CONFIG_CONTENTS: &str = r#"