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 all 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)
5 changes: 2 additions & 3 deletions services/test_results.py
Original file line number Diff line number Diff line change
Expand Up @@ -402,12 +402,11 @@ def latest_test_instances_for_a_given_commit(db_session, commit_id):
def should_write_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)
or FLAKY_SHADOW_MODE.check_value(identifier=repoid, default=False)
)
) and read_yaml_field(commit_yaml, ("test_analytics", "flake_detection"), True)


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)
) and read_yaml_field(commit_yaml, ("test_analytics", "flake_detection"), True)
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
40 changes: 36 additions & 4 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,46 @@


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_config, flake_detection,flaky_shadow_mode,tests_exist,outcome",
[
(False, True, True, True, False),
(True, False, False, False, False),
(True, False, False, True, False),
(True, True, False, True, True),
(True, False, True, True, True),
(True, True, True, True, True),
],
)
def test_update_pull_commits_merged(
self,
dbsession,
mocker,
flake_detection_config,
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 @@ -78,7 +110,7 @@ def test_update_pull_commits_merged(self, dbsession, mocker, flake_detection):
current_yaml = UserYaml.from_dict(
{
"test_analytics": {
"flake_detection": flake_detection,
"flake_detection": flake_detection_config,
}
}
)
Expand All @@ -89,7 +121,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
32 changes: 20 additions & 12 deletions tasks/tests/unit/test_test_results_finisher.py
Original file line number Diff line number Diff line change
Expand Up @@ -903,7 +903,7 @@

@pytest.mark.integration
@pytest.mark.parametrize(
"flake_detection", ["FLAKY_TEST_DETECTION", "FLAKY_SHADOW_MODE"]
"flake_detection", [None, "FLAKY_TEST_DETECTION", "FLAKY_SHADOW_MODE"]
)
def test_upload_finisher_task_call_main_branch(
self,
Expand All @@ -919,13 +919,16 @@
test_results_setup,
flake_detection,
):
mock_feature = mocker.patch(f"services.test_results.{flake_detection}")
mock_feature.check_value.return_value = True
if flake_detection:
mock_feature = mocker.patch(f"services.test_results.{flake_detection}")
mock_feature.check_value.return_value = True

Check warning on line 924 in tasks/tests/unit/test_test_results_finisher.py

View check run for this annotation

Codecov Notifications / codecov/patch

tasks/tests/unit/test_test_results_finisher.py#L922-L924

Added lines #L922 - L924 were not covered by tests
commit_yaml = {
"codecov": {"max_report_age": False},
}
if flake_detection == "FLAKY_TEST_DETECTION":
commit_yaml["test_analytics"] = {"flake_detection": True}
elif flake_detection is None:
commit_yaml["test_analytics"] = {"flake_detection": False}

Check warning on line 931 in tasks/tests/unit/test_test_results_finisher.py

View check run for this annotation

Codecov Notifications / codecov/patch

tasks/tests/unit/test_test_results_finisher.py#L930-L931

Added lines #L930 - L931 were not covered by tests

repoid, commit, pull, test_instances = test_results_setup

Expand All @@ -949,12 +952,17 @@

assert expected_result == result

test_results_mock_app.tasks[
"app.tasks.flakes.ProcessFlakesTask"
].apply_async.assert_called_with(
kwargs={
"repo_id": repoid,
"commit_id_list": [commit.commitid],
"branch": "main",
},
)
if flake_detection is None:
test_results_mock_app.tasks[

Check warning on line 956 in tasks/tests/unit/test_test_results_finisher.py

View check run for this annotation

Codecov Notifications / codecov/patch

tasks/tests/unit/test_test_results_finisher.py#L955-L956

Added lines #L955 - L956 were not covered by tests
"app.tasks.flakes.ProcessFlakesTask"
].apply_async.assert_not_called()
else:
test_results_mock_app.tasks[

Check warning on line 960 in tasks/tests/unit/test_test_results_finisher.py

View check run for this annotation

Codecov Notifications / codecov/patch

tasks/tests/unit/test_test_results_finisher.py#L960

Added line #L960 was not covered by tests
"app.tasks.flakes.ProcessFlakesTask"
].apply_async.assert_called_with(
kwargs={
"repo_id": repoid,
"commit_id_list": [commit.commitid],
"branch": "main",
},
)
Loading