From e9dfffe27b39c735613e1d609470b5dead9dbdb6 Mon Sep 17 00:00:00 2001 From: jonaswre Date: Sat, 1 Aug 2026 19:00:22 +0000 Subject: [PATCH 1/2] fix(ci): make the daily flaky sweep able to detect a flake flaky.yaml runs tests.yaml with `--run-ignored all`, which runs every #[ignore]d test. nextest cannot read the ignore REASON, so the sweep also ran tests ignored for reasons that have nothing to do with flakiness. Those fail on every run, so the sweep was red by construction -- on 2026-07-29, -30 and -31, and it would have been red every night after. An alarm that is always on reports nothing. Detecting a NEW flake is the only thing this sweep exists to do, and it could not do it. The workspace has 25 #[ignore]d tests in four distinct categories: "flaky" 5 the sweep's actual subject "not yet passing ..." 11 patchbay; known-broken "manual mainline ..." 1 dials the public BitTorrent mainline DHT "todo" 1 unimplemented Verified by `cargo nextest list --run-ignored ignored-only` that the sweep's real scope was 7 tests: the 5 flaky ones plus integration_mainline and downloader_get_many_smoke. The 11 patchbay tests were already out via `default-filter = 'not binary(patchbay)'`. Exclude the 2 non-flaky ignores with a filterset. Confirmed empirically that -E composes with default-filter rather than replacing it, so patchbay stays excluded: with the filter applied, ignored-only lists exactly the 5 flaky tests and 0 patchbay tests remain visible. Add scripts/tests/check-flaky-sweep-scope.sh, wired into ci.yml, because this list has to track the tree in BOTH directions and neither direction is self-announcing: - a new non-flakiness #[ignore] that nobody excludes puts the sweep back to permanently red; - a test ignored "flaky" that ends up excluded silently stops being watched, which is worse than a red sweep, because it looks fine. The check classifies by ignore reason and asserts both. Verified against three injected regressions: an unexcluded "needs hardware" ignore, a flaky test added to the exclusion list, and removal of the patchbay default-filter the excluded patchbay tests depend on. Co-Authored-By: Claude Opus 5 --- .github/workflows/ci.yml | 2 + .github/workflows/tests.yaml | 22 ++++- scripts/tests/check-flaky-sweep-scope.sh | 101 +++++++++++++++++++++++ 3 files changed, 124 insertions(+), 1 deletion(-) create mode 100755 scripts/tests/check-flaky-sweep-scope.sh diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index c5be425c8a1..44bf0b4de5d 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -134,6 +134,8 @@ jobs: run: scripts/tests/check-protocol-provenance.sh - name: Check tests do not contact n0 infrastructure run: scripts/tests/check-hermetic-tests.sh + - name: Check the flaky sweep watches exactly the flaky tests + run: scripts/tests/check-flaky-sweep-scope.sh - name: Check exact relay TLS provider bundles run: scripts/tests/check-relay-tls-features.sh - name: Check boundary checker contract diff --git a/.github/workflows/tests.yaml b/.github/workflows/tests.yaml index 181262ef673..5f5cd1f6168 100644 --- a/.github/workflows/tests.yaml +++ b/.github/workflows/tests.yaml @@ -119,10 +119,30 @@ jobs: - name: run tests run: | mkdir -p output - cargo nextest run --workspace ${{ env.FEATURES }} --lib --bins --tests --profile ci --run-ignored ${{ inputs.flaky && 'all' || 'default' }} ${{ inputs.flaky && '--verbose' || '' }} --no-fail-fast --message-format ${{ inputs.flaky && 'libtest-json' || 'human' }} > output/${{ matrix.name }}_${{ matrix.features }}_${{ matrix.rust }}.json + cargo nextest run --workspace ${{ env.FEATURES }} --lib --bins --tests --profile ci --run-ignored ${{ inputs.flaky && 'all' || 'default' }} ${FLAKY_EXCLUDE:+-E "$FLAKY_EXCLUDE"} ${{ inputs.flaky && '--verbose' || '' }} --no-fail-fast --message-format ${{ inputs.flaky && 'libtest-json' || 'human' }} > output/${{ matrix.name }}_${{ matrix.features }}_${{ matrix.rust }}.json env: RUST_LOG: ${{ runner.debug && 'TRACE' || 'DEBUG'}} NEXTEST_EXPERIMENTAL_LIBTEST_JSON: 1 + # `--run-ignored all` (the flaky sweep) runs every #[ignore]d test, + # but nextest cannot read the ignore REASON -- so it also runs tests + # ignored for reasons that have nothing to do with flakiness. Those + # fail every single run, which made the daily sweep permanently red + # and therefore incapable of reporting a NEW flake: an alarm that is + # always on reports nothing. It was red on 2026-07-29, -30 and -31. + # + # Exclude the ignores that are not flakiness: + # integration_mainline dials the public BitTorrent mainline + # DHT; deliberately manual (binary + # `mainline`, so exclude the binary). + # downloader_get_many_smoke ignored "todo" -- unimplemented. + # + # The 11 patchbay tests ignored "not yet passing" are already out via + # `default-filter = 'not binary(patchbay)'` in .config/nextest.toml, + # which still applies alongside -E. + # + # scripts/tests/check-flaky-sweep-scope.sh keeps this list in step + # with the #[ignore] reasons in the tree. + FLAKY_EXCLUDE: ${{ inputs.flaky && 'not test(downloader_get_many_smoke) and not binary(mainline)' || '' }} - name: upload results if: ${{ failure() && inputs.flaky }} diff --git a/scripts/tests/check-flaky-sweep-scope.sh b/scripts/tests/check-flaky-sweep-scope.sh new file mode 100755 index 00000000000..45f88d50151 --- /dev/null +++ b/scripts/tests/check-flaky-sweep-scope.sh @@ -0,0 +1,101 @@ +#!/usr/bin/env bash +# The daily flaky sweep must run exactly the tests ignored FOR FLAKINESS. +# +# flaky.yaml runs tests.yaml with `--run-ignored all`, which runs every +# #[ignore]d test. nextest cannot read the ignore reason, so without a filter +# the sweep also runs tests ignored because they are manual, unimplemented, or +# known-broken. Those fail on every run, which is how the sweep came to be +# permanently red (2026-07-29, -30, -31) -- and a red-by-construction sweep +# cannot report a NEW flake, which is the only thing it exists to do. +# +# tests.yaml therefore excludes the non-flaky ignores by name. That list has to +# stay in step with the tree, in BOTH directions: +# +# - a new #[ignore = "todo"]-style test that nobody excludes puts the sweep +# back to permanently red; +# - a test ignored "flaky" that ends up excluded silently stops being +# watched, which is worse than the sweep being red, because it looks fine. +# +# This asserts both. Classification is by ignore reason: a reason beginning +# "flaky" means sweep it, anything else means exclude it. +set -euo pipefail + +cd "$(dirname "$0")/../.." + +workflow=".github/workflows/tests.yaml" +nextest_config=".config/nextest.toml" + +filter_line=$(grep -E '^\s*FLAKY_EXCLUDE:' "$workflow" || true) +if [[ -z "$filter_line" ]]; then + echo "FAIL: no FLAKY_EXCLUDE filter found in $workflow" >&2 + echo " The sweep needs one, or it runs every ignored test and is red forever." >&2 + exit 1 +fi + +failed=0 + +# Collect `#[ignore = "reason"]` plus the fn it applies to. Attributes may sit +# between the ignore and the fn (#[traced_test], #[cfg(...)]), so scan forward. +# vendor/ is excluded: those are third-party trees this sweep does not own. +scan=$(git ls-files '*.rs' | grep -v '^vendor/' | while read -r file; do + awk -v F="$file" ' + match($0, /#\[ignore[[:space:]]*=[[:space:]]*"/) { + line = $0 + sub(/.*#\[ignore[[:space:]]*=[[:space:]]*"/, "", line) + sub(/".*/, "", line) + reason = line + for (i = 1; i <= 8; i++) { + if ((getline nxt) <= 0) break + if (match(nxt, /fn [a-zA-Z0-9_]+/)) { + name = substr(nxt, RSTART + 3, RLENGTH - 3) + print F "\t" name "\t" reason + break + } + } + } + ' "$file" +done) + +if [[ -z "$scan" ]]; then + echo "FAIL: found no #[ignore = \"...\"] tests at all -- the scan is broken, not the tree." >&2 + exit 1 +fi + +while IFS=$'\t' read -r file name reason; do + [[ -z "$name" ]] && continue + + # patchbay tests are excluded wholesale by default-filter in nextest.toml. + if [[ "$file" == *patchbay* ]]; then + if ! grep -q "not binary(patchbay)" "$nextest_config"; then + echo "FAIL: $name ($file) relies on 'not binary(patchbay)' in $nextest_config, which is gone" >&2 + failed=1 + fi + continue + fi + + if [[ "$reason" == flaky* ]]; then + # Must NOT be excluded -- the sweep exists to watch these. + if grep -qF "$name" <<<"$filter_line"; then + echo "FAIL: $name is ignored \"$reason\" but is excluded from the flaky sweep" >&2 + echo " $file" >&2 + echo " A flaky test that the sweep does not run is unwatched, and looks fine." >&2 + failed=1 + fi + else + # Must be excluded, by test name or by its binary. + binary=$(basename "$file" .rs) + if ! grep -qF "$name" <<<"$filter_line" && ! grep -qF "binary($binary)" <<<"$filter_line"; then + echo "FAIL: $name is ignored \"$reason\" -- not flakiness -- but the sweep runs it" >&2 + echo " $file" >&2 + echo " It will fail every night and keep the sweep red, hiding real flakes." >&2 + echo " Add 'not test($name)' (or 'not binary($binary)') to FLAKY_EXCLUDE in $workflow." >&2 + failed=1 + fi + fi +done <<<"$scan" + +[[ $failed -ne 0 ]] && exit 1 + +swept=$(awk -F'\t' '$3 ~ /^flaky/ && $1 !~ /patchbay/' <<<"$scan" | wc -l) +excluded=$(awk -F'\t' '$3 !~ /^flaky/ && $1 !~ /patchbay/' <<<"$scan" | wc -l) +echo "ok: flaky sweep watches $swept flaky test(s); $excluded non-flaky ignore(s) excluded" From 4bf135cdc6e3d4031c6280ec9fed65f3f4499198 Mon Sep 17 00:00:00 2001 From: jonaswre Date: Sat, 1 Aug 2026 19:31:59 +0000 Subject: [PATCH 2/2] fix(ci): emit the flaky-sweep filterset shell-independently The first version passed the filterset through a shell variable with bash parameter expansion: `${FLAKY_EXCLUDE:+-E "$FLAKY_EXCLUDE"}`. That step runs under pwsh on the windows matrix entries, where this is not expansion at all -- pwsh parses it as an undefined variable and substitutes nothing, with no error. For a normal run that is harmless, because the variable is empty anyway. But the flaky sweep runs on the windows entries too, so the filter would have silently not applied there and the sweep would have stayed red on Windows -- the exact failure being fixed, still present on a third of the matrix, and invisible because nothing errors. Emit the argument from a ${{ }} GitHub expression instead. Single quotes survive both shells. Verified the expression renders to `-E 'not test(downloader_get_many_smoke) and not binary(mainline)'` when inputs.flaky is true and to nothing when it is false. Teach the guard both things: read the filterset from the command line rather than an env var, and reject bash parameter expansion anywhere in this workflow's run blocks. Comment lines are stripped before that scan, because the comment documenting this hazard would otherwise match itself -- which it did on the first attempt. Co-Authored-By: Claude Opus 5 --- .github/workflows/tests.yaml | 50 ++++++++++++++---------- scripts/tests/check-flaky-sweep-scope.sh | 21 ++++++++-- 2 files changed, 47 insertions(+), 24 deletions(-) diff --git a/.github/workflows/tests.yaml b/.github/workflows/tests.yaml index 5f5cd1f6168..5b75dedd0e4 100644 --- a/.github/workflows/tests.yaml +++ b/.github/workflows/tests.yaml @@ -116,33 +116,41 @@ jobs: run: | cargo nextest list --workspace ${{ env.FEATURES }} --lib --bins --tests --run-ignored ignored-only + # The `-E` filterset below only applies to the flaky sweep + # (`--run-ignored all`), which runs every #[ignore]d test. nextest cannot + # read the ignore REASON, so without it the sweep also runs tests ignored + # for reasons that are not flakiness. Those fail every single run, which + # made the daily sweep permanently red -- and an alarm that is always on + # cannot report a NEW flake, the only thing it exists for. It was red on + # 2026-07-29, -30 and -31. + # + # Excluded, because they are not flaky: + # integration_mainline dials the public BitTorrent mainline DHT; + # deliberately manual. Excluded by its + # binary, `mainline`. + # downloader_get_many_smoke ignored "todo" -- unimplemented. + # + # The 11 patchbay tests ignored "not yet passing" are already excluded by + # `default-filter = 'not binary(patchbay)'` in .config/nextest.toml, which + # still applies alongside -E (verified -- -E composes with it rather than + # replacing it). + # + # This is emitted by a GitHub expression rather than a shell variable on + # purpose: this step runs under pwsh on the windows matrix entries, where + # bash parameter expansion (`${VAR:+...}`) is not expansion at all -- pwsh + # reads it as an undefined variable and silently substitutes nothing, so + # the filter would quietly not apply on Windows. Single quotes survive + # both shells. + # + # scripts/tests/check-flaky-sweep-scope.sh keeps this list in step with + # the #[ignore] reasons in the tree. - name: run tests run: | mkdir -p output - cargo nextest run --workspace ${{ env.FEATURES }} --lib --bins --tests --profile ci --run-ignored ${{ inputs.flaky && 'all' || 'default' }} ${FLAKY_EXCLUDE:+-E "$FLAKY_EXCLUDE"} ${{ inputs.flaky && '--verbose' || '' }} --no-fail-fast --message-format ${{ inputs.flaky && 'libtest-json' || 'human' }} > output/${{ matrix.name }}_${{ matrix.features }}_${{ matrix.rust }}.json + cargo nextest run --workspace ${{ env.FEATURES }} --lib --bins --tests --profile ci --run-ignored ${{ inputs.flaky && 'all' || 'default' }} ${{ inputs.flaky && '-E ''not test(downloader_get_many_smoke) and not binary(mainline)''' || '' }} ${{ inputs.flaky && '--verbose' || '' }} --no-fail-fast --message-format ${{ inputs.flaky && 'libtest-json' || 'human' }} > output/${{ matrix.name }}_${{ matrix.features }}_${{ matrix.rust }}.json env: RUST_LOG: ${{ runner.debug && 'TRACE' || 'DEBUG'}} NEXTEST_EXPERIMENTAL_LIBTEST_JSON: 1 - # `--run-ignored all` (the flaky sweep) runs every #[ignore]d test, - # but nextest cannot read the ignore REASON -- so it also runs tests - # ignored for reasons that have nothing to do with flakiness. Those - # fail every single run, which made the daily sweep permanently red - # and therefore incapable of reporting a NEW flake: an alarm that is - # always on reports nothing. It was red on 2026-07-29, -30 and -31. - # - # Exclude the ignores that are not flakiness: - # integration_mainline dials the public BitTorrent mainline - # DHT; deliberately manual (binary - # `mainline`, so exclude the binary). - # downloader_get_many_smoke ignored "todo" -- unimplemented. - # - # The 11 patchbay tests ignored "not yet passing" are already out via - # `default-filter = 'not binary(patchbay)'` in .config/nextest.toml, - # which still applies alongside -E. - # - # scripts/tests/check-flaky-sweep-scope.sh keeps this list in step - # with the #[ignore] reasons in the tree. - FLAKY_EXCLUDE: ${{ inputs.flaky && 'not test(downloader_get_many_smoke) and not binary(mainline)' || '' }} - name: upload results if: ${{ failure() && inputs.flaky }} diff --git a/scripts/tests/check-flaky-sweep-scope.sh b/scripts/tests/check-flaky-sweep-scope.sh index 45f88d50151..9c33f91947f 100755 --- a/scripts/tests/check-flaky-sweep-scope.sh +++ b/scripts/tests/check-flaky-sweep-scope.sh @@ -25,10 +25,25 @@ cd "$(dirname "$0")/../.." workflow=".github/workflows/tests.yaml" nextest_config=".config/nextest.toml" -filter_line=$(grep -E '^\s*FLAKY_EXCLUDE:' "$workflow" || true) +# The filterset is emitted by a GitHub expression on the nextest command line +# (not a shell variable -- this step runs under pwsh on Windows, where bash +# parameter expansion silently yields nothing). Grab that line. +filter_line=$(grep -E "cargo nextest run .*-E ''not " "$workflow" || true) if [[ -z "$filter_line" ]]; then - echo "FAIL: no FLAKY_EXCLUDE filter found in $workflow" >&2 - echo " The sweep needs one, or it runs every ignored test and is red forever." >&2 + echo "FAIL: no flaky-sweep -E filterset found on the nextest command in $workflow" >&2 + echo " Without it the sweep runs every ignored test and is red forever." >&2 + echo " Expected a \${{ inputs.flaky && '-E ''not ...''' || '' }} fragment." >&2 + exit 1 +fi + +# A shell-expansion form would parse here but silently do nothing on Windows. +# Comment lines are stripped first: the comment above that step documents this +# exact hazard, and matching the documentation would be a false positive. +if grep -vE '^[[:space:]]*#' "$workflow" | grep -qE '\$\{[A-Z_]+:[+-]'; then + echo "FAIL: $workflow uses bash parameter expansion (\${VAR:+...})." >&2 + echo " This step runs under pwsh on the windows matrix entries, which does" >&2 + echo " not expand it -- the value silently vanishes there. Emit the argument" >&2 + echo " from a \${{ }} GitHub expression instead." >&2 exit 1 fi