-
Notifications
You must be signed in to change notification settings - Fork 28
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
[GQL] Add filters to the testResults field on the Repository #824
Conversation
@@ -579,8 +584,22 @@ async def resolve_test_results( | |||
filters=None, | |||
**kwargs, | |||
): | |||
queryset = await sync_to_async(aggregate_test_results)( | |||
repoid=repository.repoid, branch=filters.get("branch") if filters else None | |||
parameter = None |
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.
do we need to handle a default case here?
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.
the default case is a noop so i don't think so
|
||
|
||
@test_results_aggregates_bindable.field("totalRunTime") | ||
def resolve_name(obj, _) -> float: |
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.
nit: I think our naming convention for these functions is resolve_bindable_field_name
|
||
|
||
@flake_aggregates_bindable.field("flakeCount") | ||
def resolve_name(obj, _) -> int: |
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.
reminder to update these function names to the actual resolve value
@@ -1,6 +1,8 @@ | |||
enum TestResultsOrderingParameter { | |||
LAST_DURATION | |||
AVG_DURATION |
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.
is it possible to add an annotation to avg_duration marking it as deprecated, and then creating a follow up ticket for removal. Apps can handle the removal
@@ -0,0 +1,5 @@ | |||
enum TestResultsFilterParameter { | |||
FLAKY_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.
do we need one of these for Skips too?
@@ -593,3 +612,24 @@ async def resolve_test_results( | |||
else OrderingDirection.DESC, | |||
**kwargs, | |||
) | |||
|
|||
|
|||
@repository_bindable.field("testResultsHeaders") |
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.
thought we had agreed to keep the name generic vs. "headers" in case design changes or we want to use aspects of this object elsewhere
- TestResultsHeaders represents the information displayed in the Test Analytics UI that is related to test results, total duration of tests, accumulated duration of the 5% slowest tests, total number of failures, and total number of skips - a field of type TestResultsHeaders was added to the Repository model
- rename the TestResultsHeaders GQL model to TestResultsAggregates - create the FlakeAggregates that contains flakeCount and flakeRate and uses the flaky_failure_count field from the DailyTestRollup
we want to be able to filter by flaky tests, failed tests and the slowest tests, because we want the clickable headers in the UI
dd0ef86
to
63d364f
Compare
Codecov ReportAttention: Patch coverage is ✅ All tests successful. No failed tests found.
📢 Thoughts on this report? Let us know! |
Codecov ReportAttention: Patch coverage is
✅ All tests successful. No failed tests found.
Additional details and impacted files@@ Coverage Diff @@
## main #824 +/- ##
==========================================
+ Coverage 96.26% 96.28% +0.01%
==========================================
Files 818 818
Lines 18778 18818 +40
==========================================
+ Hits 18076 18118 +42
+ Misses 702 700 -2
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
utils/test_results.py
Outdated
) | ||
|
||
thirty_days_ago = dt.date.today() - dt.timedelta(days=30) | ||
|
||
SLOW_TEST_PERCENTILE = 95 | ||
|
||
|
||
def slow_test_threshold(num_tests: int): |
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.
this can be simplified to
percentile = (100-95)/100
slow_tests_to_return = floor(percentile * total_tests)
return max(slow_tests_to_return,1)
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.
lgtm
- refactor slow test threshold function to be more readable - rework skipped test filter to show tests that are skipping tests all 100% of the time - fix the fail rate and flake rate division by zero error when pass + fail = 0
we want to be able to filter by flaky tests, failed tests and the
slowest tests, because we want the clickable headers in the UI