fix(test): stabilize flaky trace view keyboard navigation test#111917
Draft
JoshuaKGoldberg wants to merge 18 commits intomasterfrom
Draft
fix(test): stabilize flaky trace view keyboard navigation test#111917JoshuaKGoldberg wants to merge 18 commits intomasterfrom
JoshuaKGoldberg wants to merge 18 commits intomasterfrom
Conversation
Adds infrastructure for validating flaky test fixes in CI: - `itRepeatsWhenFlaky()`: a test wrapper in tests/js/sentry-test/ that runs a test 50x when the RERUN_KNOWN_FLAKY_TESTS env var is set, otherwise runs once as a normal it() - CI wiring: frontend.yml sets RERUN_KNOWN_FLAKY_TESTS=true when the PR has the "Frontend: Rerun Flaky Tests" label - ESLint: configured jest/no-standalone-expect to recognize itRepeatsWhenFlaky as a test block Wraps all 13 known flaky tests (identified from 30 days of CI failures on master) with itRepeatsWhenFlaky so fixes can be stress-tested: - eventReplay/index.spec.tsx (6 occ) - stackTrace.spec.tsx (5 occ) - resultsSearchQueryBuilder.spec.tsx (5 occ, 2 tests) - metricsTab.spec.tsx (4 occ) - customerDetails.spec.tsx (4 occ) - eventsSearchBar.spec.tsx (3 occ) - trace.spec.tsx (3 occ, previously skipped) - allMonitors.spec.tsx (2 occ) - spansSearchBar.spec.tsx (2 occ) - react-native/metrics.spec.tsx (2 occ) - useReplaysFromIssue.spec.tsx (2 occ) - spanEvidencePreview.spec.tsx (2 occ) - groupingInfoSection.spec.tsx (2 occ) Made-with: Cursor
JoshuaKGoldberg
added
Scope: Frontend
JoshuaKGoldberg
changed the title
Un-skips and fixes the flaky "arrowup+shift scrolls to the start of the list" test by using waitFor with longer timeouts for the virtualized row assertions. The findByText assertions could time out before the virtualized list finished re-rendering after a large scroll. Made-with: Cursor
JoshuaKGoldberg
force-pushed
the
fix/flaky-trace-keyboard-nav-spec
branch
from
1b2ad25 to
ee10dda
Compare
JoshuaKGoldberg
changed the base branch from
fix/flaky-test-rerun-infra
to
master
JoshuaKGoldberg
removed
the
Frontend: Rerun Flaky Tests
JoshuaKGoldberg
added
the
Frontend: Rerun Flaky Tests
during search, expanding a row retriggers search used within(container).findByText(/transaction-op-0/i), which throws when multiple nodes match the same op label (e.g. virtualized rows plus other UI). Match keyboardNavigationTestSetup by scoping to the virtualized list and using findAllByText for transaction-op- rows. Fixes CI failure in Jest (1) for trace.spec. Fixes BROWSE-411. Made-with: Cursor
… rerun Wait for the search result iterator to advance to the second match after arrowdown before asserting the highlighted row index. Combine highlight assertions into a single waitFor so the index is checked on the same DOM snapshot as the highlight count. Re-query virtualized rows immediately before each click so clicks target current nodes after list updates. Raise the known-flake timeout for the row-click test to 12s so repeated search resolution under load stays within Jest limits. Fixes BROWSE-411. Made-with: Cursor
The search result iterator can remain on the first match index while arrowdown moves the highlighted row, so waiting for a 2/ prefix was incorrect and failed in CI. Allow a longer waitFor in assertHighlightedRowAtIndex for the highlighted-persistence test and raise that known-flake test Jest timeout so slow virtualized updates can finish. Fixes BROWSE-411. Made-with: Cursor
ArrowDown from the search field did not reliably move the active search highlight to the second result within the wait window under CI load. Click the second visible trace row after results resolve instead, which matches user intent and stabilizes the setup for the persistence assertions. Fixes BROWSE-411. Made-with: Cursor
assertHighlightedRowAtIndex used the full render container while row clicks used getVirtualizedRows(virtualizedContainer), so visible-row indices could disagree when other TraceRow nodes exist outside the list. Fixes BROWSE-411. Made-with: Cursor
Visible row index 2 is not always the second search hit, and DOM node identity does not survive re-renders. Click the second .SearchResult row, record the highlighted row TraceOperation text, then assert that text is unchanged after narrowing the query. Fixes BROWSE-411. Made-with: Cursor
…test Click the search field before appending act so typing applies to the input. Wait for focus before pasting the no-match query. Restore container-scoped highlight count for the no-results case to match prior behavior. Fixes BROWSE-411. Made-with: Cursor
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
1 participant
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
trace view › keyboard navigation › arrowup+shift scrolls to the start of the listwhich was previously skipped withit.skiptransaction-op-0is visible. The flake occurred because thefindByTextassertion could time out before the virtualized list finished re-rendering after a large scrollRoot Cause
After
Shift+ArrowUpscrolls the virtualized list from item 99 back to item 0 across 100 rows, the test immediately calledfindByText(/transaction-op-0/i)which has a default 1s timeout. The virtualized list may not have finished re-rendering the first row within that window, causing the flake.Additionally, the test used inconsistent patterns compared to its working sibling test (
arrowdown+shift scrolls to the end of the list):getVirtualizedRows(virtualizedContainer)instead ofcontainer.querySelectorAll(VISIBLE_TRACE_ROW_SELECTOR)rows[1]instead ofrows[0]Fix
Shift+ArrowUp, wait forrows[0]to receive focus (viawaitFor) before checking for the text. Focus settling confirms the virtualized list has completed scrolling and rendering. This matches the pattern used in the workingarrowdown on last node jumps to starttest.container.querySelectorAll(VISIBLE_TRACE_ROW_SELECTOR)to match the sibling test pattern.rows[0]: Match the sibling test for consistency.it.skipand the associated eslint-disable comment.Fixes BROWSE-411
Note that CI Jest tests are failing because the Frontend: Rerun Flaky TestsKnown flaky tests should be run many times, just to be safe.
label is causing other, still-flaky tests to be run.
Made with Cursor