Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ref: Migrate useBranchCoverageMeasurements to TSQ V5 #3711

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -1,4 +1,8 @@
import { QueryClient, QueryClientProvider } from '@tanstack/react-query'
import {
QueryClientProvider as QueryClientProviderV5,
QueryClient as QueryClientV5,
} from '@tanstack/react-queryV5'
import { render, screen } from '@testing-library/react'
import { graphql, HttpResponse } from 'msw'
import { setupServer } from 'msw/node'
Expand Down Expand Up @@ -77,15 +81,20 @@ const server = setupServer()
const queryClient = new QueryClient({
defaultOptions: { queries: { retry: false, suspense: true } },
})
const queryClientV5 = new QueryClientV5({
defaultOptions: { queries: { retry: false } },
})

const wrapper: React.FC<React.PropsWithChildren> = ({ children }) => (
<QueryClientProvider client={queryClient}>
<MemoryRouter initialEntries={['/gh/test-org/test-repo']}>
<Route path="/:provider/:owner/:repo">
<Suspense fallback={null}>{children}</Suspense>
</Route>
</MemoryRouter>
</QueryClientProvider>
<QueryClientProviderV5 client={queryClientV5}>
<QueryClientProvider client={queryClient}>
<MemoryRouter initialEntries={['/gh/test-org/test-repo']}>
<Route path="/:provider/:owner/:repo">
<Suspense fallback={null}>{children}</Suspense>
</Route>
</MemoryRouter>
</QueryClientProvider>
</QueryClientProviderV5>
)

beforeAll(() => {
Expand All @@ -94,6 +103,7 @@ beforeAll(() => {

afterEach(() => {
queryClient.clear()
queryClientV5.clear()
server.resetHandlers()
})

Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { keepPreviousData } from '@tanstack/react-queryV5'
import { useParams } from 'react-router-dom'

import { useBranches } from 'services/branches'
Expand Down Expand Up @@ -30,23 +31,19 @@ function CoverageTrend() {
defaultBranch: overview?.defaultBranch ?? '',
})

const { data, isLoading } = useRepoCoverageTimeseries({
const { data, isPending, isSuccess } = useRepoCoverageTimeseries({
branch: selection?.name,
options: {
enabled: !!selection?.name,
suspense: false,
keepPreviousData: true,
},
options: { enabled: !!selection?.name, placeholderData: keepPreviousData },
})

return (
<SummaryField>
<TrendDropdown />
<div className="flex items-center gap-2 pb-[1.3rem]">
{/* ^ CSS doesn't want to render like the others without a p tag in the dom. */}
{isLoading ? (
{isPending ? (
<Spinner />
) : data?.measurements?.length > 0 ? (
) : isSuccess && data.measurements.length > 0 ? (
<TotalsNumber value={data?.coverageChange ?? 0} light showChange />
) : (
<p className="text-sm font-medium">
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,8 @@
import { QueryClient, QueryClientProvider } from '@tanstack/react-query'
import {
QueryClientProvider as QueryClientProviderV5,
QueryClient as QueryClientV5,
} from '@tanstack/react-queryV5'
import { renderHook, waitFor } from '@testing-library/react'
import { graphql, HttpResponse } from 'msw'
import { setupServer } from 'msw/node'
Expand Down Expand Up @@ -58,14 +62,20 @@ const server = setupServer()
const queryClient = new QueryClient({
defaultOptions: { queries: { retry: false } },
})
const queryClientV5 = new QueryClientV5({
defaultOptions: { queries: { retry: false } },
})

const wrapper =
(searchParams = ''): React.FC<React.PropsWithChildren> =>
({ children }) => (
<QueryClientProvider client={queryClient}>
<MemoryRouter initialEntries={[`/gh/caleb/mighty-nein${searchParams}`]}>
<Route path="/:provider/:owner/:repo">{children}</Route>
</MemoryRouter>
</QueryClientProvider>
<QueryClientProviderV5 client={queryClientV5}>
<QueryClientProvider client={queryClient}>
<MemoryRouter initialEntries={[`/gh/caleb/mighty-nein${searchParams}`]}>
<Route path="/:provider/:owner/:repo">{children}</Route>
</MemoryRouter>
</QueryClientProvider>
</QueryClientProviderV5>
)

beforeAll(() => {
Expand All @@ -74,6 +84,7 @@ beforeAll(() => {

afterEach(() => {
queryClient.clear()
queryClientV5.clear()
server.resetHandlers()
})

Expand All @@ -89,7 +100,9 @@ describe('useRepoCoverageTimeseries', () => {
const config = vi.fn()

function setup(
{ noCoverageData = false }: SetupArgs = { noCoverageData: false }
{ noCoverageData = false }: SetupArgs = {
noCoverageData: false,
}
) {
server.use(
graphql.query('GetRepoOverview', () => {
Expand Down Expand Up @@ -147,26 +160,19 @@ describe('useRepoCoverageTimeseries', () => {
})
})

describe('with no coverage data', () => {
it('returns an empty array', async () => {
setup({ noCoverageData: true })
const { result } = renderHook(
() => useRepoCoverageTimeseries({ branch: 'c3' }),
{ wrapper: wrapper('') }
)

await waitFor(() => expect(result.current.data?.measurements).toEqual([]))
})

it('returns 0 for coverage change', async () => {
describe('with null coverage data', () => {
it('returns measurements with 0 for coverage', async () => {
setup({ noCoverageData: true })
const { result } = renderHook(
() => useRepoCoverageTimeseries({ branch: 'c3' }),
{ wrapper: wrapper('') }
)

await waitFor(() =>
expect(result.current.data?.coverageChange).toEqual(0)
expect(result.current.data?.measurements).toEqual([
{ coverage: 0, date: new Date('2023-01-01T00:00:00.000Z') },
{ coverage: 0, date: new Date('2023-01-02T00:00:00.000Z') },
])
)
})
})
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,11 @@
import {
keepPreviousData,
useQuery as useQueryV5,
} from '@tanstack/react-queryV5'
import { useMemo } from 'react'
import { useParams } from 'react-router-dom'

import { useBranchCoverageMeasurements } from 'services/charts/useBranchCoverageMeasurements'
import { BranchCoverageMeasurementsQueryOpts } from 'services/charts/BranchCoverageMeasurementsQueryOpts'
import { useLocationParams } from 'services/navigation'
import { useRepoOverview } from 'services/repo'
import {
Expand All @@ -21,8 +25,7 @@ interface UseRepoCoverageTimeseriesArgs {
branch: string
options?: {
enabled?: boolean
suspense?: boolean
keepPreviousData?: boolean
placeholderData?: typeof keepPreviousData
}
}

Expand All @@ -49,59 +52,42 @@ export function useRepoCoverageTimeseries({
return createTimeSeriesQueryVars({ trend, oldestCommit, today })
}, [overview?.oldestCommitAt, params?.trend, today])

const { data, ...rest } = useBranchCoverageMeasurements({
provider,
owner,
repo,
branch,
after: queryVars?.after,
before: today,
interval: queryVars.interval,
opts: {
enabled: !!overview?.oldestCommitAt,
staleTime: 30000,
keepPreviousData: false,
suspense: false,
...options,
},
})

return useMemo(() => {
let coverage = []

if (!data?.measurements) {
return {
...rest,
data: { measurements: [] },
return useQueryV5({
...BranchCoverageMeasurementsQueryOpts({
provider,
owner,
repo,
branch,
after: queryVars?.after,
before: today,
interval: queryVars.interval,
}),
select: (data) => {
let coverage = []
if (data.measurements?.[0]?.max === null) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The optional chains are kinda throwing me off here because we're doing a hard set on the values in the line below 🤔

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is validating that there is an entry in the array that has a field max on it that is null so that we can override the value. You're not able to use optional chaining while assigning a value.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep I get that, it's a little unclear bc we're checking for null explicitly whereas if the value doesn't exist at any other point it'd return undefined. Just one of those "gotchas" / "landmines"

data.measurements[0].max = 0
}
}

if (data?.measurements?.[0]?.max === null) {
data.measurements[0].max = 0
}
// set initial coverage percentage
let prevPercent = data?.measurements?.[0]?.max ?? 0
coverage = data?.measurements?.map((measurement) => {
const coverage = measurement?.max ?? prevPercent

// set set initial t
let prevPercent = data?.measurements?.[0]?.max ?? 0
coverage = data?.measurements?.map((measurement) => {
const coverage = measurement?.max ?? prevPercent
// can save on a few reassignments
if (prevPercent !== coverage) {
prevPercent = coverage
}

// can save on a few reassignments
if (prevPercent !== coverage) {
prevPercent = coverage
}

return {
date: new Date(measurement?.timestamp),
coverage,
}
})
return { date: new Date(measurement?.timestamp), coverage }
})

const coverageChange =
(coverage.at(-1)?.coverage ?? 0) - (coverage.at(0)?.coverage ?? 0)
const coverageChange =
(coverage.at(-1)?.coverage ?? 0) - (coverage.at(0)?.coverage ?? 0)

return {
...rest,
data: { measurements: coverage, coverageChange },
}
}, [data, rest])
return { measurements: coverage, coverageChange }
},
enabled: !!overview?.oldestCommitAt,
staleTime: 30000,
...options,
})
}
Original file line number Diff line number Diff line change
@@ -1,4 +1,8 @@
import { QueryClient, QueryClientProvider } from '@tanstack/react-query'
import {
QueryClientProvider as QueryClientProviderV5,
QueryClient as QueryClientV5,
} from '@tanstack/react-queryV5'
import { render, screen, within } from '@testing-library/react'
import { graphql, HttpResponse } from 'msw'
import { setupServer } from 'msw/node'
Expand Down Expand Up @@ -26,21 +30,27 @@ vi.mock('recharts', async () => {
}
})

const queryClient = new QueryClient({
defaultOptions: { queries: { retry: false } },
})
const queryClientV5 = new QueryClientV5({
defaultOptions: { queries: { retry: false } },
})

const wrapper =
(
initialEntries = ['/gh/codecov/bells-hells/tree/main']
): React.FC<React.PropsWithChildren> =>
({ children }) => (
<QueryClientProvider client={queryClient}>
<MemoryRouter initialEntries={initialEntries}>
<Route path="/:provider/:owner/:repo">{children}</Route>
</MemoryRouter>
</QueryClientProvider>
<QueryClientProviderV5 client={queryClientV5}>
<QueryClientProvider client={queryClient}>
<MemoryRouter initialEntries={initialEntries}>
<Route path="/:provider/:owner/:repo">{children}</Route>
</MemoryRouter>
</QueryClientProvider>
</QueryClientProviderV5>
)

const queryClient = new QueryClient({
defaultOptions: { queries: { retry: false } },
})
const server = setupServer()

beforeAll(() => {
Expand Down Expand Up @@ -73,6 +83,7 @@ beforeEach(() => {

afterEach(() => {
queryClient.clear()
queryClientV5.clear()
server.resetHandlers()
})

Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { keepPreviousData } from '@tanstack/react-queryV5'
import { format } from 'date-fns'
import { useParams } from 'react-router-dom'
import { Area, AreaChart, CartesianGrid, XAxis, YAxis } from 'recharts'
Expand Down Expand Up @@ -43,17 +44,16 @@ function CoverageChart() {
defaultBranch: overview?.defaultBranch ?? '',
})

const { data, isPreviousData, isLoading, isError } =
const { data, isPlaceholderData, isPending, isError } =
useRepoCoverageTimeseries({
branch: selection?.name,
options: {
enabled: !!selection?.name,
suspense: false,
keepPreviousData: true,
placeholderData: keepPreviousData,
},
})

if (!isPreviousData && isLoading) {
if (!isPlaceholderData && isPending) {
return <Placeholder />
}

Expand All @@ -65,7 +65,8 @@ function CoverageChart() {
)
}

if (data?.measurements.length < 1) {
const dataCount = data?.measurements?.length ?? 0
if (dataCount < 1) {
return (
<div className="flex min-h-[250px] items-center justify-center xl:min-h-[380px]">
<p className="text-center">
Expand Down
Loading