XXX this fails tests: Respect env defined in config.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,24 @@ replace-with = "vendored-sources"
 
 [source.vendored-sources]
 directory = "vendor"
+
+[env]
+CONFIG_WORKSPACE_DIR = { value = "", relative = true }
+
+__NEXTEST_ENV_VAR_FOR_TESTING_NOT_IN_PARENT_ENV = "test-PASSED-value-set-by-main-config"
+__NEXTEST_ENV_VAR_FOR_TESTING_IN_PARENT_ENV_NO_OVERRIDE = "test-FAILED-value-set-by-main-config"
+__NEXTEST_ENV_VAR_FOR_TESTING_IN_PARENT_ENV_OVERRIDDEN = { value = "test-PASSED-value-set-by-main-config", force = true }
+__NEXTEST_ENV_VAR_FOR_TESTING_IN_PARENT_ENV_RELATIVE_NO_OVERRIDE = { value = "test-FAILED-value-set-by-main-config", relative = true }
+__NEXTEST_ENV_VAR_FOR_TESTING_IN_PARENT_ENV_RELATIVE_OVERRIDDEN = { value = "test-PASSED-value-set-by-main-config", force = true, relative = true }
+
+# See extra-config.toml in this directory for more about the __NEXTEST_TESTING_EXTRA_CONFIG variables.
+# The extra config should always override the main config, no matter what "force" is.
+__NEXTEST_TESTING_EXTRA_CONFIG_FORCE_IN_EXTRA = "test-FAILED-value-set-by-main-config"
+__NEXTEST_TESTING_EXTRA_CONFIG_FORCE_IN_MAIN = { value = "test-FAILED-value-set-by-main-config", force = true }
+__NEXTEST_TESTING_EXTRA_CONFIG_FORCE_IN_BOTH = { value = "test-FAILED-value-set-by-main-config", force = true }
+__NEXTEST_TESTING_EXTRA_CONFIG_FORCE_NONE = "test-FAILED-value-set-by-main-config"
+
+__NEXTEST_TESTING_EXTRA_CONFIG_OVERRIDE_FORCE_IN_EXTRA = "test-FAILED-value-set-by-main-config"
+__NEXTEST_TESTING_EXTRA_CONFIG_OVERRIDE_FORCE_IN_MAIN = { value = "test-FAILED-value-set-by-main-config", force = true }
+__NEXTEST_TESTING_EXTRA_CONFIG_OVERRIDE_FORCE_IN_BOTH = { value = "test-FAILED-value-set-by-main-config", force = true }
+__NEXTEST_TESTING_EXTRA_CONFIG_OVERRIDE_FORCE_NONE = "test-FAILED-value-set-by-main-config"

fixtures/nextest-tests/.cargo/extra-config.toml

@@ -0,0 +1,17 @@
+[env]
+EXTRA_CONFIG_WORKS = "1"
+
+# These __NEXTEST_TESTING_EXTRA_CONFIG variables are used to ensure that the extra config *always*
+# overrides the main config, no matter what force is set to in either.
+
+# These variables are not defined in the environment.
+__NEXTEST_TESTING_EXTRA_CONFIG_FORCE_IN_EXTRA = { value = "test-PASSED-value-set-by-extra-config", force = true }
+__NEXTEST_TESTING_EXTRA_CONFIG_FORCE_IN_MAIN = "test-PASSED-value-set-by-extra-config"
+__NEXTEST_TESTING_EXTRA_CONFIG_FORCE_IN_BOTH = { value = "test-PASSED-value-set-by-extra-config", force = true }
+__NEXTEST_TESTING_EXTRA_CONFIG_FORCE_NONE = "test-PASSED-value-set-by-extra-config"
+
+# These values are defined in the environment so they should be overridden as required.
+__NEXTEST_TESTING_EXTRA_CONFIG_OVERRIDE_FORCE_IN_EXTRA = { value = "test-PASSED-value-set-by-extra-config", force = true }
+__NEXTEST_TESTING_EXTRA_CONFIG_OVERRIDE_FORCE_IN_MAIN = "test-FAILED-value-set-by-extra-config"
+__NEXTEST_TESTING_EXTRA_CONFIG_OVERRIDE_FORCE_IN_BOTH = { value = "test-PASSED-value-set-by-extra-config", force = true }
+__NEXTEST_TESTING_EXTRA_CONFIG_OVERRIDE_FORCE_NONE = "test-FAILED-value-set-by-extra-config"

fixtures/nextest-tests/tests/basic.rs

@@ -1,5 +1,9 @@
 // Copyright (c) The nextest Contributors
-use std::{env, io::Read, path::Path};
+use std::{
+    env,
+    io::Read,
+    path::{Path, PathBuf},
+};
 
 #[test]
 fn test_success() {}
@@ -156,6 +160,77 @@ 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() {
+        let config_workspace_dir = PathBuf::from(
+            std::env::var("CONFIG_WORKSPACE_DIR")
+                .expect("CONFIG_WORKSPACE_DIR should be present, defined in [env]"),
+        );
+        assert_eq!(
+            config_workspace_dir,
+            PathBuf::from(std::env::var("CARGO_MANIFEST_DIR").unwrap())
+        );
+        assert_eq!(
+            std::env::var("__NEXTEST_ENV_VAR_FOR_TESTING_NOT_IN_PARENT_ENV").as_deref(),
+            Ok("test-PASSED-value-set-by-main-config")
+        );
+        assert_eq!(
+            std::env::var("__NEXTEST_ENV_VAR_FOR_TESTING_IN_PARENT_ENV_NO_OVERRIDE").as_deref(),
+            Ok("test-PASSED-value-set-by-environment")
+        );
+        assert_eq!(
+            std::env::var("__NEXTEST_ENV_VAR_FOR_TESTING_IN_PARENT_ENV_OVERRIDDEN").as_deref(),
+            Ok("test-PASSED-value-set-by-main-config")
+        );
+        assert_eq!(
+            std::env::var("__NEXTEST_ENV_VAR_FOR_TESTING_IN_PARENT_ENV_RELATIVE_NO_OVERRIDE")
+                .as_deref(),
+            Ok("test-PASSED-value-set-by-environment")
+        );
+        let overridden_path =
+            std::env::var("__NEXTEST_ENV_VAR_FOR_TESTING_IN_PARENT_ENV_RELATIVE_OVERRIDDEN")
+                .unwrap();
+        assert_eq!(
+            &overridden_path,
+            config_workspace_dir
+                .join("test-PASSED-value-set-by-main-config")
+                .to_str()
+                .unwrap(),
+        );
+
+        assert_eq!(
+            std::env::var("__NEXTEST_TESTING_EXTRA_CONFIG_FORCE_IN_EXTRA").as_deref(),
+            Ok("test-PASSED-value-set-by-extra-config"),
+        );
+        assert_eq!(
+            std::env::var("__NEXTEST_TESTING_EXTRA_CONFIG_FORCE_IN_MAIN").as_deref(),
+            Ok("test-PASSED-value-set-by-extra-config")
+        );
+        assert_eq!(
+            std::env::var("__NEXTEST_TESTING_EXTRA_CONFIG_FORCE_IN_BOTH").as_deref(),
+            Ok("test-PASSED-value-set-by-extra-config")
+        );
+        assert_eq!(
+            std::env::var("__NEXTEST_TESTING_EXTRA_CONFIG_FORCE_NONE").as_deref(),
+            Ok("test-PASSED-value-set-by-extra-config")
+        );
+        assert_eq!(
+            std::env::var("__NEXTEST_TESTING_EXTRA_CONFIG_OVERRIDE_FORCE_IN_EXTRA").as_deref(),
+            Ok("test-PASSED-value-set-by-extra-config")
+        );
+        assert_eq!(
+            std::env::var("__NEXTEST_TESTING_EXTRA_CONFIG_OVERRIDE_FORCE_IN_MAIN").as_deref(),
+            Ok("test-PASSED-value-set-by-environment")
+        );
+        assert_eq!(
+            std::env::var("__NEXTEST_TESTING_EXTRA_CONFIG_OVERRIDE_FORCE_IN_BOTH").as_deref(),
+            Ok("test-PASSED-value-set-by-extra-config")
+        );
+        assert_eq!(
+            std::env::var("__NEXTEST_TESTING_EXTRA_CONFIG_OVERRIDE_FORCE_NONE").as_deref(),
+            Ok("test-PASSED-value-set-by-environment")
+        );
+    }
 }
 
 #[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 {