diff --git a/.config/nextest.toml b/.config/nextest.toml index dcc6801a144..4ab2d5b575a 100644 --- a/.config/nextest.toml +++ b/.config/nextest.toml @@ -62,7 +62,7 @@ max-threads = 8 [test-groups.unused-group] max-threads = 8 -[script.build-seed-archive] +[script.setup.build-seed-archive] # This command builds a seed archive that's used by the integration-tests # package. This archive is not currently used by the older nextest-runner # integration framework, but that should really go away at some point. diff --git a/fixtures/nextest-tests/.config/nextest.toml b/fixtures/nextest-tests/.config/nextest.toml index 6f383e74746..91836c110f7 100644 --- a/fixtures/nextest-tests/.config/nextest.toml +++ b/fixtures/nextest-tests/.config/nextest.toml @@ -91,8 +91,8 @@ max-threads = 4 [test-groups.unused] max-threads = 20 -[script.my-script-unix] +[script.setup.my-script-unix] command = './scripts/my-script.sh' -[script.my-script-windows] +[script.setup.my-script-windows] command = 'cmd /c "scripts\\my-script.bat"' diff --git a/nextest-runner/src/config/config_impl.rs b/nextest-runner/src/config/config_impl.rs index fbd7cd7f645..7f645d162c1 100644 --- a/nextest-runner/src/config/config_impl.rs +++ b/nextest-runner/src/config/config_impl.rs @@ -4,16 +4,17 @@ use super::{ ArchiveConfig, CompiledByProfile, CompiledData, CompiledDefaultFilter, ConfigExperimental, CustomTestGroup, DefaultJunitImpl, DeserializedOverride, DeserializedProfileScriptConfig, - JunitConfig, JunitImpl, NextestVersionDeserialize, RetryPolicy, ScriptConfig, ScriptId, - SettingSource, SetupScripts, SlowTimeout, TestGroup, TestGroupConfig, TestSettings, + JunitConfig, JunitImpl, NextestVersionDeserialize, RetryPolicy, ScriptId, SettingSource, + SetupScriptConfig, SetupScripts, SlowTimeout, TestGroup, TestGroupConfig, TestSettings, TestThreads, ThreadsRequired, ToolConfigFile, }; use crate::{ + config::{PreTimeoutScript, PreTimeoutScriptConfig, ScriptType}, errors::{ provided_by_tool, ConfigParseError, ConfigParseErrorKind, ProfileNotFound, UnknownConfigScriptError, UnknownTestGroupError, }, - list::TestList, + list::{TestInstance, TestList}, platform::BuildPlatforms, reporter::{FinalStatusLevel, StatusLevel, TestOutputDisplay}, }; @@ -217,7 +218,7 @@ impl NextestConfig { let mut compiled = CompiledByProfile::for_default_config(); let mut known_groups = BTreeSet::new(); - let mut known_scripts = BTreeSet::new(); + let mut known_scripts = BTreeMap::new(); // Next, merge in tool configs. for ToolConfigFile { config_file, tool } in tool_config_files_rev { @@ -289,7 +290,7 @@ impl NextestConfig { experimental: &BTreeSet, unknown_callback: &mut impl FnMut(&Utf8Path, Option<&str>, &BTreeSet), known_groups: &mut BTreeSet, - known_scripts: &mut BTreeSet, + known_scripts: &mut BTreeMap, ) -> Result<(), ConfigParseError> { // Try building default builder + this file to get good error attribution and handle // overrides additively. @@ -328,8 +329,8 @@ impl NextestConfig { known_groups.extend(valid_groups); - // If scripts are present, check that the experimental feature is enabled. - if !this_config.scripts.is_empty() + // If setup scripts are present, check that the experimental feature is enabled. + if !this_config.scripts.setup.is_empty() && !experimental.contains(&ConfigExperimental::SetupScripts) { return Err(ConfigParseError::new( @@ -341,9 +342,33 @@ impl NextestConfig { )); } - // Check that setup scripts are named as expected. - let (valid_scripts, invalid_scripts): (BTreeSet<_>, _) = - this_config.scripts.keys().cloned().partition(|script| { + // If pre-timeout scripts are present, check that the experimental feature is enabled. + if !this_config.scripts.pre_timeout.is_empty() + && !experimental.contains(&ConfigExperimental::PreTimeoutScripts) + { + return Err(ConfigParseError::new( + config_file, + tool, + ConfigParseErrorKind::ExperimentalFeatureNotEnabled { + feature: ConfigExperimental::PreTimeoutScripts, + }, + )); + } + + if let Some(dup) = this_config.scripts.duplicate_ids().next() { + return Err(ConfigParseError::new( + config_file, + tool, + ConfigParseErrorKind::DuplicateConfigScriptName(dup.clone()), + )); + } + + // Check that scripts are named as expected. + let (valid_scripts, invalid_scripts): (BTreeSet<_>, _) = this_config + .scripts + .all_script_ids() + .cloned() + .partition(|script| { if let Some(tool) = tool { // The first component must be the tool name. script @@ -365,7 +390,10 @@ impl NextestConfig { return Err(ConfigParseError::new(config_file, tool, kind)); } - known_scripts.extend(valid_scripts); + known_scripts.extend(valid_scripts.into_iter().map(|id| { + let script_type = this_config.scripts.script_type(&id); + (id, script_type) + })); let this_config = this_config.into_config_impl(); @@ -434,45 +462,76 @@ impl NextestConfig { // Check that scripts are known. let mut unknown_script_errors = Vec::new(); - let mut check_script_ids = |profile_name: &str, scripts: &[ScriptId]| { - if !scripts.is_empty() && !experimental.contains(&ConfigExperimental::SetupScripts) { - return Err(ConfigParseError::new( - config_file, - tool, - ConfigParseErrorKind::ExperimentalFeatureNotEnabled { - feature: ConfigExperimental::SetupScripts, - }, - )); - } - for script in scripts { - if !known_scripts.contains(script) { - unknown_script_errors.push(UnknownConfigScriptError { - profile_name: profile_name.to_owned(), - name: script.clone(), - }); + let mut check_script_ids = + |profile_name: &str, script_type: ScriptType, scripts: &[ScriptId]| { + let config_feature = match script_type { + ScriptType::Setup => ConfigExperimental::SetupScripts, + ScriptType::PreTimeout => ConfigExperimental::PreTimeoutScripts, + }; + if !scripts.is_empty() && !experimental.contains(&config_feature) { + return Err(ConfigParseError::new( + config_file, + tool, + ConfigParseErrorKind::ExperimentalFeatureNotEnabled { + feature: config_feature, + }, + )); + } + for script in scripts { + match known_scripts.get(script) { + None => { + unknown_script_errors.push(UnknownConfigScriptError { + profile_name: profile_name.to_owned(), + name: script.clone(), + }); + } + Some(actual_script_type) if *actual_script_type != script_type => { + return Err(ConfigParseError::new( + config_file, + tool, + ConfigParseErrorKind::WrongConfigScriptType { + script: script.clone(), + attempted: script_type, + actual: actual_script_type.clone(), + }, + )); + } + Some(_) => (), + } } - } - Ok(()) - }; + Ok(()) + }; this_compiled .default .scripts .iter() - .try_for_each(|scripts| check_script_ids("default", &scripts.setup))?; + .try_for_each(|scripts| { + check_script_ids("default", ScriptType::Setup, &scripts.setup)?; + check_script_ids( + "default", + ScriptType::PreTimeout, + scripts.pre_timeout.as_slice(), + ) + })?; this_compiled .other .iter() .try_for_each(|(profile_name, data)| { - data.scripts - .iter() - .try_for_each(|scripts| check_script_ids(profile_name, &scripts.setup)) + data.scripts.iter().try_for_each(|scripts| { + check_script_ids(profile_name, ScriptType::Setup, &scripts.setup)?; + check_script_ids( + profile_name, + ScriptType::PreTimeout, + scripts.pre_timeout.as_slice(), + ) + }) })?; // If there were any unknown scripts, error out. if !unknown_script_errors.is_empty() { - let known_scripts = known_scripts.iter().cloned().collect(); + let known_scripts = known_scripts.keys().cloned().collect(); return Err(ConfigParseError::new( config_file, tool, @@ -578,8 +637,7 @@ pub struct EarlyProfile<'cfg> { default_profile: &'cfg DefaultProfileImpl, custom_profile: Option<&'cfg CustomProfileImpl>, test_groups: &'cfg BTreeMap, - // This is ordered because the scripts are used in the order they're defined. - scripts: &'cfg IndexMap, + scripts: &'cfg Scripts, // Invariant: `compiled_data.default_filter` is always present. pub(super) compiled_data: CompiledData, } @@ -646,7 +704,7 @@ pub struct EvaluatableProfile<'cfg> { custom_profile: Option<&'cfg CustomProfileImpl>, test_groups: &'cfg BTreeMap, // This is ordered because the scripts are used in the order they're defined. - scripts: &'cfg IndexMap, + scripts: &'cfg Scripts, // Invariant: `compiled_data.default_filter` is always present. pub(super) compiled_data: CompiledData, // The default filter that's been resolved after considering overrides (i.e. @@ -683,8 +741,8 @@ impl<'cfg> EvaluatableProfile<'cfg> { } /// Returns the global script configuration. - pub fn script_config(&self) -> &'cfg IndexMap { - self.scripts + pub fn script_config(&self) -> &'cfg Scripts { + &self.scripts } /// Returns the retry count for this profile. @@ -777,6 +835,11 @@ impl<'cfg> EvaluatableProfile<'cfg> { SetupScripts::new(self, test_list) } + /// Returns the pre-timeout script for the specified test, if it exists. + pub fn pre_timeout_script(&self, test_instance: &TestInstance<'_>) -> Option { + PreTimeoutScript::new(self, test_instance) + } + /// Returns settings for individual tests. pub fn settings_for(&self, query: &TestQuery<'_>) -> TestSettings { TestSettings::new(self, query) @@ -809,7 +872,7 @@ impl<'cfg> EvaluatableProfile<'cfg> { pub(super) struct NextestConfigImpl { store: StoreConfigImpl, test_groups: BTreeMap, - scripts: IndexMap, + scripts: Scripts, default_profile: DefaultProfileImpl, other_profiles: HashMap, } @@ -863,7 +926,7 @@ struct NextestConfigDeserialize { #[serde(default)] test_groups: BTreeMap, #[serde(default, rename = "script")] - scripts: IndexMap, + scripts: Scripts, #[serde(rename = "profile")] profiles: HashMap, } @@ -962,7 +1025,7 @@ impl DefaultProfileImpl { &self.overrides } - pub(super) fn setup_scripts(&self) -> &[DeserializedProfileScriptConfig] { + pub(super) fn scripts(&self) -> &[DeserializedProfileScriptConfig] { &self.scripts } } @@ -1024,6 +1087,47 @@ impl CustomProfileImpl { } } +/// The scripts defined in a profile. +#[derive(Clone, Debug, Default, Deserialize)] +#[serde(rename_all = "kebab-case")] +pub struct Scripts { + // These maps are ordered because scripts are used in the order they're defined. + /// The setup scripts defined in a profile. + #[serde(default)] + pub setup: IndexMap, + /// The pre-timeout scripts defined in a profile. + #[serde(default)] + pub pre_timeout: IndexMap, +} + +impl Scripts { + /// Determines the type of the script with the given ID. + /// + /// Panics if the ID is invalid. + fn script_type(&self, id: &ScriptId) -> ScriptType { + if self.setup.contains_key(id) { + ScriptType::Setup + } else if self.pre_timeout.contains_key(id) { + ScriptType::PreTimeout + } else { + panic!("Scripts::script_type called with invalid script ID: {id}") + } + } + + /// Returns an iterator over the names of all scripts of all types. + fn all_script_ids(&self) -> impl Iterator { + self.setup.keys().chain(self.pre_timeout.keys()) + } + + /// Returns an iterator over names that are used by more than one type of + /// script. + fn duplicate_ids(&self) -> impl Iterator { + self.pre_timeout + .keys() + .filter(|k| self.setup.contains_key(*k)) + } +} + #[cfg(test)] mod tests { use super::*; diff --git a/nextest-runner/src/config/nextest_version.rs b/nextest-runner/src/config/nextest_version.rs index 84dc3b0b346..fcac7c71dff 100644 --- a/nextest-runner/src/config/nextest_version.rs +++ b/nextest-runner/src/config/nextest_version.rs @@ -232,11 +232,13 @@ impl NextestVersionConfig { pub enum ConfigExperimental { /// Enable support for setup scripts. SetupScripts, + /// Enable support for pre-timeout scripts. + PreTimeoutScripts, } impl ConfigExperimental { fn known() -> impl Iterator { - vec![Self::SetupScripts].into_iter() + vec![Self::SetupScripts, Self::PreTimeoutScripts].into_iter() } } @@ -246,6 +248,7 @@ impl FromStr for ConfigExperimental { fn from_str(s: &str) -> Result { match s { "setup-scripts" => Ok(Self::SetupScripts), + "pre-timeout-scripts" => Ok(Self::PreTimeoutScripts), _ => Err(()), } } @@ -255,6 +258,7 @@ impl fmt::Display for ConfigExperimental { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { match self { Self::SetupScripts => write!(f, "setup-scripts"), + Self::PreTimeoutScripts => write!(f, "pre-timeout-scripts"), } } } diff --git a/nextest-runner/src/config/overrides.rs b/nextest-runner/src/config/overrides.rs index d9d021d5b39..a79aac63d0a 100644 --- a/nextest-runner/src/config/overrides.rs +++ b/nextest-runner/src/config/overrides.rs @@ -295,7 +295,7 @@ impl CompiledByProfile { "default", Some(config.default_profile().default_filter()), config.default_profile().overrides(), - config.default_profile().setup_scripts(), + config.default_profile().scripts(), &mut errors, ); let other: HashMap<_, _> = config @@ -486,13 +486,13 @@ impl CompiledData { pub(super) fn chain(self, other: Self) -> Self { let profile_default_filter = self.profile_default_filter.or(other.profile_default_filter); let mut overrides = self.overrides; - let mut setup_scripts = self.scripts; + let mut scripts = self.scripts; overrides.extend(other.overrides); - setup_scripts.extend(other.scripts); + scripts.extend(other.scripts); Self { profile_default_filter, overrides, - scripts: setup_scripts, + scripts, } } @@ -506,15 +506,15 @@ impl CompiledData { .into_iter() .map(|override_| override_.apply_build_platforms(build_platforms)) .collect(); - let setup_scripts = self + let scripts = self .scripts .into_iter() - .map(|setup_script| setup_script.apply_build_platforms(build_platforms)) + .map(|script| script.apply_build_platforms(build_platforms)) .collect(); CompiledData { profile_default_filter, overrides, - scripts: setup_scripts, + scripts, } } } diff --git a/nextest-runner/src/config/scripts.rs b/nextest-runner/src/config/scripts.rs index 5911524592b..38dbb21b374 100644 --- a/nextest-runner/src/config/scripts.rs +++ b/nextest-runner/src/config/scripts.rs @@ -13,7 +13,7 @@ use crate::{ ChildStartError, ConfigCompileError, ConfigCompileErrorKind, ConfigCompileSection, InvalidConfigScriptName, }, - list::TestList, + list::{TestInstance, TestList}, platform::BuildPlatforms, reporter::events::SetupScriptEnvMap, test_command::{apply_ld_dyld_env, create_command}, @@ -91,7 +91,7 @@ impl<'profile> SetupScripts<'profile> { // Build up a map of enabled scripts along with their data, by script ID. let mut enabled_scripts = IndexMap::new(); - for (script_id, config) in script_config { + for (script_id, config) in &script_config.setup { if enabled_ids.contains(script_id) { let compiled = by_script_id .remove(script_id) @@ -139,7 +139,7 @@ pub(crate) struct SetupScript<'profile> { pub(crate) id: ScriptId, /// The configuration for the script. - pub(crate) config: &'profile ScriptConfig, + pub(crate) config: &'profile SetupScriptConfig, /// The compiled filters to use to check which tests this script is enabled for. pub(crate) compiled: Vec<&'profile CompiledProfileScripts>, @@ -166,7 +166,7 @@ pub(crate) struct SetupScriptCommand { impl SetupScriptCommand { /// Creates a new `SetupScriptCommand` for a setup script. pub(crate) fn new( - config: &ScriptConfig, + config: &SetupScriptConfig, double_spawn: &DoubleSpawnInfo, test_list: &TestList<'_>, ) -> Result { @@ -243,9 +243,111 @@ impl<'profile> SetupScriptExecuteData<'profile> { } } +/// Data about a pre-timeout script, returned by an [`EvaluatableProfile`]. +#[derive(Debug)] +pub struct PreTimeoutScript<'profile> { + /// The script ID. + pub id: ScriptId, + + /// The configuration for the script. + pub config: &'profile PreTimeoutScriptConfig, +} + +impl<'profile> PreTimeoutScript<'profile> { + pub(super) fn new( + profile: &'profile EvaluatableProfile<'_>, + test_instance: &TestInstance<'_>, + ) -> Option { + let test_query = test_instance.to_test_query(); + + let profile_scripts = &profile.compiled_data.scripts; + if profile_scripts.is_empty() { + return None; + } + + let env = profile.filterset_ecx(); + for compiled in profile_scripts { + if let Some(script_id) = &compiled.pre_timeout { + if compiled.is_enabled(&test_query, &env) { + let config = &profile.script_config().pre_timeout[script_id]; + return Some(PreTimeoutScript { + id: script_id.clone(), + config, + }); + } + } + } + + None + } +} + +/// Represents a to-be-run pre-timeout script command with a certain set of arguments. +pub(crate) struct PreTimeoutScriptCommand { + /// The command to be run. + command: std::process::Command, + /// Double-spawn context. + double_spawn: Option, +} + +impl PreTimeoutScriptCommand { + /// Creates a new `PreTimeoutScriptCommand` for a setup script. + pub(crate) fn new( + config: &PreTimeoutScriptConfig, + double_spawn: &DoubleSpawnInfo, + test_list: &TestList<'_>, + test_instance: &TestInstance<'_>, + test_pid: u32, + ) -> Result { + let mut cmd = create_command(config.program().to_owned(), config.args(), double_spawn); + + // NB: we will always override user-provided environment variables with the + // `CARGO_*` and `NEXTEST_*` variables set directly on `cmd` below. + test_list.cargo_env().apply_env(&mut cmd); + + cmd.current_dir(&test_instance.suite_info.cwd) + // This environment variable is set to indicate that tests are being run under nextest. + .env("NEXTEST", "1") + .env("NEXTEST_PRE_TIMEOUT_TEST_PID", test_pid.to_string()) + .env("NEXTEST_PRE_TIMEOUT_TEST_NAME", test_instance.name) + .env("NEXTEST_PRE_TIMEOUT_TEST_BINARY_ID", test_instance.suite_info.binary_id.as_str()) + .env("NEXTEST_PRE_TIMEOUT_TEST_PACKAGE", test_instance.suite_info.package.name()) + .env("NEXTEST_PRE_TIMEOUT_TEST_BINARY", &test_instance.suite_info.binary_name) + .env("NEXTEST_PRE_TIMEOUT_TEST_BINARY_KIND", test_instance.suite_info.kind.as_str()); + + // XXX: set special pre-timeout script environment variables. + + apply_ld_dyld_env(&mut cmd, test_list.updated_dylib_path()); + + let double_spawn = double_spawn.spawn_context(); + + Ok(Self { + command: cmd, + double_spawn, + }) + } + + /// Returns the command to be run. + #[inline] + pub(crate) fn command_mut(&mut self) -> &mut std::process::Command { + &mut self.command + } + + pub(crate) fn spawn(self) -> std::io::Result { + let mut command = tokio::process::Command::from(self.command); + let res = command.spawn(); + if let Some(ctx) = self.double_spawn { + ctx.finish(); + } + let child = res?; + Ok(child) + } +} + #[derive(Clone, Debug)] pub(crate) struct CompiledProfileScripts { pub(super) setup: Vec, + pub(super) pre_timeout: Option, pub(super) data: ProfileScriptData, state: State, } @@ -289,6 +391,7 @@ impl CompiledProfileScripts { match (host_spec, target_spec, filter_expr) { (Ok(host_spec), Ok(target_spec), Ok(expr)) => Some(Self { setup: source.setup.clone(), + pre_timeout: source.pre_timeout.clone(), data: ProfileScriptData { host_spec, target_spec, @@ -330,6 +433,7 @@ impl CompiledProfileScripts { CompiledProfileScripts { setup: self.setup, + pre_timeout: self.pre_timeout, data: self.data, state: FinalConfig { host_eval, @@ -399,6 +503,21 @@ impl fmt::Display for ScriptId { } } +#[derive(Clone, Copy, Debug, Eq, PartialEq, Hash, PartialOrd, Ord)] +pub enum ScriptType { + Setup, + PreTimeout, +} + +impl fmt::Display for ScriptType { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + match self { + ScriptType::Setup => f.write_str("setup"), + ScriptType::PreTimeout => f.write_str("pre-timeout"), + } + } +} + #[derive(Clone, Debug)] pub(super) struct ProfileScriptData { host_spec: MaybeTargetSpec, @@ -421,14 +540,17 @@ pub(super) struct DeserializedProfileScriptConfig { /// The setup script or scripts to run. #[serde(deserialize_with = "deserialize_script_ids")] setup: Vec, + + /// The pre-timeout script to run. + pre_timeout: Option, } -/// Deserialized form of script configuration before compilation. +/// Deserialized form of setup script configuration before compilation. /// /// This is defined as a top-level element. #[derive(Clone, Debug, Deserialize)] #[serde(rename_all = "kebab-case")] -pub struct ScriptConfig { +pub struct SetupScriptConfig { /// The command to run. The first element is the program and the second element is a list /// of arguments. #[serde(deserialize_with = "deserialize_command")] @@ -452,10 +574,10 @@ pub struct ScriptConfig { /// JUnit configuration for this script. #[serde(default)] - pub junit: ScriptJunitConfig, + pub junit: SetupScriptJunitConfig, } -impl ScriptConfig { +impl SetupScriptConfig { /// Returns the name of the program. #[inline] pub fn program(&self) -> &str { @@ -478,7 +600,7 @@ impl ScriptConfig { /// A JUnit override configuration. #[derive(Copy, Clone, Debug, Deserialize)] #[serde(rename_all = "kebab-case")] -pub struct ScriptJunitConfig { +pub struct SetupScriptJunitConfig { /// Whether to store successful output. /// /// Defaults to true. @@ -492,7 +614,7 @@ pub struct ScriptJunitConfig { pub store_failure_output: bool, } -impl Default for ScriptJunitConfig { +impl Default for SetupScriptJunitConfig { fn default() -> Self { Self { store_success_output: true, @@ -501,6 +623,40 @@ impl Default for ScriptJunitConfig { } } +/// Deserialized form of pre-timeout script configuration before compilation. +/// +/// This is defined as a top-level element. +#[derive(Clone, Debug, Deserialize)] +#[serde(rename_all = "kebab-case")] +pub struct PreTimeoutScriptConfig { + /// The command to run. The first element is the program and the second element is a list + /// of arguments. + #[serde(deserialize_with = "deserialize_command")] + pub command: (String, Vec), + + /// An optional slow timeout for this command. + #[serde(default, deserialize_with = "super::deserialize_slow_timeout")] + pub slow_timeout: Option, + + /// An optional leak timeout for this command. + #[serde(default, with = "humantime_serde::option")] + pub leak_timeout: Option, +} + +impl PreTimeoutScriptConfig { + /// Returns the name of the program. + #[inline] + pub fn program(&self) -> &str { + &self.command.0 + } + + /// Returns the arguments to the command. + #[inline] + pub fn args(&self) -> &[String] { + &self.command.1 + } +} + fn default_true() -> bool { true } @@ -614,14 +770,14 @@ mod tests { # order defined below. setup = ["baz", "foo", "@tool:my-tool:toolscript"] - [script.foo] + [script.setup.foo] command = "command foo" - [script.bar] + [script.setup.bar] command = ["cargo", "run", "-p", "bar"] slow-timeout = { period = "60s", terminate-after = 2 } - [script.baz] + [script.setup.baz] command = "baz" slow-timeout = "1s" leak-timeout = "1s" @@ -631,7 +787,7 @@ mod tests { }; let tool_config_contents = indoc! {r#" - [script.'@tool:my-tool:toolscript'] + [script.setup.'@tool:my-tool:toolscript'] command = "tool-command" "# }; @@ -745,7 +901,7 @@ mod tests { #[test_case( indoc! {r#" - [script.foo] + [script.setup.foo] command = "" "#}, "invalid value: string \"\", expected a Unix shell command or a list of arguments" @@ -754,7 +910,7 @@ mod tests { )] #[test_case( indoc! {r#" - [script.foo] + [script.setup.foo] command = [] "#}, "invalid length 0, expected a Unix shell command or a list of arguments" @@ -763,15 +919,15 @@ mod tests { )] #[test_case( indoc! {r#" - [script.foo] + [script.setup.foo] "#}, - "script.foo: missing field `command`" + "script.setup.foo: missing field `command`" ; "missing command" )] #[test_case( indoc! {r#" - [script.foo] + [script.setup.foo] command = "my-command" slow-timeout = 34 "#}, @@ -781,7 +937,7 @@ mod tests { )] #[test_case( indoc! {r#" - [script.'@tool:foo'] + [script.setup.'@tool:foo'] command = "my-command" "#}, r#"invalid configuration script name: tool identifier not of the form "@tool:tool-name:identifier": `@tool:foo`"# @@ -790,7 +946,7 @@ mod tests { )] #[test_case( indoc! {r#" - [script.'#foo'] + [script.setup.'#foo'] command = "my-command" "#}, r"invalid configuration script name: invalid identifier `#foo`" @@ -820,7 +976,7 @@ mod tests { #[test_case( indoc! {r#" - [script.foo] + [script.setup.foo] command = "my-command" [[profile.default.scripts]] @@ -836,7 +992,7 @@ mod tests { )] #[test_case( indoc! {r#" - [script.foo] + [script.setup.foo] command = "my-command" [[profile.default.scripts]] @@ -853,7 +1009,7 @@ mod tests { )] #[test_case( indoc! {r#" - [script.foo] + [script.setup.foo] command = "my-command" [[profile.default.scripts]] @@ -872,7 +1028,7 @@ mod tests { )] #[test_case( indoc! {r#" - [script.foo] + [script.setup.foo] command = "my-command" [[profile.ci.overrides]] @@ -945,7 +1101,7 @@ mod tests { #[test_case( indoc! {r#" - [script.'@tool:foo:bar'] + [script.setup.'@tool:foo:bar'] command = "my-command" [[profile.ci.overrides]] @@ -986,7 +1142,7 @@ mod tests { #[test_case( indoc! {r#" - [script.'blarg'] + [script.setup.'blarg'] command = "my-command" [[profile.ci.overrides]] @@ -1036,7 +1192,7 @@ mod tests { #[test_case( indoc! {r#" - [script.foo] + [script.setup.foo] command = "my-command" [[profile.default.scripts]] diff --git a/nextest-runner/src/errors.rs b/nextest-runner/src/errors.rs index 1ac085e02d3..3d8d17b0c4b 100644 --- a/nextest-runner/src/errors.rs +++ b/nextest-runner/src/errors.rs @@ -5,7 +5,7 @@ use crate::{ cargo_config::{TargetTriple, TargetTripleSource}, - config::{ConfigExperimental, CustomTestGroup, ScriptId, TestGroup}, + config::{ConfigExperimental, CustomTestGroup, ScriptId, ScriptType, TestGroup}, helpers::{display_exited_with, dylib_path_envvar}, redact::Redactor, reuse_build::{ArchiveFormat, ArchiveStep}, @@ -124,6 +124,21 @@ pub enum ConfigParseErrorKind { #[error( "invalid config scripts defined by tool: {}\n(config scripts must start with '@tool::')", .0.iter().join(", "))] InvalidConfigScriptsDefinedByTool(BTreeSet), + /// The same config script name was used across config script types. + #[error("config script name used more than once: {0}\n(config script names must be unique across all script types)")] + DuplicateConfigScriptName(ScriptId), + /// The same config script name was used across config script types. + #[error("cannot use config script as a {attempted} script: {script}\n(config script is a {actual} script)")] + WrongConfigScriptType { + /// The name of the config script. + script: ScriptId, + + /// The script type that the user attempted to use the script as. + attempted: ScriptType, + + /// The actual script type. + actual: ScriptType, + }, /// Some config scripts were unknown. #[error( "unknown config scripts specified by config (destructure this variant for more details)" diff --git a/nextest-runner/src/reporter/aggregator/junit.rs b/nextest-runner/src/reporter/aggregator/junit.rs index 1a8e86867ad..f2a1d9d046c 100644 --- a/nextest-runner/src/reporter/aggregator/junit.rs +++ b/nextest-runner/src/reporter/aggregator/junit.rs @@ -100,6 +100,12 @@ impl<'cfg> MetadataJunit<'cfg> { } } } + TestEventKind::PreTimeoutScriptStarted { .. } + | TestEventKind::PreTimeoutScriptSlow { .. } + | TestEventKind::PreTimeoutScriptFinished { .. } => { + // JUnit reports don't have a good place to include information + // about pre-timeout scripts, so we elide them from the report. + } TestEventKind::InfoStarted { .. } | TestEventKind::InfoResponse { .. } | TestEventKind::InfoFinished { .. } => {} diff --git a/nextest-runner/src/reporter/displayer.rs b/nextest-runner/src/reporter/displayer.rs index 9de796277e1..027b78343ef 100644 --- a/nextest-runner/src/reporter/displayer.rs +++ b/nextest-runner/src/reporter/displayer.rs @@ -771,11 +771,74 @@ impl<'a> TestReporterImpl<'a> { // Always display failing setup script output if it exists. We may change this in // the future. if !run_status.result.is_success() { - self.write_setup_script_execute_status( - script_id, command, args, run_status, writer, + self.write_script_execute_status( + script_id, + command, + args, + run_status.result.is_success(), + &run_status.output, + writer, )?; } } + TestEventKind::PreTimeoutScriptStarted { test_instance, .. } => { + writeln!( + writer, + "{:>12} {}", + "PRETMT".style(self.styles.pass), + self.display_test_instance(test_instance.id()), + )?; + } + TestEventKind::PreTimeoutScriptSlow { + test_instance, + elapsed, + will_terminate, + } => { + if !*will_terminate && self.status_levels.status_level >= StatusLevel::Slow { + write!(writer, "{:>12} ", "PRETMT SLOW".style(self.styles.skip))?; + } else if *will_terminate { + write!(writer, "{:>12} ", "PRETMT TERM".style(self.styles.fail))?; + } + + self.write_slow_duration(*elapsed, writer)?; + writeln!(writer, "{}", self.display_test_instance(test_instance.id()))?; + } + TestEventKind::PreTimeoutScriptFinished { + script_id, + command, + args, + test_instance, + run_status, + } => { + match run_status.result { + ExecutionResult::Pass => { + write!(writer, "{:>12} ", "PRETMT PASS".style(self.styles.pass))?; + } + ExecutionResult::Leak => { + write!(writer, "{:>12} ", "PRETMT LEAK".style(self.styles.skip))?; + } + other => { + let status_str = short_status_str(other); + write!( + writer, + "{:>12} ", + format!("PRETMT {status_str}").style(self.styles.fail), + )?; + } + } + + self.write_duration(run_status.time_taken, writer)?; + writeln!(writer, "{}", self.display_test_instance(test_instance.id()))?; + + self.write_script_execute_status( + script_id, + command, + args, + run_status.result.is_success(), + &run_status.output, + writer, + )?; + } TestEventKind::TestStarted { test_instance, .. } => { // In no-capture mode, print out a test start event. if self.no_capture { @@ -1544,35 +1607,39 @@ impl<'a> TestReporterImpl<'a> { writer: &mut dyn Write, ) -> io::Result<()> { let status_str = "status".style(self.styles.count); + + let mut print_running = |pid: u32, time_taken: Duration, slow_after: Option| { + let running_style = if output_has_errors { + self.styles.fail + } else if slow_after.is_some() { + self.styles.skip + } else { + self.styles.pass + }; + write!( + writer, + "{status_str}: {attempt_str}{kind} {} for {:.3?}s as PID {}", + "running".style(running_style), + time_taken.as_secs_f64(), + pid.style(self.styles.count), + )?; + if let Some(slow_after) = slow_after { + write!( + writer, + " (marked slow after {:.3?}s)", + slow_after.as_secs_f64() + )?; + } + writeln!(writer)?; + Ok::<_, io::Error>(()) + }; + match state { UnitState::Running { pid, time_taken, slow_after, - } => { - let running_style = if output_has_errors { - self.styles.fail - } else if slow_after.is_some() { - self.styles.skip - } else { - self.styles.pass - }; - write!( - writer, - "{status_str}: {attempt_str}{kind} {} for {:.3?}s as PID {}", - "running".style(running_style), - time_taken.as_secs_f64(), - pid.style(self.styles.count), - )?; - if let Some(slow_after) = slow_after { - write!( - writer, - " (marked slow after {:.3?}s)", - slow_after.as_secs_f64() - )?; - } - writeln!(writer)?; - } + } => print_running(*pid, *time_taken, *slow_after)?, UnitState::Exiting { pid, time_taken, @@ -1608,6 +1675,42 @@ impl<'a> TestReporterImpl<'a> { )?; } } + UnitState::PreTimeout { + pid, + time_taken, + slow_after, + script_state, + script_output, + } => { + // Print test status. + print_running(*pid, *time_taken, Some(*slow_after))?; + + // Print note about running pre-timeout script. + write!( + writer, + "{}: test has reached timeout, pre-timeout script is running:", + "note".style(self.styles.count) + )?; + + // Print state of the pre-timeout script, indented one level + // further: + let mut writer = IndentWriter::new_skip_initial(" ", writer); + writeln!(&mut writer)?; + self.write_unit_state( + UnitKind::Script, + "", + script_state, + script_output.has_errors(), + &mut writer, + )?; + if script_state.has_valid_output() { + self.write_child_execution_output( + &self.output_spec_for_info(UnitKind::Script), + script_output, + &mut writer, + )?; + } + } UnitState::Terminating(state) => { self.write_terminating_state(kind, attempt_str, state, writer)?; } @@ -1815,16 +1918,17 @@ impl<'a> TestReporterImpl<'a> { write!(writer, "[>{:>7.3?}s] ", duration.as_secs_f64()) } - fn write_setup_script_execute_status( + fn write_script_execute_status( &self, script_id: &ScriptId, command: &str, args: &[String], - run_status: &SetupScriptExecuteStatus, + succeeded: bool, + output: &ChildExecutionOutput, writer: &mut dyn Write, ) -> io::Result<()> { - let spec = self.output_spec_for_script(script_id, command, args, run_status); - self.write_child_execution_output(&spec, &run_status.output, writer) + let spec = self.output_spec_for_script(script_id, command, args, succeeded); + self.write_child_execution_output(&spec, &output, writer) } fn write_test_execute_status( @@ -2109,9 +2213,9 @@ impl<'a> TestReporterImpl<'a> { script_id: &ScriptId, command: &str, args: &[String], - run_status: &SetupScriptExecuteStatus, + succeeded: bool, ) -> ChildOutputSpec { - let header_style = if run_status.result.is_success() { + let header_style = if succeeded { self.styles.pass } else { self.styles.fail diff --git a/nextest-runner/src/reporter/events.rs b/nextest-runner/src/reporter/events.rs index 5a157496cfe..ad0f1945193 100644 --- a/nextest-runner/src/reporter/events.rs +++ b/nextest-runner/src/reporter/events.rs @@ -124,6 +124,42 @@ pub enum TestEventKind<'a> { run_status: SetupScriptExecuteStatus, }, + /// A pre-timeout script started. + PreTimeoutScriptStarted { + /// The test instance that timed out. + test_instance: TestInstance<'a>, + }, + + /// A pre-timeout script was slow. + PreTimeoutScriptSlow { + /// The test instance that timed out. + test_instance: TestInstance<'a>, + + /// The amount of time elapsed since the start of execution. + elapsed: Duration, + + /// True if the script has hit its timeout and is about to be terminated. + will_terminate: bool, + }, + + /// A pre-timeout script completed execution. + PreTimeoutScriptFinished { + /// The script ID. + script_id: ScriptId, + + /// The command to run. + command: &'a str, + + /// The arguments to the command. + args: &'a [String], + + /// The test instance that timed out. + test_instance: TestInstance<'a>, + + /// The execution status of the pre-timeout script. + run_status: PreTimeoutScriptExecuteStatus, + }, + // TODO: add events for BinaryStarted and BinaryFinished? May want a slightly different way to // do things, maybe a couple of reporter traits (one for the run as a whole and one for each // binary). @@ -701,6 +737,25 @@ pub struct SetupScriptEnvMap { pub env_map: BTreeMap, } +/// Information about the execution of a setup script. +#[derive(Clone, Debug)] +pub struct PreTimeoutScriptExecuteStatus { + /// Output for this pre-timeout 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, + + /// The time it took for the script to run. + pub time_taken: Duration, + + /// Whether this script counts as slow. + pub is_slow: bool, +} + /// Data related to retries for a test. #[derive(Clone, Copy, Debug, Eq, PartialEq, PartialOrd, Ord)] pub struct RetryData { @@ -961,6 +1016,25 @@ pub enum UnitState { remaining: Duration, }, + /// The test has exceeded its timeout and nextest is executing the test's + /// pre-timeout script. (Only relevant for tests.) + PreTimeout { + /// The process ID of the test. + pid: u32, + + /// The amount of time the unit has been running. + time_taken: Duration, + + /// The duration after which the test was marked as slow. + slow_after: Duration, + + /// The state of the pre-timeout script. + script_state: Box, + + /// The output of the pre-timeout script. + script_output: ChildExecutionOutput, + }, + /// The child process is being terminated by nextest. Terminating(UnitTerminatingState), @@ -1000,6 +1074,7 @@ impl UnitState { match self { UnitState::Running { .. } | UnitState::Exiting { .. } + | UnitState::PreTimeout { .. } | UnitState::Terminating(_) | UnitState::Exited { .. } => true, UnitState::DelayBeforeNextAttempt { .. } => false, diff --git a/nextest-runner/src/runner/dispatcher.rs b/nextest-runner/src/runner/dispatcher.rs index 47bbba14782..bf3d3bc50f8 100644 --- a/nextest-runner/src/runner/dispatcher.rs +++ b/nextest-runner/src/runner/dispatcher.rs @@ -9,7 +9,7 @@ use super::{RunUnitRequest, RunnerTaskState, ShutdownRequest}; use crate::{ - config::{MaxFail, ScriptConfig, ScriptId}, + config::{MaxFail, ScriptId, SetupScriptConfig}, input::{InputEvent, InputHandler}, list::{TestInstance, TestInstanceId, TestList}, reporter::events::{ @@ -406,6 +406,32 @@ where HandleEventResponse::None } } + InternalEvent::Executor(ExecutorEvent::PreTimeoutScriptStarted { test_instance }) => { + self.callback_none_response(TestEventKind::PreTimeoutScriptStarted { + test_instance, + }) + } + InternalEvent::Executor(ExecutorEvent::PreTimeoutScriptSlow { + test_instance, + elapsed, + will_terminate, + }) => self.callback_none_response(TestEventKind::PreTimeoutScriptSlow { + test_instance, + elapsed, + will_terminate: will_terminate.is_some(), + }), + InternalEvent::Executor(ExecutorEvent::PreTimeoutScriptFinished { + test_instance, + script_id, + config, + status, + }) => self.callback_none_response(TestEventKind::PreTimeoutScriptFinished { + test_instance, + script_id, + command: config.program(), + args: config.args(), + run_status: status, + }), InternalEvent::Executor(ExecutorEvent::Started { test_instance, req_rx_tx, @@ -540,7 +566,7 @@ where fn new_setup_script( &mut self, id: ScriptId, - config: &'a ScriptConfig, + config: &'a SetupScriptConfig, index: usize, total: usize, req_tx: UnboundedSender>, @@ -796,7 +822,7 @@ struct ContextSetupScript<'a> { id: ScriptId, // Store these details primarily for debugging. #[expect(dead_code)] - config: &'a ScriptConfig, + config: &'a SetupScriptConfig, #[expect(dead_code)] index: usize, #[expect(dead_code)] diff --git a/nextest-runner/src/runner/executor.rs b/nextest-runner/src/runner/executor.rs index 0e91301c720..95ea372c080 100644 --- a/nextest-runner/src/runner/executor.rs +++ b/nextest-runner/src/runner/executor.rs @@ -14,8 +14,8 @@ use super::HandleSignalResult; use crate::{ config::{ - EvaluatableProfile, RetryPolicy, ScriptConfig, ScriptId, SetupScriptCommand, - SetupScriptExecuteData, SlowTimeout, TestSettings, + EvaluatableProfile, PreTimeoutScriptCommand, PreTimeoutScriptConfig, RetryPolicy, ScriptId, + SetupScriptCommand, SetupScriptConfig, SetupScriptExecuteData, SlowTimeout, TestSettings, }, double_spawn::DoubleSpawnInfo, errors::{ChildError, ChildFdError, ChildStartError, ErrorList}, @@ -25,7 +25,8 @@ use crate::{ TestInfoResponse, UnitKind, UnitState, }, runner::{ - parse_env_file, ExecutorEvent, InternalExecuteStatus, InternalSetupScriptExecuteStatus, + parse_env_file, ExecutorEvent, InternalExecuteStatus, + InternalPreTimeoutScriptExecuteStatus, InternalSetupScriptExecuteStatus, InternalTerminateReason, RunUnitQuery, RunUnitRequest, SignalRequest, UnitExecuteStatus, }, target_runner::TargetRunner, @@ -573,6 +574,270 @@ impl<'a> ExecutorContext<'a> { }) } + /// Run a pre-timeoutx script in its own process. + #[instrument(level = "debug", skip(self, resp_tx, req_rx))] + async fn run_pre_timeout_script<'b>( + &self, + script: PreTimeoutScriptPacket<'a, 'b>, + resp_tx: &UnboundedSender>, + req_rx: &mut UnboundedReceiver>, + ) { + let test_instance = script.test.test_instance; + let _ = resp_tx.send(ExecutorEvent::PreTimeoutScriptStarted { test_instance }); + + let mut stopwatch = crate::time::stopwatch(); + + let status = match self + .run_pre_timeout_script_inner(script.clone(), &mut stopwatch, resp_tx, req_rx) + .await + { + Ok(status) => status, + Err(error) => InternalPreTimeoutScriptExecuteStatus { + slow_after: None, + output: ChildExecutionOutput::StartError(error), + result: ExecutionResult::ExecFail, + stopwatch_end: stopwatch.snapshot(), + }, + }; + + let _ = resp_tx.send(ExecutorEvent::PreTimeoutScriptFinished { + test_instance, + script_id: script.script_id, + config: script.config, + status: status.into_external(), + }); + } + + #[instrument(level = "debug", skip(self, resp_tx, req_rx))] + async fn run_pre_timeout_script_inner<'b>( + &self, + script: PreTimeoutScriptPacket<'a, 'b>, + stopwatch: &mut StopwatchStart, + resp_tx: &UnboundedSender>, + req_rx: &mut UnboundedReceiver>, + ) -> Result { + // XXX: this function is 95% the same as run_setup_script_inner. Do we + // need to try to factor out a shared helper method? + + let mut cmd = script.make_command( + &self.double_spawn, + &self.test_list, + &script.test.test_instance, + script.test_pid, + )?; + let command_mut = cmd.command_mut(); + + // If creating a job fails, we might be on an old system. Ignore this -- job objects are a + // best-effort thing. + let job = super::os::Job::create().ok(); + + // For now, pre-timeout scripts always capture stdout and stderr. + command_mut.stdout(std::process::Stdio::piped()); + command_mut.stderr(std::process::Stdio::piped()); + + let mut child = cmd + .spawn() + .map_err(|error| ChildStartError::Spawn(Arc::new(error)))?; + let child_pid = child + .id() + .expect("child has never been polled so must return a PID"); + + // If assigning the child to the job fails, ignore this. This can happen if the process has + // exited. + let _ = super::os::assign_process_to_job(&child, job.as_ref()); + + let mut status: Option = None; + let slow_timeout = script + .config + .slow_timeout + .unwrap_or(self.profile.slow_timeout()); + let leak_timeout = script + .config + .leak_timeout + .unwrap_or(self.profile.leak_timeout()); + + let mut interval_sleep = std::pin::pin!(crate::time::pausable_sleep(slow_timeout.period)); + + let mut timeout_hit = 0; + + let child_fds = ChildFds::new_split(child.stdout.take(), child.stderr.take()); + let mut child_acc = ChildAccumulator::new(child_fds); + + let mut cx = UnitContext { + packet: UnitPacket::PreTimeoutScript(script.clone()), + slow_after: None, + }; + + let (res, leaked) = { + let res = loop { + tokio::select! { + () = child_acc.fill_buf(), if !child_acc.fds.is_done() => {} + res = child.wait() => { + // The pre-timeout script finished executing. + break res; + } + _ = &mut interval_sleep, if status.is_none() => { + // Mark the script as slow. + cx.slow_after = Some(slow_timeout.period); + + timeout_hit += 1; + let will_terminate = if let Some(terminate_after) = slow_timeout.terminate_after { + NonZeroUsize::new(timeout_hit as usize) + .expect("timeout_hit was just incremented") + >= terminate_after + } else { + false + }; + + if !slow_timeout.grace_period.is_zero() { + let _ = resp_tx.send(script.slow_event( + // Pass in the slow timeout period times timeout_hit, since + // stopwatch.elapsed() tends to be slightly longer. + timeout_hit * slow_timeout.period, + will_terminate.then_some(slow_timeout.grace_period), + )); + } + + if will_terminate { + // Attempt to terminate the slow script. As there is + // a race between shutting down a slow test and its + // own completion, we silently ignore errors to + // avoid printing false warnings. + // + // The return result of terminate_child is not used + // here, since it is always marked as a timeout. + _ = super::os::terminate_child( + &cx, + &mut child, + &mut child_acc, + InternalTerminateReason::Timeout, + stopwatch, + req_rx, + job.as_ref(), + slow_timeout.grace_period, + ).await; + status = Some(ExecutionResult::Timeout); + if slow_timeout.grace_period.is_zero() { + break child.wait().await; + } + // Don't break here to give the wait task a chance to finish. + } else { + interval_sleep.as_mut().reset_last_duration(); + } + } + recv = req_rx.recv() => { + // The sender stays open longer than the whole loop, and the buffer is big + // enough for all messages ever sent through this channel, so a RecvError + // should never happen. + let req = recv.expect("a RecvError should never happen here"); + + match req { + RunUnitRequest::Signal(req) => { + #[cfg_attr(not(windows), expect(unused_variables))] + let res = handle_signal_request( + &cx, + &mut child, + &mut child_acc, + req, + stopwatch, + interval_sleep.as_mut(), + req_rx, + job.as_ref(), + slow_timeout.grace_period + ).await; + + // On Unix, the signal the process exited with + // will be picked up by child.wait. On Windows, + // termination by job object will show up as + // exit code 1 -- we need to be clearer about + // that in the UI. + // + // TODO: Can we do something useful with res on + // Unix? For example, it's possible that the + // signal we send is not the same as the signal + // the child exits with. This might be a good + // thing to store in whatever test event log we + // end up building. + #[cfg(windows)] + { + if matches!( + res, + HandleSignalResult::Terminated(super::TerminateChildResult::Killed) + ) { + status = Some(ExecutionResult::Fail { + abort_status: Some(AbortStatus::JobObject), + leaked: false, + }); + } + } + } + RunUnitRequest::OtherCancel => { + // Ignore non-signal cancellation requests -- + // let the script finish. + } + RunUnitRequest::Query(RunUnitQuery::GetInfo(sender)) => { + _ = sender.send(script.info_response( + UnitState::Running { + pid: child_pid, + time_taken: stopwatch.snapshot().active, + slow_after: cx.slow_after, + }, + child_acc.snapshot_in_progress(UnitKind::WAITING_ON_SCRIPT_MESSAGE), + )); + } + } + } + } + }; + + // Build a tentative status using status and the exit status. + let tentative_status = status.or_else(|| { + res.as_ref() + .ok() + .map(|res| create_execution_result(*res, &child_acc.errors, false)) + }); + + let leaked = detect_fd_leaks( + &cx, + child_pid, + &mut child_acc, + tentative_status, + leak_timeout, + stopwatch, + req_rx, + ) + .await; + + (res, leaked) + }; + + let exit_status = match res { + Ok(exit_status) => Some(exit_status), + Err(err) => { + child_acc.errors.push(ChildFdError::Wait(Arc::new(err))); + None + } + }; + + let exit_status = exit_status.expect("None always results in early return"); + + let exec_result = status + .unwrap_or_else(|| create_execution_result(exit_status, &child_acc.errors, leaked)); + + let errors: Vec<_> = child_acc.errors.into_iter().map(ChildError::from).collect(); + + Ok(InternalPreTimeoutScriptExecuteStatus { + slow_after: cx.slow_after, + output: ChildExecutionOutput::Output { + result: Some(exec_result), + output: child_acc.output.freeze(), + errors: ErrorList::new(UnitKind::WAITING_ON_SCRIPT_MESSAGE, errors), + }, + result: exec_result, + stopwatch_end: stopwatch.snapshot(), + }) + } + /// Run an individual test in its own process. #[instrument(level = "debug", skip(self, resp_tx, req_rx))] async fn run_test( @@ -654,6 +919,7 @@ impl<'a> ExecutorContext<'a> { let mut status: Option = None; let slow_timeout = test.settings.slow_timeout(); let leak_timeout = test.settings.leak_timeout(); + let pre_timeout_script = self.profile.pre_timeout_script(&test.test_instance); // Use a pausable_sleep rather than an interval here because it's much // harder to pause and resume an interval. @@ -687,6 +953,25 @@ impl<'a> ExecutorContext<'a> { false }; + if will_terminate { + if let Some(script) = &pre_timeout_script { + let packet = PreTimeoutScriptPacket { + script_id: script.id.clone(), + test: test.clone(), + test_pid: child_pid, + test_stopwatch: stopwatch, + test_slow_after: slow_timeout.period, + test_output: child_acc.snapshot_in_progress(UnitKind::WAITING_ON_TEST_MESSAGE), + config: script.config, + }; + self.run_pre_timeout_script(packet, resp_tx, req_rx).await; + } + } + + // Send the will-terminate event *after* the pre-timeout + // script has completed (if it exists), so that the user + // observes the state transitions in this order: running + // slow -> pre-timeout -> terminating. if !slow_timeout.grace_period.is_zero() { let _ = resp_tx.send(test.slow_event( // Pass in the slow timeout period times timeout_hit, since @@ -905,16 +1190,16 @@ impl Iterator for BackoffIter { /// Either a test or a setup script, along with information about how long the /// test took. -pub(super) struct UnitContext<'a> { - packet: UnitPacket<'a>, +pub(super) struct UnitContext<'a, 'b> { + packet: UnitPacket<'a, 'b>, // TODO: This is a bit of a mess. It isn't clear where this kind of state // should live -- many parts of the request-response system need various // pieces of this code. slow_after: Option, } -impl<'a> UnitContext<'a> { - pub(super) fn packet(&self) -> &UnitPacket<'a> { +impl<'a, 'b> UnitContext<'a, 'b> { + pub(super) fn packet(&self) -> &UnitPacket<'a, 'b> { &self.packet } @@ -925,21 +1210,23 @@ impl<'a> UnitContext<'a> { ) -> InfoResponse<'a> { match &self.packet { UnitPacket::SetupScript(packet) => packet.info_response(state, output), + UnitPacket::PreTimeoutScript(packet) => packet.info_response(state, output), UnitPacket::Test(packet) => packet.info_response(state, output), } } } #[derive(Clone, Debug)] -pub(super) enum UnitPacket<'a> { +pub(super) enum UnitPacket<'a, 'b> { SetupScript(SetupScriptPacket<'a>), + PreTimeoutScript(PreTimeoutScriptPacket<'a, 'b>), Test(TestPacket<'a>), } -impl UnitPacket<'_> { +impl UnitPacket<'_, '_> { pub(super) fn kind(&self) -> UnitKind { match self { - Self::SetupScript(_) => UnitKind::Script, + Self::SetupScript(_) | Self::PreTimeoutScript(_) => UnitKind::Script, Self::Test(_) => UnitKind::Test, } } @@ -989,7 +1276,7 @@ impl<'a> TestPacket<'a> { #[derive(Clone, Debug)] pub(super) struct SetupScriptPacket<'a> { script_id: ScriptId, - config: &'a ScriptConfig, + config: &'a SetupScriptConfig, } impl<'a> SetupScriptPacket<'a> { @@ -1026,6 +1313,55 @@ impl<'a> SetupScriptPacket<'a> { } } +#[derive(Clone, Debug)] +pub(super) struct PreTimeoutScriptPacket<'a, 'b> { + script_id: ScriptId, + test: TestPacket<'a>, + test_pid: u32, + test_stopwatch: &'b StopwatchStart, + test_slow_after: Duration, + test_output: ChildExecutionOutput, + config: &'a PreTimeoutScriptConfig, +} + +impl<'a, 'b> PreTimeoutScriptPacket<'a, 'b> { + /// Turns self into a command that can be executed. + fn make_command( + &self, + double_spawn: &DoubleSpawnInfo, + test_list: &TestList<'_>, + test_instance: &TestInstance<'_>, + test_pid: u32, + ) -> Result { + PreTimeoutScriptCommand::new(self.config, double_spawn, test_list, test_instance, test_pid) + } + + fn slow_event(&self, elapsed: Duration, will_terminate: Option) -> ExecutorEvent<'a> { + ExecutorEvent::PreTimeoutScriptSlow { + test_instance: self.test.test_instance, + elapsed, + will_terminate, + } + } + + pub(super) fn info_response( + &self, + state: UnitState, + output: ChildExecutionOutput, + ) -> InfoResponse<'a> { + // The provided state is about the pre-timeout script, so we need to wrap + // it with the state of the test unit. + let state = UnitState::PreTimeout { + pid: self.test_pid, + time_taken: self.test_stopwatch.snapshot().active, + slow_after: self.test_slow_after, + script_state: Box::new(state), + script_output: output, + }; + self.test.info_response(state, self.test_output.clone()) + } +} + /// Drains the request receiver of any messages. fn drain_req_rx<'a>( mut receiver: UnboundedReceiver>, @@ -1130,7 +1466,7 @@ async fn handle_delay_between_attempts<'a>( /// could do more sophisticated checks around e.g. if any processes with the /// same PGID are around. async fn detect_fd_leaks<'a>( - cx: &UnitContext<'a>, + cx: &UnitContext<'a, '_>, child_pid: u32, child_acc: &mut ChildAccumulator, tentative_result: Option, @@ -1202,7 +1538,7 @@ async fn detect_fd_leaks<'a>( // can cause more harm than good. #[expect(clippy::too_many_arguments)] async fn handle_signal_request<'a>( - cx: &UnitContext<'a>, + cx: &UnitContext<'a, '_>, child: &mut Child, child_acc: &mut ChildAccumulator, req: SignalRequest, diff --git a/nextest-runner/src/runner/internal_events.rs b/nextest-runner/src/runner/internal_events.rs index bdd22d20d24..da74ca7029b 100644 --- a/nextest-runner/src/runner/internal_events.rs +++ b/nextest-runner/src/runner/internal_events.rs @@ -9,12 +9,12 @@ use super::{SetupScriptPacket, TestPacket}; use crate::{ - config::{ScriptConfig, ScriptId}, + config::{PreTimeoutScriptConfig, ScriptId, SetupScriptConfig}, list::TestInstance, reporter::{ events::{ - ExecuteStatus, ExecutionResult, InfoResponse, RetryData, SetupScriptEnvMap, - SetupScriptExecuteStatus, UnitState, + ExecuteStatus, ExecutionResult, InfoResponse, PreTimeoutScriptExecuteStatus, RetryData, + SetupScriptEnvMap, SetupScriptExecuteStatus, UnitState, }, TestOutputDisplay, }, @@ -41,7 +41,7 @@ use tokio::{ pub(super) enum ExecutorEvent<'a> { SetupScriptStarted { script_id: ScriptId, - config: &'a ScriptConfig, + config: &'a SetupScriptConfig, index: usize, total: usize, // See the note in the `Started` variant. @@ -49,13 +49,13 @@ pub(super) enum ExecutorEvent<'a> { }, SetupScriptSlow { script_id: ScriptId, - config: &'a ScriptConfig, + config: &'a SetupScriptConfig, elapsed: Duration, will_terminate: Option, }, SetupScriptFinished { script_id: ScriptId, - config: &'a ScriptConfig, + config: &'a SetupScriptConfig, index: usize, total: usize, status: SetupScriptExecuteStatus, @@ -80,6 +80,20 @@ pub(super) enum ExecutorEvent<'a> { elapsed: Duration, will_terminate: Option, }, + PreTimeoutScriptStarted { + test_instance: TestInstance<'a>, + }, + PreTimeoutScriptSlow { + test_instance: TestInstance<'a>, + elapsed: Duration, + will_terminate: Option, + }, + PreTimeoutScriptFinished { + test_instance: TestInstance<'a>, + script_id: ScriptId, + config: &'a PreTimeoutScriptConfig, + status: PreTimeoutScriptExecuteStatus, + }, AttemptFailedWillRetry { test_instance: TestInstance<'a>, failure_output: TestOutputDisplay, @@ -179,6 +193,25 @@ impl InternalSetupScriptExecuteStatus<'_> { } } +pub(super) struct InternalPreTimeoutScriptExecuteStatus { + pub(super) slow_after: Option, + pub(super) output: ChildExecutionOutput, + pub(super) result: ExecutionResult, + pub(super) stopwatch_end: StopwatchSnapshot, +} + +impl InternalPreTimeoutScriptExecuteStatus { + pub(super) fn into_external(self) -> PreTimeoutScriptExecuteStatus { + PreTimeoutScriptExecuteStatus { + 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(), + } + } +} + /// Events sent from the dispatcher to individual unit execution tasks. #[derive(Clone, Debug)] pub(super) enum RunUnitRequest<'a> { diff --git a/nextest-runner/src/runner/unix.rs b/nextest-runner/src/runner/unix.rs index 4c01e8f3028..de2d38b14bf 100644 --- a/nextest-runner/src/runner/unix.rs +++ b/nextest-runner/src/runner/unix.rs @@ -78,7 +78,7 @@ pub(super) fn raise_stop() { // Windows) or grace_period (only relevant on Unix) together. #[expect(clippy::too_many_arguments)] pub(super) async fn terminate_child<'a>( - cx: &UnitContext<'a>, + cx: &UnitContext<'a, '_>, child: &mut Child, child_acc: &mut ChildAccumulator, reason: InternalTerminateReason, diff --git a/site/mkdocs.yml b/site/mkdocs.yml index f66c61cf0ff..82fe94df37b 100644 --- a/site/mkdocs.yml +++ b/site/mkdocs.yml @@ -112,7 +112,10 @@ nav: - "Mutual exclusion and rate-limiting": docs/configuration/test-groups.md - "Environment variables": docs/configuration/env-vars.md - "Extra arguments": docs/configuration/extra-args.md - - docs/configuration/setup-scripts.md + - "Scripts": + - "Overview": docs/scripts/index.md + - docs/scripts/setup.md + - docs/scripts/pre-timeout.md - Machine-readable output: - "About output formats": docs/machine-readable/index.md - "JUnit support": docs/machine-readable/junit.md diff --git a/site/src/docs/configuration/env-vars.md b/site/src/docs/configuration/env-vars.md index 52d6d807ffb..da7e2489342 100644 --- a/site/src/docs/configuration/env-vars.md +++ b/site/src/docs/configuration/env-vars.md @@ -165,6 +165,6 @@ Nextest sets the dynamic library path at runtime, similar to [what Cargo does](h Nextest includes the following paths: -- Search paths included from any build script with the [`rustc-link-search` instruction](https://doc.rust-lang.org/cargo/reference/build-scripts.html#rustc-link-search). Paths outside of the target directory are removed. If additional libraries on the system are needed in the search path, consider using a [setup script ](setup-scripts.md) to configure the environment. +- Search paths included from any build script with the [`rustc-link-search` instruction](https://doc.rust-lang.org/cargo/reference/build-scripts.html#rustc-link-search). Paths outside of the target directory are removed. If additional libraries on the system are needed in the search path, consider using a [setup script ](../scripts/setup.md) to configure the environment. - The base output directory, such as `target/debug`, and the "deps" directory. This enables support for `dylib` dependencies and rustc compiler plugins. - The rustc sysroot library path, to enable proc-macro tests and binaries compiled with `-C prefer-dynamic` to work. diff --git a/site/src/docs/scripts/index.md b/site/src/docs/scripts/index.md new file mode 100644 index 00000000000..59aae9a8382 --- /dev/null +++ b/site/src/docs/scripts/index.md @@ -0,0 +1,125 @@ +--- +icon: material/script-text +--- + +# Scripts + +Nextest supports running _scripts_ when certain events occur during a test run. Scripts can be scoped to particular tests via [filtersets](../filtersets/index.md). + +Nextest currently recognizes two types of scripts: + +* [_Setup scripts_](setup.md), which execute at the start of a test run. +* [_Pre-timeout scripts_](pre-timeout.md), which execute before nextest terminates a test that has exceeded its timeout. + +Scripts are configured in two parts: _defining scripts_, and _specifying rules_ for when they should be executed. + +## Defining scripts + +Scripts are defined using the top-level `script.` configuration. + +For example, to define a setup script named "my-script", which runs `my-script.sh`: + +```toml title="Setup script definition in .config/nextest.toml" +[script.setup.my-script] +command = 'my-script.sh' +# Additional options... +``` + +See [_Defining setup scripts_](setup.md#defining-setup-scripts) for the additional options available for configuring setup scripts. + +To instead define a pre-timeout script named "my-script", which runs `my-script.sh`: + +```toml title="Pre-timeout script definition in .config/nextest.toml" +[script.pre-timeout.my-script] +command = 'my-script.sh' +# Additional options... +``` + +See [_Defining pre-timeout scripts_](pre-timeout.md#defining-pre-timeout-scripts) for the additional options available for configuring pre-timeout scripts. + +### Command specification + +All script types support the `command` option, which specifies how to invoke the script. Commands can either be specified using Unix shell rules, or as a list of arguments. In the following example, `script1` and `script2` are equivalent. + +```toml +[script..script1] +command = 'script.sh -c "Hello, world!"' + +[script..script2] +command = ['script.sh', '-c', 'Hello, world!'] +``` + +### Timeouts + +All script types support the following timeout options: + +- **`slow-timeout`**: Mark a script [as slow](../features/slow-tests.md) or [terminate it](../features/slow-tests.md#terminating-tests-after-a-timeout), using the same configuration as for tests. By default, scripts are not marked as slow or terminated (this is different from the slow timeout for tests). +- **`leak-timeout`**: Mark scripts [leaky](../features/leaky-tests.md) after a timeout, using the same configuration as for tests. By default, the leak timeout is 100ms. + + +```toml title="Script definition with timeouts" +[script..my-script] +command = 'script.sh' +slow-timeout = { period = "60s", terminate-after = 2 } +leak-timeout = "1s" +``` + +### Namespacing + +Script names must be unique across all script types. + +This means that you cannot use the same name for a setup script and a pre-timeout script: + +```toml title="Pre-timeout script definition in .config/nextest.toml" +[script.setup.my-script] +command = 'setup.sh' + +# Reusing the `my-script` name for a pre-timeout script is NOT permitted. +[script.pre-timeout.my-script] +command = 'pre-timeout.sh' +``` + +## Specifying rules + +In configuration, you can create rules for when to use scripts on a per-profile basis. This is done via the `profile..scripts` array. For example, you can configure a setup script that generates a database if tests from the `db-tests` package, or any packages that depend on it, are run. + +```toml title="Basic rules" +[[profile.default.scripts]] +filter = 'rdeps(db-tests)' +setup = 'db-generate' +``` + +(This example uses the `rdeps` [filterset](../filtersets/index.md) predicate.) + +Scripts can also filter based on platform, using the rules listed in [_Specifying platforms_](../configuration/specifying-platforms.md): + +```toml title="Platform-specific rules" +[[profile.default.scripts]] +platform = { host = "cfg(unix)" } +setup = 'script1' +``` + +A set of scripts can also be specified. All scripts in the set will be executed. + +```toml title="Multiple setup scripts" +[[profile.default.scripts]] +filter = 'test(/^script_tests::/)' +setup = ['script1', 'script2'] +``` + +Executing pre-timeout scripts follows the same pattern. For example, you can configure a pre-timeout script for every test that contains `slow` in its name. + +```toml title="Basic pre-timeout rules" +[[profile.default.scripts]] +filter = 'test(slow)' +pre-timeout = 'capture-backtrace' +``` + +A single rule can specify any number of setup scripts and any number of pre-timeout scripts. + +```toml title="Combination rules" +[[profile.default.scripts]] +filter = 'test(slow)' +setup = ['setup-1', 'setup-2'] +pre-timeout = ['pre-timeout-1', 'pre-timeout-2'] +``` diff --git a/site/src/docs/scripts/pre-timeout.md b/site/src/docs/scripts/pre-timeout.md new file mode 100644 index 00000000000..9ed71871162 --- /dev/null +++ b/site/src/docs/scripts/pre-timeout.md @@ -0,0 +1,71 @@ +--- +icon: material/timer-sand-empty +status: experimental +--- + + + +# Pre-timeout scripts + +!!! experimental "Experimental: This feature is not yet stable" + + - **Enable with:** Add `experimental = ["pre-timeout-scripts"]` to `.config/nextest.toml` + - **Tracking issue:** [#TODO](https://github.com/nextest-rs/nextest/issues/TODO) + + +Nextest runs *pre-timeout scripts* before terminating a test that has exceeded +its timeout. + +Pre-timeout scripts are useful for automatically collecting backtraces, logs, etc. that can assist in debugging why a test is slow or hung. + +## Defining pre-timeout scripts + +Pre-timeout scripts are defined using the top-level `script.pre-timeout` configuration. For example, to define a script named "my-script", which runs `my-script.sh`: + +```toml title="Script definition in .config/nextest.toml" +[script.pre-timeout.my-script] +command = 'my-script.sh' +``` + +See [_Defining scripts_](index.md#defining-scripts) for options that are common to all scripts. + +Pre-timeout scripts do not support additional configuration options. + +Notably, pre-timeout scripts always capture stdout and stderr. Support for not capturing stdout and stderr may be added in the future in order to support use cases like interactive debugging of a hung test. + +### Example + +To invoke GDB to dump backtraces before a hanging test is terminated: + +```toml title="Advanced pre-timeout script definition" +[script.pre-timeout.gdb-dump] +command = ['sh', '-c', 'gdb -p $NEXTEST_PRE_TIMEOUT_TEST_PID -batch -ex "thread apply all backtrace"'] +# TODO options +``` + +## Specifying pre-timeout script rules + +See [_Specifying rules_](index.md#specifying-rules). + +## Pre-timeout script execution + +A given pre-timeout script _S_ is a candidate for execution when the current profile has at least one rule where the `platform` predicates match the current execution environment, the script _S_ is listed in `pre-timeout`, and a test matching the `filter` _T_ has reached its configured timeout. + +Only the _first_ candidate pre-timeout script will be executed for each such _T_. Nextest orders execution candidates by their definition order (_not_ the order they're specified in the rules). + +If any pre-timeout script exits with a non-zero exit code, an error is logged but the test run continues. + +Nextest will proceed with graceful termination of the test only once the pre-timeout script terminates. See [_How nextest terminates tests_](#defining-pre-timeout-scripts). If the pre-timeout script itself is slow, nextest will apply the same termination protocol to the pre-timeout script. + +The pre-timeout script is not responsible for terminating the test process, but it is permissible for it to do so. + +Nextest executes pre-timeout scripts with the same working directory as the test and sets the following variables in the script's environment: + +* **`NEXTEST_PRE_TIMEOUT_TEST_PID`**: the ID of the process running the test. +* **`NEXTEST_PRE_TIMEOUT_TEST_NAME`**: the name of the running test. +* **`NEXTEST_PRE_TIMEOUT_TEST_BINARY_ID`**: the ID of the binary in which the test is located. +* **`NEXTEST_PRE_TIMEOUT_TEST_PACKAGE`**: the name of the package (crate) containing the test. +* **`NEXTEST_PRE_TIMEOUT_TEST_BINARY`**: the name of the binary hosting the test. +* **`NEXTEST_PRE_TIMEOUT_TEST_BINARY_KIND`**: the kind of the binary hosting the test. + + diff --git a/site/src/docs/configuration/setup-scripts.md b/site/src/docs/scripts/setup.md similarity index 62% rename from site/src/docs/configuration/setup-scripts.md rename to site/src/docs/scripts/setup.md index ea27db506a0..5d2cf88e224 100644 --- a/site/src/docs/configuration/setup-scripts.md +++ b/site/src/docs/scripts/setup.md @@ -12,77 +12,38 @@ status: experimental - **Enable with:** Add `experimental = ["setup-scripts"]` to `.config/nextest.toml` - **Tracking issue:** [#978](https://github.com/nextest-rs/nextest/issues/978) -Nextest supports running _setup scripts_ before tests are run. Setup scripts can be scoped to -particular tests via [filtersets](../filtersets/index.md). +Nextest runs *setup scripts* before tests are run. -Setup scripts are configured in two parts: _defining scripts_, and _setting up rules_ for when they should be executed. +## Defining setup scripts -## Defining scripts +Setup scripts are defined using the top-level `script.setup` configuration. For example, to define a script named "my-script", which runs `my-script.sh`: -Setup scripts are defined using the top-level `script` configuration. For example, to define a script named "my-script", which runs `my-script.sh`: - -```toml title="Setup script definition in .config/nextest.toml" -[script.my-script] +```toml title="Script definition in .config/nextest.toml" +[script.setup.my-script] command = 'my-script.sh' ``` -Commands can either be specified using Unix shell rules, or as a list of arguments. In the following example, `script1` and `script2` are equivalent. +See [_Defining scripts_](index.md#defining-scripts) for options that are common to all scripts. -```toml -[script.script1] -command = 'script.sh -c "Hello, world!"' - -[script.script2] -command = ['script.sh', '-c', 'Hello, world!'] -``` +Setup scripts support the following additional configuration options: -Setup scripts can have the following configuration options attached to them: - -- **`slow-timeout`**: Mark a setup script [as slow](../features/slow-tests.md) or [terminate it](../features/slow-tests.md#terminating-tests-after-a-timeout), using the same configuration as for tests. By default, setup scripts are not marked as slow or terminated (this is different from the slow timeout for tests). -- **`leak-timeout`**: Mark setup scripts [leaky](../features/leaky-tests.md) after a timeout, using the same configuration as for tests. By default, the leak timeout is 100ms. - **`capture-stdout`**: `true` if the script's standard output should be captured, `false` if not. By default, this is `false`. - **`capture-stderr`**: `true` if the script's standard error should be captured, `false` if not. By default, this is `false`. ### Example ```toml title="Advanced setup script definition" -[script.db-generate] +[script.setup.db-generate] command = 'cargo run -p db-generate' -slow-timeout = { period = "60s", terminate-after = 2 } -leak-timeout = "1s" capture-stdout = true capture-stderr = false ``` -## Setting up rules - -In configuration, you can create rules for when to use scripts on a per-profile basis. This is done via the `profile..scripts` array. For example, you can set up a script that generates a database if tests from the `db-tests` package, or any packages that depend on it, are run. - -```toml title="Basic rules" -[[profile.default.scripts]] -filter = 'rdeps(db-tests)' -setup = 'db-generate' -``` - -(This example uses the `rdeps` [filterset](../filtersets/index.md) predicate.) - -Setup scripts can also filter based on platform, using the rules listed in [_Specifying platforms_](../configuration/specifying-platforms.md): +## Specifying setup script rules -```toml title="Platform-specific rules" -[[profile.default.scripts]] -platform = { host = "cfg(unix)" } -setup = 'script1' -``` - -A set of scripts can also be specified. All scripts in the set will be executed. - -```toml title="Multiple setup scripts" -[[profile.default.scripts]] -filter = 'test(/^script_tests::/)' -setup = ['script1', 'script2'] -``` +See [_Specifying rules_](index.md#specifying-rules). -## Script execution +## Setup script execution A given setup script _S_ is only executed if the current profile has at least one rule where the `filter` and `platform` predicates match the current execution environment, and the setup script _S_ is listed in `setup`.