diff --git a/static/app/views/performance/newTraceDetails/trace.spec.tsx b/static/app/views/performance/newTraceDetails/trace.spec.tsx index 223e423b8b3634..55b3f4b2ae446e 100644 --- a/static/app/views/performance/newTraceDetails/trace.spec.tsx +++ b/static/app/views/performance/newTraceDetails/trace.spec.tsx @@ -4,6 +4,7 @@ import {TransactionEventFixture} from 'sentry-fixture/event'; import {ProjectFixture} from 'sentry-fixture/project'; import { + fireEvent, render, screen, userEvent, @@ -28,6 +29,19 @@ import {DEFAULT_TRACE_VIEW_PREFERENCES} from 'sentry/views/performance/newTraceD import type {TraceFullDetailed} from './traceApi/types'; +jest.mock('sentry/utils/profiling/hooks/useVirtualizedTree/virtualizedTreeUtils', () => { + const actual = jest.requireActual( + 'sentry/utils/profiling/hooks/useVirtualizedTree/virtualizedTreeUtils' + ); + return { + ...actual, + requestAnimationTimeout: (cb: () => void, _delay: number) => { + const id = setTimeout(() => cb(), 0); + return {id}; + }, + }; +}); + class MockResizeObserver { callback: ResizeObserverCallback; constructor(callback: ResizeObserverCallback) { @@ -837,20 +851,24 @@ function printVirtualizedList(container: HTMLElement) { async function assertHighlightedRowAtIndex( virtualizedContainer: HTMLElement, - index: number + index: number, + options?: {timeout?: number} ) { - await waitFor(() => { - expect(virtualizedContainer.querySelectorAll('.TraceRow.Highlight')).toHaveLength(1); - }); - await waitFor(() => { - const highlighted_row = virtualizedContainer.querySelector( - ACTIVE_SEARCH_HIGHLIGHT_ROW - ); - const r = Array.from( - virtualizedContainer.querySelectorAll(VISIBLE_TRACE_ROW_SELECTOR) - ); - expect(r.indexOf(highlighted_row!)).toBe(index); - }); + await waitFor( + () => { + const highlights = virtualizedContainer.querySelectorAll('.TraceRow.Highlight'); + expect(highlights).toHaveLength(1); + const highlighted_row = virtualizedContainer.querySelector( + ACTIVE_SEARCH_HIGHLIGHT_ROW + ); + expect(highlighted_row).toBeTruthy(); + const r = Array.from( + virtualizedContainer.querySelectorAll(VISIBLE_TRACE_ROW_SELECTOR) + ); + expect(r.indexOf(highlighted_row!)).toBe(index); + }, + typeof options?.timeout === 'number' ? {timeout: options.timeout} : {} + ); } describe('trace view', () => { @@ -1451,25 +1469,24 @@ describe('trace view', () => { }); }); - // eslint-disable-next-line jest/no-disabled-tests - it.skip('arrowup+shift scrolls to the start of the list', async () => { - const {virtualizedContainer} = await keyboardNavigationTestSetup(); + it.isKnownFlake('arrowup+shift scrolls to the start of the list', async () => { + const {container, virtualizedContainer} = await keyboardNavigationTestSetup(); - let rows = getVirtualizedRows(virtualizedContainer); + let rows = container.querySelectorAll(VISIBLE_TRACE_ROW_SELECTOR); + await userEvent.click(rows[0]!); - await userEvent.click(rows[1]!); await waitFor(() => { - rows = getVirtualizedRows(virtualizedContainer); - expect(rows[1]).toHaveFocus(); + rows = container.querySelectorAll(VISIBLE_TRACE_ROW_SELECTOR); + expect(rows[0]).toHaveFocus(); }); await userEvent.keyboard('{Shift>}{arrowdown}{/Shift}'); + expect( await within(virtualizedContainer).findByText(/transaction-op-99/i) ).toBeInTheDocument(); - await waitFor(() => { - rows = getVirtualizedRows(virtualizedContainer); + rows = container.querySelectorAll(VISIBLE_TRACE_ROW_SELECTOR); expect(rows[rows.length - 1]).toHaveFocus(); }); @@ -1478,9 +1495,8 @@ describe('trace view', () => { expect( await within(virtualizedContainer).findByText(/transaction-op-0/i) ).toBeInTheDocument(); - await waitFor(() => { - rows = getVirtualizedRows(virtualizedContainer); + rows = container.querySelectorAll(VISIBLE_TRACE_ROW_SELECTOR); expect(rows[0]).toHaveFocus(); }); }); @@ -1536,8 +1552,7 @@ describe('trace view', () => { await assertHighlightedRowAtIndex(container, 1); }); - // eslint-disable-next-line jest/no-disabled-tests - it.skip('supports roving with arrowup and arrowdown', async () => { + it.isKnownFlake('supports roving with arrowup and arrowdown', async () => { const {container} = await searchTestSetup(); const searchInput = await screen.findByPlaceholderText('Search in trace'); @@ -1608,39 +1623,92 @@ describe('trace view', () => { await assertHighlightedRowAtIndex(container, 6); }); - // TODO Abdullah Khan: This is flaky, we need to fix it - // eslint-disable-next-line jest/no-disabled-tests - it.skip('highlighted is persisted on node while it is part of the search results', async () => { - const {container} = await searchTestSetup(); - const searchInput = await screen.findByPlaceholderText('Search in trace'); - await userEvent.type(searchInput, 'trans'); - await waitFor(() => expect(searchInput).toHaveValue('trans')); - // Wait for the search results to resolve - await searchToResolve(); - - await userEvent.keyboard('{arrowdown}'); - await searchToResolve(); + it.isKnownFlake( + 'highlighted is persisted on node while it is part of the search results', + async () => { + const {container, virtualizedContainer} = await searchTestSetup(); + const searchInput = await screen.findByPlaceholderText('Search in trace'); + await userEvent.type(searchInput, 'trans'); + await waitFor(() => expect(searchInput).toHaveValue('trans')); + // Wait for the search results to resolve + await searchToResolve(); + + // Use iterator navigation instead of clicking the second visible virtualized row: + // the row at DOM index [1] is not always the second search match under CI timing. + await userEvent.click(searchInput); + await userEvent.keyboard('{arrowdown}'); + await waitFor( + () => { + expect(screen.getByTestId('trace-search-result-iterator')).toHaveTextContent( + /2\/\d+/ + ); + }, + {timeout: 10_000} + ); - await assertHighlightedRowAtIndex(container, 2); + let persistedTransactionOp = ''; + await waitFor( + () => { + const active = virtualizedContainer.querySelector( + ACTIVE_SEARCH_HIGHLIGHT_ROW + ); + expect(active).toBeTruthy(); + persistedTransactionOp = active! + .querySelector('.TraceOperation')! + .textContent.trim(); + expect(persistedTransactionOp.length).toBeGreaterThan(0); + }, + {timeout: 10_000} + ); - await userEvent.type(searchInput, 'act'); - await waitFor(() => expect(searchInput).toHaveValue('transact')); - await searchToResolve(); + await userEvent.click(searchInput); + await waitFor(() => expect(searchInput).toHaveValue('trans')); + // Single change event avoids intermediate queries (transa, transac, …) resetting the match. + fireEvent.change(searchInput, {target: {value: 'transact'}}); + await waitFor(() => expect(searchInput).toHaveValue('transact'), { + timeout: 10_000, + }); + await searchToResolve(); + + // Highlighting is persisted on the row + await waitFor( + () => { + const active = virtualizedContainer.querySelector( + ACTIVE_SEARCH_HIGHLIGHT_ROW + ); + expect(active).toBeTruthy(); + expect(active!.querySelector('.TraceOperation')!.textContent.trim()).toBe( + persistedTransactionOp + ); + }, + {timeout: 15_000} + ); - // Highlighting is persisted on the row - await assertHighlightedRowAtIndex(container, 2); + await userEvent.clear(searchInput); + await waitFor(() => expect(searchInput).toHaveValue('')); + await userEvent.click(searchInput); + await userEvent.paste('this wont match anything'); + await waitFor(() => expect(searchInput).toHaveValue('this wont match anything'), { + timeout: 10_000, + }); + await searchToResolve(); - await userEvent.clear(searchInput); - await userEvent.click(searchInput); - await userEvent.paste('this wont match anything'); - await waitFor(() => expect(searchInput).toHaveValue('this wont match anything')); - await searchToResolve(); + await waitFor(() => { + expect(screen.getByTestId('trace-search-result-iterator')).toHaveTextContent( + 'no results' + ); + }); - // When there is no match, the highlighting is removed - await waitFor(() => { - expect(container.querySelectorAll('.TraceRow.Highlight')).toHaveLength(0); - }); - }); + // When there is no match, the highlighting is removed + await waitFor( + () => { + expect(container.querySelectorAll('.TraceRow.Highlight')).toHaveLength(0); + }, + {timeout: 10_000} + ); + }, + 28_000 + ); it('auto highlights the first result when search begins', async () => { const {container} = await searchTestSetup(); @@ -1657,44 +1725,49 @@ describe('trace view', () => { await assertHighlightedRowAtIndex(container, 1); }); - // TODO Abdullah Khan: This is flaky, and when it flakes it takes over 90s to run - // eslint-disable-next-line jest/no-disabled-tests - it.skip('clicking a row that is also a search result updates the result index', async () => { - const {container, virtualizedContainer} = await searchTestSetup(); + it.isKnownFlake( + 'clicking a row that is also a search result updates the result index', + async () => { + const {virtualizedContainer} = await searchTestSetup(); - const searchInput = await screen.findByPlaceholderText('Search in trace'); - await userEvent.type(searchInput, 'transaction-op-1'); - await waitFor(() => expect(searchInput).toHaveValue('transaction-op-1')); + const searchInput = await screen.findByPlaceholderText('Search in trace'); + await userEvent.type(searchInput, 'transaction-op-1'); + await waitFor(() => expect(searchInput).toHaveValue('transaction-op-1')); - await searchToResolve(); + await searchToResolve(); - await assertHighlightedRowAtIndex(container, 2); - const rows = getVirtualizedRows(virtualizedContainer); - // By default, we highlight the first result - expect(await screen.findByTestId('trace-search-result-iterator')).toHaveTextContent( - '1/2' - ); + await assertHighlightedRowAtIndex(virtualizedContainer, 2); + // By default, we highlight the first result + expect( + await screen.findByTestId('trace-search-result-iterator') + ).toHaveTextContent('1/2'); - // Click on a random row in the list that is not a search result - await userEvent.click(rows[5]!); - await waitFor(() => { - expect(screen.queryByTestId('trace-search-result-iterator')).toHaveTextContent( - '-/2' - ); - }); + // Click on a random row in the list that is not a search result + await waitFor(() => { + expect(getVirtualizedRows(virtualizedContainer)[5]).toBeTruthy(); + }); + await userEvent.click(getVirtualizedRows(virtualizedContainer)[5]!); + await waitFor(() => { + expect(screen.queryByTestId('trace-search-result-iterator')).toHaveTextContent( + '-/2' + ); + }); - // Click on a the row in the list that is a search result - await userEvent.click(rows[2]!); - await waitFor(() => { - expect(screen.queryByTestId('trace-search-result-iterator')).toHaveTextContent( - '1/2' - ); - }); - }); + // Click on a the row in the list that is a search result + await waitFor(() => { + expect(getVirtualizedRows(virtualizedContainer)[2]).toBeTruthy(); + }); + await userEvent.click(getVirtualizedRows(virtualizedContainer)[2]!); + await waitFor(() => { + expect(screen.queryByTestId('trace-search-result-iterator')).toHaveTextContent( + '1/2' + ); + }); + }, + 12_000 + ); - // Really flakey, blocking deploys - // eslint-disable-next-line jest/no-disabled-tests - it.skip('during search, expanding a row retriggers search', async () => { + it.isKnownFlake('during search, expanding a row retriggers search', async () => { mockPerformanceSubscriptionDetailsResponse(); mockProjectDetailsResponse(); @@ -1788,12 +1861,14 @@ describe('trace view', () => { } ); - const {container} = render(, { + render(, { initialRouterConfig, }); - // Awaits for the placeholder rendering rows to be removed - await within(container).findByText(/transaction-op-0/i); + const virtualizedContainer = getVirtualizedContainer(); + await within(virtualizedContainer).findAllByText(/transaction-op-/i, undefined, { + timeout: 5000, + }); const searchInput = await screen.findByPlaceholderText('Search in trace'); await userEvent.type(searchInput, 'op-0'); @@ -1867,7 +1942,6 @@ describe('trace view', () => { await userEvent.paste('transaction-op-none'); await searchToResolve(); await waitFor(() => { - // eslint-disable-next-line testing-library/no-container expect(container.querySelectorAll('.TraceRow.Highlight')).toHaveLength(0); }); }, 20_000); @@ -1968,37 +2042,38 @@ describe('trace view', () => { }); }); - // TODO Abdullah Khan: This is flaky, and when it flakes it takes over 90s to run - // eslint-disable-next-line jest/no-disabled-tests - it.skip('clicking a node that is already open in a tab switches to that tab and persists the previous node', async () => { - const {virtualizedContainer} = await simpleTestSetup(); - const rows = getVirtualizedRows(virtualizedContainer); - expect(screen.queryAllByTestId(DRAWER_TABS_TEST_ID)).toHaveLength(0); + it.isKnownFlake( + 'clicking a node that is already open in a tab switches to that tab and persists the previous node', + async () => { + const {virtualizedContainer} = await simpleTestSetup(); + const rows = getVirtualizedRows(virtualizedContainer); + expect(screen.queryAllByTestId(DRAWER_TABS_TEST_ID)).toHaveLength(0); - await userEvent.click(rows[5]!); - await waitFor(() => { - expect(screen.queryAllByTestId(DRAWER_TABS_TEST_ID)).toHaveLength(1); - }); + await userEvent.click(rows[5]!); + await waitFor(() => { + expect(screen.queryAllByTestId(DRAWER_TABS_TEST_ID)).toHaveLength(1); + }); - await userEvent.click(await screen.findByTestId(DRAWER_TABS_PIN_BUTTON_TEST_ID)); - await userEvent.click(rows[7]!); + await userEvent.click(await screen.findByTestId(DRAWER_TABS_PIN_BUTTON_TEST_ID)); + await userEvent.click(rows[7]!); - await waitFor(() => { - expect(screen.queryAllByTestId(DRAWER_TABS_TEST_ID)).toHaveLength(2); - }); - expect(screen.queryAllByTestId(DRAWER_TABS_TEST_ID)[1]).toHaveAttribute( - 'aria-selected', - 'true' - ); - - await userEvent.click(rows[5]!); - await waitFor(() => { + await waitFor(() => { + expect(screen.queryAllByTestId(DRAWER_TABS_TEST_ID)).toHaveLength(2); + }); expect(screen.queryAllByTestId(DRAWER_TABS_TEST_ID)[1]).toHaveAttribute( 'aria-selected', 'true' ); - }); - expect(screen.queryAllByTestId(DRAWER_TABS_TEST_ID)).toHaveLength(2); - }); + + await userEvent.click(rows[5]!); + await waitFor(() => { + expect(screen.queryAllByTestId(DRAWER_TABS_TEST_ID)[1]).toHaveAttribute( + 'aria-selected', + 'true' + ); + }); + expect(screen.queryAllByTestId(DRAWER_TABS_TEST_ID)).toHaveLength(2); + } + ); }); });