Skip to content

test: unflake the garbage collection tests - #1115

Merged
rekmarks-consensys-1 merged 3 commits into
mainfrom
rekm/unflake-gc-tests
Sep 18, 2026
Merged

rekmarks-consensys-1 merged 3 commits into
mainfrom
rekm/unflake-gc-tests

Conversation

@rekmarks-consensys-1

@rekmarks-consensys-1 rekmarks-consensys-1 commented Sep 18, 2026

Copy link
Copy Markdown
Contributor

Garbage Collection > an object shared by two importers > survives until both importers let go flakes in CI:

AssertionError: expected [ 'v2', 'v3' ] to strictly equal [ 'v3' ]

The test reaped the importer vat once, ran a fixed three cranks, and then asserted. Whether an importer reports dropImports/retireImports during bringOutYourDead depends on the engine having actually collected the dropped presence by that moment, and one round of gc() does not guarantee that — a stale stack slot is enough to keep the presence alive for a pass. When it survived, the kernel still listed the importer.

  • reapAndSettle now takes an isSettled predicate and re-reaps, up to MAX_REAP_ATTEMPTS rounds of CRANKS_PER_REAP cranks, returning as soon as the kernel has recorded the drop.
  • Hoisted it to the outer describe and applied it to the dropImports and retireImports assertions in should trigger GC syscalls through bringOutYourDead, which used the same fixed pattern.

Assertions are unchanged, so a genuine failure still reports the same diff rather than a timeout. Worst case adds ~30s to tests with 60s and 40s timeouts; the happy path is unaffected.

Instrumenting five local runs of the pre-change file showed the second importer needing three reap rounds in two of them and one to two rounds otherwise, so a single reap is measurably not enough. Six runs of the file after the change passed, and the package lints clean. Both were run before the rebase onto the better-sqlite3 13 bump on main; CI covers the post-rebase state.

🤖 Generated with Claude Code


Note

Low Risk
Test-only changes to timing and synchronization; no production kernel or GC behavior is modified.

Overview
Deflakes kernel garbage-collection tests by replacing a single reapVats plus three fixed cranks with a shared reapAndSettle helper that retries (up to 10 rounds, 3 cranks each) until an isSettled predicate matches kernel state.

The helper is hoisted to the top-level Garbage Collection suite and wired into should trigger GC syscalls through bringOutYourDead (drop/retire import ref counts) and survives until both importers let go (importer list / ref counts). That test’s duplicate local helper is removed. The bringOutYourDead case timeout rises from 40s to 60s to cover worst-case retries; final assertions are unchanged.

Reviewed by Cursor Bugbot for commit ef8aff1. Bugbot is set up for automated code reviews on this repo. Configure here.

@rekmarks-consensys-1
rekmarks-consensys-1 requested a review from a team as a code owner September 18, 2026 17:02
@rekmarks-consensys-1 rekmarks-consensys-1 added the no-changelog Indicates that no changelog updates are required, and that related CI checks should be skipped. label Sep 18, 2026
`reapAndSettle` reaped once and ran a fixed three cranks before the test
asserted, so it failed whenever the engine had not collected the dropped
presence by the time `bringOutYourDead` ran. Reap until the kernel has
recorded the drop instead.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The same fixed reap-then-three-cranks pattern gated the dropImports and
retireImports assertions, so hoist `reapAndSettle` and use it there.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Give `reapAndSettle` an options bag, wait on the full expected ref count
rather than one field, and raise the `bringOutYourDead` test's timeout to
60s, which two 10-attempt reap loops can otherwise exhaust on a slow
runner.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@github-actions

Copy link
Copy Markdown
Contributor

Coverage Report

Status Category Percentage Covered / Total
🔵 Lines 73.04%
🟰 ±0%
9789 / 13401
🔵 Statements 72.89%
🟰 ±0%
9953 / 13653
🔵 Functions 73.6%
🟰 ±0%
2301 / 3126
🔵 Branches 67.25%
🟰 ±0%
4022 / 5980
File CoverageNo changed files found.
Generated in workflow #4842 for commit ef8aff1 by the Vitest Coverage Report Action

@rekmarks-consensys-1
rekmarks-consensys-1 added this pull request to the merge queue Sep 18, 2026
Merged via the queue into main with commit 3d5aea9 Sep 18, 2026
30 checks passed
@rekmarks-consensys-1
rekmarks-consensys-1 deleted the rekm/unflake-gc-tests branch September 18, 2026 17:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

no-changelog Indicates that no changelog updates are required, and that related CI checks should be skipped.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants