feat: update timed out test counting logic

Since timed out tests can now be treated as either successes or
failures, update the logic for handling them. Note that before
`self.failed` and `self.timed_out` were separate, but now `self.failed`
includes `self.failed_timed_out`.

Author: eduardorittner

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

@@ -83,12 +83,11 @@ where
 /// The result of controlling slow timeout behavior.
 ///
 /// For most situations, a timed out test should be marked failing. However, there are certain
-/// classes of tests which are expected to run indefinetely long (or just too long), like fuzzing
-/// and property tests, which both explore 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. A timeout in this context doesn't necessarily mean that there are no
-/// failing inputs, just that they weren't found up until that moment, which is still valuable
-/// information.
+/// classes of tests which are expected to run indefinetely long (or just too 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.
+/// A timeout in this context doesn't necessarily mean that there are no failing inputs, just 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 {

nextest-runner/src/reporter/events.rs

@@ -553,7 +553,7 @@ pub struct RunStats {
     /// The number of tests that passed on retry.
     pub flaky: usize,
 
-    /// The number of tests that failed.
+    /// The number of tests that failed. Includes `leaky_failed`
     pub failed: usize,
 
     /// The number of failed tests that were slow.
@@ -720,7 +720,20 @@ impl RunStats {
                     self.failed_slow += 1;
                 }
             }
-            ExecutionResult::Timeout { .. } => self.failed_timed_out += 1,
+            ExecutionResult::Timeout {
+                result: SlowTimeoutResult::Pass,
+            } => {
+                self.passed += 1;
+                self.passed_timed_out += 1;
+                if run_statuses.len() > 1 {
+                    self.flaky += 1;
+                }
+            }
+            ExecutionResult::Timeout {
+                result: SlowTimeoutResult::Fail,
+            } => {
+                self.failed_timed_out += 1;
+            }
             ExecutionResult::ExecFail => self.exec_failed += 1,
         }
     }

nextest-runner/src/runner/executor.rs

@@ -830,7 +830,7 @@ impl<'a> ExecutorContext<'a> {
                                 job.as_ref(),
                                 slow_timeout.grace_period,
                             ).await;
-                            status = Some(ExecutionResult::Timeout {result: slow_timeout.on_timeout});
+                            status = Some(ExecutionResult::Timeout { result: slow_timeout.on_timeout });
                             if slow_timeout.grace_period.is_zero() {
                                 break child.wait().await;
                             }

nextest-runner/tests/integration/basic.rs

@@ -749,3 +749,95 @@ fn test_termination() -> Result<()> {
 
     Ok(())
 }
+
+#[test]
+fn test_timeout_result() -> Result<()> {
+    test_init();
+
+    let pcx = ParseContext::new(&PACKAGE_GRAPH);
+    let expr = Filterset::parse(
+        "test(/^test_slow_timeout/)".to_owned(),
+        &pcx,
+        FiltersetKind::Test,
+    )
+    .unwrap();
+    let test_filter = TestFilterBuilder::new(
+        RunIgnored::Only,
+        None,
+        TestFilterPatterns::default(),
+        vec![expr],
+    )
+    .unwrap();
+
+    let test_list = FIXTURE_TARGETS.make_test_list(
+        "with-timeout-success",
+        &test_filter,
+        &TargetRunner::empty(),
+    )?;
+    let config = load_config();
+    let profile = config
+        .profile("with-timeout-success")
+        .expect("with-timeout-success config is valid");
+    let build_platforms = BuildPlatforms::new_with_no_target().unwrap();
+    let profile = profile.apply_build_platforms(&build_platforms);
+
+    let runner = TestRunnerBuilder::default()
+        .build(
+            &test_list,
+            &profile,
+            vec![],
+            SignalHandlerKind::Noop,
+            InputHandlerKind::Noop,
+            DoubleSpawnInfo::disabled(),
+            TargetRunner::empty(),
+        )
+        .unwrap();
+
+    let (instance_statuses, run_stats) = execute_collect(runner);
+    assert_eq!(
+        run_stats.passed_timed_out, 3,
+        "3 tests timed out and passed"
+    );
+    for test_name in [
+        "test_slow_timeout",
+        "test_slow_timeout_2",
+        "test_slow_timeout_subprocess",
+    ] {
+        let (_, instance_value) = instance_statuses
+            .iter()
+            .find(|&(&(_, name), _)| name == test_name)
+            .unwrap_or_else(|| panic!("{test_name} should be present"));
+        let valid = match &instance_value.status {
+            InstanceStatus::Skipped(_) => panic!("{test_name} should have been run"),
+            InstanceStatus::Finished(run_statuses) => {
+                // This test should not have been retried since retries aren't configured.
+                assert_eq!(
+                    run_statuses.len(),
+                    1,
+                    "{test_name} should have been run exactly once",
+                );
+                let run_status = run_statuses.last_status();
+                // The test should have taken less than 5 seconds (most relevant for
+                // test_slow_timeout_subprocess -- without job objects it gets stuck on Windows
+                // until the subprocess exits.)
+                assert!(
+                    run_status.time_taken < Duration::from_secs(5),
+                    "{test_name} should have taken less than 5 seconds, actually took {:?}",
+                    run_status.time_taken
+                );
+                run_status.result
+                    == ExecutionResult::Timeout {
+                        result: SlowTimeoutResult::Pass,
+                    }
+            }
+        };
+        if !valid {
+            panic!(
+                "for test_slow_timeout, mismatch in status: expected timeout, actual {:?}",
+                instance_value.status
+            );
+        }
+    }
+
+    Ok(())
+}

site/src/docs/features/slow-tests.md

@@ -46,7 +46,7 @@ slow-timeout = { period = "30s", terminate-after = 4 }
 
 <!-- md:version 0.9.112 -->
 
-By default, tests that time out are treated as failures. For fuzz and property tests with very large state spaces (or on a constrained environment like CI), it may be useful to treat timeouts as successes, since they're not expected to test every possible input. A timeout in this context is not the same as a failure, it just means they weren't able to find any input that caused the test to fail. You can configure this behavior using the `on-timeout` parameter. For example, to treat timeouts as successes:
+By default, tests that time out are treated as failures. For fuzz tests with very large state spaces (or on a constrained environment like CI), it may be useful to treat timeouts as successes, since they're not expected to test every possible input. A timeout in this context is not the same as a failure, it just means they weren't able to find any input that caused the test to fail. You can configure this behavior using the `on-timeout` parameter. For example, to treat timeouts as successes:
 
 ```toml title="Timeouts as successes"
 [profile.default]