8000 feat: don't do flake detection on private repos not on the pro plan by joseph-sentry · Pull Request #747 · codecov/worker · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content
This repository was archived by the owner on May 5, 2025. It is now read-only.

feat: don't do flake detection on private repos not on the pro plan #747

Merged
merged 4 commits into from
Nov 14, 2024

Conversation

joseph-sentry
Copy link
Contributor
  • 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

@joseph-sentry joseph-sentry requested a review from a team September 27, 2024 18:05
@codecov-qa
Copy link
codecov-qa bot commented Sep 27, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 97.91%. Comparing base (6c80e6b) to head (07c7b2a).
Report is 16 commits behind head on main.

✅ All tests successful. No failed tests found.

Impacted file tree graph

@@           Coverage Diff           @@
##             main     #747   +/-   ##
=======================================
  Coverage   97.91%   97.91%           
=======================================
  Files         444      444           
  Lines       35403    35416   +13     
=======================================
+ Hits        34664    34677   +13     
  Misses        739      739           
Flag Coverage Δ
integration 41.88% <72.41%> (+0.01%) ⬆️
unit 90.72% <62.06%> (+<0.01%) ⬆️

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

Components Coverage Δ
NonTestCode 95.89% <100.00%> (+<0.01%) ⬆️
OutsideTasks 95.73% <100.00%> (+<0.01%) ⬆️
Files with missing lines Coverage Δ
rollouts/__init__.py 100.00% <ø> (ø)
services/test_results.py 90.20% <100.00%> (+0.20%) ⬆️
services/tests/test_test_results.py 100.00% <100.00%> (ø)
tasks/sync_pull.py 94.38% <100.00%> (ø)
tasks/test_results_finisher.py 93.60% <100.00%> (-0.08%) ⬇️
tasks/tests/unit/test_sync_pull.py 100.00% <100.00%> (ø)
tasks/tests/unit/test_test_results_finisher.py 100.00% <100.00%> (ø)

Copy link
codecov bot commented Sep 27, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 97.91%. Comparing base (6c80e6b) to head (07c7b2a).
Report is 16 commits behind head on main.

✅ All tests successful. No failed tests found.

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##             main     #747   +/-   ##
=======================================
  Coverage   97.91%   97.91%           
=======================================
  Files         444      444           
  Lines       35403    35416   +13     
=======================================
+ Hits        34664    34677   +13     
  Misses        739      739           
Flag Coverage Δ
integration 41.88% <72.41%> (+0.01%) ⬆️
unit 90.72% <62.06%> (+<0.01%) ⬆️

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

Components Coverage Δ
NonTestCode 95.89% <100.00%> (+<0.01%) ⬆️
OutsideTasks 95.73% <100.00%> (+<0.01%) ⬆️
Files with missing lines Coverage Δ
rollouts/__init__.py 100.00% <ø> (ø)
services/test_results.py 90.20% <100.00%> (+0.20%) ⬆️
services/tests/test_test_results.py 100.00% <100.00%> (ø)
tasks/sync_pull.py 94.38% <100.00%> (ø)
tasks/test_results_finisher.py 93.60% <100.00%> (-0.08%) ⬇️
tasks/tests/unit/test_sync_pull.py 100.00% <100.00%> (ø)
tasks/tests/unit/test_test_results_finisher.py 100.00% <100.00%> (ø)

Copy link
codecov-public-qa bot commented Sep 27, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 97.91%. Comparing base (6c80e6b) to head (07c7b2a).
Report is 16 commits behind head on main.

✅ All tests successful. No failed tests found.

Impacted file tree graph

@@           Coverage Diff           @@
##             main     #747   +/-   ##
=======================================
  Coverage   97.91%   97.91%           
=======================================
  Files         444      444           
  Lines       35403    35416   +13     
=======================================
+ Hits        34664    34677   +13     
  Misses        739      739           
Flag Coverage Δ
integration 41.88% <72.41%> (+0.01%) ⬆️
unit 90.72% <62.06%> (+<0.01%) ⬆️

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

Components Coverage Δ
NonTestCode 95.89% <100.00%> (+<0.01%) ⬆️
OutsideTasks 95.73% <100.00%> (+<0.01%) ⬆️
Files with missing lines Coverage Δ
rollouts/__init__.py 100.00% <ø> (ø)
services/test_results.py 90.20% <100.00%> (+0.20%) ⬆️
services/tests/test_test_results.py 100.00% <100.00%> (ø)
tasks/sync_pull.py 94.38% <100.00%> (ø)
tasks/test_results_finisher.py 93.60% <100.00%> (-0.08%) ⬇️
tasks/tests/unit/test_sync_pull.py 100.00% <100.00%> (ø)
tasks/tests/unit/test_test_results_finisher.py 100.00% <100.00%> (ø)

@codecov-notifications
Copy link
codecov-notifications bot commented Sep 27, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

✅ All tests successful. No failed tests found.

Impacted file tree graph

@@           Coverage Diff           @@
##             main     #747   +/-   ##
=======================================
  Coverage   97.91%   97.91%           
=======================================
  Files         444      444           
  Lines       35403    35416   +13     
=======================================
+ Hits        34664    34677   +13     
  Misses        739      739           
Flag Coverage Δ
integration 41.88% <72.41%> (+0.01%) ⬆️
unit 90.72% <62.06%> (+<0.01%) ⬆️

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

Components Coverage Δ
NonTestCode 95.89% <100.00%> (+<0.01%) ⬆️
OutsideTasks 95.73% <100.00%> (+<0.01%) ⬆️
Files with missing lines Coverage Δ
rollouts/__init__.py 100.00% <ø> (ø)
services/test_results.py 90.20% <100.00%> (+0.20%) ⬆️
services/tests/test_test_results.py 100.00% <100.00%> (ø)
tasks/sync_pull.py 94.38% <100.00%> (ø)
tasks/test_results_finisher.py 93.60% <100.00%> (-0.08%) ⬇️
tasks/tests/unit/test_sync_pull.py 100.00% <100.00%> (ø)
tasks/tests/unit/test_test_results_finisher.py 100.00% <100.00%> (ø)

Copy link
Contributor
@Swatinem Swatinem left a 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

Comment on lines 265 to 268
if private:
repo.private = True
else:
repo.private = False
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
if private:
repo.private = True
else:
repo.private = False
repo.private = private

or maybe even put that into the RepositoryFactory invocation?


matching_flakes = list(
db_session.query(Flake)
.filter( # 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.

Suggested change
.filter( # type:ignore
.filter(

Comment on lines 412 to 413
for flake in matching_flakes:
flaky_test_ids[flake.testid] = FlakeInfo(flake.fail_count, flake.count)
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
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}

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(
Copy link
Contributor
@ajay-sentry ajay-sentry Oct 7, 2024

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

should_feature = FLAKY_TEST_DETECTION.check_value(
identifier=repo.repoid, default=True
)
should_plan = not_private_and_free_or_team(repo)
Copy link
Contributor

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

should_config = read_yaml_field(
commit_yaml, ("test_analytics", "flake_detection"), True
)
should_feature = FLAKY_TEST_DETECTION.check_value(
Copy link
Contributor

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

Copy link
Contributor
@ajay-sentry ajay-sentry left a 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
Copy link

✅ All tests successful. No failed tests were found.

📣 Thoughts on this report? Let Codecov know! | Powered by Codecov

@joseph-sentry joseph-sentry added this pull request to the merge queue Nov 14, 2024
Merged via the queue into main with commit 06190da Nov 14, 2024
26 of 27 checks passed
@joseph-sentry joseph-sentry deleted the joseph/flaky-plan branch November 14, 2024 20:05
6BFD
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
0