fix flaky test and correct clearEventListeners return type (#708) #1264
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.
fixes #708
This PR fixes the flaky test observed in the issue above. Its flakiness was due to
comlink
's worker function proxy, which turnedclearEventListeners
into an async function, but was not properly awaited in the test code, and its type definition didn't define it as an async function. By awaiting theclearEventListeners
function, then running the assertions, I was able to eliminate flakiness caused by the event listeners. To verify the correctness I used a AI generated bash script that runs the test case a number of times, records the failures, and calculates the #passes/total runs.Running the command above after this patch I get:
Before this patch, and removing
test.skip
I get:Where both failures looked like the screenshot on #708, and sometimes it flakes at a much higher rate. I have locally run more (1000+) iterations comparing both versions and am comfortable saying that this fixes the flakiness.
Additional Notes
AbortSignals
for registering and removing event listeners in entrypoint.ts to simplify implementation.More Details on Crash