Skip to content
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

Update flaky shadow mode #677

Merged
merged 3 commits into from
Sep 3, 2024
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 12 additions & 1 deletion database/tests/factories/reports.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import factory

from database.models.reports import CompareFlag, RepositoryFlag
from database.models.reports import CompareFlag, RepositoryFlag, Test
from database.tests.factories.core import CompareCommitFactory, RepositoryFactory


Expand All @@ -18,3 +18,14 @@ class Meta:

commit_comparison = factory.SubFactory(CompareCommitFactory)
repositoryflag = factory.SubFactory(RepositoryFlagFactory)


class TestFactory(factory.Factory):
class Meta:
model = Test

name = factory.Sequence(lambda n: f"test_{n}")
testsuite = "testsuite"
flags_hash = "flags_hash"
id_ = factory.Sequence(lambda n: f"id_{n}")
repository = factory.SubFactory(RepositoryFactory)
8 changes: 5 additions & 3 deletions services/test_results.py
Original file line number Diff line number Diff line change
Expand Up @@ -408,6 +408,8 @@ def should_write_flaky_detection(repoid: int, commit_yaml: UserYaml) -> bool:


def should_read_flaky_detection(repoid: int, commit_yaml: UserYaml) -> bool:
return FLAKY_TEST_DETECTION.check_value(
identifier=repoid, default=False
) and read_yaml_field(commit_yaml, ("test_analytics", "flake_detection"), False)
return (
FLAKY_TEST_DETECTION.check_value(identifier=repoid, default=False)
and read_yaml_field(commit_yaml, ("test_analytics", "flake_detection"), False)
or FLAKY_SHADOW_MODE.check_value(identifier=repoid, default=False)
Copy link
Contributor

@michelletran-codecov michelletran-codecov Sep 3, 2024

Choose a reason for hiding this comment

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

If there's some problem where we want to turn off displaying the results to users, then which flag would we want to flip?

Copy link
Contributor

Choose a reason for hiding this comment

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

Turning off FLAKY_SHADOW_MODE would also disable writing. So, I think we might want another flag to just disable the reads.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

i updated the changes such that FLAKY_SHADOW_MODE retains it's current functionality on the write path and we're going to use FLAKY_DETECTION to toggle the beta, so we can separately disable the read path without affecting the write path

)
10 changes: 6 additions & 4 deletions tasks/sync_pull.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
from shared.yaml.user_yaml import OwnerContext

from app import celery_app
from database.models import Commit, Pull, Repository
from database.models import Commit, Pull, Repository, Test
from helpers.exceptions import RepositoryWithoutValidBotError
from helpers.github_installation import get_installation_name_for_owner_for_task
from helpers.metrics import metrics
Expand Down Expand Up @@ -377,9 +377,11 @@ def update_pull_commits(
synchronize_session=False,
)
)
self.trigger_process_flakes(
repoid, pull.head, pull_dict["head"]["branch"], current_yaml
)

if db_session.query(Test).filter(Test.repoid == repoid).count() > 0:
self.trigger_process_flakes(
repoid, pull.head, pull_dict["head"]["branch"], current_yaml
)

# set the rest of the commits to deleted (do not show in the UI)
deleted_count = (
Expand Down
36 changes: 33 additions & 3 deletions tasks/tests/unit/test_sync_pull.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
from shared.torngit.exceptions import TorngitClientError

from database.tests.factories import CommitFactory, PullFactory, RepositoryFactory
from database.tests.factories.reports import TestFactory
from helpers.exceptions import RepositoryWithoutValidBotError
from services.repository import EnrichedPull
from services.yaml import UserYaml
Expand All @@ -18,15 +19,44 @@


class TestPullSyncTask(object):
@pytest.mark.parametrize("flake_detection", [False, True])
def test_update_pull_commits_merged(self, dbsession, mocker, flake_detection):
@pytest.mark.parametrize(
"flake_detection,flaky_shadow_mode,tests_exist,outcome",
[
(False, False, False, False),
(False, True, False, False),
(True, False, False, False),
(False, True, True, True),
(True, True, True, True),
],
)
def test_update_pull_commits_merged(
self,
dbsession,
mocker,
flake_detection,
flaky_shadow_mode,
tests_exist,
outcome,
):
if flake_detection:
mock_feature = mocker.patch("services.test_results.FLAKY_TEST_DETECTION")
mock_feature.check_value.return_value = True

if flaky_shadow_mode:
_flaky_shadow_mode_feature = mocker.patch(
"services.test_results.FLAKY_SHADOW_MODE"
)
_flaky_shadow_mode_feature.check_value.return_value = True

repository = RepositoryFactory.create()
dbsession.add(repository)
dbsession.flush()

if tests_exist:
t = TestFactory(repository=repository)
dbsession.add(t)
dbsession.flush()

base_commit = CommitFactory.create(repository=repository)
head_commit = CommitFactory.create(repository=repository)
pull = PullFactory.create(
Expand Down Expand Up @@ -89,7 +119,7 @@ def test_update_pull_commits_merged(self, dbsession, mocker, flake_detection):
mock_repo_provider, enriched_pull, commits, commits_at_base, current_yaml
)

if flake_detection:
if outcome:
apply_async.assert_called_once_with(
kwargs=dict(
repo_id=repository.repoid,
Expand Down
Loading