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 "