Repository navigation
ci: regenerate parallel-allowlist from post-#36175 build data - #36585
Conversation
The allowlist was generated from builds 82146-83745, which predate the parallel-batch feature itself (#36175). Tests that measure process-wide state (heapStats().objectTypeCounts, rss(), expectMaxObjectTypeCount) or have tight timing budgets were included because they had never run in-process alongside other test files before. Across 400 recent builds the top parallel-batch-only flakes ("passed alone" after failing in the batch) are: streams-leak.test.ts 348/400 builds shell/leak.test.ts 340/400 socket-retention.test.ts 334/400 fetch-tls-abortsignal-timeout.test.ts 282/400 spawn-streaming-stdout.test.ts 267/400 08965.test.ts 261/400 Regenerated with scripts/update-parallel-allowlist.mjs from builds 85133-86444 (300 PR builds, post-feature). 282 dirs -> 215, 1642 files -> 1296 in the batch, 153 -> 160 explicit excludes. The generator already filters on "zero flaky/failed annotations in the window"; the previous run simply had no data to filter on.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (1)
WalkthroughThe generated parallel-test configuration now contains updated build metadata, directory allowlists, and excluded-test files across Bun, Node.js, third-party, web, and regression suites. ChangesParallel test allowlist
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
Comment |
There was a problem hiding this comment.
LGTM — mechanical regeneration of the parallel-batch allowlist from newer CI data.
What was reviewed:
- Consumer in
scripts/runner.node.mjs:388-394— the new""dir entry is handled correctly (matches top-levelpackage-json-lint.test.tsonly). - Self-test
test/internal/parallel-allowlist.test.tsinvariants: all 160 excludeFiles exist on disk and have their dirname indirs; counts match_meta.stats(215/160). - All ten top-offender tests from the PR description are excluded (either in
excludeFilesor their dir dropped).
Extended reasoning...
Overview
Single-file change to test/parallel-allowlist.json, a generated CI configuration file that tells scripts/runner.node.mjs which test files may run in the shared bun test --parallel batch. The generator script (scripts/update-parallel-allowlist.mjs) is unchanged — only its output is refreshed against builds 85133-86444, which now include real parallel-batch flake data (the previous window predated the batch's introduction in #36175).
Net effect: 282→215 dirs, 153→160 explicit excludes, 1642→1296 batch-eligible files. Tests dropped from the batch fall back to serial execution — no coverage is lost.
Security risks
None. This is CI test-scheduling metadata; it does not touch runtime, build, or any shipped code path.
Level of scrutiny
Low. The failure mode of a wrong entry is a yellow flake annotation + serial retry (the exact symptom this PR is fixing), not a broken build or missed test. I verified structural correctness against both the runner's consumer logic and the repo's own self-test:
jqcounts match_meta.statsexactly (215 dirs, 160 excludes).- Every
excludeFilesentry exists on disk and itsdirname()is present indirs(sotest/internal/parallel-allowlist.test.tspasses). - The new
""dir entry is intentional:isParallelAllowlistedusesslash === -1 ? "" : path.slice(0, slash), so it matches only top-level test files — currently justpackage-json-lint.test.ts. - Spot-checked all ten top-offender tests from the PR table: each is now excluded either via
excludeFiles(streams-leak,shell/leak,socket-retention,fetch-tls-abortsignal-timeout,20875,node-http-pinned-write,27272) or by dropping their dir (js/bun/spawn,regression/issue/08965,js/third_party/esbuild).
Other factors
The PR description's mechanism analysis (RSS/heapStats assertions being process-wide) is sound and matches why these tests can't be made parallel-safe without subprocess isolation. This is exactly the kind of periodic data refresh the generator was built for.
|
Found 3 issues this PR may fix:
🤖 Generated with Claude Code |
|
Re the find-issues bot: none of #32092, #34095, #34690 are fixed by this PR.
Leaving the PR description as-is. |
|
Heads up: the regen dropped |
update-parallel-allowlist.mjs rebuilds excludeFiles from scratch based on flake annotations in the scanned build window. A file that is already excluded runs serially, produces no annotation, and so gets dropped on the next regen, which is how #36585 lost inspect-error-leak. Add a guard that fails CI if a regen drops any of the known in-process RSS/process- state leak tests, so the oscillation is caught at the regen PR rather than as a downstream flake.
…e-exclude streams-leak scripts/update-parallel-allowlist.mjs rebuilds excludeFiles from scratch from flake annotations in the scanned build window. A file that is already excluded runs serially, produces no annotation, and so is dropped on the next regen, then flakes once re-included. #36585 dropped inspect-error-leak this way (flaked on nearly every build from 86456 until the #39236 regen re-excluded it); #39236 in turn dropped streams-leak, which has flaked in the parallel batch on every macOS lane of every main build since (98887 onward). Add a guard in test/internal/parallel-allowlist.test.ts listing the tests that assert on in-process rss()/heapStats() and so cannot be batch-safe; a regen that drops one of them fails there, naming the file. Re-add streams-leak to excludeFiles so the guard passes on this tree.
scripts/update-parallel-allowlist.mjs rebuilds excludeFiles from scratch from the flake annotations in its scan window. A file that is already excluded runs serially, never produces a batch annotation, and so is dropped by the next regen, then flakes once it is back in the batch. #36585 dropped inspect-error-leak and sourcetextmodule-leak this way and both have flaked in the batch on most builds since (x64-asan timeouts, Windows worker exit 9); #39236 re-excluded them, dropped streams-leak instead, and has now been reverted (#39369), putting the first two back in the batch. Add a guard in test/internal/parallel-allowlist.test.ts listing the dedicated leak tests that assert on rss()/heapStats() inside the test process; a regen that drops one of them fails there naming the file. Re-add the three pinned files that the current table runs in the batch (inspect-error-leak, sourcetextmodule-leak, zlib/leak) so the guard passes on this tree.
… more in-process leak tests The runner reads only parallel-allowlist.json, so a denylist entry keeps a file out of the batch only once a regen has copied it into excludeFiles. Assert that every denylisted file in a listed dir is in excludeFiles (and that denylist entries are real files), so a by-hand denylist addition or a table produced by an older script fails in CI instead of running the file in the batch. Against the table from before #39373 the check names the 97 files that PR's body describes as batch-flaking. Add the three in-process RSS/heapStats leak tests whose batch failures predate the window the denylist was collected from (98887..99420), so the next regen cannot promote them: shell/leak (#36585: failed in the batch in 340 of 400 builds), request-clone-leak (excluded by hand in #36759 after batch failures on x64-asan and Windows) and zlib/leak (build 86483). All three are already in excludeFiles; this only makes the exclusion stick.
Problem
The parallel-batch allowlist (
test/parallel-allowlist.json) was generated from builds 82146-83745, which predate #36175 (the PR that introduced the parallel batch). The generator's rule is "zero flaky/failed annotations in the scanned builds", but at that point no test had ever run inside a sharedbun test --parallelprocess, so the allowlist couldn't know which tests are incompatible with in-process parallelism.The result: tests that measure process-wide state (
heapStats().objectTypeCounts,rss(),expectMaxObjectTypeCount) or have tight wall-clock budgets were included, and they now flake on nearly every build. Across the last 400 completed PR builds, the top "failed in the parallel batch; passed alone" annotations by distinct build count:test/js/web/streams/streams-leak.test.tstest/js/bun/shell/leak.test.tstest/js/bun/net/socket-retention.test.tstest/js/web/fetch/fetch-tls-abortsignal-timeout.test.tstest/js/bun/spawn/spawn-streaming-stdout.test.tstest/regression/issue/08965/08965.test.tstest/regression/issue/20875.test.tstest/js/node/http/node-http-pinned-write.test.tstest/regression/issue/27272.test.tstest/js/third_party/esbuild/esbuild-child_process.test.tsEach of these generates a yellow annotation + serial retry on most builds.
Fix
Re-ran
scripts/update-parallel-allowlist.mjs(300 PR builds, 85133-86444). 609 distinct files flaked or failed in that window; the generator drops them per its existing rule.All six of the top offenders above are now excluded (either via
excludeFilesor their dir dropped).Verification
streams-leak.test.tsassertsrss() < 250MB; in the batch it sees 390MB-2.6GB from neighbouring tests.shell/leak.test.ts's#11816tests assertheapStats().objectTypeCounts.ShellInterpreter <= 3in-process; any concurrent test using$contributes.socket-retention.test.tsandspawn-streaming-stdout.test.tsuseexpectMaxObjectTypeCountsimilarly. These are correct tests in an isolated process and cannot be made parallel-safe without moving the assertions into a subprocess; excluding them from the batch is what the generator already does once it has data.no test proof · iteration 0 · docs-only change; test-proof not applicable