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