fix(ci): make the daily flaky sweep able to detect a flake - #19
Merged
Conversation
|
Documentation for this PR has been generated and is available at: https://holon-technologies.github.io/iroh/pr/19/docs/krikos/ Last updated: 2026-08-01T20:40:50Z |
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 <noreply@anthropic.com>
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 <noreply@anthropic.com>
jonaswre
force-pushed
the
fix/flaky-sweep-scope
branch
from
August 1, 2026 20:32
fe0ca5e to
4bf135c
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
flaky.yamlrunstests.yamlwith--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.The sweep was therefore red by construction — on 2026-07-29, -30 and -31, and every night after. An alarm that is always on reports nothing, and detecting a new flake is the only thing this sweep exists to do.
Scope, measured
Verified with
cargo nextest list --run-ignored ignored-onlyrather than inferred from source. The workspace has 25 ignored tests in four categories:"flaky""not yet passing…"(patchbay)default-filter"manual mainline testnet coverage…""todo"The sweep's real scope was 7 tests: the 5 flaky ones plus
integration_mainlineanddownloader_get_many_smoke. Those last two are what kept it red.The change
Exclude the two non-flaky ignores with a filterset.
Confirmed empirically that
-Ecomposes withdefault-filterrather than replacing it — with the filter applied, ignored-only lists exactly the 5 flaky tests, and 0 patchbay tests are visible. This mattered: had-Eoverridden the default filter, the fix would have pulled all 11 patchbay tests into the sweep and made it worse.The guard
scripts/tests/check-flaky-sweep-scope.sh(wired intoci.yml) classifies by ignore reason and asserts the exclusion list tracks the tree in both directions, because neither is self-announcing:#[ignore]that nobody excludes → sweep goes permanently red again;"flaky"that ends up excluded → silently stops being watched, which is worse than a red sweep, because it looks fine.Verified against three injected regressions, all caught:
#[ignore = "needs hardware"]testsync_big) added to the exclusion listnot binary(patchbay)default-filter the patchbay ignores depend on64/64 local checks pass (was 63 — the new check is auto-discovered by
run-all-local-checks.sh).What this does not fix
The sweep will still go red whenever a genuinely flaky test flakes — that is the signal working.
sync_full_basic,sync_restart_node,sync_big,test_roundtrip_bytes_smallandconnect_via_relay_becomes_direct_and_sends_directare real unfixed defects. The sweep will now tell you which one, instead of drowning them in a guaranteed failure.🤖 Generated with Claude Code