Fix flaky ipfs-cache warm tests (main is red) - #310
Merged
Conversation
settle() drained 20 setImmediate turns to wait for warm()'s fire-and-forget chain. That barrier expires in ~0.5ms on an idle loop, while the chain it guards contains real threadpool I/O — so on a loaded CI runner (node --test forks one process per file across 2 cores) the fs read had not landed and the assertions read an empty result. Turn count was never a proxy for wall clock. Poll for the effect each test actually asserts instead, with a 5s deadline so a genuine hang still fails the assertion rather than hanging the run. The warm-caches case waits on the entry appearing under its final name: the fetch call landing is not enough, since a get() racing the pending write would re-fetch and break the very hit the test checks for.
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
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.
mainwent red on the #309 merge run — but on a test unrelated to that PR:warm pulls images in the background and swallows failures(expected: 1, actual: 0). #309's own PR run passed the same file four minutes earlier. It's a long-standing flake that finally lost the dice.Mechanism
settle()waited forwarm()'s fire-and-forget chain by draining 20setImmediateturns. That barrier expires in ~0.5 ms on an idle loop (measured), while the chain it guards contains real threadpool I/O — the cache-missfsread. Turn count was never a proxy for wall clock.On a 2-core CI runner,
node --testforks one process per test file, so the threadpool and disk are heavily contended and that read takes milliseconds. The barrier had long since expired,h.errorswas still empty, and the assertion read 0.Fix
Poll for the effect each test actually asserts, with a 5s deadline so a genuine hang still fails the assertion rather than hanging the run.
The
warm caches what a later get would have fetchedcase waits on the entry appearing under its final name — waiting on the fetch call isn't enough, since aget()racing the pending write would re-fetch and break the very cache hit the test checks for.Verification
ipfs-cache.test.ts20x locally: 14/14 pass every runnpm run test:backend625/625, lint + typecheck cleanHonest caveat: I could not reproduce the original failure locally (12 parallel runs of the old code all passed on this machine) — it needs the contention of a small CI runner. The argument for the fix is structural rather than a red-to-green repro: the new barrier is condition-based, so it cannot expire early no matter the timing.