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

fix: TA finishing touches and just in case fixes #3487

Merged
merged 1 commit into from
Nov 12, 2024
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
Expand Up @@ -223,7 +223,7 @@ describe('CodecovCLI', () => {
expect(link).toBeInTheDocument()
expect(link).toHaveAttribute(
'href',
'https://docs.codecov.com/docs/test-result-ingestion-beta#failed-test-reporting'
'https://docs.codecov.com/docs/test-analytics#failed-test-reporting'
)
})
})
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -270,7 +270,7 @@ describe('FailedTestsTable', () => {
const nameColumn = await screen.findByText('Test name')
expect(nameColumn).toBeInTheDocument()

const durationColumn = await screen.findByText('Avg duration')
const durationColumn = await screen.findByText('Avg. duration')
expect(durationColumn).toBeInTheDocument()

const failureRateColumn = await screen.findByText('Failure rate')
Expand Down Expand Up @@ -366,7 +366,7 @@ describe('FailedTestsTable', () => {
wrapper: wrapper(queryClient),
})

const durationColumn = await screen.findByText('Avg duration')
const durationColumn = await screen.findByText('Avg. duration')
await user.click(durationColumn)

await waitFor(() => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,7 @@ const getColumns = ({
cell: (info) => info.renderValue(),
}),
columnHelper.accessor('avgDuration', {
header: () => 'Avg duration',
header: () => 'Avg. duration',
cell: (info) => `${(info.renderValue() ?? 0).toFixed(3)}s`,
}),
columnHelper.accessor('failureRate', {
Expand Down Expand Up @@ -238,42 +238,40 @@ const FailedTestsTable = () => {
const tableData = useMemo(() => {
if (!testData?.testResults) return []

return (
testData.testResults.map((result) => {
const value = (result.flakeRate ?? 0) * 100
const isFlakeInt = Number.isInteger(value)

const FlakeRateContent = (
<Tooltip delayDuration={0} skipDelayDuration={100}>
<Tooltip.Root>
<Tooltip.Trigger className="underline decoration-dotted decoration-1 underline-offset-4">
{isFlakeInt ? `${value}%` : `${value.toFixed(2)}%`}
</Tooltip.Trigger>
<Tooltip.Portal>
<Tooltip.Content
className="bg-ds-gray-primary p-2 text-xs text-ds-gray-octonary"
side="right"
>
{result.totalPassCount} Passed, {result.totalFailCount} Failed
({result.totalFlakyFailCount} Flaky), {result.totalSkipCount}{' '}
Skipped
<Tooltip.Arrow className="size-4 fill-ds-gray-primary" />
</Tooltip.Content>
</Tooltip.Portal>
</Tooltip.Root>
</Tooltip>
)

return {
name: result.name,
avgDuration: result.avgDuration,
failureRate: result.failureRate,
flakeRate: FlakeRateContent,
commitsFailed: result.commitsFailed,
updatedAt: result.updatedAt,
}
}) ?? []
)
return testData.testResults.map((result) => {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

huge diff but it's just the removal of ??

const value = (result.flakeRate ?? 0) * 100
const isFlakeInt = Number.isInteger(value)

const FlakeRateContent = (
<Tooltip delayDuration={0} skipDelayDuration={100}>
<Tooltip.Root>
<Tooltip.Trigger className="underline decoration-dotted decoration-1 underline-offset-4">
{isFlakeInt ? `${value}%` : `${value.toFixed(2)}%`}
</Tooltip.Trigger>
<Tooltip.Portal>
<Tooltip.Content
className="bg-ds-gray-primary p-2 text-xs text-ds-gray-octonary"
side="right"
>
{result.totalPassCount} Passed, {result.totalFailCount} Failed (
{result.totalFlakyFailCount} Flaky), {result.totalSkipCount}{' '}
Skipped
<Tooltip.Arrow className="size-4 fill-ds-gray-primary" />
</Tooltip.Content>
</Tooltip.Portal>
</Tooltip.Root>
</Tooltip>
)

return {
name: result.name,
avgDuration: result.avgDuration,
failureRate: result.failureRate,
flakeRate: FlakeRateContent,
commitsFailed: result.commitsFailed,
updatedAt: result.updatedAt,
}
})
}, [testData?.testResults])

const columns = useMemo(
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { useState } from 'react'
import { useMemo, useState } from 'react'
import { useHistory, useParams } from 'react-router-dom'

import { Branch, useBranch, useBranches } from 'services/branches'
Expand Down Expand Up @@ -76,15 +76,20 @@ const BranchSelector = () => {
)
}

const sortedBranchList = overview?.defaultBranch
? [
const sortedBranchList = useMemo(() => {
if (!branchList?.branches) return []
Copy link
Contributor Author

Choose a reason for hiding this comment

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

adds a check for !branchList?.branches to clean things up a bit

And wraps the whole thing in a useMemo


if (overview?.defaultBranch) {
return [
// Pins the default branch to the top of the list always, filters it from results otherwise
{ name: overview.defaultBranch, head: null },
...(branchList?.branches?.filter(
...branchList.branches.filter(
(branch) => branch.name !== overview.defaultBranch
) ?? []),
),
]
: (branchList?.branches ?? [])
}
return branchList.branches
}, [overview?.defaultBranch, branchList.branches])

return (
<div className="flex w-full flex-col gap-1 px-4 lg:w-64 xl:w-80">
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -196,7 +196,7 @@ describe('SelectorSection', () => {

expect(link).toHaveAttribute(
'href',
'https://docs.codecov.com/docs/test-result-ingestion-beta#data-retention'
'https://docs.codecov.com/docs/test-analytics#data-retention'
)
})
})
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,12 +15,14 @@ const FlakeAggregatesSchema = z.object({
__typename: z.literal('Repository'),
testAnalytics: z
.object({
flakeAggregates: z.object({
flakeCount: z.number(),
flakeCountPercentChange: z.number().nullable(),
flakeRate: z.number(),
flakeRatePercentChange: z.number().nullable(),
}),
flakeAggregates: z
.object({
flakeCount: z.number(),
flakeCountPercentChange: z.number().nullable(),
flakeRate: z.number(),
flakeRatePercentChange: z.number().nullable(),
})
.nullable(),
Copy link
Contributor Author

Choose a reason for hiding this comment

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

nullable just in case

})
.nullable(),
}),
Expand Down Expand Up @@ -100,6 +102,7 @@ export const useFlakeAggregates = ({
status: 404,
data: {},
dev: 'useFlakeAggregates - 404 Failed to parse data',
error: parsedData.error,
} satisfies NetworkErrorObject)
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -233,6 +233,7 @@ export const useInfiniteTestResults = ({
status: 404,
data: {},
dev: 'useInfiniteTestResults - 404 Failed to parse data',
error: parsedData.error,
} satisfies NetworkErrorObject)
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,18 +22,20 @@ const TestResultsAggregatesSchema = z.object({
defaultBranch: z.string().nullable(),
testAnalytics: z
.object({
testResultsAggregates: z.object({
totalDuration: z.number(),
totalDurationPercentChange: z.number().nullable(),
slowestTestsDuration: z.number(),
slowestTestsDurationPercentChange: z.number().nullable(),
totalSlowTests: z.number(),
totalSlowTestsPercentChange: z.number().nullable(),
totalFails: z.number(),
totalFailsPercentChange: z.number().nullable(),
totalSkips: z.number(),
totalSkipsPercentChange: z.number().nullable(),
}),
testResultsAggregates: z
.object({
totalDuration: z.number(),
totalDurationPercentChange: z.number().nullable(),
slowestTestsDuration: z.number(),
slowestTestsDurationPercentChange: z.number().nullable(),
totalSlowTests: z.number(),
totalSlowTestsPercentChange: z.number().nullable(),
totalFails: z.number(),
totalFailsPercentChange: z.number().nullable(),
totalSkips: z.number(),
totalSkipsPercentChange: z.number().nullable(),
})
.nullable(),
Copy link
Contributor Author

Choose a reason for hiding this comment

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

nullable just in case

})
.nullable(),
}),
Expand Down Expand Up @@ -115,6 +117,7 @@ export const useTestResultsAggregates = ({
status: 404,
data: {},
dev: 'useTestResultsAggregates - 404 Failed to parse data',
error: parsedData.error,
} satisfies NetworkErrorObject)
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,7 @@ export const useTestResultsFlags = ({ term }: { term?: string }) => {
status: 404,
data: {},
dev: 'useTestResultsFlags - 404 Failed to parse data',
error: parsedData.error,
} satisfies NetworkErrorObject)
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,7 @@ export const useTestResultsTestSuites = ({ term }: { term?: string }) => {
status: 404,
data: {},
dev: 'useTestResultsTestSuites - 404 Failed to parse data',
error: parsedData.error,
} satisfies NetworkErrorObject)
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -213,7 +213,7 @@ describe('GitHubActions', () => {
expect(link).toBeInTheDocument()
expect(link).toHaveAttribute(
'href',
'https://docs.codecov.com/docs/test-result-ingestion-beta#failed-test-reporting'
'https://docs.codecov.com/docs/test-analytics#failed-test-reporting'
)
})
})
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -89,8 +89,8 @@ describe('useStaticNavLinks', () => {
${links.quickStart} | ${'https://docs.codecov.com/docs/quick-start'}
${links.installSelfHosted} | ${'https://docs.codecov.com/docs/installing-codecov-self-hosted'}
${links.login} | ${`/login`}
${links.testsAnalytics} | ${'https://docs.codecov.com/docs/test-result-ingestion-beta#failed-test-reporting'}
${links.testsAnalyticsDataRetention} | ${'https://docs.codecov.com/docs/test-result-ingestion-beta#data-retention'}
${links.testsAnalytics} | ${'https://docs.codecov.com/docs/test-analytics#failed-test-reporting'}
${links.testsAnalyticsDataRetention} | ${'https://docs.codecov.com/docs/test-analytics#data-retention'}
${links.expiredReports} | ${'https://docs.codecov.com/docs/codecov-yaml#section-expired-reports'}
${links.unusableReports} | ${'https://docs.codecov.com/docs/error-reference#unusable-reports'}
${links.demoRepo} | ${'/github/codecov/gazebo'}
Expand Down
5 changes: 2 additions & 3 deletions src/services/navigation/useNavLinks/useStaticNavLinks.ts
Original file line number Diff line number Diff line change
Expand Up @@ -460,14 +460,13 @@ export function useStaticNavLinks() {
testsAnalytics: {
text: 'Tests Analytics',
path: () =>
'https://docs.codecov.com/docs/test-result-ingestion-beta#failed-test-reporting',
'https://docs.codecov.com/docs/test-analytics#failed-test-reporting',
isExternalLink: true,
openNewTab: true,
},
testsAnalyticsDataRetention: {
text: 'Test Analytics Data Retention',
path: () =>
'https://docs.codecov.com/docs/test-result-ingestion-beta#data-retention',
path: () => 'https://docs.codecov.com/docs/test-analytics#data-retention',
isExternalLink: true,
openNewTab: true,
},
Expand Down
Loading