Skip to content

Commit

Permalink
generalize timeout grace period to all shutdown events
Browse files Browse the repository at this point in the history
  • Loading branch information
erratic-pattern committed Oct 15, 2023
1 parent 107c657 commit c87440e
Show file tree
Hide file tree
Showing 7 changed files with 100 additions and 46 deletions.
9 changes: 9 additions & 0 deletions nextest-runner/default-config.toml
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,15 @@ slow-timeout = { period = "60s" }
# See <https://nexte.st/book/leaky-tests> for more information.
leak-timeout = "100ms"

# Control how long to wait for graceful shutdown of a test. Only supported on Unix platforms.
#
# When a test shutdown occurs, nextest waits for the grace period for the test to shut down.
# If the test doesn't shut itself down within that time, nextest sends SIGKILL (kill -9)
# to the process group to terminate it immediately.
#
# See <https://nexte.st/book/slow-tests.html#how-nextest-terminates-tests>
shutdown-grace-period = "10s"

[profile.default.junit]
# Output a JUnit report into the given file inside 'store.dir/<profile-name>'.
# If unspecified, JUnit is not written out.
Expand Down
13 changes: 13 additions & 0 deletions nextest-runner/src/config/config_impl.rs
Original file line number Diff line number Diff line change
Expand Up @@ -659,6 +659,13 @@ impl<'cfg> NextestProfile<'cfg, FinalConfig> {
.unwrap_or(self.default_profile.leak_timeout)
}

/// Returns the time to wait after sending a SIGTERM to a child process before sending a SIGKILL
pub fn shutdown_grace_period(&self) -> Duration {
self.custom_profile
.and_then(|profile| profile.shutdown_grace_period)
.unwrap_or(self.default_profile.shutdown_grace_period)
}

/// Returns the test status level.
pub fn status_level(&self) -> StatusLevel {
self.custom_profile
Expand Down Expand Up @@ -874,6 +881,7 @@ pub(super) struct DefaultProfileImpl {
fail_fast: bool,
slow_timeout: SlowTimeout,
leak_timeout: Duration,
shutdown_grace_period: Duration,
overrides: Vec<DeserializedOverride>,
scripts: Vec<DeserializedProfileScriptConfig>,
junit: DefaultJunitImpl,
Expand Down Expand Up @@ -908,6 +916,9 @@ impl DefaultProfileImpl {
leak_timeout: p
.leak_timeout
.expect("leak-timeout present in default profile"),
shutdown_grace_period: p
.shutdown_grace_period
.expect("shutdown-grace-period present in default profile"),
overrides: p.overrides,
scripts: p.scripts,
junit: DefaultJunitImpl {
Expand Down Expand Up @@ -968,6 +979,8 @@ pub(super) struct CustomProfileImpl {
slow_timeout: Option<SlowTimeout>,
#[serde(default, with = "humantime_serde::option")]
leak_timeout: Option<Duration>,
#[serde(default, with = "humantime_serde::option")]
shutdown_grace_period: Option<Duration>,
#[serde(default)]
overrides: Vec<DeserializedOverride>,
#[serde(default)]
Expand Down
31 changes: 27 additions & 4 deletions nextest-runner/src/config/overrides.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ pub struct TestSettings<Source = ()> {
retries: (RetryPolicy, Source),
slow_timeout: (SlowTimeout, Source),
leak_timeout: (Duration, Source),
shutdown_grace_period: (Duration, Source),
test_group: (TestGroup, Source),
success_output: (TestOutputDisplay, Source),
failure_output: (TestOutputDisplay, Source),
Expand Down Expand Up @@ -88,6 +89,11 @@ impl TestSettings {
self.leak_timeout.0
}

/// Returns the terminate grace period for this test.
pub fn shutdown_grace_period(&self) -> Duration {
self.shutdown_grace_period.0
}

/// Returns the test group for this test.
pub fn test_group(&self) -> &TestGroup {
&self.test_group.0
Expand Down Expand Up @@ -127,6 +133,7 @@ impl<Source: Copy> TestSettings<Source> {
let mut retries = None;
let mut slow_timeout = None;
let mut leak_timeout = None;
let mut shutdown_grace_period = None;
let mut test_group = None;
let mut success_output = None;
let mut failure_output = None;
Expand Down Expand Up @@ -172,6 +179,11 @@ impl<Source: Copy> TestSettings<Source> {
leak_timeout = Some(Source::track_override(l, override_));
}
}
if shutdown_grace_period.is_none() {
if let Some(s) = override_.data.shutdown_grace_period {
shutdown_grace_period = Some(Source::track_override(s, override_));
}
}
if test_group.is_none() {
if let Some(t) = &override_.data.test_group {
test_group = Some(Source::track_override(t.clone(), override_));
Expand Down Expand Up @@ -207,6 +219,8 @@ impl<Source: Copy> TestSettings<Source> {
slow_timeout.unwrap_or_else(|| Source::track_profile(profile.slow_timeout()));
let leak_timeout =
leak_timeout.unwrap_or_else(|| Source::track_profile(profile.leak_timeout()));
let shutdown_grace_period = shutdown_grace_period
.unwrap_or_else(|| Source::track_profile(profile.shutdown_grace_period()));
let test_group = test_group.unwrap_or_else(|| Source::track_profile(TestGroup::Global));
let success_output =
success_output.unwrap_or_else(|| Source::track_profile(profile.success_output()));
Expand All @@ -226,6 +240,7 @@ impl<Source: Copy> TestSettings<Source> {
retries,
slow_timeout,
leak_timeout,
shutdown_grace_period,
test_group,
success_output,
failure_output,
Expand Down Expand Up @@ -254,6 +269,10 @@ impl<Source: Copy> TestSettings<Source> {
self.leak_timeout
}

pub(crate) fn shutdown_grace_period_with_source(&self) -> (Duration, Source) {
self.shutdown_grace_period
}

Check warning on line 274 in nextest-runner/src/config/overrides.rs

View check run for this annotation

Codecov / codecov/patch

nextest-runner/src/config/overrides.rs#L272-L274

Added lines #L272 - L274 were not covered by tests

/// Returns the test group for this test, with the source attached.
pub(crate) fn test_group_with_source(&self) -> &(TestGroup, Source) {
&self.test_group
Expand Down Expand Up @@ -401,6 +420,7 @@ pub(super) struct ProfileOverrideData {
retries: Option<RetryPolicy>,
slow_timeout: Option<SlowTimeout>,
leak_timeout: Option<Duration>,
shutdown_grace_period: Option<Duration>,
pub(super) test_group: Option<TestGroup>,
success_output: Option<TestOutputDisplay>,
failure_output: Option<TestOutputDisplay>,
Expand Down Expand Up @@ -450,6 +470,7 @@ impl CompiledOverride<PreBuildPlatform> {
retries: source.retries,
slow_timeout: source.slow_timeout,
leak_timeout: source.leak_timeout,
shutdown_grace_period: source.shutdown_grace_period,
test_group: source.test_group.clone(),
success_output: source.success_output,
failure_output: source.failure_output,
Expand Down Expand Up @@ -558,6 +579,8 @@ pub(super) struct DeserializedOverride {
slow_timeout: Option<SlowTimeout>,
#[serde(default, with = "humantime_serde::option")]
leak_timeout: Option<Duration>,
#[serde(default, with = "humantime_serde::option")]
shutdown_grace_period: Option<Duration>,
#[serde(default)]
test_group: Option<TestGroup>,
#[serde(default)]
Expand Down Expand Up @@ -652,7 +675,8 @@ mod tests {
platform = 'aarch64-apple-darwin' # this is the target platform
filter = "test(test)"
retries = { backoff = "exponential", count = 20, delay = "1s", max-delay = "20s" }
slow-timeout = { period = "120s", terminate-after = 1, grace-period = "0s" }
slow-timeout = { period = "120s", terminate-after = 1 }
shutdown-grace-period = "0s"
success-output = "immediate-final"
junit = { store-success-output = true }
Expand Down Expand Up @@ -727,11 +751,11 @@ mod tests {
overrides.slow_timeout(),
SlowTimeout {
period: Duration::from_secs(60),
terminate_after: None,
grace_period: Duration::from_secs(10),
terminate_after: None
}
);
assert_eq!(overrides.leak_timeout(), Duration::from_millis(300));
assert_eq!(overrides.shutdown_grace_period(), Duration::from_secs(10));
assert_eq!(overrides.test_group(), &test_group("my-group"));
assert_eq!(overrides.success_output(), TestOutputDisplay::Never);
assert_eq!(overrides.failure_output(), TestOutputDisplay::Final);
Expand Down Expand Up @@ -769,7 +793,6 @@ mod tests {
SlowTimeout {
period: Duration::from_secs(120),
terminate_after: Some(NonZeroUsize::new(1).unwrap()),
grace_period: Duration::ZERO,
}
);
assert_eq!(overrides.leak_timeout(), Duration::from_millis(300));
Expand Down
4 changes: 4 additions & 0 deletions nextest-runner/src/config/scripts.rs
Original file line number Diff line number Diff line change
Expand Up @@ -505,6 +505,9 @@ pub struct ScriptConfig {
#[serde(default, with = "humantime_serde::option")]
pub leak_timeout: Option<Duration>,

#[serde(default, with = "humantime_serde::option")]
pub shutdown_grace_period: Option<Duration>,

/// Whether to capture standard output for this command.
#[serde(default)]
pub capture_stdout: bool,
Expand Down Expand Up @@ -651,6 +654,7 @@ mod tests {
[script.bar]
command = ["cargo", "run", "-p", "bar"]
slow-timeout = { period = "60s", terminate-after = 2 }
shutdown-grace-period = "5s"
[script.baz]
command = "baz"
Expand Down
28 changes: 10 additions & 18 deletions nextest-runner/src/config/slow_timeout.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,6 @@ pub struct SlowTimeout {
pub(crate) period: Duration,
#[serde(default)]
pub(crate) terminate_after: Option<NonZeroUsize>,
#[serde(with = "humantime_serde", default = "default_grace_period")]
pub(crate) grace_period: Duration,
}

impl SlowTimeout {
Expand All @@ -22,14 +20,9 @@ impl SlowTimeout {
// See far_future() in pausable_sleep.rs for why this is roughly 30 years.
period: Duration::from_secs(86400 * 365 * 30),
terminate_after: None,
grace_period: Duration::from_secs(10),
};
}

fn default_grace_period() -> Duration {
Duration::from_secs(10)
}

pub(super) fn deserialize_slow_timeout<'de, D>(
deserializer: D,
) -> Result<Option<SlowTimeout>, D::Error>
Expand Down Expand Up @@ -59,7 +52,6 @@ where
Ok(Some(SlowTimeout {
period,
terminate_after: None,
grace_period: default_grace_period(),
}))
}
}
Expand Down Expand Up @@ -88,7 +80,7 @@ mod tests {

#[test_case(
"",
Ok(SlowTimeout { period: Duration::from_secs(60), terminate_after: None, grace_period: Duration::from_secs(10) }),
Ok(SlowTimeout { period: Duration::from_secs(60), terminate_after: None }),
None
; "empty config is expected to use the hardcoded values"
Expand All @@ -98,7 +90,7 @@ mod tests {
[profile.default]
slow-timeout = "30s"
"#},
Ok(SlowTimeout { period: Duration::from_secs(30), terminate_after: None, grace_period: Duration::from_secs(10) }),
Ok(SlowTimeout { period: Duration::from_secs(30), terminate_after: None }),
None
; "overrides the default profile"
Expand All @@ -111,8 +103,8 @@ mod tests {
[profile.ci]
slow-timeout = { period = "60s", terminate-after = 3 }
"#},
Ok(SlowTimeout { period: Duration::from_secs(30), terminate_after: None, grace_period: Duration::from_secs(10) }),
Some(SlowTimeout { period: Duration::from_secs(60), terminate_after: Some(NonZeroUsize::new(3).unwrap()), grace_period: Duration::from_secs(10) })
Ok(SlowTimeout { period: Duration::from_secs(30), terminate_after: None }),
Some(SlowTimeout { period: Duration::from_secs(60), terminate_after: Some(NonZeroUsize::new(3).unwrap()) })
; "adds a custom profile 'ci'"
)]
Expand All @@ -124,21 +116,21 @@ mod tests {
[profile.ci]
slow-timeout = "30s"
"#},
Ok(SlowTimeout { period: Duration::from_secs(60), terminate_after: Some(NonZeroUsize::new(3).unwrap()), grace_period: Duration::from_secs(10) }),
Some(SlowTimeout { period: Duration::from_secs(30), terminate_after: None, grace_period: Duration::from_secs(10) })
Ok(SlowTimeout { period: Duration::from_secs(60), terminate_after: Some(NonZeroUsize::new(3).unwrap())}),
Some(SlowTimeout { period: Duration::from_secs(30), terminate_after: None })
; "ci profile uses string notation"
)]
#[test_case(
indoc! {r#"
[profile.default]
slow-timeout = { period = "60s", terminate-after = 3, grace-period = "1s" }
slow-timeout = { period = "60s", terminate-after = 3 }
[profile.ci]
slow-timeout = "30s"
"#},
Ok(SlowTimeout { period: Duration::from_secs(60), terminate_after: Some(NonZeroUsize::new(3).unwrap()), grace_period: Duration::from_secs(1) }),
Some(SlowTimeout { period: Duration::from_secs(30), terminate_after: None, grace_period: Duration::from_secs(10) })
Ok(SlowTimeout { period: Duration::from_secs(60), terminate_after: Some(NonZeroUsize::new(3).unwrap())}),
Some(SlowTimeout { period: Duration::from_secs(30), terminate_after: None })
; "timeout grace period"
)]
Expand All @@ -147,7 +139,7 @@ mod tests {
[profile.default]
slow-timeout = { period = "60s" }
"#},
Ok(SlowTimeout { period: Duration::from_secs(60), terminate_after: None, grace_period: Duration::from_secs(10) }),
Ok(SlowTimeout { period: Duration::from_secs(60), terminate_after: None }),
None
; "partial table"
Expand Down
Loading

0 comments on commit c87440e

Please sign in to comment.