feat: treat timeouts as failure or success based on config

fixture-data/src/nextest_tests.rs

@@ -47,6 +47,10 @@ pub static EXPECTED_TEST_SUITES: LazyLock<IdOrdMap<TestSuiteFixture>> = LazyLock
                     "test_slow_timeout_subprocess",
                     TestCaseFixtureStatus::IgnoredPass,
                 ),
+                TestCaseFixture::new(
+                    "test_flaky_slow_timeout_mod_3",
+                    TestCaseFixtureStatus::IgnoredFail
+                ),
                 TestCaseFixture::new("test_stdin_closed", TestCaseFixtureStatus::Pass),
                 TestCaseFixture::new("test_subprocess_doesnt_exit", TestCaseFixtureStatus::Leak),
                 TestCaseFixture::new("test_subprocess_doesnt_exit_fail", TestCaseFixtureStatus::FailLeak),

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

@@ -88,6 +88,22 @@ filter = 'test(=test_slow_timeout_2)'
 slow-timeout = { period = "500ms", terminate-after = 2 }
 test-group = '@global'
 
+[profile.with-timeout-success]
+slow-timeout = { period = "10ms", terminate-after = 2, on-timeout = "pass" }
+
+[[profile.with-timeout-success.overrides]]
+filter = 'test(=test_slow_timeout_2)'
+slow-timeout = { period = "500ms", terminate-after = 2, on-timeout = "fail" }
+test-group = '@global'
+
+[[profile.with-timeout-success.overrides]]
+filter = 'test(=test_slow_timeout_subprocess)'
+slow-timeout = { period = "10ms", terminate-after = 2, on-timeout = "fail" }
+
+[profile.with-timeout-retries-success]
+slow-timeout = { period = "10ms", terminate-after = 2, on-timeout = "pass" }
+retries = 2
+
 [profile.with-junit]
 retries = 2
 

fixtures/nextest-tests/tests/basic.rs

@@ -319,6 +319,17 @@ fn test_slow_timeout_subprocess() {
     cmd.output().unwrap();
 }
 
+#[test]
+#[ignore]
+fn test_flaky_slow_timeout_mod_3() {
+    let nextest_attempt = nextest_attempt();
+    if nextest_attempt % 3 != 0 {
+        panic!("Failed because attempt {} % 3 != 0", nextest_attempt)
+    }
+    // The timeout for the with-timeout-success profile is set to 2 seconds.
+    std::thread::sleep(std::time::Duration::from_secs(4));
+}
+
 #[test]
 fn test_result_failure() -> Result<(), std::io::Error> {
     Err(std::io::Error::new(

nextest-runner/default-config.toml

@@ -88,7 +88,11 @@ fail-fast = true
 # which will cause slow tests to be terminated after the specified number of
 # periods have passed.
 # Example: slow-timeout = { period = "60s", terminate-after = 2 }
-slow-timeout = { period = "60s" }
+#
+# The 'on-timeout' parameter controls whether timeouts are treated as failures (the default)
+# or successes.
+# Example: slow-timeout = { period = "60s", on-timeout = "pass" }
+slow-timeout = { period = "60s", on-timeout = "fail" }
 
 # Treat a test as leaky if after the process is shut down, standard output and standard error
 # aren't closed within this duration.

nextest-runner/src/config/elements/slow_timeout.rs

@@ -15,6 +15,8 @@ pub struct SlowTimeout {
     pub(crate) terminate_after: Option<NonZeroUsize>,
     #[serde(with = "humantime_serde", default = "default_grace_period")]
     pub(crate) grace_period: Duration,
+    #[serde(default)]
+    pub(crate) on_timeout: SlowTimeoutResult,
 }
 
 impl SlowTimeout {
@@ -24,6 +26,7 @@ impl SlowTimeout {
         period: far_future_duration(),
         terminate_after: None,
         grace_period: Duration::from_secs(10),
+        on_timeout: SlowTimeoutResult::Fail,
     };
 }
 
@@ -61,6 +64,7 @@ where
                     period,
                     terminate_after: None,
                     grace_period: default_grace_period(),
+                    on_timeout: SlowTimeoutResult::Fail,
                 }))
             }
         }
@@ -76,6 +80,25 @@ where
     deserializer.deserialize_any(V)
 }
 
+/// The result of controlling slow timeout behavior.
+///
+/// In most situations a timed out test should be marked failing. However, there are certain
+/// classes of tests which are expected to run indefinitely long, like fuzzing, which explores a
+/// huge state space. For these tests it's nice to be able to treat a timeout as a success since
+/// they generally check for invariants and other properties of the code under test during their
+/// execution. A timeout in this context doesn't mean that there are no failing inputs, it just
+/// means that they weren't found up until that moment, which is still valuable information.
+#[derive(Clone, Copy, Debug, Deserialize, Default, PartialEq, Eq)]
+#[serde(rename_all = "kebab-case")]
+pub enum SlowTimeoutResult {
+    #[default]
+    /// The test is marked as failed.
+    Fail,
+
+    /// The test is marked as passed.
+    Pass,
+}
+
 #[cfg(test)]
 mod tests {
     use super::*;
@@ -87,7 +110,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, grace_period: Duration::from_secs(10), on_timeout: SlowTimeoutResult::Fail }),
         None
 
         ; "empty config is expected to use the hardcoded values"
@@ -97,7 +120,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, grace_period: Duration::from_secs(10), on_timeout: SlowTimeoutResult::Fail }),
         None
 
         ; "overrides the default profile"
@@ -110,8 +133,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, grace_period: Duration::from_secs(10), on_timeout: SlowTimeoutResult::Fail }),
+        Some(SlowTimeout { period: Duration::from_secs(60), terminate_after: Some(NonZeroUsize::new(3).unwrap()), grace_period: Duration::from_secs(10), on_timeout: SlowTimeoutResult::Fail })
 
         ; "adds a custom profile 'ci'"
     )]
@@ -123,8 +146,8 @@ 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()), grace_period: Duration::from_secs(10), on_timeout: SlowTimeoutResult::Fail }),
+        Some(SlowTimeout { period: Duration::from_secs(30), terminate_after: None, grace_period: Duration::from_secs(10), on_timeout: SlowTimeoutResult::Fail })
 
         ; "ci profile uses string notation"
     )]
@@ -136,8 +159,8 @@ 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(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()), grace_period: Duration::from_secs(1), on_timeout: SlowTimeoutResult::Fail }),
+        Some(SlowTimeout { period: Duration::from_secs(30), terminate_after: None, grace_period: Duration::from_secs(10), on_timeout: SlowTimeoutResult::Fail })
 
         ; "timeout grace period"
     )]
@@ -146,7 +169,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, grace_period: Duration::from_secs(10), on_timeout: SlowTimeoutResult::Fail }),
         None
 
         ; "partial table"
@@ -161,6 +184,39 @@ mod tests {
 
         ; "zero terminate-after should fail"
     )]
+    #[test_case(
+        indoc! {r#"
+            [profile.default]
+            slow-timeout = { period = "60s", on-timeout = "pass" }
+        "#},
+        Ok(SlowTimeout { period: Duration::from_secs(60), terminate_after: None, grace_period: Duration::from_secs(10), on_timeout: SlowTimeoutResult::Pass }),
+        None
+
+        ; "timeout result success"
+    )]
+    #[test_case(
+        indoc! {r#"
+            [profile.default]
+            slow-timeout = { period = "60s", on-timeout = "fail" }
+        "#},
+        Ok(SlowTimeout { period: Duration::from_secs(60), terminate_after: None, grace_period: Duration::from_secs(10), on_timeout: SlowTimeoutResult::Fail }),
+        None
+
+        ; "timeout result failure"
+    )]
+    #[test_case(
+        indoc! {r#"
+            [profile.default]
+            slow-timeout = { period = "60s", on-timeout = "pass" }
+
+            [profile.ci]
+            slow-timeout = { period = "30s", on-timeout = "fail" }
+        "#},
+        Ok(SlowTimeout { period: Duration::from_secs(60), terminate_after: None, grace_period: Duration::from_secs(10), on_timeout: SlowTimeoutResult::Pass }),
+        Some(SlowTimeout { period: Duration::from_secs(30), terminate_after: None, grace_period: Duration::from_secs(10), on_timeout: SlowTimeoutResult::Fail })
+
+        ; "override on-timeout option"
+    )]
     #[test_case(
         indoc! {r#"
             [profile.default]

nextest-runner/src/config/overrides/imp.rs

@@ -1019,7 +1019,11 @@ impl<'de> Deserialize<'de> for PlatformStrings {
 #[cfg(test)]
 mod tests {
     use super::*;
-    use crate::config::{core::NextestConfig, elements::LeakTimeoutResult, utils::test_helpers::*};
+    use crate::config::{
+        core::NextestConfig,
+        elements::{LeakTimeoutResult, SlowTimeoutResult},
+        utils::test_helpers::*,
+    };
     use camino_tempfile::tempdir;
     use indoc::indoc;
     use std::{num::NonZeroUsize, time::Duration};
@@ -1066,6 +1070,11 @@ mod tests {
             filter = "test(override5)"
             retries = 8
 
+            # Override 6 -- timeout result success
+            [[profile.default.overrides]]
+            filter = "test(timeout_success)"
+            slow-timeout = { period = "30s", on-timeout = "pass" }
+
             [profile.default.junit]
             path = "my-path.xml"
 
@@ -1108,6 +1117,7 @@ mod tests {
             overrides.slow_timeout(),
             SlowTimeout {
                 period: Duration::from_secs(60),
+                on_timeout: SlowTimeoutResult::default(),
                 terminate_after: None,
                 grace_period: Duration::from_secs(10),
             }
@@ -1159,6 +1169,7 @@ mod tests {
                 period: Duration::from_secs(120),
                 terminate_after: Some(NonZeroUsize::new(1).unwrap()),
                 grace_period: Duration::ZERO,
+                on_timeout: SlowTimeoutResult::default(),
             }
         );
         assert_eq!(
@@ -1197,6 +1208,22 @@ mod tests {
         let overrides = profile.settings_for(&query);
         assert_eq!(overrides.retries(), RetryPolicy::new_without_delay(8));
 
+        // This query matches override 6.
+        let query = TestQuery {
+            binary_query: target_binary_query.to_query(),
+            test_name: "timeout_success",
+        };
+        let overrides = profile.settings_for(&query);
+        assert_eq!(
+            overrides.slow_timeout(),
+            SlowTimeout {
+                period: Duration::from_secs(30),
+                on_timeout: SlowTimeoutResult::Pass,
+                terminate_after: None,
+                grace_period: Duration::from_secs(10),
+            }
+        );
+
         // This query does not match any overrides.
         let query = TestQuery {
             binary_query: target_binary_query.to_query(),

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

@@ -5,7 +5,7 @@
 
 use crate::{
     config::{
-        elements::{JunitConfig, LeakTimeoutResult},
+        elements::{JunitConfig, LeakTimeoutResult, SlowTimeoutResult},
         scripts::ScriptId,
     },
     errors::{DisplayErrorChain, WriteEventError},
@@ -339,7 +339,9 @@ fn non_success_kind_and_type(kind: UnitKind, result: ExecutionResult) -> (NonSuc
             NonSuccessKind::Failure,
             format!("{kind} failure with exit code {code}"),
         ),
-        ExecutionResult::Timeout => (NonSuccessKind::Failure, format!("{kind} timeout")),
+        ExecutionResult::Timeout {
+            result: SlowTimeoutResult::Fail,
+        } => (NonSuccessKind::Failure, format!("{kind} timeout")),
         ExecutionResult::ExecFail => (NonSuccessKind::Error, "execution failure".to_owned()),
         ExecutionResult::Leak {
             result: LeakTimeoutResult::Pass,
@@ -353,7 +355,10 @@ fn non_success_kind_and_type(kind: UnitKind, result: ExecutionResult) -> (NonSuc
             NonSuccessKind::Error,
             format!("{kind} exited with code 0, but leaked handles so was marked failed"),
         ),
-        ExecutionResult::Pass => {
+        ExecutionResult::Pass
+        | ExecutionResult::Timeout {
+            result: SlowTimeoutResult::Pass,
+        } => {
             unreachable!("this is a failure status")
         }
     }