Skip to content

feat(Codecov): add test analytics table #91832

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

Merged
merged 5 commits into from
May 22, 2025
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
6 changes: 6 additions & 0 deletions static/app/views/codecov/tests/settings.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
import type {ValidSort} from 'sentry/views/codecov/tests/testAnalyticsTable/testAnalyticsTable';

export const DEFAULT_SORT: ValidSort = {
field: 'commitsFailed',
kind: 'desc',
};
Original file line number Diff line number Diff line change
@@ -0,0 +1,79 @@
import type {ReactNode} from 'react';
import styled from '@emotion/styled';

import Link from 'sentry/components/links/link';
import QuestionTooltip from 'sentry/components/questionTooltip';
import {IconArrow} from 'sentry/icons';
import {space} from 'sentry/styles/space';
import type {Sort} from 'sentry/utils/discover/fields';
import {useLocation} from 'sentry/utils/useLocation';

type HeaderParams = {
alignment: string;
fieldName: string;
label: string;
sort: undefined | Sort;
tooltip?: string | ReactNode;
};

function SortableHeader({fieldName, label, sort, tooltip, alignment}: HeaderParams) {
const location = useLocation();

const arrowDirection = sort?.kind === 'asc' ? 'up' : 'down';
const sortArrow = <IconArrow size="xs" direction={arrowDirection} />;

return (
<HeaderCell alignment={alignment}>
<StyledLink
role="columnheader"
aria-sort={
sort?.field.endsWith(fieldName)
? sort?.kind === 'asc'
? 'ascending'
: 'descending'
: 'none'
}
to={{
pathname: location.pathname,
query: {
...location.query,
sort: sort?.field.endsWith(fieldName)
? sort?.kind === 'desc'
? fieldName
: '-' + fieldName
: '-' + fieldName,
},
}}
>
{label} {sort?.field === fieldName && sortArrow}
</StyledLink>
{tooltip ? (
<StyledQuestionTooltip size="xs" position="top" title={tooltip} isHoverable />
) : null}
</HeaderCell>
);
}

const HeaderCell = styled('div')<{alignment: string}>`
display: block;
width: 100%;
text-align: ${p => (p.alignment === 'left' ? 'left' : 'right')};
`;

const StyledLink = styled(Link)`
color: inherit;

:hover {
color: inherit;
}

svg {
vertical-align: top;
}
`;

const StyledQuestionTooltip = styled(QuestionTooltip)`
margin-left: ${space(0.5)};
`;

export default SortableHeader;
79 changes: 79 additions & 0 deletions static/app/views/codecov/tests/testAnalyticsTable/tableBody.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,79 @@
import styled from '@emotion/styled';

import {Tag} from 'sentry/components/core/badge/tag';
import {DateTime} from 'sentry/components/dateTime';
import PerformanceDuration from 'sentry/components/performanceDuration';
import {space} from 'sentry/styles/space';
import {
type Column,
RIGHT_ALIGNED_FIELDS,
type Row,
} from 'sentry/views/codecov/tests/testAnalyticsTable/testAnalyticsTable';

interface TableBodyProps {
column: Column;
row: Row;
}

export function renderTableBody({column, row}: TableBodyProps) {
const key = column.key;
const value = row[key];
const alignment = RIGHT_ALIGNED_FIELDS.has(key) ? 'right' : 'left';

if (key === 'testName') {
return <Container alignment={alignment}>{value}</Container>;
}

if (key === 'averageDurationMs') {
return (
<NumberContainer>
<PerformanceDuration milliseconds={Number(value)} abbreviation />
</NumberContainer>
);
}

if (key === 'flakeRate') {
const isBrokenTest = row.isBrokenTest;
return (
<NumberContainer>
{isBrokenTest && <StyledTag type={'highlight'}>Broken test</StyledTag>}
{value}%
</NumberContainer>
);
}

if (key === 'commitsFailed') {
return <Container alignment={alignment}>{value}</Container>;
}

if (key === 'lastRun') {
return (
<DateContainer>
<DateTime date={value} year seconds timeZone />
</DateContainer>
);
}

return <Container alignment={alignment}>{value}</Container>;
}

export const Container = styled('div')<{alignment: string}>`
${p => p.theme.overflowEllipsis};
font-family: ${p => p.theme.text.familyMono};
text-align: ${p => (p.alignment === 'left' ? 'left' : 'right')};
`;

const DateContainer = styled('div')`
color: ${p => p.theme.tokens.content.muted};
text-align: 'left';
`;

const NumberContainer = styled('div')`
text-align: right;
font-variant-numeric: tabular-nums;
${p => p.theme.overflowEllipsis};
`;

const StyledTag = styled(Tag)`
margin-right: ${space(1.5)};
`;
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
import {tct} from 'sentry/locale';
import type {Sort} from 'sentry/utils/discover/fields';
import SortableHeader from 'sentry/views/codecov/tests/testAnalyticsTable/sortableHeader';
import type {Column} from 'sentry/views/codecov/tests/testAnalyticsTable/testAnalyticsTable';
import {RIGHT_ALIGNED_FIELDS} from 'sentry/views/codecov/tests/testAnalyticsTable/testAnalyticsTable';

type TableHeaderParams = {
column: Column;
sort?: Sort;
};

type FlakyTestTooltipProps = {
date: string;
};

function FlakyTestsTooltip({date}: FlakyTestTooltipProps) {
return (
<p>
{tct(
`Shows how often a flake occurs by tracking how many times a test goes from fail to pass or pass to fail on a given branch and commit within the last [date]`,
{date}
)}
.
</p>
);
}

export const renderTableHeader = ({column, sort}: TableHeaderParams) => {
const {key, name} = column;
// TODO: adjust when the date selector is completed
const date = '30 days';

const alignment = RIGHT_ALIGNED_FIELDS.has(key) ? 'right' : 'left';

return (
<SortableHeader
alignment={alignment}
sort={sort}
fieldName={key}
label={name}
{...(key === 'flakeRate' && {
tooltip: <FlakyTestsTooltip date={date} />,
})}
/>
);
};
Original file line number Diff line number Diff line change
@@ -0,0 +1,97 @@
import type {GridColumnHeader} from 'sentry/components/gridEditable';
import GridEditable, {COL_WIDTH_UNDEFINED} from 'sentry/components/gridEditable';
import {t} from 'sentry/locale';
import type {Sort} from 'sentry/utils/discover/fields';
import {renderTableBody} from 'sentry/views/codecov/tests/testAnalyticsTable/tableBody';
import {renderTableHeader} from 'sentry/views/codecov/tests/testAnalyticsTable/tableHeader';

type TestAnalyticsTableResponse = {
averageDurationMs: number;
commitsFailed: number;
flakeRate: number;
isBrokenTest: boolean;
lastRun: string;
testName: string;
};

export type Row = Pick<
TestAnalyticsTableResponse,
| 'testName'
| 'averageDurationMs'
| 'flakeRate'
| 'commitsFailed'
| 'lastRun'
| 'isBrokenTest'
>;
Comment on lines +17 to +25
Copy link
Contributor

@calvin-codecov calvin-codecov May 20, 2025

Choose a reason for hiding this comment

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

Is there a specific reason why we're using Pick here and picking everything instead of using the TestAnalyticsTableResponse type directly or setting Row = TestAnalyticsTableResponse? Are we implying that new response fields may not automatically want to be added to Row?

Copy link
Contributor Author

@adrianviquez adrianviquez May 21, 2025

Choose a reason for hiding this comment

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

My read here was that your row values don't need to necessarily follow the backend response. I think makes sense that an api response (or one after it's transformed from the hooks) exclusively contain the values you'll use in the table. We might fetch more data but we only care about the items relevant to the table.

I think this gives room for the api response to expand as needed without coupling it w/ the table/ui. There's an argument to how that shouldn't happen but yeah I think that'd lead to the API being more coupled to the design

Copy link
Contributor

Choose a reason for hiding this comment

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

That's fair. Cool, I'd say just leave it then for the implication that it can be decoupled.

export type Column = GridColumnHeader<
'testName' | 'averageDurationMs' | 'flakeRate' | 'commitsFailed' | 'lastRun'
>;

export type ValidSort = Sort & {
field: (typeof SORTABLE_FIELDS)[number];
};

const COLUMNS_ORDER: Column[] = [
{key: 'testName', name: t('Test Name'), width: COL_WIDTH_UNDEFINED},
{key: 'averageDurationMs', name: t('Avg. Duration'), width: COL_WIDTH_UNDEFINED},
Copy link
Contributor

Choose a reason for hiding this comment

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

any idea if t() would know how to translate abbreviations like "Avg."? I'm guessing it wouldn't

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't have a strong feel for it, but it might worth bringing up in a design sync as a consideration when thinking on abbreviations. Seems to me like this could be some antipattern design wise?

Copy link
Contributor

Choose a reason for hiding this comment

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

I'll make a note to bring up

{key: 'flakeRate', name: t('Flake Rate'), width: COL_WIDTH_UNDEFINED},
{key: 'commitsFailed', name: t('Commits Failed'), width: COL_WIDTH_UNDEFINED},
{key: 'lastRun', name: t('Last Run'), width: COL_WIDTH_UNDEFINED},
];

export const RIGHT_ALIGNED_FIELDS = new Set([
'averageDurationMs',
'flakeRate',
'commitsFailed',
]);

export const SORTABLE_FIELDS = [
'testName',
'averageDurationMs',
'flakeRate',
'commitsFailed',
'lastRun',
] as const;

export function isAValidSort(sort: Sort): sort is ValidSort {
return (SORTABLE_FIELDS as unknown as string[]).includes(sort.field);
}

interface Props {
response: {
data: Row[];
isLoading: boolean;
error?: Error | null;
};
sort: ValidSort;
}

export default function TestAnalyticsTable({response, sort}: Props) {
const {data, isLoading} = response;

return (
<GridEditable
aria-label={t('Test Analytics')}
isLoading={isLoading}
error={response.error}
data={data}
columnOrder={COLUMNS_ORDER}
// TODO: This isn't used as per the docs but is still required. Test if
// it affects sorting when backend is ready.
columnSortBy={[
{
key: sort.field,
order: sort.kind,
},
]}
grid={{
renderHeadCell: column =>
renderTableHeader({
column,
sort,
}),
renderBodyCell: (column, row) => renderTableBody({column, row}),
}}
/>
);
}
49 changes: 49 additions & 0 deletions static/app/views/codecov/tests/tests.tsx
Original file line number Diff line number Diff line change
@@ -1,10 +1,18 @@
import styled from '@emotion/styled';

import {DatePicker} from 'sentry/components/codecov/datePicker/datePicker';
// import TestAnalyticsTable from 'sentry/components/codecov/testAnalytics/testAnalyticsTable';
import PageFilterBar from 'sentry/components/organizations/pageFilterBar';
import PageFiltersContainer from 'sentry/components/organizations/pageFilters/container';
import {space} from 'sentry/styles/space';
import {decodeSorts} from 'sentry/utils/queryString';
import {useLocation} from 'sentry/utils/useLocation';
import {DEFAULT_SORT} from 'sentry/views/codecov/tests/settings';
import {Summaries} from 'sentry/views/codecov/tests/summaries/summaries';
import type {ValidSort} from 'sentry/views/codecov/tests/testAnalyticsTable/testAnalyticsTable';
import TestAnalyticsTable, {
isAValidSort,
} from 'sentry/views/codecov/tests/testAnalyticsTable/testAnalyticsTable';

const DEFAULT_CODECOV_DATETIME_SELECTION = {
start: null,
Expand All @@ -13,7 +21,47 @@ const DEFAULT_CODECOV_DATETIME_SELECTION = {
period: '24h',
};

// TODO: Sorting will only work once this is connected to the API
const fakeApiResponse = {
data: [
{
testName:
'tests.symbolicator.test_unreal_full.SymbolicatorUnrealIntegrationTest::test_unreal_crash_with_attachments',
averageDurationMs: 4,
flakeRate: 0.4,
commitsFailed: 1,
lastRun: '2025-04-17T22:26:19.486793+00:00',
isBrokenTest: false,
},
{
testName:
'graphql_api/tests/test_owner.py::TestOwnerType::test_fetch_current_user_is_not_okta_authenticated',
averageDurationMs: 4370,
flakeRate: 0,
commitsFailed: 5,
lastRun: '2025-04-16T22:26:19.486793+00:00',
isBrokenTest: true,
},
{
testName: 'graphql_api/tests/test_owner.py',
averageDurationMs: 10032,
flakeRate: 1,
commitsFailed: 3,
lastRun: '2025-02-16T22:26:19.486793+00:00',
isBrokenTest: false,
},
],
isLoading: false,
isError: false,
};

export default function TestsPage() {
const location = useLocation();

const sorts: [ValidSort] = [
decodeSorts(location.query?.sort).find(isAValidSort) ?? DEFAULT_SORT,
];

return (
<LayoutGap>
<p>Test Analytics</p>
Expand All @@ -26,6 +74,7 @@ export default function TestsPage() {
</PageFiltersContainer>
{/* TODO: Conditionally show these if the branch we're in is the main branch */}
<Summaries />
<TestAnalyticsTable response={fakeApiResponse} sort={sorts[0]} />
</LayoutGap>
);
}
Expand Down
Loading