Skip to content
This repository was archived by the owner on Jun 13, 2025. It is now read-only.

Conversation

@joseph-sentry
Copy link
Contributor

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.

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.
@joseph-sentry joseph-sentry requested a review from a team as a code owner October 29, 2024 17:31
@codecov
Copy link

codecov bot commented Oct 29, 2024

Codecov Report

Attention: Patch coverage is 93.04348% with 8 lines in your changes missing coverage. Please review.

Project coverage is 96.27%. Comparing base (4e9c42d) to head (009d9ae).
Report is 4 commits behind head on main.

✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
utils/test_results.py 90.36% 8 Missing ⚠️
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     
Flag Coverage Δ
unit 92.48% <93.04%> (+0.01%) ⬆️
unit-latest-uploader 92.48% <93.04%> (+0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@codecov-notifications
Copy link

Codecov Report

Attention: Patch coverage is 93.04348% with 8 lines in your changes missing coverage. Please review.

✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
utils/test_results.py 90.36% 8 Missing ⚠️

📢 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
Copy link
Contributor

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
Copy link
Contributor

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}")
Copy link
Contributor

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
Copy link
Contributor

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}")
Copy link
Contributor

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(
Copy link
Contributor

Choose a reason for hiding this comment

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

from... numbers?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants