Conversation
…en handle The modified-date-range tests used a lower bound that equals the default range on exactly one calendar day, 2026-10-02, so they failed in CI that day. The first-page failure test now gets a 500 handler so its retries fail fast instead of leaving a 120s axios timer open, and it asserts the rejection rather than passing when nothing throws. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
luixlive
left a comment
There was a problem hiding this comment.
Thanks for the fix Chris! Changes seem good and a low effort review from Claude found no blocker. Feel free to merge it ✅
If you have a couple minutes, Clauded provided 3 low-severity suggestions to improve the quality of the tests:
-
Have MSW fail requests that have no handler (
parallelPaginatedRequest.test.js:31)
This is the root cause of the open handle the PR fixes. As it is, any future test that calls a URL without a handler will hit the real network and bring the open handle back. server.listen({ onUnhandledRequest: 'error' }) makes that impossible. -
Check the retries (
parallelPaginatedRequest.test.js:84)
The test only checks the final error, so it would still pass if the retry loop stopped retrying. Counting requests in the 500 handler (expecting 3) would cover the behavior the test is named for. -
Derive the "modified" date from the default (
EventFilter/index.test.js:311)
MovingINITIAL_FILTER_STATE's lower bound back one day states the test's intent and works for any clock or env value. 2020-01-01 is very unlikely to break, though.
What does this PR do?
Fixes two test-only problems seen in the CI run for #1737 on 2026-10-02.
src/EventFilter/index.test.js— "names the dates trigger state while the date range is modified" and "offers to reset while the date range is modified" set the lower date to2026-09-01. The filter's default lower bound is the start of the day 31 days ago, so on 2026-10-02 that was exactly the default and the range read as unmodified. The tests now use2020-01-01, which can never match the rolling default.src/utils/parallelPaginatedRequest.test.js— "throws error if first page can not be resolved" requested a URL with no MSW handler, so the request fell through to the network and left axios's 120s timeout as the open handle--detectOpenHandlesreports in every CI run. It also could not fail, since its assertion sat inside acatch. It now registers a 500 handler so the retries fail fast, and asserts the rejection withrejects.toThrow.Evidence
TZ=UTC CI=true jest --detectOpenHandles --forceExit, and Jest no longer reports the open handle.Relevant link(s)
🤖 Generated with Claude Code