fix(#4082): wait on real tooltip state in work side menu browser tests#4083
Draft
Spark450 wants to merge 1 commit into
Draft
fix(#4082): wait on real tooltip state in work side menu browser tests#4083Spark450 wants to merge 1 commit into
Spark450 wants to merge 1 commit into
Conversation
The tooltip show/hide browser tests mixed fake timers with real browser hover interactions. The .show class is applied after a real setTimeout following a browser-delivered mouseenter, so advancing fake time by a fixed amount raced the real event and intermittently never fired the timer, leaving .show unapplied. Drop fake timers and wait on the actual .show class state via vi.waitFor against real timers. Also remove the toBeTruthy() truthy assertions on query results, which test nothing meaningful. Fixes #4082 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
Preview links
Built from commit 17affc1. Previews are removed automatically when this PR closes. |
Collaborator
Author
|
@vanessatran-ddi I very quickly got in over my head here, I can leave this hanging around if you think it could be something we can use but if not, happy to close. |
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
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.
Before (the change)
The work side menu tooltip browser tests (
libs/react-components/specs/work-side-menu.browser.spec.tsx) usedvi.useFakeTimers()with a fixedvi.advanceTimersByTimeAsync(400)before assertingtooltipEl?.classList.contains("show").The
.showclass is applied by a realsetTimeoutthat runs only after the browser delivers a realmouseenterfrom the Playwrighthover()interaction. Because the real event is not governed by fake timers, advancing fake time by a fixed amount raced the real event: when the mouseenter had not yet been delivered, the 300ms timer was never registered, so advancing fake time fired nothing and.showwas never applied. The assertion then failed intermittently right after the fixed advance.The same tests also used
expect(tooltipEl).toBeTruthy(), a truthy assertion on a query result that our testing standards flag as meaningless.After (the change)
The three tooltip tests now drop fake timers and wait on the actual
.showclass state viavi.waitForagainst real timers. The redundanttoBeTruthy()assertions are removed, since asserting on.showalready requires the element to exist.This is a test only change. No component, prop, event, or wrapper behavior changed, so React and Angular wrappers are untouched.
Make sure that you've checked the boxes below before you submit the PR
Steps needed to test
npx vitest --run --project='*-headless' work-side-menu.browser -t "tooltip".expect(tooltipEl?.classList.contains("show")).toBe(true).Verified locally: 6/6 tooltip assertions pass across 3 repeat runs, and the full
work-side-menu.browserspec passes 26/26.Fixes #4082
🤖 Generated with Claude Code