-
Notifications
You must be signed in to change notification settings - Fork 11
feat: don't do flake detection on private repos not on the pro plan #747
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
✅ All tests successful. No failed tests found. @@ Coverage Diff @@
## main #747 +/- ##
=======================================
Coverage 97.91% 97.91%
=======================================
Files 444 444
Lines 35403 35416 +13
=======================================
+ Hits 34664 34677 +13
Misses 739 739
Flags with carried forward coverage won't be shown. Click here to find out more.
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
✅ All tests successful. No failed tests found. Additional details and impacted files@@ Coverage Diff @@
## main #747 +/- ##
=======================================
Coverage 97.91% 97.91%
=======================================
Files 444 444
Lines 35403 35416 +13
=======================================
+ Hits 34664 34677 +13
Misses 739 739
Flags with carried forward coverage won't be shown. Click here to find out more.
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
✅ All tests successful. No failed tests found. @@ Coverage Diff @@
## main #747 +/- ##
=======================================
Coverage 97.91% 97.91%
=======================================
Files 444 444
Lines 35403 35416 +13
=======================================
+ Hits 34664 34677 +13
Misses 739 739
Flags with carried forward coverage won't be shown. Click here to find out more.
|
Codecov ReportAll modified and coverable lines are covered by tests ✅ ✅ All tests successful. No failed tests found. @@ Coverage Diff @@
## main #747 +/- ##
=======================================
Coverage 97.91% 97.91%
=======================================
Files 444 444
Lines 35403 35416 +13
=======================================
+ Hits 34664 34677 +13
Misses 739 739
Flags with carried forward coverage won't be shown. Click here to find out more.
|
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, some styling nits
services/tests/test_test_results.py
Outdated
if private: | ||
repo.private = True | ||
else: | ||
repo.private = False |
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.
if private: | |
repo.private = True | |
else: | |
repo.private = False | |
repo.private = private |
or maybe even put that into the RepositoryFactory
invocation?
tasks/test_results_finisher.py
Outdated
|
||
matching_flakes = list( | ||
db_session.query(Flake) | ||
.filter( # 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.
.filter( # type:ignore | |
.filter( |
tasks/test_results_finisher.py
Outdated
for flake in matching_flakes: | ||
flaky_test_ids[flake.testid] = FlakeInfo(flake.fail_count, flake.count) |
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 flake in matching_flakes: | |
flaky_test_ids[flake.testid] = FlakeInfo(flake.fail_count, flake.count) | |
flaky_test_ids = {flake.testid: FlakeInfo(flake.fail_count, flake.count) for flake in matching_flakes} |
services/test_results.py
Outdated
identifier=repoid, default=False | ||
) and read_yaml_field(commit_yaml, ("test_analytics", "flake_detection"), True) | ||
def should_do_flaky_detection(repo: Repository, commit_yaml: UserYaml) -> bool: | ||
should_config = read_yaml_field( |
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.
has_flakey_configured instead of should_config imo
services/test_results.py
Outdated
should_feature = FLAKY_TEST_DETECTION.check_value( | ||
identifier=repo.repoid, default=True | ||
) | ||
should_plan = not_private_and_free_or_team(repo) |
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.
has_valid_plan_or_repo instead of should_plan imo
services/test_results.py
Outdated
should_config = read_yaml_field( | ||
commit_yaml, ("test_analytics", "flake_detection"), True | ||
) | ||
should_feature = FLAKY_TEST_DETECTION.check_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.
feature_enabled instead of should_feature imo
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.
so.... close....
- remove flaky_shadow_mode feature flag - simplify should_{write,read}_flaky_detection functions to general should_do_flake_detection that checks whether to do either - add check to make sure repo is not private and on a free plan or team plan - the new logic is: - if a user has explicitly set flake_detection to false in their yaml we don't do flake detection - else: - if a user is part of the feature flag we do flake detection - else if the repo is private then we require them to be on a pro plan - else we do flake detection
e1f9b74
to
07c7b2a
Compare
✅ All tests successful. No failed tests were found. 📣 Thoughts on this report? Let Codecov know! | Powered by Codecov |
general should_do_flake_detection that checks whether to do either
team plan
yaml we don't do flake detection
pro plan