add a fail-fast flag (#42)

Summary:
This resolves #41. Full disclosure: this change was coded by an AI agent. Original prompt:

> Please build a feature for fast failures. It should be a test case configuration option (similar to "detached") named  "fail_fast". When set, if the test case fails for any reason (exit status, snapshot, etc), the entire test document immediately stops.

I reviewed the code and tests, and tested the result in my actual scrut deployment.

Pull Request resolved: #42

Reviewed By: AndreasBackx

Differential Revision: D87975693

Pulled By: ukautz

fbshipit-source-id: 45446d6c09ac9e9f44ff5f42a964db3c0bc4533c

Author: CGamesPlay

selftest/complex/inline-configuration/test-testcase-fail-fast.md

@@ -0,0 +1,47 @@
+# Validate per-testcase fail_fast configuration
+
+Tests in this file validate that the `fail_fast` option stops execution of the entire test document when a test case fails.
+
+```scrut
+$ alias scrut_test='$SCRUT_BIN test --match-markdown="*.mdtest"'
+```
+
+## fail_fast stops on first failure
+
+```scrut
+$ scrut_test "$TESTDIR"/test-testcase-fail-fast.mdtest 2>&1
+// =============================================================================
+// @ *test-testcase-fail-fast.mdtest:* (glob)
+// -----------------------------------------------------------------------------
+// # Second test fails with fail_fast
+// -----------------------------------------------------------------------------
+// $ echo "Test 2"
+// =============================================================================
+
+1     | - Wrong output
+   1  | + Test 2
+
+
+Result: 1 document(s) with 3 testcase(s): 1 succeeded, 1 failed and 1 skipped
+[50]
+```
+
+## without fail_fast all tests run
+
+```scrut
+$ scrut_test "$TESTDIR"/test-testcase-no-fail-fast.mdtest 2>&1
+// =============================================================================
+// @ *test-testcase-no-fail-fast.mdtest:* (glob)
+// -----------------------------------------------------------------------------
+// # Second test fails without fail_fast
+// -----------------------------------------------------------------------------
+// $ echo "Test 2"
+// =============================================================================
+
+1     | - Wrong output
+   1  | + Test 2
+
+
+Result: 1 document(s) with 3 testcase(s): 2 succeeded, 1 failed and 0 skipped
+[50]
+```

selftest/complex/inline-configuration/test-testcase-fail-fast.mdtest

@@ -0,0 +1,24 @@
+# This test validates fail_fast stops execution on first failure
+
+This file has 3 tests: first passes, second fails with fail_fast enabled, third should be skipped.
+
+## First test passes
+
+```scrut
+$ echo "Test 1"
+Test 1
+```
+
+## Second test fails with fail_fast
+
+```scrut {fail_fast: true}
+$ echo "Test 2"
+Wrong output
+```
+
+## Third test should be skipped
+
+```scrut
+$ echo "Test 3"
+Test 3
+```

selftest/complex/inline-configuration/test-testcase-no-fail-fast.mdtest

@@ -0,0 +1,24 @@
+# This test validates normal behavior without fail_fast
+
+This file has 3 tests: first passes, second fails WITHOUT fail_fast, third should still run.
+
+## First test passes
+
+```scrut
+$ echo "Test 1"
+Test 1
+```
+
+## Second test fails without fail_fast
+
+```scrut
+$ echo "Test 2"
+Wrong output
+```
+
+## Third test should still run
+
+```scrut
+$ echo "Test 3"
+Test 3
+```

src/bin/commands/test.rs

@@ -294,51 +294,24 @@ impl Args {
 
                     // ... because test timed out
                     ExecutionError::Timeout(timeout, outputs) => {
-                        // append outcomes for each testcase that was executed (i.e. all testcase
-                        // until and including the one that timed out)
-                        outcomes.extend(outputs.iter().zip(testcases.iter()).map(
-                            |(output, testcase)| {
-                                let result = if matches!(output.exit_code, ExitStatus::Timeout(_)) {
-                                    count_failed += 1;
+                        handle_early_termination(
+                            &outputs,
+                            &testcases,
+                            &mut outcomes,
+                            test.path.display().to_string(),
+                            escaping.clone(),
+                            test.parser_type,
+                            &mut count_success,
+                            &mut count_failed,
+                            &mut count_skipped,
+                            |output, testcase| {
+                                if matches!(output.exit_code, ExitStatus::Timeout(_)) {
                                     Err(TestCaseError::Timeout)
                                 } else {
-                                    let result = testcase.validate(output);
-                                    if result.is_err() {
-                                        count_failed += 1;
-                                        result
-                                    } else {
-                                        count_success += 1;
-                                        Ok(())
-                                    }
-                                };
-                                Outcome {
-                                    location: Some(test.path.display().to_string()),
-                                    testcase: (*testcase).clone(),
-                                    output: output.clone(),
-                                    escaping: escaping.clone(),
-                                    format: test.parser_type,
-                                    result,
+                                    testcase.validate(output)
                                 }
                             },
-                        ));
-
-                        // append outcomes for each testcase that was not executed (i.e. all
-                        // testcases after the one that timed out)
-                        let missing = testcases.len() - outputs.len();
-                        if missing > 0 {
-                            outcomes.extend(testcases.iter().skip(outputs.len()).map(|testcase| {
-                                Outcome {
-                                    location: Some(test.path.display().to_string()),
-                                    testcase: (*testcase).clone(),
-                                    output: ("", "", None).into(),
-                                    escaping: escaping.clone(),
-                                    format: test.parser_type,
-                                    result: Err(TestCaseError::Skipped),
-                                }
-                            }));
-
-                            count_skipped += missing;
-                        }
+                        );
 
                         let (location, timeout) = match timeout {
                             ExecutionTimeout::Index(idx) => (
@@ -361,6 +334,29 @@ impl Args {
                         continue;
                     }
 
+                    // ... because test failed with fail_fast enabled
+                    ExecutionError::Failed(idx, outputs) => {
+                        handle_early_termination(
+                            &outputs,
+                            &testcases,
+                            &mut outcomes,
+                            test.path.display().to_string(),
+                            escaping.clone(),
+                            test.parser_type,
+                            &mut count_success,
+                            &mut count_failed,
+                            &mut count_skipped,
+                            |output, testcase| testcase.validate(output),
+                        );
+
+                        pw.println(format!(
+                            "⚡ {}: stopped at testcase #{} due to fail_fast",
+                            style(test.path.to_string_lossy()).red(),
+                            idx + 1,
+                        ));
+                        continue;
+                    }
+
                     // ... because of a final error
                     _ => bail!("failing in {:?}: {}", test.path, err),
                 },
@@ -480,6 +476,65 @@ impl Args {
     }
 }
 
+/// Helper function to handle early termination cases (timeout, fail_fast).
+/// Validates outputs that were collected, marks remaining tests as skipped.
+fn handle_early_termination<F>(
+    outputs: &[scrut::output::Output],
+    testcases: &[&TestCase],
+    outcomes: &mut Vec<Outcome>,
+    location: String,
+    escaping: scrut::escaping::Escaper,
+    format: ParserType,
+    count_success: &mut usize,
+    count_failed: &mut usize,
+    count_skipped: &mut usize,
+    mut validate_output: F,
+) where
+    F: FnMut(&scrut::output::Output, &TestCase) -> Result<(), TestCaseError>,
+{
+    // append outcomes for each testcase that was executed
+    outcomes.extend(
+        outputs
+            .iter()
+            .zip(testcases.iter())
+            .map(|(output, testcase)| {
+                let result = validate_output(output, testcase);
+                if result.is_err() {
+                    *count_failed += 1;
+                } else {
+                    *count_success += 1;
+                }
+                Outcome {
+                    location: Some(location.clone()),
+                    testcase: (*testcase).clone(),
+                    output: output.clone(),
+                    escaping: escaping.clone(),
+                    format,
+                    result,
+                }
+            }),
+    );
+
+    // append outcomes for testcases not executed
+    let missing = testcases.len() - outputs.len();
+    if missing > 0 {
+        outcomes.extend(
+            testcases
+                .iter()
+                .skip(outputs.len())
+                .map(|testcase| Outcome {
+                    location: Some(location.clone()),
+                    testcase: (*testcase).clone(),
+                    output: ("", "", None).into(),
+                    escaping: escaping.clone(),
+                    format,
+                    result: Err(TestCaseError::Skipped),
+                }),
+        );
+        *count_skipped += missing;
+    }
+}
+
 fn prefix_with_directory(prefix: &Path, paths: &[PathBuf]) -> Vec<PathBuf> {
     paths
         .iter()

src/config.rs

@@ -242,6 +242,11 @@ pub struct TestCaseConfig {
     #[serde(skip_serializing_if = "Option::is_none")]
     pub detached_kill_signal: Option<KillSignal>,
 
+    /// If true, stops execution of the entire test document immediately if this
+    /// test case fails for any reason (exit status, snapshot validation, etc).
+    #[serde(skip_serializing_if = "Option::is_none")]
+    pub fail_fast: Option<bool>,
+
     /// A set of environment variable names and values that will be explicitly set
     /// for the test.
     #[serde(skip_serializing_if = "BTreeMap::is_empty")]
@@ -332,6 +337,7 @@ impl TestCaseConfig {
             && self.keep_crlf.is_none()
             && self.timeout.is_none()
             && self.detached.is_none()
+            && self.fail_fast.is_none()
             && self.wait.is_none()
             && self.skip_document_code.is_none()
             && self.strip_ansi_escaping.is_none()
@@ -358,6 +364,7 @@ impl TestCaseConfig {
                 .detached_kill_signal
                 .clone()
                 .or_else(|| defaults.detached_kill_signal.clone()),
+            fail_fast: self.fail_fast.or(defaults.fail_fast),
             wait: self.wait.clone().or_else(|| defaults.wait.clone()),
             skip_document_code: self.skip_document_code.or(defaults.skip_document_code),
             strip_ansi_escaping: self.strip_ansi_escaping.or(defaults.strip_ansi_escaping),
@@ -406,6 +413,9 @@ impl TestCaseConfig {
         if self.detached != other.detached {
             diff.detached = self.detached;
         }
+        if self.fail_fast != other.fail_fast {
+            diff.fail_fast = self.fail_fast;
+        }
         if self.skip_document_code != other.skip_document_code {
             diff.skip_document_code = self.skip_document_code;
         }
@@ -451,6 +461,9 @@ impl TestCaseConfig {
         if let Some(value) = self.detached {
             output.push(format!("detached: {}", value))
         }
+        if let Some(value) = self.fail_fast {
+            output.push(format!("fail_fast: {}", value))
+        }
         if let Some(value) = self.skip_document_code {
             output.push(format!("skip_document_code: {}", value))
         }
@@ -484,6 +497,10 @@ impl TestCaseConfig {
         self.skip_document_code
             .unwrap_or(DEFAULT_SKIP_DOCUMENT_CODE)
     }
+
+    pub fn get_fail_fast(&self) -> bool {
+        self.fail_fast.unwrap_or(false)
+    }
 }
 
 impl Display for TestCaseConfig {
@@ -553,6 +570,7 @@ append:
 defaults:
   detached: true
   detached_kill_signal: quit
+  fail_fast: true
   environment:
     BAZ: zoing
     FOO: bar
@@ -594,6 +612,7 @@ total_timeout: 5m 3s
                     },
                     detached: Some(true),
                     detached_kill_signal: Some(KillSignal::test_default()),
+                    fail_fast: Some(true),
                     wait: Some(TestCaseWait {
                         timeout: Duration::from_secs(2 * 60 + 1),
                         path: Some(PathBuf::from("the-wait-path")),
@@ -624,6 +643,7 @@ total_timeout: 5m 3s
                 },
                 detached: Some(true),
                 detached_kill_signal: Some(KillSignal::test_default()),
+                fail_fast: Some(true),
                 wait: Some(TestCaseWait {
                     timeout: Duration::from_secs(2 * 60 + 1),
                     path: Some(PathBuf::from("the-wait-path")),
@@ -641,6 +661,7 @@ total_timeout: 5m 3s
     const FULL_TESTCASE_CONFIG: &str = "
 detached: true
 detached_kill_signal: quit
+fail_fast: true
 environment:
   BAZ: zoing
   FOO: bar
@@ -672,6 +693,7 @@ wait:
                 },
                 detached: Some(true),
                 detached_kill_signal: Some(KillSignal::test_default()),
+                fail_fast: Some(true),
                 wait: Some(TestCaseWait {
                     timeout: Duration::from_secs(2 * 60 + 1),
                     path: Some(PathBuf::from("the-wait-path")),
@@ -696,6 +718,7 @@ wait:
             },
             detached: Some(true),
             detached_kill_signal: Some(KillSignal::test_default()),
+            fail_fast: Some(true),
             wait: Some(TestCaseWait {
                 timeout: Duration::from_secs(2 * 60 + 1),
                 path: Some(PathBuf::from("the-wait-path")),
@@ -711,7 +734,7 @@ wait:
 
     #[test]
     fn test_testcase_config_yaml_one_liner() {
-        let tests = vec![
+        let tests = [
             (TestCaseConfig::empty(), "{}"),
             (
                 TestCaseConfig {
@@ -736,6 +759,7 @@ wait:
                     keep_crlf: Some(true),
                     detached: Some(false),
                     detached_kill_signal: None,
+                    fail_fast: Some(false),
                     environment: BTreeMap::from([("foo".to_string(), "bar".to_string())]),
                     skip_document_code: Some(123),
                     strip_ansi_escaping: Some(true),
@@ -745,7 +769,7 @@ wait:
                         path: Some(PathBuf::from("/tmp/wait")),
                     }),
                 },
-                "{output_stream: stderr, keep_crlf: true, timeout: 3m 54s, detached: false, skip_document_code: 123, strip_ansi_escaping: true, wait: {timeout: 2m 3s, path: /tmp/wait}, environment: {foo: \"bar\"}}",
+                "{output_stream: stderr, keep_crlf: true, timeout: 3m 54s, detached: false, fail_fast: false, skip_document_code: 123, strip_ansi_escaping: true, wait: {timeout: 2m 3s, path: /tmp/wait}, environment: {foo: \"bar\"}}",
             ),
         ];
         for (idx, (config, expected)) in tests.iter().enumerate() {