Skip to content

Commit

Permalink
feat(ta): new prev result format in finisher
Browse files Browse the repository at this point in the history
this commit modifies the test results finisher to handle the new format
of previous_results that the processor will be outputting in the future

it also adds some typing to identify both the type of the old format and
the type of the new format

we want to go from a list[list[dict[str, str] | list[Any]]] to
list[bool] where the only key in the dict of the first type is:
"successful": bool

because of the rollout strategy we have to start with this change that
will deploy code to the finisher that can handle both formats then
we will push the change to the processor to start producing the new
format and finally we will remove the code that handles the old format
from the finisher
  • Loading branch information
joseph-sentry committed Dec 16, 2024
1 parent eb66157 commit bc0f90b
Show file tree
Hide file tree
Showing 2 changed files with 31 additions and 24 deletions.
29 changes: 19 additions & 10 deletions tasks/test_results_finisher.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import logging
from dataclasses import dataclass
from typing import Any
from typing import Any, TypedDict

from asgiref.sync import async_to_sync
from shared.reports.types import UploadType
Expand Down Expand Up @@ -51,11 +51,15 @@ class FlakeUpdateInfo:
newly_calculated_flakes: dict[str, set[FlakeSymptomType]]


class TAProcessorResult(TypedDict):
successful: bool


class TestResultsFinisherTask(BaseCodecovTask, name=test_results_finisher_task_name):
def run_impl(
self,
db_session: Session,
chord_result: dict[str, Any],
chord_result: list[bool] | list[list[TAProcessorResult | list[Any]]],
*,
repoid: int,
commitid: str,
Expand Down Expand Up @@ -111,7 +115,7 @@ def process_impl_within_lock(
repoid: int,
commitid: str,
commit_yaml: UserYaml,
previous_result: dict[str, Any],
previous_result: list[bool] | list[list[TAProcessorResult | list[Any]]],
**kwargs,
):
log.info("Running test results finishers", extra=self.extra_dict)
Expand Down Expand Up @@ -151,7 +155,7 @@ def process_impl_within_lock(
db_session.add(totals)
db_session.flush()

if self.check_if_no_success(previous_result):
if not check_if_any_success(previous_result):
# every processor errored, nothing to notify on
queue_notify = False

Expand Down Expand Up @@ -340,12 +344,17 @@ def get_flaky_tests(
}
return flaky_test_ids

def check_if_no_success(self, previous_result):
return all(
testrun_list["successful"] is False
for result in previous_result
for testrun_list in result
)

def check_if_any_success(
chord_result: list[bool] | list[list[TAProcessorResult | list[Any]]],
) -> bool:
for result in chord_result:
if isinstance(result, list):
if any(isinstance(r, dict) and r["successful"] for r in result):
return True
elif isinstance(result, bool) and result:
return True
return False


RegisteredTestResultsFinisherTask = celery_app.register_task(TestResultsFinisherTask())
Expand Down
26 changes: 12 additions & 14 deletions tasks/tests/unit/test_test_results_finisher.py
Original file line number Diff line number Diff line change
Expand Up @@ -339,6 +339,7 @@ def test_results_setup_no_instances(mocker, dbsession):
class TestUploadTestFinisherTask(object):
@pytest.mark.integration
@pytest.mark.django_db(databases={"default"})
@pytest.mark.parametrize("successful", [True, [{"successful": True}]])
def test_upload_finisher_task_call(
self,
mocker,
Expand All @@ -351,6 +352,7 @@ def test_upload_finisher_task_call(
test_results_mock_app,
mock_repo_provider_comments,
test_results_setup,
successful,
):
mock_feature = mocker.patch("services.test_results.FLAKY_TEST_DETECTION")
mock_feature.check_value.return_value = False
Expand All @@ -360,7 +362,7 @@ def test_upload_finisher_task_call(
result = TestResultsFinisherTask().run_impl(
dbsession,
[
[{"successful": True}],
successful,
],
repoid=repoid,
commitid=commit.commitid,
Expand Down Expand Up @@ -486,7 +488,7 @@ def test_upload_finisher_task_call_no_failures(
result = TestResultsFinisherTask().run_impl(
dbsession,
[
[{"successful": True}],
True,
],
repoid=repoid,
commitid=commit.commitid,
Expand Down Expand Up @@ -541,9 +543,7 @@ def test_upload_finisher_task_call_no_success(

result = TestResultsFinisherTask().run_impl(
dbsession,
[
[{"successful": False}],
],
[False],
repoid=repoid,
commitid=commit.commitid,
commit_yaml={"codecov": {"max_report_age": False}},
Expand Down Expand Up @@ -627,7 +627,7 @@ def test_upload_finisher_task_call_upgrade_comment(
result = TestResultsFinisherTask().run_impl(
dbsession,
[
[{"successful": True}],
True,
],
repoid=repoid,
commitid=commit.commitid,
Expand Down Expand Up @@ -681,7 +681,7 @@ def test_upload_finisher_task_call_existing_comment(
result = TestResultsFinisherTask().run_impl(
dbsession,
[
[{"successful": True}],
True,
],
repoid=repoid,
commitid=commit.commitid,
Expand Down Expand Up @@ -807,7 +807,7 @@ def test_upload_finisher_task_call_comment_fails(
result = TestResultsFinisherTask().run_impl(
dbsession,
[
[{"successful": True}],
True,
],
repoid=repoid,
commitid=commit.commitid,
Expand Down Expand Up @@ -883,7 +883,7 @@ def test_upload_finisher_task_call_with_flaky(
result = TestResultsFinisherTask().run_impl(
dbsession,
[
[{"successful": True}],
True,
],
repoid=repoid,
commitid=commit.commitid,
Expand Down Expand Up @@ -972,7 +972,7 @@ def test_upload_finisher_task_call_main_branch(
result = TestResultsFinisherTask().run_impl(
dbsession,
[
[{"successful": True}],
True,
],
repoid=repoid,
commitid=commit.commitid,
Expand Down Expand Up @@ -1032,9 +1032,7 @@ def test_upload_finisher_task_call_computed_name(

result = TestResultsFinisherTask().run_impl(
dbsession,
[
[{"successful": True}],
],
[True],
repoid=repoid,
commitid=commit.commitid,
commit_yaml={"codecov": {"max_report_age": False}},
Expand Down Expand Up @@ -1160,7 +1158,7 @@ def test_upload_finisher_task_call_main_with_plan(
result = TestResultsFinisherTask().run_impl(
dbsession,
[
[{"successful": True}],
True,
],
repoid=repoid,
commitid=commit.commitid,
Expand Down

0 comments on commit bc0f90b

Please sign in to comment.