Skip to content

Fix the date-dependent EventFilter tests and the paginated request open handle - #1750

Open
chrisj-er wants to merge 1 commit into
developfrom
fix-date-dependent-tests
Open

chrisj-er wants to merge 1 commit into
developfrom
fix-date-dependent-tests

Conversation

@chrisj-er

Copy link
Copy Markdown
Contributor

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 to 2026-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 use 2020-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 --detectOpenHandles reports in every CI run. It also could not fail, since its assertion sat inside a catch. It now registers a 500 handler so the retries fail fast, and asserts the rejection with rejects.toThrow.

Evidence

  • Both suites pass under TZ=UTC CI=true jest --detectOpenHandles --forceExit, and Jest no longer reports the open handle.
  • ESLint clean on both files.

Relevant link(s)

🤖 Generated with Claude Code

…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>
@chrisj-er
chrisj-er requested a review from luixlive October 5, 2026 17:36

@luixlive luixlive left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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:

  1. 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.

  2. 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.

  3. Derive the "modified" date from the default (EventFilter/index.test.js:311)
    Moving INITIAL_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.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants