From 6de74a981f650fcb0d7af364cb5e31f64d06bb67 Mon Sep 17 00:00:00 2001 From: ajay-sentry <159853603+ajay-sentry@users.noreply.github.com> Date: Fri, 26 Jul 2024 08:47:49 -0700 Subject: [PATCH] Feat: Fix Failed Test Table Sort, link Hook, and add Spec (#3054) Co-authored-by: Rula Abu Hasna --- .../FailedTestsTable.spec.tsx | 368 ++++++++++++++++++ .../FailedTestsTable/FailedTestsTable.tsx | 322 ++++++++------- .../RepoPage/FailedTestsTab/hooks/index.ts | 7 +- .../hooks/useInfiniteTestResults.tsx | 31 +- 4 files changed, 560 insertions(+), 168 deletions(-) create mode 100644 src/pages/RepoPage/FailedTestsTab/FailedTestsTable/FailedTestsTable.spec.tsx diff --git a/src/pages/RepoPage/FailedTestsTab/FailedTestsTable/FailedTestsTable.spec.tsx b/src/pages/RepoPage/FailedTestsTab/FailedTestsTable/FailedTestsTable.spec.tsx new file mode 100644 index 0000000000..2b7de6ca22 --- /dev/null +++ b/src/pages/RepoPage/FailedTestsTab/FailedTestsTable/FailedTestsTable.spec.tsx @@ -0,0 +1,368 @@ +import { QueryClient, QueryClientProvider } from '@tanstack/react-query' +import { + render, + screen, + waitFor, + waitForElementToBeRemoved, +} from '@testing-library/react' +import userEvent from '@testing-library/user-event' +import { graphql } from 'msw' +import { setupServer } from 'msw/node' +import { mockIsIntersecting } from 'react-intersection-observer/test-utils' +import { MemoryRouter, Route } from 'react-router-dom' + +import FailedTestsTable from './FailedTestsTable' + +import { OrderingDirection, OrderingParameter } from '../hooks' + +const node1 = { + updatedAt: '2023-01-01T00:00:00Z', + name: 'test-1', + commitsFailed: 1, + failureRate: 0.1, + avgDuration: 10, +} + +const node2 = { + updatedAt: '2023-01-02T00:00:00Z', + name: 'test-2', + commitsFailed: 2, + failureRate: 0.2, + avgDuration: 20, +} + +const node3 = { + updatedAt: '2023-01-03T00:00:00Z', + name: 'test-3', + commitsFailed: 3, + failureRate: 0.3, + avgDuration: 30, +} + +const server = setupServer() +const wrapper = + (queryClient: QueryClient): React.FC => + ({ children }) => + ( + + + {children} + + + ) + +beforeAll(() => { + server.listen() +}) + +afterEach(() => { + server.resetHandlers() +}) + +afterAll(() => { + server.close() +}) + +let consoleError: any +let consoleWarn: any + +beforeEach(() => { + // Mock console.error and console.warn + consoleError = jest.spyOn(console, 'error').mockImplementation(() => {}) + consoleWarn = jest.spyOn(console, 'warn').mockImplementation(() => {}) +}) + +afterEach(() => { + // Restore console methods + consoleError.mockRestore() + consoleWarn.mockRestore() +}) + +interface SetupArgs { + noEntries?: boolean + bundleAnalysisEnabled?: boolean +} + +describe('FailedTestsTable', () => { + function setup({ noEntries = false }: SetupArgs) { + const queryClient = new QueryClient() + + const user = userEvent.setup() + const mockSort = jest.fn() + + server.use( + graphql.query('GetTestResults', (req, res, ctx) => { + mockSort(req.variables) + if (noEntries) { + return res( + ctx.status(200), + ctx.data({ + owner: { + repository: { + __typename: 'Repository', + testResults: { + edges: [], + pageInfo: { + hasNextPage: false, + endCursor: null, + }, + }, + }, + }, + }) + ) + } + + const dataReturned = { + owner: { + repository: { + __typename: 'Repository', + testResults: { + edges: req.variables.after + ? [ + { + node: node3, + }, + ] + : [ + { + node: node1, + }, + { + node: node2, + }, + ], + pageInfo: { + hasNextPage: req.variables.after ? false : true, + endCursor: req.variables.after + ? 'aa' + : 'MjAyMC0wOC0xMSAxNzozMDowMiswMDowMHwxMDA=', + }, + }, + }, + }, + } + return res(ctx.status(200), ctx.data(dataReturned)) + }) + ) + + return { queryClient, user, mockSort } + } + + describe('renders table headers', () => { + it('renders each column name', async () => { + const { queryClient } = setup({}) + render(, { + wrapper: wrapper(queryClient), + }) + + const nameColumn = await screen.findByText('Test name') + expect(nameColumn).toBeInTheDocument() + + const durationColumn = await screen.findByText('Average duration') + expect(durationColumn).toBeInTheDocument() + + const failureRateColumn = await screen.findByText('Failure rate') + expect(failureRateColumn).toBeInTheDocument() + + const commitFailedColumn = await screen.findByText('Commits failed') + expect(commitFailedColumn).toBeInTheDocument() + + const lastRunColumn = await screen.findByText('Last run') + expect(lastRunColumn).toBeInTheDocument() + }) + }) + + describe('renders table body', () => { + it('renders the first element', async () => { + const { queryClient } = setup({}) + render(, { + wrapper: wrapper(queryClient), + }) + + const loading = await screen.findByText('Loading') + mockIsIntersecting(loading, false) + + const nameColumn = await screen.findByText('test-1') + expect(nameColumn).toBeInTheDocument() + + const durationColumn = await screen.findByText('10.000s') + expect(durationColumn).toBeInTheDocument() + + const failureRateColumn = await screen.findByText('10.00%') + expect(failureRateColumn).toBeInTheDocument() + + const commitFailedColumn = await screen.findByText('1') + expect(commitFailedColumn).toBeInTheDocument() + + const lastRunColumn = await screen.findAllByText('over 1 year ago') + expect(lastRunColumn.length).toBeGreaterThan(0) + }) + }) + + describe('no data is returned', () => { + it('still returns an empty table', async () => { + const { queryClient } = setup({ noEntries: true }) + render(, { + wrapper: wrapper(queryClient), + }) + + expect(consoleError).not.toHaveBeenCalled() + expect(consoleWarn).not.toHaveBeenCalled() + }) + }) + + describe('ability to sort', () => { + it('can sort on duration column', async () => { + const { queryClient, user, mockSort } = setup({ noEntries: true }) + render(, { + wrapper: wrapper(queryClient), + }) + + const durationColumn = await screen.findByText('Average duration') + await user.click(durationColumn) + + await waitFor(() => { + expect(mockSort).toHaveBeenCalledWith( + expect.objectContaining({ + ordering: { + direction: OrderingDirection.DESC, + parameter: OrderingParameter.AVG_DURATION, + }, + }) + ) + }) + + await user.click(durationColumn) + + await waitFor(() => { + expect(mockSort).toHaveBeenCalledWith( + expect.objectContaining({ + ordering: { + direction: OrderingDirection.ASC, + parameter: OrderingParameter.AVG_DURATION, + }, + }) + ) + }) + }) + + it('can sort on failure rate column', async () => { + const { queryClient, user, mockSort } = setup({ noEntries: true }) + render(, { + wrapper: wrapper(queryClient), + }) + + const failureRateColumn = await screen.findByText('Failure rate') + await user.click(failureRateColumn) + + await waitFor(() => { + expect(mockSort).toHaveBeenCalledWith( + expect.objectContaining({ + ordering: { + direction: OrderingDirection.DESC, + parameter: OrderingParameter.FAILURE_RATE, + }, + }) + ) + }) + + await user.click(failureRateColumn) + + await waitFor(() => { + expect(mockSort).toHaveBeenCalledWith( + expect.objectContaining({ + ordering: { + direction: OrderingDirection.ASC, + parameter: OrderingParameter.FAILURE_RATE, + }, + }) + ) + }) + }) + + it('can sort on commits failed column', async () => { + const { queryClient, user, mockSort } = setup({ noEntries: true }) + render(, { + wrapper: wrapper(queryClient), + }) + + const commitsFailedColumn = await screen.findByText('Commits failed') + await user.click(commitsFailedColumn) + + await waitFor(() => { + expect(mockSort).toHaveBeenCalledWith( + expect.objectContaining({ + ordering: { + direction: OrderingDirection.DESC, + parameter: OrderingParameter.COMMITS_WHERE_FAIL, + }, + }) + ) + }) + + await user.click(commitsFailedColumn) + + await waitFor(() => { + expect(mockSort).toHaveBeenCalledWith( + expect.objectContaining({ + ordering: { + direction: OrderingDirection.ASC, + parameter: OrderingParameter.COMMITS_WHERE_FAIL, + }, + }) + ) + }) + }) + + it('can sort on last run column', async () => { + const { queryClient, user, mockSort } = setup({ noEntries: true }) + render(, { + wrapper: wrapper(queryClient), + }) + + const lastRunColumn = await screen.findByText('Last run') + await user.click(lastRunColumn) + + await waitFor(() => { + expect(mockSort).toHaveBeenCalledWith( + expect.objectContaining({ + ordering: { + direction: OrderingDirection.DESC, + parameter: OrderingParameter.UPDATED_AT, + }, + }) + ) + }) + + await user.click(lastRunColumn) + + await waitFor(() => { + expect(mockSort).toHaveBeenCalledWith( + expect.objectContaining({ + ordering: { + direction: OrderingDirection.ASC, + parameter: OrderingParameter.UPDATED_AT, + }, + }) + ) + }) + }) + }) + + describe('infinite scrolling', () => { + it('loads next page', async () => { + const { queryClient } = setup({}) + render(, { + wrapper: wrapper(queryClient), + }) + + const loading = await screen.findByText('Loading') + mockIsIntersecting(loading, true) + await waitForElementToBeRemoved(loading) + + const thirdCommit = await screen.findByText('test-3') + expect(thirdCommit).toBeInTheDocument() + }) + }) +}) diff --git a/src/pages/RepoPage/FailedTestsTab/FailedTestsTable/FailedTestsTable.tsx b/src/pages/RepoPage/FailedTestsTab/FailedTestsTable/FailedTestsTable.tsx index 01b90eedc6..1ab886a98f 100644 --- a/src/pages/RepoPage/FailedTestsTab/FailedTestsTable/FailedTestsTable.tsx +++ b/src/pages/RepoPage/FailedTestsTab/FailedTestsTable/FailedTestsTable.tsx @@ -1,40 +1,46 @@ -import type { SortingState } from '@tanstack/react-table' import { createColumnHelper, flexRender, getCoreRowModel, getSortedRowModel, + SortingState, useReactTable, } from '@tanstack/react-table' import cs from 'classnames' -import { useState } from 'react' -// import { useInView } from 'react-intersection-observer' -// import { useParams } from 'react-router-dom' +import { useEffect, useMemo, useState } from 'react' +import { useInView } from 'react-intersection-observer' +import { useParams } from 'react-router-dom' -import { OrderingDirection } from 'services/repos' +import { formatTimeToNow } from 'shared/utils/dates' import Icon from 'ui/Icon' import Spinner from 'ui/Spinner' +import { + OrderingDirection, + OrderingParameter, + useInfiniteTestResults, +} from '../hooks' + const Loader = () => (
) -// function LoadMoreTrigger({ -// intersectionRef, -// }: { -// intersectionRef: React.Ref -// }) { -// return ( -// -// Loading -// -// ) -// } +function LoadMoreTrigger({ + intersectionRef, +}: { + intersectionRef: React.Ref +}) { + return ( + + Loading + + ) +} export function getSortingOption( sorting: Array<{ id: string; desc: boolean }> @@ -46,28 +52,26 @@ export function getSortingOption( ? OrderingDirection.DESC : OrderingDirection.ASC - let ordering - if (state.id === 'testName') { - ordering = 'NAME' - } + let parameter: keyof typeof OrderingParameter = + OrderingParameter.COMMITS_WHERE_FAIL - if (state.id === 'lastDuration') { - ordering = 'LAST DURATION' + if (state.id === 'avgDuration') { + parameter = OrderingParameter.AVG_DURATION } if (state.id === 'failureRate') { - ordering = 'FAILURE RATE' + parameter = OrderingParameter.FAILURE_RATE } if (state.id === 'commitsFailed') { - ordering = 'COMMITS FAILED' + parameter = OrderingParameter.COMMITS_WHERE_FAIL } if (state.id === 'updatedAt') { - ordering = 'LAST RUN' + parameter = OrderingParameter.UPDATED_AT } - return { direction, ordering } + return { direction, parameter } } return undefined @@ -75,9 +79,9 @@ export function getSortingOption( interface FailedTestsColumns { name: string - avgDuration: number - failureRate: number - commitsFailed: number + avgDuration: number | null + failureRate: number | null + commitsFailed: number | null updatedAt: string } @@ -85,177 +89,163 @@ const columnHelper = createColumnHelper() const columns = [ columnHelper.accessor('name', { - header: 'Test name', + header: () => 'Test name', cell: (info) => info.renderValue(), }), columnHelper.accessor('avgDuration', { - header: 'Average duration', - cell: (info) => info.renderValue(), + header: () => 'Average duration', + cell: (info) => `${(info.renderValue() ?? 0).toFixed(3)}s`, }), columnHelper.accessor('failureRate', { - header: 'Failure rate', - cell: (info) => info.renderValue(), + header: () => 'Failure rate', + cell: (info) => { + const value = (info.renderValue() ?? 0) * 100 + const isInt = Number.isInteger(info.renderValue()) + return isInt ? `${value}%` : `${value.toFixed(2)}%` + }, }), columnHelper.accessor('commitsFailed', { - header: 'Commits failed', - cell: (info) => info.renderValue(), + header: () => 'Commits failed', + cell: (info) => (info.renderValue() ? info.renderValue() : 0), }), columnHelper.accessor('updatedAt', { - header: 'Last run', - cell: (info) => info.renderValue(), + header: () => 'Last run', + cell: (info) => formatTimeToNow(info.renderValue()), }), ] +interface URLParams { + provider: string + owner: string + repo: string +} + const FailedTestsTable = () => { - // const { ref, inView } = useInView() + const { ref, inView } = useInView() const [sorting, setSorting] = useState([ { id: 'commitsFailed', desc: true, }, ]) - // const { owner } = useParams<{ owner: string }>() - - // const { - // data: reposData, - // fetchNextPage, - // hasNextPage, - // isFetching, - // isFetchingNextPage, - // } = useReposTeam({ - // activated: false, - // sortItem: getSortingOption(sorting), - // owner, - // }) + const { provider, owner, repo } = useParams() + + const { + data: testData, + fetchNextPage, + hasNextPage, + isFetchingNextPage, + isLoading, + } = useInfiniteTestResults({ + provider, + owner, + repo, + ordering: getSortingOption(sorting), + opts: { + suspense: false, + }, + }) - // const tableData = useMemo(() => { - // const data = reposData?.pages?.map((page) => page?.repos).flat() - // return data ?? [] - // }, [reposData?.pages]) + const tableData = useMemo(() => { + return testData?.testResults + }, [testData]) const table = useReactTable({ columns, - data: [ - { - name: 'blah', - avgDuration: 123, - failureRate: 1, - commitsFailed: 4, - updatedAt: '2021-01-01', - }, - { - name: 'cool', - avgDuration: 123, - failureRate: 2, - commitsFailed: 3, - updatedAt: '2022-01-01', - }, - { - name: 'rad guy', - avgDuration: 123, - failureRate: 3, - commitsFailed: 2, - updatedAt: '2023-01-01', - }, - { - name: 'awww ok', - avgDuration: 123, - failureRate: 4, - commitsFailed: 1, - updatedAt: '2024-01-01', - }, - ], + data: tableData ?? [], state: { sorting, }, onSortingChange: setSorting, getCoreRowModel: getCoreRowModel(), getSortedRowModel: getSortedRowModel(), + // debugAll: true, }) - // useEffect(() => { - // if (inView && hasNextPage) { - // fetchNextPage() - // } - // }, [fetchNextPage, inView, hasNextPage]) - - const isLoading = false + useEffect(() => { + if (inView && hasNextPage) { + fetchNextPage() + } + }, [fetchNextPage, inView, hasNextPage]) return ( -
- - - - - - - - - - {table.getHeaderGroups().map((headerGroup) => ( - - {headerGroup.headers.map((header) => ( - + + {isLoading ? ( + + + + ) : ( + table.getRowModel().rows.map((row) => ( + + {row.getVisibleCells().map((cell) => ( + + ))} + + )) + )} + +
-
+
+ + + + + + + + + + {table.getHeaderGroups().map((headerGroup) => ( + + {headerGroup.headers.map((header) => ( + - ))} - - ))} - - - {isLoading ? ( - - - - ) : ( - table.getRowModel().rows.map((row) => ( - - {row.getVisibleCells().map((cell) => ( - + {flexRender( + header.column.columnDef.header, + header.getContext() + )} + + + + + ))} - )) - )} - -
- {flexRender( - header.column.columnDef.header, - header.getContext() - )} - - - - -
- -
- {flexRender(cell.column.columnDef.cell, cell.getContext())} -
- {/* {isFetching ? : null} - {hasNextPage ? : null} */} -
+ ))} +
+ +
+ {flexRender( + cell.column.columnDef.cell, + cell.getContext() + )} +
+
+ {isFetchingNextPage ? : null} + {hasNextPage ? : null} + ) } diff --git a/src/pages/RepoPage/FailedTestsTab/hooks/index.ts b/src/pages/RepoPage/FailedTestsTab/hooks/index.ts index 8e38e0e716..b356624d52 100644 --- a/src/pages/RepoPage/FailedTestsTab/hooks/index.ts +++ b/src/pages/RepoPage/FailedTestsTab/hooks/index.ts @@ -1 +1,6 @@ -export { useInfiniteTestResults } from './useInfiniteTestResults' +export { + useInfiniteTestResults, + OrderingDirection, + TestResultsOrdering, + OrderingParameter, +} from './useInfiniteTestResults' diff --git a/src/pages/RepoPage/FailedTestsTab/hooks/useInfiniteTestResults.tsx b/src/pages/RepoPage/FailedTestsTab/hooks/useInfiniteTestResults.tsx index 94c0cf294d..63df1e1f9b 100644 --- a/src/pages/RepoPage/FailedTestsTab/hooks/useInfiniteTestResults.tsx +++ b/src/pages/RepoPage/FailedTestsTab/hooks/useInfiniteTestResults.tsx @@ -2,6 +2,7 @@ import { useInfiniteQuery, type UseInfiniteQueryOptions, } from '@tanstack/react-query' +import { useMemo } from 'react' import { z } from 'zod' import { @@ -21,6 +22,23 @@ const TestResultSchema = z.object({ avgDuration: z.number().nullable(), }) +export const OrderingDirection = { + DESC: 'DESC', + ASC: 'ASC', +} as const + +export const OrderingParameter = { + AVG_DURATION: 'AVG_DURATION', + FAILURE_RATE: 'FAILURE_RATE', + COMMITS_WHERE_FAIL: 'COMMITS_WHERE_FAIL', + UPDATED_AT: 'UPDATED_AT', +} as const + +export const TestResultsOrdering = z.object({ + direction: z.nativeEnum(OrderingDirection), + parameter: z.nativeEnum(OrderingParameter), +}) + type TestResult = z.infer const GetTestResultsSchema = z.object({ @@ -53,6 +71,7 @@ query GetTestResults( $owner: String! $repo: String! $filters: TestResultsFilters + $ordering: TestResultsOrdering $first: Int $after: String $last: Int @@ -64,6 +83,7 @@ query GetTestResults( ... on Repository { testResults( filters: $filters + ordering: $ordering first: $first after: $after last: $last @@ -102,6 +122,7 @@ interface UseTestResultsArgs { filters?: { branch?: string } + ordering?: z.infer first?: number after?: string last?: number @@ -121,6 +142,7 @@ export const useInfiniteTestResults = ({ after, last, before, + ordering, opts = {}, }: UseTestResultsArgs) => { const { data, ...rest } = useInfiniteQuery({ @@ -134,6 +156,7 @@ export const useInfiniteTestResults = ({ after, last, before, + ordering, ], queryFn: ({ pageParam = after, signal }) => Api.graphql({ @@ -145,6 +168,7 @@ export const useInfiniteTestResults = ({ owner, repo, filters, + ordering, first, after: pageParam, last, @@ -206,9 +230,14 @@ export const useInfiniteTestResults = ({ ...opts, }) + const memoedData = useMemo( + () => data?.pages?.flatMap((page) => page.testResults) ?? [], + [data] + ) + return { data: { - testResults: data?.pages?.flatMap((page) => page.testResults) ?? [], + testResults: memoedData, }, ...rest, }