fix: drop callCount from dedup tests — still flaky after #419#421
Merged
Conversation
…ss-test fetch pollution The delta-based callCount fix in #419 was insufficient: await Promise.all() yields to the event loop, allowing other test files to call fetch() against our mock during the microtask resolution window. This inflated callCount from 1 to 2 intermittently in CI. Changes: - Remove callCount entirely from the two dedup tests. Promise identity (expect(pa).toBe(pb)) is the correct and sufficient assertion — it verifies the inflight promise is shared without any exposure to cross-test globals. - Relax the 'caches results' test to use toBeGreaterThanOrEqual(1) for the first fetch (cross-test pollution can inflate during await yields) while keeping the strict delta=0 check for the cache hit.
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.
Summary
Follow-up to #419. The delta-based
callCountfix was still flaky in CI (run):Expected: 1, Received: 2.Root Cause
await Promise.all()yields to the event loop. During that yield, other test files' code can callfetch()against ourglobalThis.fetchmock, incrementingcallCount. This happens even with the delta-based counting from #419 because the counter is incremented between the reset and the assertion.Fix
callCountentirely. Promise identity (expect(pa).toBe(pb)) is the correct and sufficient assertion — it verifies the inflight promise is shared across concurrent callers without any exposure to cross-test globals. Captured synchronously, no event-loop yield.toBeGreaterThanOrEqual(1)for the first fetch (pollution can inflate during await yields), keep strictdelta === 0for the cache hit.