From bc0f90bceb3d0715fa3c2b7d89db25f66fbd9243 Mon Sep 17 00:00:00 2001 From: joseph-sentry Date: Mon, 16 Dec 2024 13:10:18 -0500 Subject: [PATCH] feat(ta): new prev result format in finisher 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 --- tasks/test_results_finisher.py | 29 ++++++++++++------- .../tests/unit/test_test_results_finisher.py | 26 ++++++++--------- 2 files changed, 31 insertions(+), 24 deletions(-) diff --git a/tasks/test_results_finisher.py b/tasks/test_results_finisher.py index 2036b2431..44166fa7a 100644 --- a/tasks/test_results_finisher.py +++ b/tasks/test_results_finisher.py @@ -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 @@ -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, @@ -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) @@ -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 @@ -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()) diff --git a/tasks/tests/unit/test_test_results_finisher.py b/tasks/tests/unit/test_test_results_finisher.py index d22c870ac..4d37a6162 100644 --- a/tasks/tests/unit/test_test_results_finisher.py +++ b/tasks/tests/unit/test_test_results_finisher.py @@ -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, @@ -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 @@ -360,7 +362,7 @@ def test_upload_finisher_task_call( result = TestResultsFinisherTask().run_impl( dbsession, [ - [{"successful": True}], + successful, ], repoid=repoid, commitid=commit.commitid, @@ -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, @@ -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}}, @@ -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, @@ -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, @@ -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, @@ -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, @@ -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, @@ -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}}, @@ -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,