feat(Codecov): add test analytics table #91832
Conversation
github-actions
bot
added
the
Scope: Frontend
| return ( | ||
| <p> | ||
| Shows how often a flake occurs by tracking how many times a test goes from fail to | ||
| pass or pass to fail ona given branch and commit within the last {dateSelector}. |
| const location = useLocation(); | ||
|
|
||
| // TODO: Sorting will only work once this is connected to the API | ||
| const fakeApiResponse = { |
There was a problem hiding this comment.
this fake response is what we expect to be returned from the hook, right? So we'll do some transformation logic or something in there rather than on the Backend
There was a problem hiding this comment.
i.e. for broken test. So what would be a computed value in the GQL resolver ends up being a computed value in our hook. Could also do it on the BE too either in codecov or in the endpoint as well
There was a problem hiding this comment.
I think we'll inevitably revise this as needed once we get a couple of routes sorted and better understand the pattern, so I'm okay leaving this as is for now and adjusting code in the backend or hook or whatever, does that make sense?
There was a problem hiding this comment.
Yeah that's chill with me
| text-align: ${p => (p.alignment === 'left' ? 'left' : 'right')}; | ||
| `; | ||
|
|
||
| const SortLink = styled(Link)` |
There was a problem hiding this comment.
nit: we could rename this like what we discussed earlier so it reduces confusion with the other SortLink component
| export type Row = Pick< | ||
| TestAnalyticsTableResponse, | ||
| | 'testName' | ||
| | 'averageDurationMs' | ||
| | 'flakeRate' | ||
| | 'commitsFailed' | ||
| | 'lastRun' | ||
| | 'isBrokenTest' | ||
| >; |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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
There was a problem hiding this comment.
That's fair. Cool, I'd say just leave it then for the implication that it can be decoupled.
|
|
||
| const COLUMNS_ORDER: Column[] = [ | ||
| {key: 'testName', name: t('Test Name'), width: COL_WIDTH_UNDEFINED}, | ||
| {key: 'averageDurationMs', name: t('Avg. Duration'), width: COL_WIDTH_UNDEFINED}, |
There was a problem hiding this comment.
any idea if t() would know how to translate abbreviations like "Avg."? I'm guessing it wouldn't
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
I'll make a note to bring up
Codecov ReportAll modified and coverable lines are covered by tests ✅ ✅ All tests successful. No failed tests found. Additional details and impacted files@@ Coverage Diff @@
## master #91832 +/- ##
==========================================
- Coverage 87.64% 86.46% -1.19%
==========================================
Files 10350 10348 -2
Lines 586586 586564 -22
Branches 22574 22574
==========================================
- Hits 514115 507166 -6949
- Misses 72026 78953 +6927
Partials 445 445 |
This PR creates the base table layout for the Test Analytics Codecov page.
Notes