[nextest-runner] add script output environment variables to JUnit report

These environment variables are useful to have in the JUnit output.

nextest-runner/src/config/scripts.rs

@@ -11,27 +11,26 @@ use crate::{
     double_spawn::{DoubleSpawnContext, DoubleSpawnInfo},
     errors::{
         ChildStartError, ConfigCompileError, ConfigCompileErrorKind, ConfigCompileSection,
-        InvalidConfigScriptName, SetupScriptOutputError,
+        InvalidConfigScriptName,
     },
     list::TestList,
     platform::BuildPlatforms,
+    reporter::events::SetupScriptEnvMap,
     test_command::{apply_ld_dyld_env, create_command},
 };
-use camino::Utf8Path;
 use camino_tempfile::Utf8TempPath;
 use guppy::graph::{cargo::BuildPlatform, PackageGraph};
 use indexmap::IndexMap;
 use nextest_filtering::{EvalContext, Filterset, FiltersetKind, ParseContext, TestQuery};
 use serde::{de::Error, Deserialize};
 use smol_str::SmolStr;
 use std::{
-    collections::{BTreeMap, HashMap, HashSet},
+    collections::{HashMap, HashSet},
     fmt,
     process::Command,
     sync::Arc,
     time::Duration,
 };
-use tokio::io::{AsyncBufReadExt, BufReader};
 
 /// Data about setup scripts, returned by an [`EvaluatableProfile`].
 pub struct SetupScripts<'profile> {
@@ -244,63 +243,6 @@ impl<'profile> SetupScriptExecuteData<'profile> {
     }
 }
 
-#[derive(Clone, Debug)]
-pub(crate) struct SetupScriptEnvMap {
-    env_map: BTreeMap<String, String>,
-}
-
-impl SetupScriptEnvMap {
-    pub(crate) async fn new(env_path: &Utf8Path) -> Result<Self, SetupScriptOutputError> {
-        let mut env_map = BTreeMap::new();
-        let f = tokio::fs::File::open(env_path).await.map_err(|error| {
-            SetupScriptOutputError::EnvFileOpen {
-                path: env_path.to_owned(),
-                error: Arc::new(error),
-            }
-        })?;
-        let reader = BufReader::new(f);
-        let mut lines = reader.lines();
-        loop {
-            let line =
-                lines
-                    .next_line()
-                    .await
-                    .map_err(|error| SetupScriptOutputError::EnvFileRead {
-                        path: env_path.to_owned(),
-                        error: Arc::new(error),
-                    })?;
-            let Some(line) = line else { break };
-
-            // Split this line into key and value.
-            let (key, value) = match line.split_once('=') {
-                Some((key, value)) => (key, value),
-                None => {
-                    return Err(SetupScriptOutputError::EnvFileParse {
-                        path: env_path.to_owned(),
-                        line: line.to_owned(),
-                    })
-                }
-            };
-
-            // Ban keys starting with `NEXTEST`.
-            if key.starts_with("NEXTEST") {
-                return Err(SetupScriptOutputError::EnvFileReservedKey {
-                    key: key.to_owned(),
-                });
-            }
-
-            env_map.insert(key.to_owned(), value.to_owned());
-        }
-
-        Ok(Self { env_map })
-    }
-
-    #[inline]
-    pub(crate) fn len(&self) -> usize {
-        self.env_map.len()
-    }
-}
-
 #[derive(Clone, Debug)]
 pub(crate) struct CompiledProfileScripts<State> {
     pub(super) setup: Vec<ScriptId>,

nextest-runner/src/reporter/aggregator/junit.rs

@@ -92,6 +92,12 @@ impl<'cfg> MetadataJunit<'cfg> {
                 // Add properties corresponding to the setup script.
                 test_suite.add_property(("command", command));
                 test_suite.add_property(("args".to_owned(), shell_words::join(args)));
+                // Also add environment variables set by the script.
+                if let Some(env_map) = run_status.env_map {
+                    for (key, value) in env_map.env_map {
+                        test_suite.add_property((format!("output-env:{key}"), value));
+                    }
+                }
             }
             TestEventKind::InfoStarted { .. }
             | TestEventKind::InfoResponse { .. }

nextest-runner/src/reporter/events.rs

@@ -15,7 +15,7 @@ use crate::{
 use chrono::{DateTime, FixedOffset};
 use nextest_metadata::MismatchReason;
 use quick_junit::ReportUuid;
-use std::{fmt, process::ExitStatus, time::Duration};
+use std::{collections::BTreeMap, fmt, process::ExitStatus, time::Duration};
 
 /// A test event.
 ///
@@ -672,19 +672,33 @@ pub struct ExecuteStatus {
 pub struct SetupScriptExecuteStatus {
     /// Output for this setup script.
     pub output: ChildExecutionOutput,
+
     /// The execution result for this setup script: pass, fail or execution error.
     pub result: ExecutionResult,
+
     /// The time at which the script started.
     pub start_time: DateTime<FixedOffset>,
+
     /// The time it took for the script to run.
     pub time_taken: Duration,
+
     /// Whether this script counts as slow.
     pub is_slow: bool,
-    /// The number of environment variables that were set by this script.
+
+    /// The map of environment variables that were set by this script.
     ///
     /// `None` if an error occurred while running the script or reading the
     /// environment map.
-    pub env_count: Option<usize>,
+    pub env_map: Option<SetupScriptEnvMap>,
+}
+
+/// A map of environment variables set by a setup script.
+///
+/// Part of [`SetupScriptExecuteStatus`].
+#[derive(Clone, Debug)]
+pub struct SetupScriptEnvMap {
+    /// The map of environment variables set by the script.
+    pub env_map: BTreeMap<String, String>,
 }
 
 /// Data related to retries for a test.

nextest-runner/src/runner/executor.rs

@@ -11,11 +11,10 @@
 //! just a better abstraction, it also provides a better user experience (less
 //! inconsistent state).
 
-use super::{InternalTerminateReason, RunUnitRequest};
 use crate::{
     config::{
         EvaluatableProfile, RetryPolicy, ScriptConfig, ScriptId, SetupScriptCommand,
-        SetupScriptEnvMap, SetupScriptExecuteData, SlowTimeout, TestSettings,
+        SetupScriptExecuteData, SlowTimeout, TestSettings,
     },
     double_spawn::DoubleSpawnInfo,
     errors::{ChildError, ChildFdError, ChildStartError, ErrorList},
@@ -25,8 +24,8 @@ use crate::{
         TestInfoResponse, UnitKind, UnitState,
     },
     runner::{
-        ExecutorEvent, InternalExecuteStatus, InternalSetupScriptExecuteStatus, RunUnitQuery,
-        SignalRequest, UnitExecuteStatus,
+        parse_env_file, ExecutorEvent, InternalExecuteStatus, InternalSetupScriptExecuteStatus,
+        InternalTerminateReason, RunUnitQuery, RunUnitRequest, SignalRequest, UnitExecuteStatus,
     },
     target_runner::TargetRunner,
     test_command::{ChildAccumulator, ChildFds},
@@ -144,7 +143,8 @@ impl<'a> ExecutorContext<'a> {
                 // that may have been sent.
                 drain_req_rx(req_rx, UnitExecuteStatus::SetupScript(&status));
 
-                let (status, env_map) = status.into_external();
+                let status = status.into_external();
+                let env_map = status.env_map.clone();
 
                 let _ = this_resp_tx.send(ExecutorEvent::SetupScriptFinished {
                     script_id,
@@ -525,7 +525,7 @@ impl<'a> ExecutorContext<'a> {
         // Read from the environment map. If there's an error here, add it to the list of child errors.
         let mut errors: Vec<_> = child_acc.errors.into_iter().map(ChildError::from).collect();
         let env_map = if exec_result.is_success() {
-            match SetupScriptEnvMap::new(&env_path).await {
+            match parse_env_file(&env_path).await {
                 Ok(env_map) => Some(env_map),
                 Err(error) => {
                     errors.push(ChildError::SetupScriptOutput(error));

nextest-runner/src/runner/internal_events.rs

@@ -9,12 +9,12 @@
 
 use super::{SetupScriptPacket, TestPacket};
 use crate::{
-    config::{ScriptConfig, ScriptId, SetupScriptEnvMap},
+    config::{ScriptConfig, ScriptId},
     list::TestInstance,
     reporter::{
         events::{
-            ExecuteStatus, ExecutionResult, InfoResponse, RetryData, SetupScriptExecuteStatus,
-            UnitState,
+            ExecuteStatus, ExecutionResult, InfoResponse, RetryData, SetupScriptEnvMap,
+            SetupScriptExecuteStatus, UnitState,
         },
         TestOutputDisplay,
     },
@@ -165,19 +165,15 @@ pub(super) struct InternalSetupScriptExecuteStatus<'a> {
 }
 
 impl InternalSetupScriptExecuteStatus<'_> {
-    pub(super) fn into_external(self) -> (SetupScriptExecuteStatus, Option<SetupScriptEnvMap>) {
-        let env_count = self.env_map.as_ref().map(|map| map.len());
-        (
-            SetupScriptExecuteStatus {
-                output: self.output,
-                result: self.result,
-                start_time: self.stopwatch_end.start_time.fixed_offset(),
-                time_taken: self.stopwatch_end.active,
-                is_slow: self.slow_after.is_some(),
-                env_count,
-            },
-            self.env_map,
-        )
+    pub(super) fn into_external(self) -> SetupScriptExecuteStatus {
+        SetupScriptExecuteStatus {
+            output: self.output,
+            result: self.result,
+            start_time: self.stopwatch_end.start_time.fixed_offset(),
+            time_taken: self.stopwatch_end.active,
+            is_slow: self.slow_after.is_some(),
+            env_map: self.env_map,
+        }
     }
 }
 

nextest-runner/src/runner/mod.rs

@@ -14,6 +14,7 @@ mod dispatcher;
 mod executor;
 mod imp;
 mod internal_events;
+mod script_helpers;
 
 #[cfg(unix)]
 #[path = "unix.rs"]
@@ -27,3 +28,4 @@ use dispatcher::*;
 use executor::*;
 pub use imp::*;
 use internal_events::*;
+use script_helpers::*;

nextest-runner/src/runner/script_helpers.rs

@@ -0,0 +1,55 @@
+// Copyright (c) The nextest Contributors
+// SPDX-License-Identifier: MIT OR Apache-2.0
+
+use crate::{errors::SetupScriptOutputError, reporter::events::SetupScriptEnvMap};
+use camino::Utf8Path;
+use std::{collections::BTreeMap, sync::Arc};
+use tokio::io::{AsyncBufReadExt, BufReader};
+
+/// Parses an environment file generated by a setup script.
+pub(super) async fn parse_env_file(
+    env_path: &Utf8Path,
+) -> Result<SetupScriptEnvMap, SetupScriptOutputError> {
+    let mut env_map = BTreeMap::new();
+    let f = tokio::fs::File::open(env_path).await.map_err(|error| {
+        SetupScriptOutputError::EnvFileOpen {
+            path: env_path.to_owned(),
+            error: Arc::new(error),
+        }
+    })?;
+    let reader = BufReader::new(f);
+    let mut lines = reader.lines();
+    loop {
+        let line =
+            lines
+                .next_line()
+                .await
+                .map_err(|error| SetupScriptOutputError::EnvFileRead {
+                    path: env_path.to_owned(),
+                    error: Arc::new(error),
+                })?;
+        let Some(line) = line else { break };
+
+        // Split this line into key and value.
+        let (key, value) = match line.split_once('=') {
+            Some((key, value)) => (key, value),
+            None => {
+                return Err(SetupScriptOutputError::EnvFileParse {
+                    path: env_path.to_owned(),
+                    line: line.to_owned(),
+                })
+            }
+        };
+
+        // Ban keys starting with `NEXTEST`.
+        if key.starts_with("NEXTEST") {
+            return Err(SetupScriptOutputError::EnvFileReservedKey {
+                key: key.to_owned(),
+            });
+        }
+
+        env_map.insert(key.to_owned(), value.to_owned());
+    }
+
+    Ok(SetupScriptEnvMap { env_map })
+}