-
Notifications
You must be signed in to change notification settings - Fork 24
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
Tests analytics page V2 #3174
base: main
Are you sure you want to change the base?
Tests analytics page V2 #3174
Conversation
Bundle ReportChanges will decrease total bundle size by 371.24kB (-2.1%) ⬇️. This is within the configured threshold ✅ Detailed changes
|
Bundle ReportChanges will decrease total bundle size by 371.24kB (-2.1%) ⬇️. This is within the configured threshold ✅ Detailed changes
|
Codecov ReportAttention: Patch coverage is
✅ All tests successful. No failed tests found. @@ Coverage Diff @@
## main #3174 +/- ##
========================================
Coverage 99.09% 99.09%
========================================
Files 804 810 +6
Lines 14187 14422 +235
Branches 4024 4112 +88
========================================
+ Hits 14058 14291 +233
- Misses 120 124 +4
+ Partials 9 7 -2
... and 15 files with indirect coverage changes
Continue to review full report in Codecov by Sentry.
|
Codecov ReportAttention: Patch coverage is
✅ All tests successful. No failed tests found. @@ Coverage Diff @@
## main #3174 +/- ##
========================================
Coverage 99.09% 99.09%
========================================
Files 804 810 +6
Lines 14187 14422 +235
Branches 4017 4105 +88
========================================
+ Hits 14058 14291 +233
- Misses 120 124 +4
+ Partials 9 7 -2
... and 15 files with indirect coverage changes
Continue to review full report in Codecov by Sentry.
|
Codecov ReportAttention: Patch coverage is
✅ All tests successful. No failed tests found. Additional details and impacted files@@ Coverage Diff @@
## main #3174 +/- ##
========================================
Coverage 99.09% 99.09%
========================================
Files 804 810 +6
Lines 14187 14422 +235
Branches 4024 4105 +81
========================================
+ Hits 14058 14291 +233
- Misses 120 124 +4
+ Partials 9 7 -2
... and 15 files with indirect coverage changes
Continue to review full report in Codecov by Sentry.
|
✅ Deploy preview for gazebo ready!Previews expire after 1 month automatically.
|
Codecov ReportAttention: Patch coverage is ✅ All tests successful. No failed tests found. @@ Coverage Diff @@
## main #3174 +/- ##
========================================
Coverage 99.09% 99.09%
========================================
Files 804 810 +6
Lines 14187 14422 +235
Branches 4017 4112 +95
========================================
+ Hits 14058 14291 +233
- Misses 120 124 +4
+ Partials 9 7 -2
... and 15 files with indirect coverage changes
Continue to review full report in Codecov by Sentry.
|
…into tests-analytics-v2
Co-authored-by: Ajay Singh <ajay.singh@sentry.io>
…into tests-analytics-v2
…elector to remove param (#3459)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Approved this massive effort - nice work, team! 🎉 🎉
I added some nits that prove I actually read it 😂 but don't need to be addressed before shipping this iteration (in fact, you probably don't want to since it can potentially introduce new errors). I also did a QA sweep last week on the preview deploy and added some small tickets to the polish epic. Amazing stuff! 🚀
const parsedData = TestResultsTestSuitesSchema.safeParse(res?.data) | ||
|
||
if (!parsedData.success) { | ||
return Promise.reject({ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not blocking or needed now - with the new pattern per here if you use the helper rejectNetworkError
it will send to sentry which is handy
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
PR up here! #3484
totalSkips={aggregates?.totalSkips} | ||
totalSkipsPercentChange={aggregates?.totalSkipsPercentChange} | ||
updateParams={updateParams} | ||
isSelected={queryParams?.parameter === 'SKIPPED_TESTS'} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not blocking - would be nice to use the constant defined elsewhere (I think TestResultsFilterParameter
)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
PR up here! #3484
cell: (info) => { | ||
const value = (info.renderValue() ?? 0) * 100 | ||
const isInt = Number.isInteger(info.renderValue()) | ||
return isInt ? `${value}%` : `${value.toFixed(2)}%` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Technically if info.renderValue() is not a number this could end up saying NaN%
. Maybe an edge case don't need to expect or can add to post-release polish as needed
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For the failure rate (the value here), the API always returns a float, so we're safe!
ref: https://github.com/codecov/codecov-api/blob/main/graphql_api/types/test_results/test_results.graphql#L5
interval?: MeasurementInterval | ||
parameter?: TestResultsFilterParameterType | ||
term?: string | ||
test_suites?: string[] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was wondering why this one was snake case - guess it's that way in api
- https://github.com/codecov/codecov-api/blob/7857053efdb545165cbce4ffc0bd9526d82a73aa/graphql_api/types/inputs/test_results_filters.graphql#L4
Seems ok as is - if we ever have time to clean it up can grab it later
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
good catch
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
testAnalytics: z | ||
.object({ | ||
testResultsAggregates: z.object({ | ||
totalDuration: z.number(), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In too deep with this now but would be nice to have units on these duration
fields next time. Maybe with the polish epic can add comments indicating these (totalDuration
, slowestTestsDuration
) represent seconds either here and/or in api
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For sure, could be a nice-to-have in the GQ schema playground too! Add comments indicating these (totalDuration, slowestTestsDuration) represent seconds #2879
Description
We are developing against this. Please open a PR of you wish to push changes to this branch.
Design: https://www.figma.com/design/TcppABudxnslgEmaXm0B9O/GH-2028?node-id=1-2&node-type=CANVAS&t=QWvqJ1kK3WG1pFLS-0
Link to Sample Entry
Legal Boilerplate
Look, I get it. The entity doing business as "Sentry" was incorporated in the State of Delaware in 2015 as Functional Software, Inc. In 2022 this entity acquired Codecov and as result Sentry is going to need some rights from me in order to utilize my contributions in this PR. So here's the deal: I retain all rights, title and interest in and to my contributions, and by keeping this boilerplate intact I confirm that Sentry can use, modify, copy, and redistribute my contributions, under Sentry's choice of terms.