fix(e2e): improve robustness of flaky tablet/mobile E2E tests#207
Merged
Conversation
## Root Causes
### search() / clearSearch() race condition (work-items-list.spec.ts tablet timeouts)
The waitForResponse listener was registered AFTER searchInput.fill(), creating a
race where the debounced 300ms request could complete before the listener was
attached. On WebKit/tablet, this caused the listener to never resolve. Fix:
register the listener BEFORE the fill action, then await it after. Also added
waitForLoaded() after the response to ensure React has flushed filtered data into
the DOM before callers read titles.
### confirmDelete() missing DELETE response wait (work-items-list.spec.ts mobile timeout)
The confirmDelete() method clicked the button and waited for the modal to hide,
but didn't wait for the DELETE API response. This caused a race where the test
proceeded to assert list state before the server confirmed deletion and before
React re-fetched the list. Fix: register a DELETE 204 response listener before
the click. In the test, the post-delete list-refresh GET listener is also
registered before confirmDelete() is called.
### waitForURL timeout too short for mobile WebKit (work-item-create.spec.ts)
The waitForURL('**/work-items/**', { timeout: 7000 }) hardcoded a 7s timeout that
was too short for mobile WebKit (project-level navigationTimeout is 15s). Removed
the hardcoded timeout so the project-level setting applies. Same fix for the
heading assertion.
### proxy login race condition (proxy-setup.spec.ts desktop)
The login tests used `expect(page).not.toHaveURL(/\/login/, { timeout: 15000 })`
after button click, which could time out if React's router update lagged behind
session establishment. Replaced with a shared loginThroughProxy() helper that
uses Promise.all to start the API response listener BEFORE clicking Submit, then
waits for URL change with waitForURL() — a condition-based wait that is more
reliable than polling not.toHaveURL. Eliminated duplicated login code across 3
tests.
Co-Authored-By: Claude <qa-integration-tester> (Sonnet 4.6) <noreply@anthropic.com>
Use a proper `import type { Page }` declaration instead of an inline
`import()` type annotation, which is forbidden by the
`@typescript-eslint/consistent-type-imports` rule.
Co-Authored-By: Claude <qa-integration-tester> (Sonnet 4.6) <noreply@anthropic.com>
Co-Authored-By: Claude <qa-integration-tester> (Sonnet 4.6) <noreply@anthropic.com>
Contributor
|
🎉 This PR is included in version 1.9.0-beta.52 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
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
2 participants
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
proxy-setup.spec.ts (
[desktop]lines 65 & 115): Replaced duplicated inline login code across 3 tests with a sharedloginThroughProxy()helper that usesPromise.allto start the API response listener before clicking Submit (prevents the race condition where the login POST response arrives before the listener is attached through the extra nginx hop). UseswaitForURL()instead ofexpect(page).not.toHaveURL()for a condition-based wait that reliably exits once the React router navigates away.WorkItemsPage.ts
search()/clearSearch(): ThewaitForResponselistener was registered AFTERsearchInput.fill(), creating a race on WebKit/tablet where the 300ms-debounced request completed before the listener was attached (listener would never resolve). Fix: register the listener before the action, then await it. Additionally,waitForLoaded()is called after the response to ensure React has flushed the filtered data into the DOM before callers read titles.WorkItemsPage.ts
confirmDelete(): Added awaitForResponselistener for the DELETE 204 response before clicking confirm, so the test reliably knows the server has processed the deletion before checking list state.work-items-list.spec.ts delete test (line 272,
[mobile]): ReplacedwaitForLoaded()afterconfirmDelete()with an explicitwaitForResponsefor the post-delete list-refresh GET, registered before the delete action — this avoids the race between modal close and list re-render.work-item-create.spec.ts (line 75,
[mobile]): Removed hardcoded{ timeout: 7000 }fromwaitForURLand the headingtoHaveTextassertion. These now use the project-levelnavigationTimeout(15s for mobile/tablet WebKit) which is sufficient for the redirect chain after form submission.Test plan
e2e-smokejob)betashows the 8 previously-failing tests as passingRoot Cause Summary
All failures were race conditions between network responses and DOM updates on slow WebKit CI runners:
waitForResponseregistered after the triggering action (missed the response)🤖 Generated with Claude Code