From da9a12a3dd6befee354bf172a14dee86b3623c8f Mon Sep 17 00:00:00 2001 From: Johnathan Corgan Date: Thu, 1 Oct 2026 03:41:04 +0000 Subject: [PATCH] Surface unit tests that passed only on retry The ci nextest profile retries a failing test twice, so a test that fails and then passes leaves the job green, with the retry visible only as "(1 flaky)" in the summary line and a FLAKY line in the log. Recent macOS runs have carried such flakes unnoticed. The Linux job's two junit reporters ignore nextest's elements, and the macOS and Windows jobs have no reporter, so no job surfaced them. testing/check-nextest-flaky.sh reads the ci profile's JUnit report and emits a warning annotation and a step-summary line for each test that passed only on retry. It exits 0 so a flake does not red the run, and 2 when there is no report to read. Each of the three unit-test jobs runs it after nextest, on success or failure but not on a cancelled run. Fixture tests over recorded nextest reports (clean, flaky, and a test that failed every attempt) run in local CI and in the GitHub CI parity job. The unit-test jobs cache the whole target directory, so a restored target/nextest/ci/junit.xml from an earlier run could sit where the check reads and hide a run that failed before writing one. Delete the report before nextest runs. --- .github/workflows/ci.yml | 55 ++++++++++++++++++ testing/check-nextest-flaky.sh | 95 +++++++++++++++++++++++++++++++ testing/ci-local.sh | 13 +++++ testing/nextest-flaky/clean.xml | 9 +++ testing/nextest-flaky/failed.xml | 72 +++++++++++++++++++++++ testing/nextest-flaky/flaky.xml | 53 +++++++++++++++++ testing/nextest-flaky/test.sh | 98 ++++++++++++++++++++++++++++++++ 7 files changed, 395 insertions(+) create mode 100644 testing/check-nextest-flaky.sh create mode 100644 testing/nextest-flaky/clean.xml create mode 100644 testing/nextest-flaky/failed.xml create mode 100644 testing/nextest-flaky/flaky.xml create mode 100644 testing/nextest-flaky/test.sh diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 1026b6c4..0bb5acf0 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -89,6 +89,10 @@ jobs: # step with ci-local.sh's run_deb_version by hand. - name: Check the Debian version derived for tags and branches run: bash testing/check-deb-version.sh + # The unit-test jobs' flaky-test reporter, against recorded nextest + # reports. Kept in step with ci-local.sh's run_nextest_flaky by hand. + - name: Check the flaky-test reporter against its fixtures + run: bash testing/nextest-flaky/test.sh fmt: name: Format check @@ -314,9 +318,26 @@ jobs: - name: Install cargo-nextest uses: taiki-e/install-action@nextest + # The cache restores target/, including the previous run's report. Remove + # it so the flaky-test check below reads only this run's, and reports a + # missing one when nextest wrote none. + - name: Remove a cached nextest report + shell: bash + run: rm -f target/nextest/ci/junit.xml + - name: Run unit tests run: cargo nextest run --all --profile ci + # The ci profile retries a failing test, so a test that passed only on + # retry leaves the job green. Annotate each one, and list it in the run + # summary, so a flake is seen without failing an unrelated run. The + # Linux job's two junit reporters ignore nextest's + # elements, and the macOS and Windows jobs have no reporter at all. + - name: Report tests that passed only on retry + if: success() || failure() + shell: bash + run: bash testing/check-nextest-flaky.sh target/nextest/ci/junit.xml + - name: Publish test report (Checks tab) uses: dorny/test-reporter@4a2e97665d5fa767581ef38eca97b9694bd4eef4 # v2 if: always() @@ -381,9 +402,26 @@ jobs: - name: Install cargo-nextest uses: taiki-e/install-action@nextest + # The cache restores target/, including the previous run's report. Remove + # it so the flaky-test check below reads only this run's, and reports a + # missing one when nextest wrote none. + - name: Remove a cached nextest report + shell: bash + run: rm -f target/nextest/ci/junit.xml + - name: Run unit tests run: cargo nextest run --all --profile ci + # The ci profile retries a failing test, so a test that passed only on + # retry leaves the job green. Annotate each one, and list it in the run + # summary, so a flake is seen without failing an unrelated run. The + # Linux job's two junit reporters ignore nextest's + # elements, and the macOS and Windows jobs have no reporter at all. + - name: Report tests that passed only on retry + if: success() || failure() + shell: bash + run: bash testing/check-nextest-flaky.sh target/nextest/ci/junit.xml + # ───────────────────────────────────────────────────────────────────────────── # Job 2c – Unit tests (Windows) # ───────────────────────────────────────────────────────────────────────────── @@ -413,9 +451,26 @@ jobs: - name: Install cargo-nextest uses: taiki-e/install-action@nextest + # The cache restores target/, including the previous run's report. Remove + # it so the flaky-test check below reads only this run's, and reports a + # missing one when nextest wrote none. + - name: Remove a cached nextest report + shell: bash + run: rm -f target/nextest/ci/junit.xml + - name: Run unit tests run: cargo nextest run --all --profile ci + # The ci profile retries a failing test, so a test that passed only on + # retry leaves the job green. Annotate each one, and list it in the run + # summary, so a flake is seen without failing an unrelated run. The + # Linux job's two junit reporters ignore nextest's + # elements, and the macOS and Windows jobs have no reporter at all. + - name: Report tests that passed only on retry + if: success() || failure() + shell: bash + run: bash testing/check-nextest-flaky.sh target/nextest/ci/junit.xml + # ───────────────────────────────────────────────────────────────────────────── # Job 2d – PowerShell lint (Windows packaging scripts) # diff --git a/testing/check-nextest-flaky.sh b/testing/check-nextest-flaky.sh new file mode 100644 index 00000000..5065a6a5 --- /dev/null +++ b/testing/check-nextest-flaky.sh @@ -0,0 +1,95 @@ +#!/bin/bash +# ── Surface tests that passed only on retry ───────────────────────────────── +# The ci nextest profile (.config/nextest.toml) retries a failing test twice, +# so a test that fails and then passes reports green. nextest says so only in +# its summary count ("N passed (1 flaky)") and a FLAKY line in the job log, +# where nobody looks on a green run; that is how a real race in a test went +# unnoticed until someone happened to read the output. The profile's own +# comment names the remedy: surface retried-but-passed tests, and keep the +# retries so a flake does not red an unrelated run. +# +# This reads the profile's JUnit report and, for every test case carrying a +# (an attempt that failed before the final one passed), emits a +# GitHub warning annotation and a line in the step summary. A test that failed +# every attempt carries instead and is the nextest step's red, not +# this script's concern. +# +# The report is parsed with awk rather than an XML library so the script runs +# unchanged on the Linux, macOS and Windows (Git Bash) runners. It relies on +# quick-junit's layout, one element per line, and on the two element names +# appearing only as elements: in text and attributes `<` is always escaped. +# +# Usage: check-nextest-flaky.sh [junit.xml] +# Default path: target/nextest/ci/junit.xml, the ci profile's report. +# Exit 0 = the report was read (flaky tests, if any, were annotated; they do +# not fail the step). Exit 2 = no report, or one with no test cases: nothing +# was checked, and that is never reported as a pass. +# ───────────────────────────────────────────────────────────────────────────── +set -uo pipefail + +REPORT="${1:-target/nextest/ci/junit.xml}" + +if [[ ! -s "$REPORT" ]]; then + echo "::error title=Flaky-test check::no JUnit report at $REPORT; flaky tests were not checked" + exit 2 +fi + +# One line per flaky test: "", +# then a final "cases" line so an empty or truncated report is +# told apart from a clean one. +if ! parsed="$(awk ' + function attr(line, key, m, v) { + if (match(line, " " key "=\"[^\"]*\"")) { + v = substr(line, RSTART + length(key) + 3, RLENGTH - length(key) - 4) + gsub(/</, "<", v); gsub(/>/, ">", v); gsub(/"/, "\"", v) + gsub(/'/, "\047", v); gsub(/&/, "\\&", v) + return v + } + return "" + } + function flush() { + if (fails > 0) printf "%d\t%s\t%s\n", fails, cls, name + fails = 0 + } + /]/ { flush(); cases++; name = attr($0, "name"); cls = attr($0, "classname") } + /]/ { fails++ } + END { flush(); printf "cases\t%d\n", cases } +' "$REPORT")"; then + echo "::error title=Flaky-test check::could not parse $REPORT; flaky tests were not checked" + exit 2 +fi + +cases="$(printf '%s\n' "$parsed" | awk -F'\t' '$1 == "cases" { print $2 }')" +if [[ -z "$cases" || "$cases" -eq 0 ]]; then + echo "::error title=Flaky-test check::$REPORT lists no test cases; flaky tests were not checked" + exit 2 +fi + +flaky=0 +summary="" +while IFS=$'\t' read -r fails cls name; do + [[ "$fails" == "cases" ]] && continue + flaky=$((flaky + 1)) + # Workflow-command values escape %, CR and LF; names carry none of the + # latter, but a % in a test name would otherwise be read as an escape. + msg="$cls $name failed $fails attempt(s) before passing on retry" + echo "::warning title=Flaky test::${msg//%/%25}" + summary+="- \`$cls $name\`: failed $fails attempt(s), then passed"$'\n' +done <<< "$parsed" + +if [[ "$flaky" -eq 0 ]]; then + echo "check-nextest-flaky: $cases test case(s), none passed only on retry" + exit 0 +fi + +echo "check-nextest-flaky: $flaky of $cases test case(s) passed only on retry" +if [[ -n "${GITHUB_STEP_SUMMARY:-}" ]]; then + { + echo "### Flaky tests ($flaky)" + echo "" + echo "These failed at least once and passed on a retry, so the run is green." + echo "" + printf '%s' "$summary" + } >> "$GITHUB_STEP_SUMMARY" +fi +exit 0 diff --git a/testing/ci-local.sh b/testing/ci-local.sh index 6479f362..c8361094 100755 --- a/testing/ci-local.sh +++ b/testing/ci-local.sh @@ -1604,6 +1604,18 @@ run_deb_version() { record "deb-version" $rc } +# The GitHub unit-test jobs run check-nextest-flaky.sh after nextest to +# surface tests that passed only on retry. Nothing local runs nextest under the +# retrying ci profile, so the checker itself never runs here; its fixture tests +# do, so a checker that stopped seeing flaky tests fails here rather than going +# quiet on GitHub. Static, about a second, and needs nothing built. +run_nextest_flaky() { + local rc=0 + info "[nextest-flaky] Checking the flaky-test reporter against its fixtures" + bash "$SCRIPT_DIR/nextest-flaky/test.sh" || rc=$? + record "nextest-flaky" $rc +} + # ── Main ─────────────────────────────────────────────────────────────────── main() { @@ -1626,6 +1638,7 @@ main() { run_comment_refs run_wait_converge run_deb_version + run_nextest_flaky if [[ "$TEST_ONLY" == true ]]; then run_tests diff --git a/testing/nextest-flaky/clean.xml b/testing/nextest-flaky/clean.xml new file mode 100644 index 00000000..aa9acb3b --- /dev/null +++ b/testing/nextest-flaky/clean.xml @@ -0,0 +1,9 @@ + + + + + + + + + diff --git a/testing/nextest-flaky/failed.xml b/testing/nextest-flaky/failed.xml new file mode 100644 index 00000000..b44c9108 --- /dev/null +++ b/testing/nextest-flaky/failed.xml @@ -0,0 +1,72 @@ + + + + + + + + thread 'tests::always_fails' (191023) panicked at src/lib.rs:22:67: +nope +note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace + thread 'tests::always_fails' (191028) panicked at src/lib.rs:22:67: +nope +note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace + +running 1 test +test tests::always_fails ... FAILED + +failures: + +failures: + tests::always_fails + +test result: FAILED. 0 passed; 1 failed; 0 ignored; 0 measured; 3 filtered out; finished in 0.00s + + + +thread 'tests::always_fails' (191028) panicked at src/lib.rs:22:67: +nope +note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace + + + thread 'tests::always_fails' (191032) panicked at src/lib.rs:22:67: +nope +note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace + +running 1 test +test tests::always_fails ... FAILED + +failures: + +failures: + tests::always_fails + +test result: FAILED. 0 passed; 1 failed; 0 ignored; 0 measured; 3 filtered out; finished in 0.01s + + + +thread 'tests::always_fails' (191032) panicked at src/lib.rs:22:67: +nope +note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace + + + +running 1 test +test tests::always_fails ... FAILED + +failures: + +failures: + tests::always_fails + +test result: FAILED. 0 passed; 1 failed; 0 ignored; 0 measured; 3 filtered out; finished in 0.00s + + + +thread 'tests::always_fails' (191023) panicked at src/lib.rs:22:67: +nope +note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace + + + + diff --git a/testing/nextest-flaky/flaky.xml b/testing/nextest-flaky/flaky.xml new file mode 100644 index 00000000..69d48e14 --- /dev/null +++ b/testing/nextest-flaky/flaky.xml @@ -0,0 +1,53 @@ + + + + + + + thread 'tests::flips_once' (174785) panicked at src/lib.rs:10:13: +first attempt fails +note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace + +running 1 test +test tests::flips_once ... FAILED + +failures: + +failures: + tests::flips_once + +test result: FAILED. 0 passed; 1 failed; 0 ignored; 0 measured; 3 filtered out; finished in 0.00s + + + +thread 'tests::flips_once' (174785) panicked at src/lib.rs:10:13: +first attempt fails +note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace + + + + + thread 'tests::also_flips' (174790) panicked at src/lib.rs:18:13: +first attempt fails <&> "quoted" +note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace + +running 1 test +test tests::also_flips ... FAILED + +failures: + +failures: + tests::also_flips + +test result: FAILED. 0 passed; 1 failed; 0 ignored; 0 measured; 3 filtered out; finished in 0.00s + + + +thread 'tests::also_flips' (174790) panicked at src/lib.rs:18:13: +first attempt fails <&> "quoted" +note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace + + + + + diff --git a/testing/nextest-flaky/test.sh b/testing/nextest-flaky/test.sh new file mode 100644 index 00000000..67535d39 --- /dev/null +++ b/testing/nextest-flaky/test.sh @@ -0,0 +1,98 @@ +#!/bin/bash +# ── Fixture tests for check-nextest-flaky.sh ──────────────────────────────── +# The checker only runs on GitHub, after the ci-profile nextest steps, and a +# flaky test there is rare: a checker that stopped seeing them would stay quiet +# for months and look exactly like a healthy one. These fixtures pin its +# behaviour instead. Each is a real JUnit report from cargo-nextest 0.9.146 +# under this repository's ci profile (retries = 2), run over a four-test crate: +# +# clean.xml every test passed on its first attempt +# flaky.xml two tests failed once and passed on retry (FLAKY 2/3) +# failed.xml one test failed all three attempts (, ) +# +# The flaky fixture must produce one warning per flaky test and a step-summary +# entry for each; the clean and failed fixtures must produce none, since a test +# that never passed is the nextest step's red, not a flake. A missing report +# must not read as clean. +# +# Exit 0 = every case behaved. Exit 1 = a case did not. +# ───────────────────────────────────────────────────────────────────────────── +set -uo pipefail + +SCRIPT_DIR="$(cd "$(dirname "$0")" && pwd)" +CHECK="$SCRIPT_DIR/../check-nextest-flaky.sh" +WORK="$(mktemp -d)" +trap 'rm -rf "$WORK"' EXIT + +FAILED=0 +ok() { echo " ok $*"; } +bad() { echo " FAIL $*"; FAILED=$((FAILED + 1)); } + +# expect : +# records ok when the command succeeds and a failure otherwise. +expect() { + local good="$1" poor="$2" + shift 2 + if "$@"; then ok "$good"; else bad "$poor"; fi + return 0 +} + +# run_case : runs the checker with a fresh step summary, leaving +# its output in $WORK/out, the summary in $WORK/summary and its status in RC. +run_case() { + : > "$WORK/summary" + GITHUB_STEP_SUMMARY="$WORK/summary" bash "$CHECK" "$1" > "$WORK/out" 2>&1 + RC=$? + WARNINGS="$(grep -c '^::warning title=Flaky test::' "$WORK/out")" + return 0 +} + +echo "check-nextest-flaky fixtures" + +run_case "$SCRIPT_DIR/flaky.xml" +expect "flaky: exit 0, so a flake does not red the run" "flaky: exit $RC, expected 0" \ + test "$RC" -eq 0 +expect "flaky: one warning per flaky test" "flaky: $WARNINGS warning(s), expected 2" \ + test "$WARNINGS" -eq 2 +for t in tests::flips_once tests::also_flips; do + if grep -q "^::warning title=Flaky test::flakydemo $t failed 1 attempt(s)" "$WORK/out" \ + && grep -q "flakydemo $t\`: failed 1 attempt(s)" "$WORK/summary"; then + ok "flaky: $t named in a warning and in the step summary" + else + bad "flaky: $t missing from the warnings or the step summary" + fi +done +if grep -q 'tests::steady\|tests::always_fails' "$WORK/out" "$WORK/summary"; then + bad "flaky: a test that passed first time was reported" +else + ok "flaky: tests that passed first time are not reported" +fi + +run_case "$SCRIPT_DIR/clean.xml" +expect "clean: exit 0, no warning, no summary" \ + "clean: exit $RC, $WARNINGS warning(s), summary $(wc -c < "$WORK/summary") bytes" \ + test "$RC" -eq 0 -a "$WARNINGS" -eq 0 -a ! -s "$WORK/summary" +expect "clean: all four cases were read" "clean: did not report reading four cases" \ + grep -q '4 test case(s), none passed only on retry' "$WORK/out" + +run_case "$SCRIPT_DIR/failed.xml" +expect "failed: a test that never passed is not reported as flaky" \ + "failed: exit $RC, $WARNINGS warning(s)" \ + test "$RC" -eq 0 -a "$WARNINGS" -eq 0 + +run_case "$WORK/no-such-report.xml" +expect "missing report: exit 2, not a clean pass" "missing report: exit $RC, expected 2" \ + test "$RC" -eq 2 -a "$WARNINGS" -eq 0 + +echo '' > "$WORK/empty.xml" +run_case "$WORK/empty.xml" +expect "report with no test cases: exit 2, not a clean pass" \ + "report with no test cases: exit $RC, expected 2" \ + test "$RC" -eq 2 + +if [[ "$FAILED" -ne 0 ]]; then + echo "check-nextest-flaky fixtures: $FAILED case(s) failed" + exit 1 +fi +echo "check-nextest-flaky fixtures: all cases passed" +exit 0