-
Notifications
You must be signed in to change notification settings - Fork 29
refactor: test results GQL endpoints #934
Conversation
this refactor simplifies the tests from test results aggregates and flake aggregates and adds some typing to the generate_test_results function. It also changes the arguments passed to the generate_test_results function so handling them is simpler and changes some things regarding error handling in that function.
Codecov ReportAttention: Patch coverage is
✅ All tests successful. No failed tests found.
Additional details and impacted files@@ Coverage Diff @@
## main #934 +/- ##
==========================================
+ Coverage 96.23% 96.27% +0.03%
==========================================
Files 823 823
Lines 18953 19082 +129
==========================================
+ Hits 18240 18371 +131
+ Misses 713 711 -2
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
Codecov ReportAttention: Patch coverage is ✅ All tests successful. No failed tests found.
📢 Thoughts on this report? Let us know! |
| ) -> TestResultsAggregates | None: | ||
| return await sync_to_async(generate_test_results_aggregates)( | ||
| repoid=repository.repoid, interval=convert_interval_to_timedelta(interval) | ||
| repoid=repository.repoid, interval=interval.value if interval else 30 |
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: Magic number -> const
| ) -> FlakeAggregates | None: | ||
| return await sync_to_async(generate_flake_aggregates)( | ||
| repoid=repository.repoid, interval=convert_interval_to_timedelta(interval) | ||
| repoid=repository.repoid, interval=interval.value if interval else 30 |
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: Magic number -> const
| return ValidationError(f"Invalid ordering field: {ordering}") | ||
| ) -> None: | ||
| if interval not in {1, 7, 30}: | ||
| raise ValidationError(f"Invalid interval: {interval}") |
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.
Could we add a couple tests for these validation states?
| ) | ||
|
|
||
| filtered_test_ids = set([bridge.test_id for bridge in bridges]) | ||
| filtered_test_ids = set([bridge.test_id for bridge in bridges]) # type: ignore |
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.
whats the ignore for? :O
|
|
||
| cursor_value = decode_cursor(after) if after else decode_cursor(before) | ||
| descending = ordering_direction == "DESC" | ||
| print(f"cursor_value: {cursor_value}") |
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 remove these prints
| f"{field}_percent_change": percent_diff( | ||
| curr_numbers[field], past_numbers[field] | ||
|
|
||
| def test_results_aggregates_from_numbers( |
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.
from... numbers?
this refactor simplifies the tests from test results aggregates and flake aggregates and adds some typing to the generate_test_results function. It also changes the arguments passed to the generate_test_results function so handling them is simpler and changes some things regarding error handling in that function.