Skip to content

Annotate flaky tests #196

Description

@marshall-mcmullen

We should have a way to annotate flaky tests and have ebash automatically re-run them.

diff --git a/CLAUDE.md b/CLAUDE.md
index eb17ab5..81ea7e9 100644
--- a/CLAUDE.md
+++ b/CLAUDE.md
@@ -39,6 +39,7 @@ type -P podman &>/dev/null && echo "podman available"
 ## Writing Tests

 Tests are bash functions prefixed with `ETEST_` in `.etest` files.
+Use `FLAKY_ETEST_` prefix for known-flaky tests (automatically retried once on failure).
 Use `DISABLED_ETEST_` prefix to exclude tests from normal runs (run with `etest --disabled`):

 ```bash
diff --git a/doc/etest.md b/doc/etest.md
index 3bf8271..cfd2c1f 100644
--- a/doc/etest.md
+++ b/doc/etest.md
@@ -155,6 +155,40 @@ match a pattern (`--exclude` / `-x`), and more.

 There are literally thousands of tests to look at for examples [here](https://github.com/elibs/ebash/tree/master/tests).

+## Test name prefixes
+
+In addition to the standard `ETEST_` prefix, etest recognizes two special prefixes that change how a test is
+discovered and run:
+
+| Prefix            | Discovered by default? | Behavior                                                                   |
+|-------------------|------------------------|----------------------------------------------------------------------------|
+| `ETEST_`          | Yes                    | Standard test. Runs once. Pass on `0`, skip on `77`, fail otherwise.       |
+| `FLAKY_ETEST_`    | Yes                    | Known-flaky test. Runs once; if it fails it is automatically retried once. |
+| `DISABLED_ETEST_` | No (use `--disabled`)  | Excluded from normal runs. Run explicitly with `etest --disabled`.         |
+
+### Flaky tests
+
+Tests that are known to fail intermittently (e.g. due to timing or external races) can be prefixed with
+`FLAKY_ETEST_` instead of `ETEST_`. A flaky test is still discovered and run by default, but if it fails on its
+first attempt it is given a second chance:
+
+- If either attempt passes, the test is reported as **PASSED** (a note is written to the test log indicating that
+  it passed on retry, so genuinely flaky tests remain easy to find).
+- Only if **both** attempts fail is the test reported as **FAILED**, with the full stack trace from the final attempt.
+
+```shell
+FLAKY_ETEST_sometimes_races()
+{
+    # Runs once; automatically retried a single time if it fails.
+    do_something_racy
+}
+```
+
+Only the test function itself is retried -- `setup`, `teardown`, `suite_setup` and `suite_teardown` run exactly
+once regardless of retries. A flaky test that needs a clean slate between attempts should perform its own cleanup at
+the top of the function. Flakiness and `--repeat` are independent: under `--repeat=N` a flaky test gets up to two
+attempts within each of the `N` iterations.
+
 ## Test verbosity

 That listing of test passes and failures looks nice, but when your test fails, it's not particularly helpful.But
diff --git a/share/etest/runners.sh b/share/etest/runners.sh
index 4d25f33..10268e8 100644
--- a/share/etest/runners.sh
+++ b/share/etest/runners.sh
@@ -52,10 +52,12 @@ run_single_test()
         verbose

     # If this file is being sourced then it's an ETEST so log it as a subtest via einfos. Otherwise log via einfo as a
-    # top-level test script.
+    # top-level test script. Strip any FLAKY_ prefix as well as the ETEST_ prefix so the subtest label reads as the
+    # bare test name (e.g. FLAKY_ETEST_foo -> foo).
     local einfo_message einfo_message_length
     if [[ -n "${source}" ]]; then
-        einfo_message=$(einfos -n "${testname#ETEST_}" 2>&1)
+        local short_testname="${testname#FLAKY_}"
+        einfo_message=$(einfos -n "${short_testname#ETEST_}" 2>&1)
     else
         einfo_message=$(EMSG_PREFIX="" einfo -n "${testname}" 2>&1)
     fi
@@ -155,13 +157,55 @@ run_single_test()

         : ${ETEST_TIMEOUT:=${timeout}}
         : ${ETEST_JOBS:=${jobs}}
-        etestmsg "Running $(lval command testidx testidx_total timeout=ETEST_TIMEOUT jobs=ETEST_JOBS)"

+        # Tests prefixed with FLAKY_ get one automatic retry: they run once, and if they fail they
+        # are given a second chance. If either attempt passes the test is considered PASSED; only if
+        # both attempts fail is it considered FAILED. Non-flaky tests use a single attempt and behave
+        # exactly as before. Note: only the test command itself is re-run -- setup, teardown and the
+        # suite_setup/suite_teardown hooks run exactly once regardless of retries.
+        local etest_attempts=1
+        [[ ${testname} == FLAKY_* ]] && etest_attempts=2
+
+        # Build the actual command to run, wrapping in etimeout only when a finite timeout is set.
+        local run_cmd=( "${command}" )
         if [[ -n "${ETEST_TIMEOUT}" && "${ETEST_TIMEOUT}" != "infinity" ]]; then
-            etimeout --timeout="${ETEST_TIMEOUT}" "${command}"
-        else
-            "${command}"
+            run_cmd=( etimeout --timeout="${ETEST_TIMEOUT}" "${command}" )
         fi
+
+        local attempt=1 cmd_rc=0
+        while [[ ${attempt} -le ${etest_attempts} ]]; do
+
+            if [[ ${attempt} -gt 1 ]]; then
+                ewarn "Flaky test failed on attempt $(( attempt - 1 ))/${etest_attempts}; retrying"
+            fi
+
+            etestmsg "Running $(lval command testidx testidx_total timeout=ETEST_TIMEOUT jobs=ETEST_JOBS attempt etest_attempts)"
+
+            if [[ ${attempt} -lt ${etest_attempts} ]]; then
+                # Non-final attempt: capture the return code WITHOUT dying so we can retry on failure.
+                $(tryrc -r=cmd_rc "${run_cmd[@]}")
+
+                # A skip (77) propagates immediately; a pass stops retrying; a failure falls through
+                # and loops around to the next (final) attempt.
+                if [[ ${cmd_rc} -eq 77 ]]; then
+                    exit 77
+                elif [[ ${cmd_rc} -eq 0 ]]; then
+                    einfo "Flaky test passed on attempt ${attempt}/${etest_attempts}"
+                    break
+                fi
+            else
+                # Final attempt: run directly so a real failure produces a full stack trace and
+                # propagates up to the catch block below, exactly as a normal test failure would.
+                "${run_cmd[@]}"
+
+                # Reaching here on the final attempt of a flaky test means a retry succeeded.
+                if [[ ${etest_attempts} -gt 1 ]]; then
+                    einfo "Flaky test passed on attempt ${attempt}/${etest_attempts}"
+                fi
+            fi
+
+            : $(( ++attempt ))
+        done
     }
     catch
     {
@@ -712,8 +756,10 @@ __run_all_tests_parallel()

                     # Determine testdir and statedir for the crashed test
                     # statedir is always ${testdir}.state (sibling directory pattern)
+                    # A non-empty testfunc from a .etest file is a test function (ETEST_, FLAKY_ETEST_ or
+                    # DISABLED_ETEST_); otherwise it's a standalone executable test script.
                     local testdir statedir
-                    if [[ "${testfunc}" == ETEST_* ]]; then
+                    if [[ "${testfile}" == *.etest && -n "${testfunc}" ]]; then
                         testdir="${workdir}/${suite}.etest/${testfunc}"
                     else
                         testdir="${workdir}/$(basename "${testfunc}")"
diff --git a/share/etest/test_list.sh b/share/etest/test_list.sh
index d2665fc..e41a115 100644
--- a/share/etest/test_list.sh
+++ b/share/etest/test_list.sh
@@ -57,13 +57,14 @@ find_matching_tests()

     # Build function list for all .etest files in a single grep pass (much faster than per-file grep)
     # Output format: "filepath:ETEST_funcname()" - we parse this to build TEST_FUNCTIONS_TO_RUN
-    # When --disabled is set, also include DISABLED_ETEST_ functions
+    # FLAKY_ETEST_ functions are discovered (and run with one automatic retry) by default.
+    # When --disabled is set, also include DISABLED_ETEST_ functions.
     if [[ ${#all_etests[@]} -gt 0 ]]; then
         local grep_line testfile function grep_pattern
         if [[ ${disabled:-0} -eq 1 ]]; then
-            grep_pattern="^(DISABLED_)?ETEST[-_][a-zA-Z0-9_-]+\(\)"
+            grep_pattern="^(DISABLED_|FLAKY_)?ETEST[-_][a-zA-Z0-9_-]+\(\)"
         else
-            grep_pattern="^ETEST[-_][a-zA-Z0-9_-]+\(\)"
+            grep_pattern="^(FLAKY_)?ETEST[-_][a-zA-Z0-9_-]+\(\)"
         fi

         # Track functions per file to detect duplicates within the same file
diff --git a/tests/etest/flaky/flaky.etest b/tests/etest/flaky/flaky.etest
new file mode 100644
index 0000000..ee71743
--- /dev/null
+++ b/tests/etest/flaky/flaky.etest
@@ -0,0 +1,284 @@
+#!/usr/bin/env bash
+#
+# Copyright 2026, Marshall McMullen <marshall.mcmullen@gmail.com>
+#
+# This program is free software: you can redistribute it and/or modify it under the terms of the Apache License
+# as published by the Apache Software Foundation, either version 2 of the License, or (at your option) any later
+# version.
+
+#-----------------------------------------------------------------------------------------------------------------------
+#
+# Tests: FLAKY_ test prefix with automatic single retry
+#
+# These tests verify that FLAKY_ETEST_ functions are discovered by default, retried once on failure,
+# and reported correctly. Each test spawns a nested etest subprocess over a fixture file and inspects
+# its output and exit code.
+#
+#-----------------------------------------------------------------------------------------------------------------------
+
+# Run nested etest in a clean environment to avoid inheriting parent etest variables.
+# Modeled after tests/etest/options/logdir.etest::run_nested_etest.
+run_nested_etest()
+{
+    mkdir -p nested_etest
+    (
+        cd nested_etest
+        env -i HOME="${HOME}" PATH="${PATH}" TERM="${TERM}" \
+            FLAKY_MARKER="${FLAKY_MARKER:-}" \
+            FLAKY_COUNTER="${FLAKY_COUNTER:-}" \
+            "${EBASH_HOME}/bin/etest" "$@" 2>&1
+    )
+}
+
+# Fixture writers. Each emits a throwaway .etest file body to the path given in $1.
+#
+# Fixtures are generated as temp files OUTSIDE the tests/ tree on purpose: the top-level CI test run discovers
+# tests by recursing the tests/ directory for *.etest files (see .ebash etest.tests and test_list.sh), so any
+# fixture committed under tests/ would be run directly -- without the FLAKY_MARKER/FLAKY_COUNTER setup the nested
+# wrapper provides -- and would fail. Generating them inline keeps them invisible to discovery; they are only ever
+# sourced by the nested etest subprocess that is explicitly handed their path.
+#
+# Each heredoc uses a '|' margin marker that "sed 's/^ *| //'" strips when writing the fixture file. This is NOT
+# cosmetic: discovery greps each .etest file for "^(FLAKY_)?ETEST..()" (test_list.sh), and that grep cannot tell a
+# heredoc body apart from real code. Without the marker the embedded "FLAKY_ETEST_*()" lines would sit at column 0
+# in THIS file and be discovered as phantom tests of the flaky suite. The marker keeps them indented here while the
+# stripped output still has the column-0 function definitions the nested etest needs to discover.
+write_fixture_flaky_pass()
+{
+    # FLAKY_ test that fails on the first attempt and passes on the second. Uses FLAKY_MARKER (an absolute path
+    # outside testdir) to track which attempt we are on.
+    sed 's/^ *| //' > "$1" <<'EOF'
+        | #!/usr/bin/env bash
+        | FLAKY_ETEST_fails_then_passes()
+        | {
+        |     if [[ -f "${FLAKY_MARKER}" ]]; then
+        |         einfo "second attempt: passing"
+        |     else
+        |         touch "${FLAKY_MARKER}"
+        |         die "first attempt: failing on purpose"
+        |     fi
+        | }
+EOF
+}
+
+write_fixture_flaky_fail()
+{
+    # FLAKY_ test that fails on both attempts.
+    sed 's/^ *| //' > "$1" <<'EOF'
+        | #!/usr/bin/env bash
+        | FLAKY_ETEST_always_fails()
+        | {
+        |     die "this test always fails"
+        | }
+EOF
+}
+
+write_fixture_flaky_first_pass()
+{
+    # FLAKY_ test that passes on the first attempt (no retry needed). Uses FLAKY_COUNTER (an absolute path outside
+    # testdir) to record invocation count.
+    sed 's/^ *| //' > "$1" <<'EOF'
+        | #!/usr/bin/env bash
+        | FLAKY_ETEST_passes_first_time()
+        | {
+        |     # Append a line to the counter file so the outer test can verify we were only called once.
+        |     echo "invoked" >> "${FLAKY_COUNTER}"
+        |     einfo "passing on first attempt"
+        | }
+EOF
+}
+
+write_fixture_normal_fail()
+{
+    # A normal (non-flaky) ETEST_ that fails once. Regression guard: verifies non-flaky tests do NOT get a retry.
+    sed 's/^ *| //' > "$1" <<'EOF'
+        | #!/usr/bin/env bash
+        | ETEST_normal_fails_once()
+        | {
+        |     die "normal test failing -- should not be retried"
+        | }
+EOF
+}
+
+#-----------------------------------------------------------------------------------------------------------------------
+# Layer A: Integration tests via nested etest subprocess
+#-----------------------------------------------------------------------------------------------------------------------
+
+# A FLAKY_ test that fails on the first attempt and passes on the second should be reported PASSED.
+ETEST_flaky_fails_then_passes()
+{
+    local marker
+    marker=$(mktemp)
+    rm -f "${marker}"
+    FLAKY_MARKER="${marker}"
+
+    local fixture
+    fixture=$(mktemp --suffix=.etest)
+    write_fixture_flaky_pass "${fixture}"
+
+    etestmsg "Running nested etest with flaky_pass fixture"
+    local output rc=0
+    output=$(run_nested_etest --verbose "${fixture}") || rc=$?
+
+    etestmsg "Checking exit code is 0 (PASSED)"
+    assert_eq 0 "${rc}" "Expected exit code 0 but got ${rc}"
+
+    etestmsg "Checking output reports PASSED"
+    assert_match "${output}" "PASSED"
+
+    etestmsg "Checking output contains retry note"
+    assert_match "${output}" "passed on attempt 2"
+
+    rm -f "${marker}" "${fixture}"
+}
+
+# A FLAKY_ test that fails on both attempts should be reported FAILED.
+ETEST_flaky_always_fails()
+{
+    local fixture
+    fixture=$(mktemp --suffix=.etest)
+    write_fixture_flaky_fail "${fixture}"
+
+    etestmsg "Running nested etest with flaky_fail fixture"
+    local output rc=0
+    output=$(run_nested_etest --verbose "${fixture}") || rc=$?
+
+    etestmsg "Checking exit code is non-zero (FAILED)"
+    assert_ne 0 "${rc}" "Expected non-zero exit code but got 0"
+
+    etestmsg "Checking output reports FAILED"
+    assert_match "${output}" "FAILED"
+
+    etestmsg "Checking output contains retry warning"
+    assert_match "${output}" "retrying"
+
+    rm -f "${fixture}"
+}
+
+# A FLAKY_ test that passes on the first attempt should be reported PASSED with only one invocation.
+ETEST_flaky_passes_first_time()
+{
+    local counter
+    counter=$(mktemp)
+    : > "${counter}"
+    FLAKY_COUNTER="${counter}"
+
+    local fixture
+    fixture=$(mktemp --suffix=.etest)
+    write_fixture_flaky_first_pass "${fixture}"
+
+    etestmsg "Running nested etest with flaky_first_pass fixture"
+    local output rc=0
+    output=$(run_nested_etest --verbose "${fixture}") || rc=$?
+
+    etestmsg "Checking exit code is 0 (PASSED)"
+    assert_eq 0 "${rc}" "Expected exit code 0 but got ${rc}"
+
+    etestmsg "Checking output reports PASSED"
+    assert_match "${output}" "PASSED"
+
+    etestmsg "Checking test was only invoked once"
+    local invocations
+    invocations=$(wc -l < "${counter}")
+    assert_eq 1 "${invocations}" "Expected 1 invocation but got ${invocations}"
+
+    rm -f "${counter}" "${fixture}"
+}
+
+# A normal (non-flaky) ETEST_ that fails should NOT be retried -- it should fail immediately.
+# This is a regression guard ensuring the retry logic only applies to FLAKY_ tests.
+ETEST_normal_test_not_retried()
+{
+    local fixture
+    fixture=$(mktemp --suffix=.etest)
+    write_fixture_normal_fail "${fixture}"
+
+    etestmsg "Running nested etest with normal_fail fixture"
+    local output rc=0
+    output=$(run_nested_etest --verbose "${fixture}") || rc=$?
+
+    etestmsg "Checking exit code is non-zero (FAILED)"
+    assert_ne 0 "${rc}" "Expected non-zero exit code but got 0"
+
+    etestmsg "Checking output reports FAILED"
+    assert_match "${output}" "FAILED"
+
+    etestmsg "Checking output does NOT contain retry warning"
+    assert_not_match "${output}" "retrying"
+
+    rm -f "${fixture}"
+}
+
+#-----------------------------------------------------------------------------------------------------------------------
+# Layer B: In-process unit checks (no subprocess needed)
+#-----------------------------------------------------------------------------------------------------------------------
+
+# The flaky predicate pattern-matches correctly.
+# Use a helper function since [[ ... ]] is a keyword and can't be passed as args to assert_true.
+_matches_flaky()
+{
+    [[ "$1" == FLAKY_* ]]
+}
+
+ETEST_flaky_predicate()
+{
+    etestmsg "FLAKY_ETEST_foo should match FLAKY_*"
+    assert_true _matches_flaky "FLAKY_ETEST_foo"
+
+    etestmsg "ETEST_foo should NOT match FLAKY_*"
+    assert_false _matches_flaky "ETEST_foo"
+
+    etestmsg "DISABLED_ETEST_foo should NOT match FLAKY_*"
+    assert_false _matches_flaky "DISABLED_ETEST_foo"
+}
+
+#-----------------------------------------------------------------------------------------------------------------------
+# Layer C: Discovery visibility (FLAKY_ listed by default, DISABLED_ not)
+#-----------------------------------------------------------------------------------------------------------------------
+
+ETEST_flaky_discovered_by_default()
+{
+    local fixture
+    fixture=$(mktemp --suffix=.etest)
+    write_fixture_flaky_pass "${fixture}"
+
+    etestmsg "Running nested etest --print on fixture"
+    local output
+    output=$(run_nested_etest --print "${fixture}")
+
+    etestmsg "Checking FLAKY_ETEST_ is listed"
+    assert_match "${output}" "FLAKY_ETEST_fails_then_passes"
+
+    rm -f "${fixture}"
+}
+
+ETEST_disabled_not_discovered_by_default()
+{
+    # Create a temporary fixture with both DISABLED_ and FLAKY_ tests. The '|' margin marker (stripped by sed) keeps
+    # these embedded test definitions from being discovered as phantom tests of THIS file -- see the fixture writers.
+    local tmpfile
+    tmpfile=$(mktemp --suffix=.etest)
+    sed 's/^ *| //' > "${tmpfile}" <<'EOF'
+        | #!/usr/bin/env bash
+        | FLAKY_ETEST_visible()
+        | {
+        |     true
+        | }
+        | DISABLED_ETEST_hidden()
+        | {
+        |     true
+        | }
+EOF
+
+    etestmsg "Running nested etest --print (no --disabled)"
+    local output
+    output=$(run_nested_etest --print "${tmpfile}")
+
+    etestmsg "Checking FLAKY_ETEST_ is listed"
+    assert_match "${output}" "FLAKY_ETEST_visible"
+
+    etestmsg "Checking DISABLED_ETEST_ is NOT listed"
+    assert_not_match "${output}" "DISABLED_ETEST_hidden"
+
+    rm -f "${tmpfile}"
+}

Metadata

Metadata

Labels

No labels
No labels

Type

No type

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions