Skip to content

chore: DAH-0000 resolve flaky SupplementalApplicationPage tests - #791

Draft
fwextensions wants to merge 2 commits into
mainfrom
chore/fix-flaky-supplemental-test
Draft

chore: DAH-0000 resolve flaky SupplementalApplicationPage tests#791
fwextensions wants to merge 2 commits into
mainfrom
chore/fix-flaky-supplemental-test

Conversation

@fwextensions

@fwextensions fwextensions commented Jan 29, 2026

Copy link
Copy Markdown
Collaborator

Description

Add proper async handling to tests that were failing intermittently due to race conditions. Tests were querying for DOM elements before async data loading completed. This was causing SupplementalApplicationPage.test.js to sometimes fail in slower environments on Windows, like the VS Code terminal, but then it would work in the Windows terminal. Ensuring that async calls are always awaited and bumping the timeouts seems to have fixed it.

Changes:

  • Add waitFor to getWrapper() to ensure form is loaded before tests run
  • Use findByRole instead of getByRole for elements that appear after async ops
  • Add await to selectEvent.openMenu() calls which are async
  • Wrap remaining state-changing interactions in await act()

Before requesting eng review

Version Control

  • branch name contains the Jira ticket number
  • PR name follows type: TICKET-NUMBER Description format, use DAH-000 if it does not need a ticket
  • PR name follows urgent: Description format if it is urgent and does not need a ticket

Code quality

  • the set of changes is small
  • all automated code checks pass (linting, tests, coverage, etc.)
  • if the PR is a bugfix, there are tests and logs around the bug

Review instructions

  • instructions specify which environment(s) it applies to
  • instructions work for PA testers
  • instructions have already been performed at least once

Request eng review

  • PR has needs review label
  • Use Housing Eng group to automatically assign reviewers, and/or assign specific engineers
  • If time sensitive, notify engineers in Slack

Before merging

Request product acceptance (PA) testing

  • PA tested in the review environment (use needs product acceptance label)
  • if PA testing cannot be done, changes are behind a feature flag

Add proper async handling to tests that were failing intermittently due
to race conditions. Tests were querying for DOM elements before async
data loading completed.

Changes:
- Add waitFor to getWrapper() to ensure form is loaded before tests run
- Use findByRole instead of getByRole for elements that appear after async ops
- Add await to selectEvent.openMenu() calls which are async
- Wrap remaining state-changing interactions in await act()
@hshaosf
hshaosf temporarily deployed to dahlia-partners-pr-791 January 29, 2026 20:00 Inactive
@fwextensions
fwextensions requested review from a team, alulabeshue-sfgov and jimlin-sfgov and removed request for a team January 29, 2026 20:00
It says it's required by eslint-plugin-standard, to replace eslint-plugin-node.
Clean up lint issues.
@hshaosf
hshaosf temporarily deployed to dahlia-partners-pr-791 January 29, 2026 20:16 Inactive

@jimlin-sfgov jimlin-sfgov left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

LGTM

On the other hand, we can choose not to fix this, and use the flakiness as a reason to get you a beefed up laptop @fwextensions


// Increase default test timeout for this file - tests involve multiple async operations
// and can be slow in resource-constrained environments (e.g., VS Code terminal)
jest.setTimeout(15000)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Thought: this is a smell for our long-standing issue of slow loading times, which we may work on one day...

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

That's what Opus thinks.

Slow patterns in current tests:

  1. Expensive cloning - Every API mock does _cloneDeep(mockedApplication) on a 406-line object
  2. Redundant async - Mocks are async but return immediately (adds microtask overhead)
  3. Full page render per test - Nested beforeEach calls getWrapper() repeatedly:
    - "when no unit is selected": 2 tests = 2 renders
    - "when unit without priority is selected": 4 tests = 4 renders
    - "when unit with priority is selected": 2 tests = 2 renders

That's 8 full renders just in Lease Section nested describes

@fwextensions
fwextensions marked this pull request as draft June 22, 2026 19:54
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.

3 participants