Allow setup scripts to define specific environment variables by metatoaster · Pull Request #3001 · nextest-rs/nextest

metatoaster · 2026-01-26T14:13:25Z

As per the comment I made on #978, I've gone ahead and implement exactly that, a way to set specific environment variables for setup scripts.

During the construction of the test for this, I've discovered a possibly undocumented silent failure (which got me down some rabbit holes, but I digress). if the setup script writes the NEXTEST prefixed variable to NEXTEST_ENV, it seems to cause all environment variables assigned to that file be ignored and thus not passed to the intended tests. I'd expect that it would rather only ignore values keyed under NEXTEST* and thus the test will see all other values under valid keys. I haven't dived too deep into the code for that part but maybe this could also be documented with the setup scripts if this is intended.

codecov · 2026-01-26T14:18:28Z

Codecov Report

❌ Patch coverage is 99.59677% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 85.20%. Comparing base (fe3c6f0) to head (dbe5d89).
⚠️ Report is 1 commits behind head on main.

Files with missing lines	Patch %	Lines
nextest-runner/src/config/scripts/imp.rs	97.72%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #3001      +/-   ##
==========================================
+ Coverage   85.11%   85.20%   +0.08%     
==========================================
  Files         157      158       +1     
  Lines       46125    46366     +241     
==========================================
+ Hits        39259    39504     +245     
+ Misses       6866     6862       -4

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

sunshowers

Thanks for doing this!

if the setup script writes the NEXTEST prefixed variable to NEXTEST_ENV, it seems to cause all environment variables assigned to that file be ignored and thus not passed to the intended tests. I'd expect that it would rather only ignore values keyed under NEXTEST* and thus the test will see all other values under valid keys.

Interesting -- I think this should just error out loudly instead (this is an experimental feature so it's fine to change it). I'd collect all env vars that are bad and explicitly error out on them.

sunshowers · 2026-01-31T23:00:02Z

+            env = {
+                MODE = "qux_mode",
+            }


env should be part of command (always requiring passing in as a map)

Yep, and this makes a lot of sense given how command is effectively shared with wrapper scripts. Naturally having wrapper scripts support environment variables will take some additional work - should that be included with this pull request?

In any case I've pushed a commit (without flattening onto the original) to reflect this - please let me know if flattening this new commit onto the existing one is required.

sunshowers · 2026-01-31T23:03:43Z

+[[profile.default.scripts]]
+platform = { host = "cfg(unix)" }
+filter = "test(=test_setup_script_env_vars)"
+setup = "my-script-unix-with-env"
+
+[[profile.default.scripts]]
+platform = { host = "cfg(windows)" }
+filter = "test(=test_setup_script_env_vars)"
+setup = "my-script-windows-with-env"
+


Do we need an extra test? Seems like we can combine this check into test_cargo_env_vars.

I had this extra definition and separate test so that I don't touch the existing definition that is working without environment variables defined, as the idea of this change is to be able to specify different environment variables for the same underlying script (e.g. my-script.sh), hence why I ultimately split this out.

That said It should be possible to combine it into test_cargo_env_vars if this is what's desired. Please let me know if this is the case.

sunshowers · 2026-01-31T23:04:25Z

+    NEXTEST = "0",
+    NEXTEST_PROFILE = "0",


I think these should error out similar to the environment that setup scripts set.

By error out, should this be done at the point of parsing?

metatoaster · 2026-02-02T00:05:09Z

Thanks for the feedback on the pull request. I will be working on the suggested changes in about a weeks time once my partner and our newborn is settled back at home hopefully soon. The delay in response is due to this evolving personal circumstance, but please know that I do want to get this change in so I may get rid of the workaround using platform specific scripts for the other project where I'm a contributor of.

sunshowers · 2026-02-02T00:21:17Z

Congratulations! Hope everyone's well 🩷

metatoaster · 2026-02-11T04:15:21Z

I think this should just error out loudly instead (this is an experimental feature so it's fine to change it). I'd collect all env vars that are bad and explicitly error out on them.

I had a bit more time to chase this down (between newborn needs) and the following are just notes and observations that I've made. The current behavior of parse_env_file simply returns an error upon encountering environment variables that start with NEXTEST, and in the executor this error is stored, where eventually it will make its way to an InfoResponse<'a>, though I am unsure how that bit gets to where it gets reported.

I also noted that when the setup script is finished it gets reported when the display reporter handles a TestEventKind::SetupScriptFinished. The run_status value is a SetupScriptExecuteStatus which contains the attribute error_summary, but it is unused at this part. I lazily used dbg! to reveal the following:

           SETUP [      1/2] my-script-unix: /tmp/nextest-fixture-2HiJEa/src/my-script.sh
    [nextest-runner/src/reporter/displayer/imp.rs:728:17] &run_status.error_summary = None
      SETUP PASS [   0.022s] my-script-unix: /tmp/nextest-fixture-2HiJEa/src/my-script.sh
           SETUP [      2/2] my-script-unix-with-env: /tmp/nextest-fixture-2HiJEa/src/my-script.sh
    [nextest-runner/src/reporter/displayer/imp.rs:728:17] &run_status.error_summary = Some(
        ErrorSummary {
            short_message: "error reading setup script output",
            description: "error reading setup script output\n  caused by:\n  - key `NEXTEST` begins with `NEXTEST`, which is reserved for internal use",
        },
    )
      SETUP PASS [   0.012s] my-script-unix-with-env: /tmp/nextest-fixture-2HiJEa/src/my-script.sh

In both options for error_summary they simply generate SETUP PASS with no indication that one of them had an error. Perhaps if either the short message or the description is written out it would serve to indicate a swallowed error.

So by "explicitly error out on them" (when a reserved key is set), should the behavior of this setup script be of an failure and trigger early abort the test run, much like how a non-zero exit code does? Or take a more lenient approach by collecting the env keys that are reserved and then ensure that they all show up under error_summary and then update the report for TestEventKind::SetupScriptFinished to include that? I can take a stab at doing either implementation if this is desired.

sunshowers · 2026-02-22T20:42:32Z

Sorry for the delay, work has kept me very busy.

So by "explicitly error out on them" (when a reserved key is set), should the behavior of this setup script be of an failure and trigger early abort the test run, much like how a non-zero exit code does

Yeah, that is definitely what we want here -- fail loudly. Working on fixing this right now -- thanks for catching!

edit: fixed in #3094.

metatoaster · 2026-02-24T03:41:13Z

Thanks for the fix! Also the pull request you just did is also quite instructive with regards to project organization. I will tidy up this pull request in due time, and let me know if this plan works for you (to address the two remaining issues raised during the code review):

Have the config parser error out on all NEXTEST reserved values that are present inside command.env
Combine the new test I added into test_cargo_env_vars (and move the standalone tests I intend to add from fixtures/nextest-tests/tests/basic.rs into integration-tests/tests/integration/main.rs).
Update the docs to reflect the reserved keys.

sunshowers · 2026-02-24T03:48:13Z

Sounds great!

metatoaster · 2026-02-26T15:32:33Z

I ended up squashing all the code commits into one, given the intrusive nature of the snapshot modifications from that one test that got added then removed (conflicting with the current main branch, the merge target) and that some of the later commits ended up being rather trivial (e.g. adding tests). Should the original commits are of interests, I pushed a separate branch with most of the individual commits based on the earlier commit that I was working on top of. Sorry for the wait, the needs of a newborn are many after all.

sunshowers · 2026-03-09T19:43:39Z

+    assert!(matches!(
+        std::env::var("CMD_ENV_VAR").as_deref(),
+        Err(&std::env::VarError::NotPresent) | Ok("not-set-in-conf"),
+    ));


Is this ever not present? If so, could this be written as an exhaustive match? If not, then just assert on the specific value.

I ended up resorting to an additional environment variable specific to test_setup_script_defined_env for the disambiguation of this particular test, which is also part of the fix that addresses the location of test_list.cargo_env(), which is done in 0d00ce9.

sunshowers · 2026-03-09T19:43:50Z

                    program,
                    args,
+                    env: BTreeMap::new(),
                    relative_to: ScriptCommandRelativeTo::None,


Ah so I realized that ScriptCommand is used for both setup and wrapper scripts, but we only apply env to setup scripts. Could you also extend env to apply to wrapper scripts (and add a test)?

Yeah I was thinking of doing this in a separate pull request, but we can do it here in one go also. It is now done in 68919b3.

sunshowers · 2026-03-09T19:46:49Z

+        // Set the additional user-provided environment variables assigned to the setup
+        // script configuration after the global values assigned above but before the
+        // test runner controlled ones which are assigned below, as per above note.
+        cmd.envs(config.command.env.iter());


To ensure that current and future keys are covered, can we use an exhaustive destructure here (and in wrapper scripts)?

Sorry I am not following this particular comment, as the intention is to assign all arbitrary keys and values in this map to the command, the keys and values here should have been validated by this point given the deserializer provided that. I thought assignment of all values with an iterator is suitable, so I am not sure what is meant by an exhaustive destructure on an arbitrary map here.

Edit: Unless of course you mean I should use cmd.env on each of the key/value pairs obtained by destructuring every single entry inside config.command.env rather than simply using cmd.envs, though if we are going down this far, there should probably be a newtype for this BTreeMap and implement fn apply_env(cmd: &mut Command) for that with the additional filtering/validation logic if this is what you are truly after, though at this point that particular map, if deserialized through serde, should have the unexpected values filtered out.

That said, should we have this newtype, the needs from this comment can simply be addressed.

sunshowers · 2026-03-09T19:49:35Z

+                    if key.starts_with("NEXTEST") {
+                        return Err(A::Error::invalid_value(
+                            serde::de::Unexpected::Str(&key),
+                            &"a key that does not begin with `NEXTEST`, which is reserved for internal use",
+                        ));


I feel like we should apply some more checks to the environment key here (e.g. not supporting cases like ' ') though we have this issue elsewhere so that can be a followup.

I ended up started working on this too as part of the above comment about environment keys, since I found relevant info about environment variables in POSIX.1-2024. Will push new commit hopefully in the next couple days.

I added 6fba33b and c0f5273 to address the need to do this check. Refer to the lengthy comment before ENV_KEY_VALIDATION which was added in 6fba33b for the background I've included with why the restriction and checks are in place. In c0f5273 I added the minimal viable implementation to propagate the new error returned from the validation back up to where they may be consumed, so the level of detail captured might be lower than expected. Though given how the validation should have been done during deserialization, the code paths that can result in invalid environment variables keys triggering the error might not actually be hit under usage through the CLI/configuration files. Anyway I am looking forward to what you think, and please let me know if and what else I should improve.

sunshowers · 2026-03-09T19:50:06Z

    );

    assert_eq!(std::env::var("MY_ENV_VAR").as_deref(), Ok("my-env-var"));
+    // The command for the script has received the value


Please add a period to the end of this comment.

Not sure how I managed to delete the rest of that sentence without noticing it myself, but this is now addressed with the fix to the ordering of test_list.cargo_env().apply_env in 0d00ce9.

sunshowers · 2026-03-09T19:50:47Z

+            }
+        }
+
+        deserializer.deserialize_any(EnvMapVisitor)


Can this be deserialize_map?

Done in 55ac2d6.

sunshowers · 2026-03-09T19:52:56Z

@@ -314,6 +314,11 @@ impl SetupScriptCommand {
        // `CARGO_*` and `NEXTEST_*` variables set directly on `cmd` below.


I believe this comment may not be true any more -- I think test_list.cargo_env().apply_env should move below cmd.envs.

Yeah, I should have investigated what test_list.cargo_env() does in greater detail, indeed it does not inhibit how I want to apply command.env. Swapping it around as suggested does not change the intended behavior where command.env overrides standard environment variables, and on top of that, doing so will allow force = true to function as intended to override values specified in command.env. I've updated the code and extended the test case to account for this which is done in 0d00ce9.

Allow specific environment variables be defined for a given setup script command such that they don't need to be wrapped by platform specific scripts (i.e. batch file vs. bash) to set them up when they may be platform agnostic. This also has the effect that allows specific environment variables be passed to specific tests via a simple wrapper program that may be configured directly on the nextest configuration. Ensure that environment variables (by extension, the `env` from Cargo's config files) will not override the values defined in the command's `env` section, and that supplied keys cannot begin with `NEXTEST`.

Use another special environment variable to test for expected values that are brought active by the integration test environment. This test also validates that the expected priority of variable assigments from Cargo's config are upheld.

Have wrapper scripts also support the passing of the map of envronment variables defined in its `command.env` to the underlying command.

Additional test to show where and what values are defined when a value is defined for both wrapper and setup scripts, that the setup script will still receive the correct value it has been defined for.

Rounding out the coverage report, might as well cover the similarly uncovered cases.

Provide `ScriptCommandEnvMap` which uses the `validate_env_var_key` helper to validate keys during deserialization and applying of the environment variables onto a `Command`. Existing usage of `.envs()` with the original `BTreeMap` are converted to using the `.apply_env()` method, but the validation errors currently ignored as the caller will need to be updated to handle this new error.

Have `TestCommand::new` return a result instead with the error being a `CommandSetupError`, where the only variant being the environment variable being invalid. The consequential errors are simply added as variant under the relevant error enums, with reuse of existing structs where possible to achieve a minimally viable implementation.

sunshowers · 2026-03-15T20:52:19Z

Looks great, thanks. I ran into the same issue with hyphens in env vars a while ago: see https://nexte.st/docs/configuration/env-vars/#environment-variables-nextest-sets (NEXTEST_BIN_EXE_<name>)

Will do a final review and then get this in for release next week.

Make it so that if a `ScriptCommandEnvMap` is successfully created, it has valid environment variable keys.

We should not set the environment if the target runner is inactive. Fix and add a test.

metatoaster · 2026-03-16T02:49:33Z

Thank you for your time for the code review and merging this in!

metatoaster force-pushed the setup-scripts-env branch from 5dda85f to b899406 Compare January 26, 2026 14:27

loci-dev mentioned this pull request Jan 26, 2026

UPSTREAM PR #3001: Allow setup scripts to define specific environment variables auroralabs-loci/nextest#11

Open

metatoaster force-pushed the setup-scripts-env branch 2 times, most recently from 968d296 to aa6bf5c Compare January 26, 2026 16:15

sunshowers reviewed Jan 31, 2026

View reviewed changes

loci-dev mentioned this pull request Feb 2, 2026

UPSTREAM PR #3001: Allow setup scripts to define specific environment variables auroralabs-loci/nextest#23

Open

metatoaster force-pushed the setup-scripts-env branch 3 times, most recently from 0fae3c2 to 8275c3c Compare February 11, 2026 00:28

metatoaster force-pushed the setup-scripts-env branch 2 times, most recently from fbe6f7a to 8bbca44 Compare February 26, 2026 15:06

metatoaster requested a review from sunshowers February 26, 2026 15:28

sunshowers reviewed Mar 9, 2026

View reviewed changes

metatoaster force-pushed the setup-scripts-env branch 6 times, most recently from 35b376c to 9489b09 Compare March 12, 2026 12:41

metatoaster added 2 commits March 14, 2026 00:27

metatoaster added 7 commits March 14, 2026 00:27

[nextest-runner] use deserialize_map instead of deserialize_any

55ac2d6

[nextest-runner] add wrapper scripts env vars

68919b3

Have wrapper scripts also support the passing of the map of envronment variables defined in its `command.env` to the underlying command.

[nextest-runner] Test for shadowing of duplicate env vars

17ee78a

Additional test to show where and what values are defined when a value is defined for both wrapper and setup scripts, that the setup script will still receive the correct value it has been defined for.

[nextest-runner] rounding out config parser tests

adb82c7

Rounding out the coverage report, might as well cover the similarly uncovered cases.

[site] document env for command in the scripts

09cf5bd

metatoaster force-pushed the setup-scripts-env branch from 9489b09 to c0f5273 Compare March 13, 2026 11:30

sunshowers added 7 commits March 15, 2026 16:31

[nextest-runner] make ScriptCommandEnvMap have newtype invariants

e5badc2

Make it so that if a `ScriptCommandEnvMap` is successfully created, it has valid environment variable keys.

[nextest-runner] fix target-runner overrides-wrapper behavior

a6320a9

We should not set the environment if the target runner is inactive. Fix and add a test.

[meta] copyedit pass

7e2fba6

[nextest-runner] always prefer per-test configs over cargo

a7a8cbe

[meta] more self-review fixes

6c95c8f

merge main into branch

8e81af3

[meta] a few more fixes

dbe5d89

sunshowers merged commit d792fb0 into nextest-rs:main Mar 16, 2026
21 checks passed

		@@ -314,6 +314,11 @@ impl SetupScriptCommand {
		// `CARGO_` and `NEXTEST_` variables set directly on `cmd` below.

Uh oh!

Conversation

metatoaster commented Jan 26, 2026

Uh oh!

codecov Bot commented Jan 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

sunshowers left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

metatoaster Feb 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

metatoaster commented Feb 2, 2026

Uh oh!

sunshowers commented Feb 2, 2026

Uh oh!

metatoaster commented Feb 11, 2026

Uh oh!

sunshowers commented Feb 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

metatoaster commented Feb 24, 2026

Uh oh!

sunshowers commented Feb 24, 2026

Uh oh!

metatoaster commented Feb 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

metatoaster Mar 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

metatoaster Mar 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

metatoaster Mar 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

metatoaster Mar 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

metatoaster Mar 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

metatoaster Mar 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

codecov Bot commented Jan 26, 2026 •

edited

Loading

metatoaster Feb 10, 2026 •

edited

Loading

sunshowers commented Feb 22, 2026 •

edited

Loading

metatoaster commented Feb 26, 2026 •

edited

Loading

metatoaster Mar 10, 2026 •

edited

Loading

metatoaster Mar 10, 2026 •

edited

Loading

metatoaster Mar 10, 2026 •

edited

Loading

metatoaster Mar 12, 2026 •

edited

Loading

metatoaster Mar 10, 2026 •

edited

Loading

metatoaster Mar 10, 2026 •

edited

Loading

metatoaster Mar 10, 2026 •

edited

Loading