From 1aa76f7dc804f1c7979e71dc80e686a04b0afd48 Mon Sep 17 00:00:00 2001 From: Chemaclass Date: Mon, 10 Aug 2026 15:37:49 +0200 Subject: [PATCH] feat(runner): report a test that only passed on retry as flaky --retry hid flakiness instead of surfacing it. A test that failed and then passed was recorded as a plain pass; the only trace was a console suffix, absent from the counters, from JUnit, TAP, JSON and HTML, and from the GitHub Actions annotations. A CI job that retried a flaky test looked exactly like a clean run. Flaky is now a facet of passed, not a seventh outcome: the test is still counted in the pass total, so the exit code is unchanged unless --fail-on-flaky is given, and the flaky count is deliberately left out of the total to keep it from exceeding the tests actually run. The first attempt's failure message is the whole diagnostic value and was being overwritten by the next attempt, so it is captured before the retry loop discards it and carried into every report. The retry count lives in the runner shell, not in the test subshell that builds the payload, so it is appended to the payload after the loop. That is what lets it cross the fork: the parallel aggregator re-derives every status from the .result file and would otherwise see a plain pass. The field is read and defaulted on its own there, because a payload without it would leave the strip a no-op and hand the numeric guard arbitrary text, marking every test failed. Closes #1012 --- .env.example | 1 + CHANGELOG.md | 1 + completions/_bashunit | 1 + completions/bashunit.bash | 2 +- docs/command-line.md | 51 ++++++++++ src/config/env.sh | 9 +- src/console/header.sh | 1 + src/console/summary.sh | 14 +++ src/main/test.sh | 4 + src/reports/collect.sh | 22 ++++- src/reports/gha.sh | 4 + src/reports/html.sh | 1 + src/reports/json.sh | 18 +++- src/reports/junit.sh | 7 ++ src/reports/tap.sh | 5 + src/runner/exec.sh | 27 +++++ src/state/counters.sh | 13 +++ src/state/parallel.sh | 15 +++ tests/acceptance/bashunit_flaky_test.sh | 125 ++++++++++++++++++++++++ tests/unit/config/parallel_test.sh | 55 +++++++++++ tests/unit/reports/reports_test.sh | 43 ++++++++ tests/unit/state/state_test.sh | 12 +++ 22 files changed, 421 insertions(+), 10 deletions(-) create mode 100644 tests/acceptance/bashunit_flaky_test.sh diff --git a/.env.example b/.env.example index 211a077d..386d1e20 100644 --- a/.env.example +++ b/.env.example @@ -43,6 +43,7 @@ BASHUNIT_PARALLEL_RUN= # Default: false BASHUNIT_STOP_ON_FAILURE= # Default: false (stop suite on first failure) BASHUNIT_RERUN_FAILED= # Default: false (replay only last run's failing tests) BASHUNIT_ORDER_BY= # Default: defined (or defects, random) +BASHUNIT_FAIL_ON_FLAKY= # Default: false (treat retry-passed tests as failed) BASHUNIT_CHANGED= # Default: false (run only test files changed since a git ref) BASHUNIT_CHANGED_REF= # Default: empty (--changed ref: origin/HEAD, then HEAD) BASHUNIT_EXCLUDE_FILTER= # Default: empty (skip tests whose name matches) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7a9514d4..d3719366 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,7 @@ ## Unreleased ### Added +- Flaky is a first-class outcome: a test that only passed after a retry is counted separately, kept inside the pass total so the exit code is unchanged, and carried into JUnit (``), TAP, JSON, HTML and GitHub Actions along with the first attempt's failure message. `--fail-on-flaky` turns such a run red (#1012) - `--order-by ` picks the execution order: `defined` (default), `defects` (last run's failures first, whole suite still runs) or `random`. `--random-order` and `--seed` keep working unchanged (#1011) - `--changed []` runs only the test files git reports as touched since `` (default `origin/HEAD`, then `HEAD`), covering committed, staged, unstaged and untracked changes. Deletions are dropped, a rename selects its new path, and a missing work tree or unresolvable ref fails the run instead of selecting nothing (#1010) - `--list` (alias `--dry-run`) prints the tests a run would execute, without running them; `--list-format json` emits file, function, name, line and tags. Honours every selection flag, including `--shard` and `--random-order --seed` ordering (#1007) diff --git a/completions/_bashunit b/completions/_bashunit index 2a16d648..f1cdbd4c 100644 --- a/completions/_bashunit +++ b/completions/_bashunit @@ -85,6 +85,7 @@ _bashunit() { '--test-timeout[Fail a test running longer than N seconds]:seconds:' \ '--retry[Rerun a failed test up to N extra times]:count:' \ '--random-order[Randomize test execution order]' \ + '--fail-on-flaky[Treat tests that only passed after a retry as failures]' \ '--order-by[Execution order]:mode:(defined defects random)' \ '--seed[Seed for random order]:seed:' \ '--shard[Run shard i of n]:shard:' \ diff --git a/completions/bashunit.bash b/completions/bashunit.bash index af4c3f11..e6b3569f 100644 --- a/completions/bashunit.bash +++ b/completions/bashunit.bash @@ -16,7 +16,7 @@ _BASHUNIT_COMPLETIONS_DOC_OPTS="--custom -e --env --boot -h --help" _BASHUNIT_COMPLETIONS_TEST_OPTS="--assert --boot --changed --coverage --coverage-exclude \ --coverage-min --coverage-paths --coverage-report --coverage-report-html \ ---debug --detailed --dry-run --env --exclude-filter --exclude-tag --fail-on-risky --failures-only \ +--debug --detailed --dry-run --env --exclude-filter --exclude-tag --fail-on-flaky --fail-on-risky --failures-only \ --filter --help --jobs --list --list-format --log-gha --log-junit --login --no-color \ --no-coverage-report --no-output --no-output-on-failure --no-parallel \ --no-progress --no-snapshot-create --order-by --output --parallel --profile \ diff --git a/docs/command-line.md b/docs/command-line.md index b1a32e16..d35775c0 100644 --- a/docs/command-line.md +++ b/docs/command-line.md @@ -98,6 +98,7 @@ bashunit test tests/ --parallel --simple | `--no-output` | Suppress all output | | `--failures-only` | Only show failures | | `--fail-on-risky` | Treat risky tests (no assertions) as failures | +| `--fail-on-flaky` | Treat flaky tests (passed only after a retry) as failures | | `--profile` | Report the slowest tests after a run | | `--no-progress` | Suppress real-time progress, show only summary | | `--show-output` | Show test output on failure (default) | @@ -822,6 +823,56 @@ BASHUNIT_RERUN_FAILED=true bashunit test tests/ ``` ::: +### Flaky tests + +> `bashunit test --retry 2 --fail-on-flaky` + +A test that failed and then passed on a retry is **flaky**: it passed, so the +run stays green, but the summary says so. + +``` +Tests: 12 passed, 1 flaky, 12 total +``` + +The flaky count is a facet of `passed`, not a seventh outcome, which is why it +is not added to the total. Without it a retried failure is indistinguishable +from a clean run, and flakiness never gets triaged. + +Every report carries the status, along with the **first attempt's** failure +message (the diagnostic value, otherwise discarded when the retry overwrites +it) and the retry count: + +| Format | Output | +|--------|--------| +| JUnit | `` inside the ``, rendered natively by Jenkins and GitLab. Not counted in `failures` | +| TAP | `ok N - name # TODO flaky (retried 1/2)` | +| JSON | `"status": "flaky"`, `"retries": N`, plus a `flaky` key in the summary | +| HTML | its own row styling | +| GitHub Actions | a `::warning` annotation | + +Add `--fail-on-flaky` to turn a flaky run red, mirroring +[`--fail-on-risky`](#test-options): + +```bash +bashunit test tests/ --retry 2 --fail-on-flaky +``` + +Notes: + +- `--retry 0` (the default) can never produce a flaky result: nothing is retried. +- Counters are correct under `--parallel`; the retry count crosses the fork in + the per-test payload. +- Flaky never changes the exit code on its own. + +::: code-group +```bash [Surface flakiness in CI] +bashunit test --retry 2 --report-junit report.xml +``` +```bash [Env variable] +BASHUNIT_FAIL_ON_FLAKY=true bashunit test tests/ --retry 2 +``` +::: + ### Order by > `bashunit test --order-by ` diff --git a/src/config/env.sh b/src/config/env.sh index ea7c4ffa..a32614fb 100644 --- a/src/config/env.sh +++ b/src/config/env.sh @@ -255,6 +255,8 @@ _BASHUNIT_DEFAULT_SHARD_INDEX="" _BASHUNIT_DEFAULT_SHARD_TOTAL="" # Replay only the tests recorded as failing by the previous run _BASHUNIT_DEFAULT_RERUN_FAILED="false" +# Treat a test that only passed after a retry as a failure for the exit code +_BASHUNIT_DEFAULT_FAIL_ON_FLAKY="false" # Execution order: defined (definition order), defects (last run's failures # first) or random (equivalent to --random-order) _BASHUNIT_DEFAULT_ORDER_BY="defined" @@ -308,8 +310,9 @@ _BASHUNIT_DEFAULT_SNAPSHOT_REPORT_UNUSED="false" # up unrelated environment values. : "${BASHUNIT_RANDOM_ORDER:=$_BASHUNIT_DEFAULT_RANDOM_ORDER}" : "${BASHUNIT_SEED:=$_BASHUNIT_DEFAULT_SEED}" -# No bare ORDER_BY alias, same reasoning as RETRY/SEED above. +# No bare ORDER_BY/FAIL_ON_FLAKY aliases, same reasoning as RETRY/SEED above. : "${BASHUNIT_ORDER_BY:=$_BASHUNIT_DEFAULT_ORDER_BY}" +: "${BASHUNIT_FAIL_ON_FLAKY:=$_BASHUNIT_DEFAULT_FAIL_ON_FLAKY}" : "${BASHUNIT_SHARD_INDEX:=$_BASHUNIT_DEFAULT_SHARD_INDEX}" : "${BASHUNIT_SHARD_TOTAL:=$_BASHUNIT_DEFAULT_SHARD_TOTAL}" # No bare RERUN_FAILED alias, same reasoning as RETRY/SEED above. The default @@ -613,6 +616,10 @@ function bashunit::env::is_fail_on_risky_enabled() { [ "$BASHUNIT_FAIL_ON_RISKY" = "true" ] } +function bashunit::env::is_fail_on_flaky_enabled() { + [ "${BASHUNIT_FAIL_ON_FLAKY:-false}" = "true" ] +} + function bashunit::env::is_profile_enabled() { [ "$BASHUNIT_PROFILE" = "true" ] } diff --git a/src/console/header.sh b/src/console/header.sh index 78a118f2..b31660ba 100644 --- a/src/console/header.sh +++ b/src/console/header.sh @@ -154,6 +154,7 @@ Options: --no-output Suppress all output --failures-only Only show failures (suppress passed/skipped/incomplete) --fail-on-risky Treat risky tests (no assertions) as failures + --fail-on-flaky Treat flaky tests (passed only after a retry) as failures --profile Report the slowest tests (count: BASHUNIT_PROFILE_COUNT, default 10) --no-progress Suppress real-time progress, show only final results --show-output Show test output on failure (default: enabled) diff --git a/src/console/summary.sh b/src/console/summary.sh index 824ef7b4..9c848ee2 100644 --- a/src/console/summary.sh +++ b/src/console/summary.sh @@ -30,6 +30,7 @@ function bashunit::console_results::render_result() { local tests_snapshot=$_BASHUNIT_TESTS_SNAPSHOT local tests_failed=$_BASHUNIT_TESTS_FAILED local tests_risky=$_BASHUNIT_TESTS_RISKY + local tests_flaky=$_BASHUNIT_TESTS_FLAKY local assertions_passed=$_BASHUNIT_ASSERTIONS_PASSED local assertions_skipped=$_BASHUNIT_ASSERTIONS_SKIPPED local assertions_incomplete=$_BASHUNIT_ASSERTIONS_INCOMPLETE @@ -70,6 +71,11 @@ function bashunit::console_results::render_result() { if [ "$tests_risky" -gt 0 ]; then printf " %s%s risky%s," "$_BASHUNIT_COLOR_RISKY" "$tests_risky" "$_BASHUNIT_COLOR_DEFAULT" fi + # Deliberately absent from total_tests: these tests are already inside the + # passed count, so adding them would make the total exceed the tests run. + if [ "$tests_flaky" -gt 0 ]; then + printf " %s%s flaky%s," "$_BASHUNIT_COLOR_INCOMPLETE" "$tests_flaky" "$_BASHUNIT_COLOR_DEFAULT" + fi printf " %s total\n" "$total_tests" printf "%sAssertions:%s" "$_BASHUNIT_COLOR_FAINT" "$_BASHUNIT_COLOR_DEFAULT" @@ -96,6 +102,14 @@ function bashunit::console_results::render_result() { return 1 fi + # Ranked above risky so a run that is both reports the outcome that turns it + # red. Without the flag flaky is a pass, so the ladder falls straight through. + if [ "$tests_flaky" -gt 0 ] && bashunit::env::is_fail_on_flaky_enabled; then + printf "\n%s%s%s\n" "$_BASHUNIT_COLOR_RETURN_ERROR" " Some tests flaky " "$_BASHUNIT_COLOR_DEFAULT" + bashunit::console_results::print_execution_time + return 1 + fi + if [ "$tests_risky" -gt 0 ]; then printf "\n%s%s%s\n" "$_BASHUNIT_COLOR_RETURN_RISKY" " Some tests risky (no assertions) " "$_BASHUNIT_COLOR_DEFAULT" bashunit::console_results::print_execution_time diff --git a/src/main/test.sh b/src/main/test.sh index 3fd88ab3..2f758fb4 100644 --- a/src/main/test.sh +++ b/src/main/test.sh @@ -278,6 +278,10 @@ function bashunit::main::cmd_test() { BASHUNIT_FAIL_ON_RISKY=true export -n BASHUNIT_FAIL_ON_RISKY ;; + --fail-on-flaky) + BASHUNIT_FAIL_ON_FLAKY=true + export -n BASHUNIT_FAIL_ON_FLAKY + ;; --profile) BASHUNIT_PROFILE=true export -n BASHUNIT_PROFILE diff --git a/src/reports/collect.sh b/src/reports/collect.sh index e636bf41..fea413f1 100644 --- a/src/reports/collect.sh +++ b/src/reports/collect.sh @@ -17,6 +17,7 @@ _BASHUNIT_REPORTS_TEST_DURATIONS=() _BASHUNIT_REPORTS_TEST_ASSERTIONS=() _BASHUNIT_REPORTS_TEST_FAILURES=() _BASHUNIT_REPORTS_TEST_LINES=() +_BASHUNIT_REPORTS_TEST_RETRIES=() function bashunit::reports::add_test_snapshot() { bashunit::reports::add_test "$1" "$2" "$3" "$4" "snapshot" @@ -42,6 +43,17 @@ function bashunit::reports::add_test_failed() { bashunit::reports::add_test "$1" "$2" "$3" "$4" "failed" "$5" } +## +# A test that passed, but not on the first attempt. Carries the retry count and +# the first attempt's failure message, which is the whole diagnostic value and +# is otherwise discarded when the retry loop overwrites the losing attempt. +# Arguments: $1 file, $2 name, $3 duration, $4 assertions, $5 first failure, +# $6 retries. +## +function bashunit::reports::add_test_flaky() { + bashunit::reports::add_test "$1" "$2" "$3" "$4" "flaky" "$5" "$6" +} + # Returns 0 when any report output is requested. function bashunit::reports::is_enabled() { [ -n "${BASHUNIT_LOG_JUNIT:-}" ] || @@ -61,6 +73,7 @@ function bashunit::reports::add_test() { local assertions="$4" local status="$5" local failure_message="${6:-}" + local retries="${7:-0}" # Capture the line number from the current test location ("file:line"), # but only when it belongs to this test's file, so a stale location from a @@ -86,7 +99,7 @@ function bashunit::reports::add_test() { # Fields are base64-encoded because a failure message carries newlines and # arbitrary text, either of which would break a delimited line. if bashunit::parallel::is_enabled; then - printf '%s|%s|%s|%s|%s|%s|%s\n' \ + printf '%s|%s|%s|%s|%s|%s|%s|%s\n' \ "$(bashunit::helper::encode_base64 "$file")" \ "$(bashunit::helper::encode_base64 "$test_name")" \ "$(bashunit::helper::encode_base64 "$status")" \ @@ -94,6 +107,7 @@ function bashunit::reports::add_test() { "$(bashunit::helper::encode_base64 "$assertions")" \ "$(bashunit::helper::encode_base64 "$failure_message")" \ "$(bashunit::helper::encode_base64 "$line")" \ + "$(bashunit::helper::encode_base64 "$retries")" \ >>"${REPORTS_OUTPUT_PATH:-/dev/null}" 2>/dev/null || true fi @@ -104,6 +118,7 @@ function bashunit::reports::add_test() { _BASHUNIT_REPORTS_TEST_DURATIONS[${#_BASHUNIT_REPORTS_TEST_DURATIONS[@]}]="$duration" _BASHUNIT_REPORTS_TEST_FAILURES[${#_BASHUNIT_REPORTS_TEST_FAILURES[@]}]="$failure_message" _BASHUNIT_REPORTS_TEST_LINES[${#_BASHUNIT_REPORTS_TEST_LINES[@]}]="$line" + _BASHUNIT_REPORTS_TEST_RETRIES[${#_BASHUNIT_REPORTS_TEST_RETRIES[@]}]="$retries" } ## @@ -115,8 +130,8 @@ function bashunit::reports::load_spooled() { bashunit::reports::is_enabled || return 0 [ -f "${REPORTS_OUTPUT_PATH:-}" ] || return 0 - local file test_name status duration assertions failure_message line n - while IFS='|' read -r file test_name status duration assertions failure_message line; do + local file test_name status duration assertions failure_message line retries n + while IFS='|' read -r file test_name status duration assertions failure_message line retries; do [ -n "$file" ] || continue local n=${#_BASHUNIT_REPORTS_TEST_FILES[@]} _BASHUNIT_REPORTS_TEST_FILES[n]=$(bashunit::helper::decode_base64 "$file") @@ -126,5 +141,6 @@ function bashunit::reports::load_spooled() { _BASHUNIT_REPORTS_TEST_ASSERTIONS[n]=$(bashunit::helper::decode_base64 "$assertions") _BASHUNIT_REPORTS_TEST_FAILURES[n]=$(bashunit::helper::decode_base64 "$failure_message") _BASHUNIT_REPORTS_TEST_LINES[n]=$(bashunit::helper::decode_base64 "$line") + _BASHUNIT_REPORTS_TEST_RETRIES[n]=$(bashunit::helper::decode_base64 "$retries") done <"$REPORTS_OUTPUT_PATH" } diff --git a/src/reports/gha.sh b/src/reports/gha.sh index 3c98e844..7815adfd 100644 --- a/src/reports/gha.sh +++ b/src/reports/gha.sh @@ -37,6 +37,10 @@ function bashunit::reports::print_gha_annotations() { level="warning" message="Test has no assertions (risky)" ;; + flaky) + level="warning" + message="Test passed only after ${_BASHUNIT_REPORTS_TEST_RETRIES[$i]:-0} retries: $failure_message" + ;; incomplete) level="notice" message="Test incomplete" diff --git a/src/reports/html.sh b/src/reports/html.sh index 3caaa894..cf771374 100644 --- a/src/reports/html.sh +++ b/src/reports/html.sh @@ -48,6 +48,7 @@ function bashunit::reports::generate_report_html() { echo " .incomplete { background-color: #d9edf7; }" echo " .snapshot { background-color: #dfe6e9; }" echo " .risky { background-color: #f5e6f5; }" + echo " .flaky { background-color: #ffe8cc; }" echo " " echo "" echo "" diff --git a/src/reports/json.sh b/src/reports/json.sh index d35fe851..d475b976 100644 --- a/src/reports/json.sh +++ b/src/reports/json.sh @@ -20,7 +20,7 @@ function bashunit::reports::generate_report_json() { local output_file="$1" local total="${#_BASHUNIT_REPORTS_TEST_NAMES[@]}" - local passed=0 failed=0 skipped=0 incomplete=0 duration_total=0 + local passed=0 failed=0 skipped=0 incomplete=0 flaky=0 duration_total=0 local i for i in "${!_BASHUNIT_REPORTS_TEST_NAMES[@]}"; do duration_total=$((duration_total + ${_BASHUNIT_REPORTS_TEST_DURATIONS[$i]:-0})) @@ -28,6 +28,12 @@ function bashunit::reports::generate_report_json() { failed) failed=$((failed + 1)) ;; skipped) skipped=$((skipped + 1)) ;; incomplete) incomplete=$((incomplete + 1)) ;; + # Flaky is counted twice on purpose: it passed, so it belongs in passed, and + # the separate tally is what makes it triageable. + flaky) + flaky=$((flaky + 1)) + passed=$((passed + 1)) + ;; # snapshot and risky ran without failing, so they count as passed here; the # per-test "status" field below preserves the exact category. *) passed=$((passed + 1)) ;; @@ -38,8 +44,8 @@ function bashunit::reports::generate_report_json() { printf '{\n' printf ' "summary": { "total": %d, "passed": %d, "failed": %d,' \ "$total" "$passed" "$failed" - printf ' "skipped": %d, "incomplete": %d, "duration_ms": %d },\n' \ - "$skipped" "$incomplete" "$duration_total" + printf ' "skipped": %d, "incomplete": %d, "flaky": %d, "duration_ms": %d },\n' \ + "$skipped" "$incomplete" "$flaky" "$duration_total" printf ' "tests": [\n' local seq=0 for i in "${!_BASHUNIT_REPORTS_TEST_NAMES[@]}"; do @@ -51,8 +57,10 @@ function bashunit::reports::generate_report_json() { message=$(bashunit::reports::__json_escape "${_BASHUNIT_REPORTS_TEST_FAILURES[$i]:-}") sep="," [ "$seq" -eq "$((total - 1))" ] && sep="" - printf ' { "file": "%s", "name": "%s", "status": "%s", "duration_ms": %d, "message": "%s" }%s\n' \ - "$file" "$name" "$status" "$duration" "$message" "$sep" + printf ' { "file": "%s", "name": "%s", "status": "%s", "duration_ms": %d,' \ + "$file" "$name" "$status" "$duration" + printf ' "retries": %d, "message": "%s" }%s\n' \ + "${_BASHUNIT_REPORTS_TEST_RETRIES[$i]:-0}" "$message" "$sep" seq=$((seq + 1)) done printf ' ]\n' diff --git a/src/reports/junit.sh b/src/reports/junit.sh index c07d1807..06dab80e 100644 --- a/src/reports/junit.sh +++ b/src/reports/junit.sh @@ -51,6 +51,13 @@ function bashunit::reports::generate_junit_xml() { local escaped_message escaped_message=$(bashunit::reports::__xml_escape "$failure_message") echo " $escaped_message" + elif [ "$status" = "flaky" ]; then + # Jenkins and GitLab render flakyFailure natively, and it does not count + # towards failures="" -- which is the point: the test passed. + local escaped_flaky + escaped_flaky=$(bashunit::reports::__xml_escape "$failure_message") + echo " $escaped_flaky" elif [ "$status" = "risky" ]; then echo " " elif [ "$status" = "skipped" ]; then diff --git a/src/reports/tap.sh b/src/reports/tap.sh index 051bfbaf..da817e65 100644 --- a/src/reports/tap.sh +++ b/src/reports/tap.sh @@ -50,6 +50,11 @@ function bashunit::reports::generate_report_tap() { incomplete) echo "ok $seq - $name # TODO" ;; + flaky) + # `ok` because it passed; the TODO directive is how TAP consumers mark a + # result that needs attention without failing the run. + echo "ok $seq - $name # TODO flaky (retried ${_BASHUNIT_REPORTS_TEST_RETRIES[$i]:-0}/${BASHUNIT_RETRY:-0})" + ;; *) echo "ok $seq - $name" ;; diff --git a/src/runner/exec.sh b/src/runner/exec.sh index 365965b7..aa48b6dc 100644 --- a/src/runner/exec.sh +++ b/src/runner/exec.sh @@ -337,6 +337,10 @@ function bashunit::runner::run_test() { bashunit::env::resolve_retry_count local retry_max=$_BASHUNIT_RETRY_VALIDATED local retries_used=0 + # The losing attempts are overwritten by the next iteration, so the first + # failure -- the only evidence of what the flakiness looks like -- is kept here + # before it is lost. + local first_attempt_result="" local measure_duration=false bashunit::runner::needs_test_duration && measure_duration=true # Retry wraps ONLY execution: a failed attempt is judged from its encoded @@ -372,10 +376,20 @@ function bashunit::runner::run_test() { [ "$_BASHUNIT_RUNNER_COUNTS_FAILED_OUT" -eq 0 ]; then break fi + # Only reached when the attempt failed, so this is the first failure. + if [ -z "$first_attempt_result" ]; then + first_attempt_result="$test_execution_result" + fi [ "$retries_used" -ge "$retry_max" ] && break retries_used=$((retries_used + 1)) done + # The retry count lives in this shell, not in the test subshell that built the + # payload, so it is appended here. Every decoder matches its key greedily and + # stops at the next `##`, which makes a trailing field additive-safe; without + # it the count never crosses the fork and --parallel could not see flakiness. + test_execution_result="$test_execution_result##TEST_RETRIES=$retries_used##" + # Closes FD 3, which was used temporarily to hold the original stdout. exec 3>&- @@ -569,6 +583,19 @@ function bashunit::runner::run_test() { fi _BASHUNIT_RETRY_NOTE="" bashunit::state::add_tests_passed + # Flaky is a facet of passed, never a replacement for it: the test did pass, so + # the exit code only changes under --fail-on-flaky. + if [ "$retries_used" -gt 0 ]; then + bashunit::state::add_tests_flaky + bashunit::runner::decode_subshell_output "$first_attempt_result" + local first_failure=$_BASHUNIT_RUNNER_SUBSHELL_OUTPUT_OUT + bashunit::runner::format_subshell_output "$first_failure" + first_failure=$_BASHUNIT_RUNNER_OUTPUT_OUT + bashunit::reports::add_test_flaky \ + "$test_file" "$label" "$duration" "$total_assertions" "$first_failure" "$retries_used" + bashunit::internal_log "Test flaky" "$label" "retries:$retries_used" + return + fi bashunit::reports::add_test_passed "$test_file" "$label" "$duration" "$total_assertions" bashunit::internal_log "Test passed" "$label" } diff --git a/src/state/counters.sh b/src/state/counters.sh index 8953d069..d68008ac 100644 --- a/src/state/counters.sh +++ b/src/state/counters.sh @@ -8,6 +8,9 @@ _BASHUNIT_TESTS_SKIPPED=0 _BASHUNIT_TESTS_INCOMPLETE=0 _BASHUNIT_TESTS_SNAPSHOT=0 _BASHUNIT_TESTS_RISKY=0 +# Flaky is a facet of passed, not a seventh outcome: it counts tests already +# tallied in _BASHUNIT_TESTS_PASSED, so it must never be added to the total. +_BASHUNIT_TESTS_FLAKY=0 _BASHUNIT_ASSERTIONS_PASSED=0 _BASHUNIT_ASSERTIONS_FAILED=0 _BASHUNIT_ASSERTIONS_SKIPPED=0 @@ -74,6 +77,16 @@ function bashunit::state::add_tests_risky() { } +function bashunit::state::get_tests_flaky() { + echo "$_BASHUNIT_TESTS_FLAKY" +} + + +function bashunit::state::add_tests_flaky() { + ((_BASHUNIT_TESTS_FLAKY++)) || true +} + + function bashunit::state::get_assertions_passed() { echo "$_BASHUNIT_ASSERTIONS_PASSED" } diff --git a/src/state/parallel.sh b/src/state/parallel.sh index 59607d9f..fc9dd9e0 100644 --- a/src/state/parallel.sh +++ b/src/state/parallel.sh @@ -68,6 +68,18 @@ function bashunit::state::aggregate_parallel_results() { exit_code="${exit_code%%##*}" exit_code=${exit_code:-0} + # Read separately from the block above, and defaulted on its own, because a + # payload without the field would leave the strip a no-op and hand the + # guard below arbitrary text -- which would mark every test failed. + local retries=0 + case "$result_line" in + *"##TEST_RETRIES="*) + retries="${result_line##*##TEST_RETRIES=}" + retries="${retries%%##*}" + case "$retries" in '' | *[!0-9]*) retries=0 ;; esac + ;; + esac + # A truncated or non-payload .result line leaves every ##KEY= strip a # no-op, so these fields hold arbitrary text. `$(( ))` on such text is a # fatal arithmetic syntax error and `[ -gt ]` reports "integer expression @@ -125,6 +137,9 @@ function bashunit::state::aggregate_parallel_results() { continue fi + if [ "$retries" -gt 0 ]; then + bashunit::state::add_tests_flaky + fi bashunit::state::add_tests_passed done done diff --git a/tests/acceptance/bashunit_flaky_test.sh b/tests/acceptance/bashunit_flaky_test.sh new file mode 100644 index 00000000..f7e02e65 --- /dev/null +++ b/tests/acceptance/bashunit_flaky_test.sh @@ -0,0 +1,125 @@ +#!/usr/bin/env bash + +# A test that only passed after a retry is a distinct outcome: it passed, so the +# exit code stays 0, but the run has to say so or CI cannot triage flakiness. +# Reuses the deterministic retry fixture (a counter file survives the attempts). + +function set_up_before_script() { + TEST_ENV_FILE="tests/acceptance/fixtures/.env.default" + FIXTURE="tests/acceptance/fixtures/test_bashunit_retry.sh" +} + +function set_up() { + COUNTER_FILE="$(mktemp)" + export BASHUNIT_RETRY_FIXTURE_COUNTER="$COUNTER_FILE" + export BASHUNIT_RETRY_FIXTURE_PASS_ON=2 + printf '0' >"$COUNTER_FILE" + REPORT_DIR="$(bashunit::temp_dir flaky_reports)" +} + +function tear_down() { + rm -f "$COUNTER_FILE" + unset BASHUNIT_RETRY_FIXTURE_COUNTER BASHUNIT_RETRY_FIXTURE_PASS_ON +} + +function test_a_test_that_only_passed_on_retry_is_counted_as_flaky() { + local output + output="$(./bashunit --no-parallel --no-color --env "$TEST_ENV_FILE" \ + --retry 1 --filter test_a_flaky "$FIXTURE")" + + assert_contains "1 passed, 1 flaky, 1 total" "$(printf '%s' "$output" | tr -s ' ')" +} + +function test_a_flaky_test_still_exits_zero_by_default() { + local ec=0 + ./bashunit --no-parallel --no-color --env "$TEST_ENV_FILE" \ + --retry 1 --filter test_a_flaky "$FIXTURE" >/dev/null 2>&1 || ec=$? + + assert_same "0" "$ec" +} + +function test_fail_on_flaky_turns_a_flaky_run_red() { + local ec=0 + ./bashunit --no-parallel --no-color --env "$TEST_ENV_FILE" \ + --retry 1 --fail-on-flaky --filter test_a_flaky "$FIXTURE" >/dev/null 2>&1 || ec=$? + + assert_general_error "" "" "$ec" +} + +function test_a_clean_run_reports_no_flaky_count() { + local output + output="$(./bashunit --no-parallel --no-color --env "$TEST_ENV_FILE" \ + --retry 1 --filter test_b_always "$FIXTURE")" + + assert_contains "1 passed, 1 total" "$(printf '%s' "$output" | tr -s ' ')" + assert_not_contains "flaky" "$output" +} + +function test_without_retries_a_failing_test_is_never_flaky() { + local output + output="$(./bashunit --no-parallel --no-color --env "$TEST_ENV_FILE" \ + --retry 0 --filter test_a_flaky "$FIXTURE")" || true + + assert_contains "1 failed" "$output" + # Not a bare "flaky": the fixture's own function name contains the word. + assert_not_contains "1 flaky" "$output" +} + +function test_flaky_counters_survive_parallel_aggregation() { + local output + output="$(./bashunit --parallel --no-color --env "$TEST_ENV_FILE" \ + --retry 1 --filter test_a_flaky "$FIXTURE")" + + assert_contains "1 passed, 1 flaky, 1 total" "$(printf '%s' "$output" | tr -s ' ')" +} + +function test_junit_reports_a_flaky_failure_carrying_the_first_attempt() { + ./bashunit --no-parallel --no-color --env "$TEST_ENV_FILE" \ + --retry 1 --filter test_a_flaky --report-junit "$REPORT_DIR/report.xml" "$FIXTURE" >/dev/null 2>&1 + + local report + report="$(cat "$REPORT_DIR/report.xml")" + + assert_contains "/dev/null 2>&1 + + local report + report="$(cat "$REPORT_DIR/report.tap")" + + assert_contains "ok 1 - " "$report" + assert_contains "# TODO flaky (retried 1/1)" "$report" +} + +function test_json_carries_the_flaky_status_the_retries_and_the_first_failure() { + ./bashunit --no-parallel --no-color --env "$TEST_ENV_FILE" \ + --retry 1 --filter test_a_flaky --report-json "$REPORT_DIR/report.json" "$FIXTURE" >/dev/null 2>&1 + + local report + report="$(cat "$REPORT_DIR/report.json")" + + assert_contains '"status": "flaky"' "$report" + assert_contains '"retries": 1' "$report" + assert_contains "failed-on-attempt-1" "$report" + assert_contains '"flaky": 1' "$report" +} + +function test_html_marks_the_flaky_row() { + ./bashunit --no-parallel --no-color --env "$TEST_ENV_FILE" \ + --retry 1 --filter test_a_flaky --report-html "$REPORT_DIR/report.html" "$FIXTURE" >/dev/null 2>&1 + + assert_contains "flaky" "$(cat "$REPORT_DIR/report.html")" +} + +function test_github_actions_annotates_a_flaky_test_as_a_warning() { + ./bashunit --no-parallel --no-color --env "$TEST_ENV_FILE" \ + --retry 1 --filter test_a_flaky --log-gha "$REPORT_DIR/gha.log" "$FIXTURE" >/dev/null 2>&1 + + assert_contains "::warning" "$(cat "$REPORT_DIR/gha.log")" +} diff --git a/tests/unit/config/parallel_test.sh b/tests/unit/config/parallel_test.sh index d46505a4..b3257b49 100644 --- a/tests/unit/config/parallel_test.sh +++ b/tests/unit/config/parallel_test.sh @@ -370,3 +370,58 @@ EOF assert_same "8" "$passed" assert_same "3" "$failed" } + +# A retried-then-passed worker carries its retry count in the payload, because +# the parent re-derives every status from the .result file and would otherwise +# record a plain pass. +function test_aggregate_counts_a_retried_pass_as_flaky() { + _create_result_file "$TEMP_DIR_PARALLEL_TEST_SUITE/script1" "test1.result" \ + "##ASSERTIONS_PASSED=1##ASSERTIONS_FAILED=0##TEST_EXIT_CODE=0##TEST_RETRIES=2##" + + local before + before=$(bashunit::state::get_tests_flaky) + + local flaky + flaky=$( + bashunit::state::aggregate_parallel_results "$TEMP_DIR_PARALLEL_TEST_SUITE" >/dev/null + bashunit::state::get_tests_flaky + ) + + assert_same "1" "$((flaky - before))" +} + +function test_aggregate_does_not_count_a_first_try_pass_as_flaky() { + _create_result_file "$TEMP_DIR_PARALLEL_TEST_SUITE/script1" "test1.result" \ + "##ASSERTIONS_PASSED=1##ASSERTIONS_FAILED=0##TEST_EXIT_CODE=0##TEST_RETRIES=0##" + + local before + before=$(bashunit::state::get_tests_flaky) + + local flaky + flaky=$( + bashunit::state::aggregate_parallel_results "$TEMP_DIR_PARALLEL_TEST_SUITE" >/dev/null + bashunit::state::get_tests_flaky + ) + + assert_same "0" "$((flaky - before))" +} + +# A payload predating the field leaves the ##TEST_RETRIES= strip a no-op, which +# would hand the numeric guard arbitrary text and mark every test failed. +function test_aggregate_treats_a_payload_without_the_retry_field_as_a_plain_pass() { + _create_result_file "$TEMP_DIR_PARALLEL_TEST_SUITE/script1" "test1.result" \ + "##ASSERTIONS_PASSED=1##ASSERTIONS_FAILED=0##TEST_EXIT_CODE=0##" + + local before_flaky before_failed + before_flaky=$(bashunit::state::get_tests_flaky) + before_failed=$(bashunit::state::get_tests_failed) + + local counts + counts=$( + bashunit::state::aggregate_parallel_results "$TEMP_DIR_PARALLEL_TEST_SUITE" >/dev/null + printf '%s %s' "$(bashunit::state::get_tests_flaky)" "$(bashunit::state::get_tests_failed)" + ) + + assert_same "0" "$((${counts%% *} - before_flaky))" + assert_same "0" "$((${counts##* } - before_failed))" +} diff --git a/tests/unit/reports/reports_test.sh b/tests/unit/reports/reports_test.sh index d42667f9..1d4a5f22 100644 --- a/tests/unit/reports/reports_test.sh +++ b/tests/unit/reports/reports_test.sh @@ -15,6 +15,7 @@ function set_up() { _BASHUNIT_REPORTS_TEST_ASSERTIONS=() _BASHUNIT_REPORTS_TEST_FAILURES=() _BASHUNIT_REPORTS_TEST_LINES=() + _BASHUNIT_REPORTS_TEST_RETRIES=() _BASHUNIT_TEST_LOCATION="" # Unset report env vars by default @@ -590,3 +591,45 @@ function test_generate_report_html_applies_status_css_classes() { assert_contains '' "$content" assert_contains '' "$content" } + +function test_generate_junit_xml_flaky_testcase() { + _mock_state_functions + BASHUNIT_LOG_JUNIT="report.xml" + + bashunit::reports::add_test "test.sh" "test_flaky" "10" "1" "flaky" "expected 1 got 2" "2" + bashunit::reports::generate_junit_xml "$_TEMP_OUTPUT_FILE" + + local content + content=$(cat "$_TEMP_OUTPUT_FILE") + + assert_contains "