diff --git a/services/lock_manager.py b/services/lock_manager.py index 7c357431e..f96b2d993 100644 --- a/services/lock_manager.py +++ b/services/lock_manager.py @@ -16,6 +16,7 @@ class LockType(Enum): BUNDLE_ANALYSIS_PROCESSING = "bundle_analysis_processing" BUNDLE_ANALYSIS_NOTIFY = "bundle_analysis_notify" + NOTIFICATION = "notify" # TODO: port existing task locking to use `LockManager` diff --git a/services/test_results.py b/services/test_results.py index 082426cf8..34e7cdcbe 100644 --- a/services/test_results.py +++ b/services/test_results.py @@ -3,15 +3,17 @@ from typing import Mapping, Sequence from shared.torngit.exceptions import TorngitClientError -from test_results_parser import Outcome, Testrun +from sqlalchemy import desc +from test_results_parser import Outcome from database.enums import ReportType -from database.models import Commit, CommitReport, RepositoryFlag, Upload +from database.models import Commit, CommitReport, RepositoryFlag, TestInstance, Upload from services.report import BaseReportService from services.repository import ( fetch_and_update_pull_request_information_from_commit, get_repo_provider_service, ) +from services.urls import get_pull_url log = logging.getLogger(__name__) @@ -94,3 +96,157 @@ def generate_test_id(repoid, testsuite, name, flags_hash): "utf-8" ) ).hexdigest() + + +class TestResultsNotifier: + def __init__(self, commit: Commit, commit_yaml, test_instances): + self.commit = commit + self.commit_yaml = commit_yaml + self.test_instances = test_instances + + async def notify(self): + commit_report = self.commit.commit_report(report_type=ReportType.TEST_RESULTS) + if not commit_report: + log.warning( + "No test results commit report found for this commit", + extra=dict( + commitid=self.commit.commitid, + report_key=commit_report.external_id, + ), + ) + + repo_service = get_repo_provider_service(self.commit.repository, self.commit) + + pull = await fetch_and_update_pull_request_information_from_commit( + repo_service, self.commit, self.commit_yaml + ) + pullid = pull.database_pull.pullid + if pull is None: + log.info( + "Not notifying since there is no pull request associated with this commit", + extra=dict( + commitid=self.commit.commitid, + report_key=commit_report.external_id, + pullid=pullid, + ), + ) + + pull_url = get_pull_url(pull.database_pull) + + message = self.build_message(pull_url, self.test_instances) + + try: + comment_id = pull.database_pull.commentid + if comment_id: + await repo_service.edit_comment(pullid, comment_id, message) + else: + res = await repo_service.post_comment(pullid, message) + pull.database_pull.commentid = res["id"] + return True + except TorngitClientError: + log.error( + "Error creating/updating PR comment", + extra=dict( + commitid=self.commit.commitid, + report_key=commit_report.external_id, + pullid=pullid, + ), + ) + return False + + def build_message(self, url, test_instances): + message = [] + + message += [ + f"## [Codecov]({url}) Report", + "", + "**Test Failures Detected**: Due to failing tests, we cannot provide coverage reports at this time.", + "", + "### :x: Failed Test Results: ", + ] + failed_tests = 0 + passed_tests = 0 + skipped_tests = 0 + + failures = dict() + + for test_instance in test_instances: + if test_instance.outcome == str( + Outcome.Failure + ) or test_instance.outcome == str(Outcome.Error): + failed_tests += 1 + job_code = test_instance.upload.job_code + flag_names = sorted(test_instance.upload.flag_names) + suffix = "" + if job_code or flag_names: + suffix = f"[{''.join(flag_names) or ''} {job_code or ''}]" + failures[ + f"{test_instance.test.testsuite}::{test_instance.test.name}{suffix}" + ] = test_instance.failure_message + elif test_instance.outcome == str(Outcome.Skip): + skipped_tests += 1 + elif test_instance.outcome == str(Outcome.Pass): + passed_tests += 1 + + results = f"Completed {len(test_instances)} tests with **`{failed_tests} failed`**, {passed_tests} passed and {skipped_tests} skipped." + + message.append(results) + + details = [ + "
View the full list of failed tests", + "", + "| **File path** | **Failure message** |", + "| :-- | :-- |", + ] + + message += details + + failure_table = [ + "| {0} | 
{1}
|".format( + self.insert_breaks(failed_test_name), + failure_message.replace("\n", "
"), + ) + for failed_test_name, failure_message in sorted( + failures.items(), key=lambda failure: failure[0] + ) + ] + + message += failure_table + + return "\n".join(message) + + def insert_breaks(self, table_value): + line_size = 70 + lines = [ + table_value[i : i + line_size] + for i in range(0, len(table_value), line_size) + ] + return "
".join(lines) + + +def latest_test_instances_for_a_given_commit(db_session, commit_id): + """ + This will result in a SQL query that looks something like this: + + SELECT DISTINCT ON (rt.test_id) rt.id, rt.external_id, rt.created_at, rt.updated_at, rt.test_id, rt.duration_seconds, rt.outcome, rt.upload_id, rt.failure_message + FROM reports_testinstance rt JOIN reports_upload ru ON ru.id = rt.upload_id JOIN reports_commitreport rc ON rc.id = ru.report_id + WHERE rc.commit_id = ORDER BY rt.test_id, ru.created_at desc + + The goal of this query is to return: "the latest test instance for each unique test based on upload creation time" + + The `DISTINCT ON` test_id with the order by test_id, enforces that we are only fetching one test instance for each test + + The ordering of the upload.create_at desc enforces that we get the latest test instance for that unique test + """ + return ( + db_session.query(TestInstance) + .join(Upload) + .join(CommitReport) + .filter( + CommitReport.commit_id == commit_id, + ) + .order_by(TestInstance.test_id) + .order_by(desc(Upload.created_at)) + .distinct(TestInstance.test_id) + .all() + ) diff --git a/tasks/notify.py b/tasks/notify.py index 4fce917a5..8ccfbf7b5 100644 --- a/tasks/notify.py +++ b/tasks/notify.py @@ -10,10 +10,12 @@ from shared.reports.readonly import ReadOnlyReport from shared.torngit.exceptions import TorngitClientError, TorngitServerFailureError from shared.yaml import UserYaml +from sqlalchemy import desc from sqlalchemy.orm.session import Session +from test_results_parser import Outcome from app import celery_app -from database.enums import CommitErrorTypes, Decoration +from database.enums import CommitErrorTypes, Decoration, ReportType from database.models import Commit, Pull from database.models.core import GITHUB_APP_INSTALLATION_DEFAULT_NAME from helpers.checkpoint_logger import from_kwargs as checkpoints_from_kwargs @@ -26,6 +28,7 @@ from services.comparison import ComparisonProxy from services.comparison.types import Comparison, FullCommit from services.decoration import determine_decoration_details +from services.lock_manager import LockManager, LockType from services.notification import NotificationService from services.redis import Redis, get_redis_connection from services.report import ReportService @@ -34,6 +37,7 @@ fetch_and_update_pull_request_information_from_commit, get_repo_provider_service, ) +from services.test_results import latest_test_instances_for_a_given_commit from services.yaml import get_current_yaml, read_yaml_field from tasks.base import BaseCodecovTask from tasks.upload_processor import UPLOAD_PROCESSING_LOCK_NAME @@ -68,13 +72,18 @@ async def run_async( "notifications": None, "reason": "has_other_notifies_coming", } - notify_lock_name = f"notify_lock_{repoid}_{commitid}" + + lock_manager = LockManager( + repoid=repoid, + commitid=commitid, + report_type=ReportType.COVERAGE, + lock_timeout=max(80, self.hard_time_limit_task), + ) + try: lock_acquired = False - with redis_connection.lock( - notify_lock_name, - timeout=max(80, self.hard_time_limit_task), - blocking_timeout=10, + with lock_manager.locked( + lock_type=LockType.NOTIFICATION, retry_num=self.request.retries ): lock_acquired = True return await self.run_async_within_lock( @@ -131,6 +140,21 @@ async def run_async_within_lock( Commit.repoid == repoid, Commit.commitid == commitid ) commit = commits_query.first() + + # check if there were any test failures + latest_test_instances = latest_test_instances_for_a_given_commit( + db_session, commit.id_ + ) + + if any( + [test.outcome == str(Outcome.Failure) for test in latest_test_instances] + ): + return { + "notify_attempted": False, + "notifications": None, + "reason": "test_failures", + } + try: repository_service = get_repo_provider_service(commit.repository) except RepositoryWithoutValidBotError: diff --git a/tasks/test_results_finisher.py b/tasks/test_results_finisher.py new file mode 100644 index 000000000..6ad6be487 --- /dev/null +++ b/tasks/test_results_finisher.py @@ -0,0 +1,144 @@ +import logging +from typing import Any, Dict + +from shared.yaml import UserYaml +from test_results_parser import Outcome + +from app import celery_app +from database.enums import ReportType +from database.models import Commit, CommitReport, Test, TestInstance, Upload +from services.lock_manager import LockManager, LockRetry, LockType +from services.test_results import ( + TestResultsNotifier, + latest_test_instances_for_a_given_commit, +) +from tasks.base import BaseCodecovTask +from tasks.notify import notify_task_name + +log = logging.getLogger(__name__) + +test_results_finisher_task_name = "app.tasks.test_results.TestResultsFinisherTask" + + +class TestResultsFinisherTask(BaseCodecovTask, name=test_results_finisher_task_name): + async def run_async( + self, + db_session, + chord_result: Dict[str, Any], + *, + repoid: int, + commitid: str, + commit_yaml: dict, + **kwargs, + ): + repoid = int(repoid) + commit_yaml = UserYaml.from_dict(commit_yaml) + + log.info( + "Starting test results finisher task", + extra=dict( + repoid=repoid, + commit=commitid, + commit_yaml=commit_yaml, + ), + ) + + lock_manager = LockManager( + repoid=repoid, + commitid=commitid, + report_type=ReportType.COVERAGE, + lock_timeout=max(80, self.hard_time_limit_task), + ) + + try: + # this needs to be the coverage notification lock + # since both tests post/edit the same comment + with lock_manager.locked( + LockType.NOTIFICATION, + retry_num=self.request.retries, + ): + return await self.process_async_within_lock( + db_session=db_session, + repoid=repoid, + commitid=commitid, + commit_yaml=commit_yaml, + previous_result=chord_result, + **kwargs, + ) + except LockRetry as retry: + self.retry(max_retries=5, countdown=retry.countdown) + + async def process_async_within_lock( + self, + *, + db_session, + repoid: int, + commitid: str, + commit_yaml: UserYaml, + previous_result: Dict[str, Any], + **kwargs, + ): + log.info( + "Running test results finishers", + extra=dict( + repoid=repoid, + commit=commitid, + commit_yaml=commit_yaml, + parent_task=self.request.parent_id, + ), + ) + + commit: Commit = ( + db_session.query(Commit).filter_by(repoid=repoid, commitid=commitid).first() + ) + assert commit, "commit not found" + + if self.check_if_no_success(previous_result): + # every processor errored, nothing to notify on + return {"notify_attempted": False, "notify_succeeded": False} + + test_instances = latest_test_instances_for_a_given_commit( + db_session, commit.id_ + ) + + if self.check_if_no_failures(test_instances): + self.app.tasks[notify_task_name].apply_async( + args=None, + kwargs=dict( + repoid=repoid, commitid=commitid, current_yaml=commit_yaml.to_dict() + ), + ) + return {"notify_attempted": False, "notify_succeeded": False} + + notifier = TestResultsNotifier(commit, commit_yaml, test_instances) + success = await notifier.notify() + + log.info( + "Finished test results notify", + extra=dict( + repoid=repoid, + commit=commitid, + commit_yaml=commit_yaml, + parent_task=self.request.parent_id, + ), + ) + + return {"notify_attempted": True, "notify_succeeded": success} + + 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_no_failures(self, testrun_list): + return all( + [instance.outcome != str(Outcome.Failure) for instance in testrun_list] + ) + + +RegisteredTestResultsFinisherTask = celery_app.register_task(TestResultsFinisherTask()) +test_results_finisher_task = celery_app.tasks[RegisteredTestResultsFinisherTask.name] diff --git a/tasks/tests/unit/test_notify_task.py b/tasks/tests/unit/test_notify_task.py index f3c459be9..b16021284 100644 --- a/tasks/tests/unit/test_notify_task.py +++ b/tasks/tests/unit/test_notify_task.py @@ -841,19 +841,25 @@ async def test_run_async_unobtainable_lock(self, dbsession, mock_redis, mocker): mocked_run_async_within_lock = mocker.patch.object( NotifyTask, "run_async_within_lock" ) + mocked_has_upcoming_notifies_according_to_redis = mocker.patch.object( + NotifyTask, "has_upcoming_notifies_according_to_redis", return_value=False + ) commit = CommitFactory.create() dbsession.add(commit) dbsession.flush() current_yaml = {"codecov": {"require_ci_to_pass": True}} task = NotifyTask() - mock_redis.lock.side_effect = LockError() - mock_redis.get.return_value = None + m = mocker.MagicMock() + m.return_value.locked.return_value.__enter__.side_effect = LockError() + mocker.patch("tasks.notify.LockManager", m) + res = await task.run_async( dbsession, repoid=commit.repoid, commitid=commit.commitid, current_yaml=current_yaml, ) + assert res == { "notifications": None, "notified": False, diff --git a/tasks/tests/unit/test_test_results_finisher.py b/tasks/tests/unit/test_test_results_finisher.py new file mode 100644 index 000000000..7ef29abfb --- /dev/null +++ b/tasks/tests/unit/test_test_results_finisher.py @@ -0,0 +1,789 @@ +import datetime +from pathlib import Path + +import pytest +from mock import AsyncMock +from shared.torngit.exceptions import TorngitClientError +from test_results_parser import Outcome + +from database.enums import ReportType +from database.models import CommitReport, RepositoryFlag, Test, TestInstance +from database.tests.factories import CommitFactory, PullFactory, UploadFactory +from services.repository import EnrichedPull +from services.test_results import generate_test_id +from tasks.test_results_finisher import TestResultsFinisherTask + +here = Path(__file__) + + +class TestUploadTestFinisherTask(object): + @pytest.mark.asyncio + @pytest.mark.integration + async def test_upload_finisher_task_call( + self, + mocker, + mock_configuration, + dbsession, + codecov_vcr, + mock_storage, + mock_redis, + celery_app, + ): + mocked_app = mocker.patch.object( + TestResultsFinisherTask, + "app", + tasks={"app.tasks.notify.Notify": mocker.MagicMock()}, + ) + + commit = CommitFactory.create( + message="hello world", + commitid="cd76b0821854a780b60012aed85af0a8263004ad", + repository__owner__unencrypted_oauth_token="test7lk5ndmtqzxlx06rip65nac9c7epqopclnoy", + repository__owner__username="joseph-sentry", + repository__owner__service="github", + repository__name="codecov-demo", + ) + dbsession.add(commit) + dbsession.flush() + + upload1 = UploadFactory.create() + dbsession.add(upload1) + dbsession.flush() + + upload2 = UploadFactory.create() + upload2.created_at = upload1.created_at + datetime.timedelta(0, 3) + dbsession.add(upload2) + dbsession.flush() + + current_report_row = CommitReport( + commit_id=commit.id_, report_type=ReportType.TEST_RESULTS.value + ) + dbsession.add(current_report_row) + dbsession.flush() + upload1.report = current_report_row + upload2.report = current_report_row + dbsession.flush() + + pull = PullFactory.create(repository=commit.repository, head=commit.commitid) + + _ = mocker.patch( + "services.test_results.fetch_and_update_pull_request_information_from_commit", + return_value=EnrichedPull( + database_pull=pull, + provider_pull={}, + ), + ) + + mocker.patch.object(TestResultsFinisherTask, "hard_time_limit_task", 0) + + m = mocker.MagicMock( + edit_comment=AsyncMock(return_value=True), + post_comment=AsyncMock(return_value={"id": 1}), + ) + mocked_repo_provider = mocker.patch( + "services.test_results.get_repo_provider_service", + return_value=m, + ) + + repoid = upload1.report.commit.repoid + upload2.report.commit.repoid = repoid + dbsession.flush() + + flag1 = RepositoryFlag(repository_id=repoid, flag_name="a") + flag2 = RepositoryFlag(repository_id=repoid, flag_name="b") + dbsession.flush() + + upload1.flags = [flag1] + upload2.flags = [flag2] + dbsession.flush() + + upload_id1 = upload1.id + upload_id2 = upload2.id + + test_id1 = generate_test_id(repoid, "test_name", "test_testsuite", "a") + test_id2 = generate_test_id(repoid, "test_name", "test_testsuite", "b") + + test1 = Test( + id_=test_id1, + repoid=repoid, + name="test_name", + testsuite="test_testsuite", + flags_hash="a", + ) + dbsession.add(test1) + dbsession.flush() + test2 = Test( + id_=test_id2, + repoid=repoid, + name="test_name", + testsuite="test_testsuite", + flags_hash="b", + ) + dbsession.add(test2) + dbsession.flush() + + test_instance1 = TestInstance( + test_id=test_id1, + outcome=str(Outcome.Failure), + failure_message="bad", + duration_seconds=1, + upload_id=upload_id1, + ) + dbsession.add(test_instance1) + dbsession.flush() + + test_instance2 = TestInstance( + test_id=test_id1, + outcome=str(Outcome.Failure), + failure_message="not that bad", + duration_seconds=1, + upload_id=upload_id2, + ) + dbsession.add(test_instance2) + dbsession.flush() + + test_instance3 = TestInstance( + test_id=test_id2, + outcome=str(Outcome.Failure), + failure_message="okay i guess", + duration_seconds=2, + upload_id=upload_id1, + ) + + dbsession.add(test_instance3) + dbsession.flush() + + result = await TestResultsFinisherTask().run_async( + dbsession, + [ + [{"successful": True}], + ], + repoid=repoid, + commitid=commit.commitid, + commit_yaml={"codecov": {"max_report_age": False}}, + ) + + expected_result = {"notify_attempted": True, "notify_succeeded": True} + m.post_comment.assert_called_with( + pull.pullid, + f"## [Codecov](https://app.codecov.io/gh/joseph-sentry/codecov-demo/pull/{pull.pullid}) Report\n\n**Test Failures Detected**: Due to failing tests, we cannot provide coverage reports at this time.\n\n### :x: Failed Test Results: \nCompleted 2 tests with **`2 failed`**, 0 passed and 0 skipped.\n
View the full list of failed tests\n\n| **File path** | **Failure message** |\n| :-- | :-- |\n| test_testsuite::test_name[a ] | 
okay i guess
|\n| test_testsuite::test_name[b ] | 
not that bad
|", + ) + + assert expected_result == result + + @pytest.mark.asyncio + @pytest.mark.integration + async def test_upload_finisher_task_call_no_failures( + self, + mocker, + mock_configuration, + dbsession, + codecov_vcr, + mock_storage, + mock_redis, + celery_app, + ): + mocked_app = mocker.patch.object( + TestResultsFinisherTask, + "app", + tasks={"app.tasks.notify.Notify": mocker.MagicMock()}, + ) + + commit = CommitFactory.create( + message="hello world", + commitid="cd76b0821854a780b60012aed85af0a8263004ad", + repository__owner__unencrypted_oauth_token="test7lk5ndmtqzxlx06rip65nac9c7epqopclnoy", + repository__owner__username="joseph-sentry", + repository__owner__service="github", + repository__name="codecov-demo", + ) + dbsession.add(commit) + dbsession.flush() + + upload1 = UploadFactory.create() + dbsession.add(upload1) + dbsession.flush() + + upload2 = UploadFactory.create() + upload2.created_at = upload2.created_at + datetime.timedelta(0, 3) + dbsession.add(upload2) + dbsession.flush() + + current_report_row = CommitReport( + commit_id=commit.id_, report_type=ReportType.TEST_RESULTS.value + ) + dbsession.add(current_report_row) + dbsession.flush() + upload1.report = current_report_row + upload2.report = current_report_row + dbsession.flush() + + pull = PullFactory.create(repository=commit.repository, head=commit.commitid) + + _ = mocker.patch( + "services.test_results.fetch_and_update_pull_request_information_from_commit", + return_value=EnrichedPull( + database_pull=pull, + provider_pull={}, + ), + ) + + mocker.patch.object(TestResultsFinisherTask, "hard_time_limit_task", 0) + + m = mocker.MagicMock( + edit_comment=AsyncMock(return_value=True), + post_comment=AsyncMock(return_value={"id": 1}), + ) + mocked_repo_provider = mocker.patch( + "services.test_results.get_repo_provider_service", + return_value=m, + ) + + repoid = upload1.report.commit.repoid + upload2.report.commit.repoid = repoid + dbsession.flush() + + flag1 = RepositoryFlag(repository_id=repoid, flag_name="a") + flag2 = RepositoryFlag(repository_id=repoid, flag_name="b") + dbsession.flush() + + upload1.flags = [flag1] + upload2.flags = [flag2] + dbsession.flush() + + upload_id1 = upload1.id + upload_id2 = upload2.id + + test_id1 = generate_test_id(repoid, "test_name", "test_testsuite", "a") + test_id2 = generate_test_id(repoid, "test_name", "test_testsuite", "b") + + test1 = Test( + id_=test_id1, + repoid=repoid, + name="test_name", + testsuite="test_testsuite", + flags_hash="a", + ) + dbsession.add(test1) + dbsession.flush() + test2 = Test( + id_=test_id2, + repoid=repoid, + name="test_name", + testsuite="test_testsuite", + flags_hash="b", + ) + dbsession.add(test2) + dbsession.flush() + + test_instance1 = TestInstance( + test_id=test_id1, + outcome=str(Outcome.Pass), + failure_message="bad", + duration_seconds=1, + upload_id=upload_id1, + ) + dbsession.add(test_instance1) + dbsession.flush() + + test_instance2 = TestInstance( + test_id=test_id1, + outcome=str(Outcome.Pass), + failure_message="not that bad", + duration_seconds=1, + upload_id=upload_id2, + ) + dbsession.add(test_instance2) + dbsession.flush() + + test_instance3 = TestInstance( + test_id=test_id2, + outcome=str(Outcome.Pass), + failure_message="okay i guess", + duration_seconds=2, + upload_id=upload_id1, + ) + + dbsession.add(test_instance3) + dbsession.flush() + + result = await TestResultsFinisherTask().run_async( + dbsession, + [ + [{"successful": True}], + ], + repoid=repoid, + commitid=commit.commitid, + commit_yaml={"codecov": {"max_report_age": False}}, + ) + + expected_result = {"notify_attempted": False, "notify_succeeded": False} + mocked_app.tasks["app.tasks.notify.Notify"].apply_async.assert_called_with( + args=None, + kwargs={ + "commitid": commit.commitid, + "current_yaml": {"codecov": {"max_report_age": False}}, + "repoid": commit.repoid, + }, + ) + + assert expected_result == result + + @pytest.mark.asyncio + @pytest.mark.integration + async def test_upload_finisher_task_call_no_success( + self, + mocker, + mock_configuration, + dbsession, + codecov_vcr, + mock_storage, + mock_redis, + celery_app, + ): + mocked_app = mocker.patch.object( + TestResultsFinisherTask, + "app", + tasks={"app.tasks.notify.Notify": mocker.MagicMock()}, + ) + + commit = CommitFactory.create( + message="hello world", + commitid="cd76b0821854a780b60012aed85af0a8263004ad", + repository__owner__unencrypted_oauth_token="test7lk5ndmtqzxlx06rip65nac9c7epqopclnoy", + repository__owner__username="joseph-sentry", + repository__owner__service="github", + repository__name="codecov-demo", + ) + dbsession.add(commit) + dbsession.flush() + + upload1 = UploadFactory.create() + dbsession.add(upload1) + dbsession.flush() + + upload2 = UploadFactory.create() + upload2.created_at = upload2.created_at + datetime.timedelta(0, 3) + dbsession.add(upload2) + dbsession.flush() + + current_report_row = CommitReport( + commit_id=commit.id_, report_type=ReportType.TEST_RESULTS.value + ) + dbsession.add(current_report_row) + dbsession.flush() + upload1.report = current_report_row + upload2.report = current_report_row + dbsession.flush() + + pull = PullFactory.create(repository=commit.repository, head=commit.commitid) + + _ = mocker.patch( + "services.test_results.fetch_and_update_pull_request_information_from_commit", + return_value=EnrichedPull( + database_pull=pull, + provider_pull={}, + ), + ) + + mocker.patch.object(TestResultsFinisherTask, "hard_time_limit_task", 0) + + m = mocker.MagicMock( + edit_comment=AsyncMock(return_value=True), + post_comment=AsyncMock(return_value={"id": 1}), + ) + mocked_repo_provider = mocker.patch( + "services.test_results.get_repo_provider_service", + return_value=m, + ) + + repoid = upload1.report.commit.repoid + upload2.report.commit.repoid = repoid + dbsession.flush() + + flag1 = RepositoryFlag(repository_id=repoid, flag_name="a") + flag2 = RepositoryFlag(repository_id=repoid, flag_name="b") + dbsession.flush() + + upload1.flags = [flag1] + upload2.flags = [flag2] + dbsession.flush() + + upload_id1 = upload1.id + upload_id2 = upload2.id + + test_id1 = generate_test_id(repoid, "test_name", "test_testsuite", "a") + test_id2 = generate_test_id(repoid, "test_name", "test_testsuite", "b") + + test1 = Test( + id_=test_id1, + repoid=repoid, + name="test_name", + testsuite="test_testsuite", + flags_hash="a", + ) + dbsession.add(test1) + dbsession.flush() + test2 = Test( + id_=test_id2, + repoid=repoid, + name="test_name", + testsuite="test_testsuite", + flags_hash="b", + ) + dbsession.add(test2) + dbsession.flush() + + test_instance1 = TestInstance( + test_id=test_id1, + outcome=str(Outcome.Pass), + failure_message="bad", + duration_seconds=1, + upload_id=upload_id1, + ) + dbsession.add(test_instance1) + dbsession.flush() + + test_instance2 = TestInstance( + test_id=test_id1, + outcome=str(Outcome.Pass), + failure_message="not that bad", + duration_seconds=1, + upload_id=upload_id2, + ) + dbsession.add(test_instance2) + dbsession.flush() + + test_instance3 = TestInstance( + test_id=test_id2, + outcome=str(Outcome.Pass), + failure_message="okay i guess", + duration_seconds=2, + upload_id=upload_id1, + ) + + dbsession.add(test_instance3) + dbsession.flush() + + result = await TestResultsFinisherTask().run_async( + dbsession, + [ + [{"successful": False}], + ], + repoid=repoid, + commitid=commit.commitid, + commit_yaml={"codecov": {"max_report_age": False}}, + ) + + expected_result = {"notify_attempted": False, "notify_succeeded": False} + + assert expected_result == result + + @pytest.mark.asyncio + @pytest.mark.integration + async def test_upload_finisher_task_call_existing_comment( + self, + mocker, + mock_configuration, + dbsession, + codecov_vcr, + mock_storage, + mock_redis, + celery_app, + ): + mocked_app = mocker.patch.object( + TestResultsFinisherTask, + "app", + tasks={"app.tasks.notify.Notify": mocker.MagicMock()}, + ) + + commit = CommitFactory.create( + message="hello world", + commitid="cd76b0821854a780b60012aed85af0a8263004ad", + repository__owner__unencrypted_oauth_token="test7lk5ndmtqzxlx06rip65nac9c7epqopclnoy", + repository__owner__username="joseph-sentry", + repository__owner__service="github", + repository__name="codecov-demo", + ) + dbsession.add(commit) + dbsession.flush() + + upload1 = UploadFactory.create() + dbsession.add(upload1) + dbsession.flush() + + upload2 = UploadFactory.create() + upload2.created_at = upload2.created_at + datetime.timedelta(0, 3) + dbsession.add(upload2) + dbsession.flush() + + current_report_row = CommitReport( + commit_id=commit.id_, report_type=ReportType.TEST_RESULTS.value + ) + dbsession.add(current_report_row) + dbsession.flush() + upload1.report = current_report_row + upload2.report = current_report_row + dbsession.flush() + + pull = PullFactory.create( + repository=commit.repository, head=commit.commitid, commentid=1, pullid=1 + ) + + _ = mocker.patch( + "services.test_results.fetch_and_update_pull_request_information_from_commit", + return_value=EnrichedPull( + database_pull=pull, + provider_pull={}, + ), + ) + + mocker.patch.object(TestResultsFinisherTask, "hard_time_limit_task", 0) + + m = mocker.MagicMock( + edit_comment=AsyncMock(return_value=True), + post_comment=AsyncMock(return_value={"id": 1}), + ) + mocked_repo_provider = mocker.patch( + "services.test_results.get_repo_provider_service", + return_value=m, + ) + + repoid = upload1.report.commit.repoid + upload2.report.commit.repoid = repoid + dbsession.flush() + + flag1 = RepositoryFlag(repository_id=repoid, flag_name="a") + flag2 = RepositoryFlag(repository_id=repoid, flag_name="b") + dbsession.flush() + + upload1.flags = [flag1] + upload2.flags = [flag2] + dbsession.flush() + + upload_id1 = upload1.id + upload_id2 = upload2.id + + test_id1 = generate_test_id(repoid, "test_name", "test_testsuite", "a") + test_id2 = generate_test_id(repoid, "test_name", "test_testsuite", "b") + + test1 = Test( + id_=test_id1, + repoid=repoid, + name="test_name", + testsuite="test_testsuite", + flags_hash="a", + ) + dbsession.add(test1) + dbsession.flush() + test2 = Test( + id_=test_id2, + repoid=repoid, + name="test_name", + testsuite="test_testsuite", + flags_hash="b", + ) + dbsession.add(test2) + dbsession.flush() + + test_instance1 = TestInstance( + test_id=test_id1, + outcome=str(Outcome.Failure), + failure_message="bad", + duration_seconds=1, + upload_id=upload_id1, + ) + dbsession.add(test_instance1) + dbsession.flush() + + test_instance2 = TestInstance( + test_id=test_id1, + outcome=str(Outcome.Failure), + failure_message="not that bad", + duration_seconds=1, + upload_id=upload_id2, + ) + dbsession.add(test_instance2) + dbsession.flush() + + test_instance3 = TestInstance( + test_id=test_id2, + outcome=str(Outcome.Failure), + failure_message="okay i guess", + duration_seconds=2, + upload_id=upload_id1, + ) + + dbsession.add(test_instance3) + dbsession.flush() + + result = await TestResultsFinisherTask().run_async( + dbsession, + [ + [{"successful": True}], + ], + repoid=repoid, + commitid=commit.commitid, + commit_yaml={"codecov": {"max_report_age": False}}, + ) + + expected_result = {"notify_attempted": True, "notify_succeeded": True} + + m.edit_comment.assert_called_with( + pull.pullid, + 1, + "## [Codecov](https://app.codecov.io/gh/joseph-sentry/codecov-demo/pull/1) Report\n\n**Test Failures Detected**: Due to failing tests, we cannot provide coverage reports at this time.\n\n### :x: Failed Test Results: \nCompleted 2 tests with **`2 failed`**, 0 passed and 0 skipped.\n
View the full list of failed tests\n\n| **File path** | **Failure message** |\n| :-- | :-- |\n| test_testsuite::test_name[a ] | 
okay i guess
|\n| test_testsuite::test_name[b ] | 
not that bad
|", + ) + + assert expected_result == result + + @pytest.mark.asyncio + @pytest.mark.integration + async def test_upload_finisher_task_call_comment_fails( + self, + mocker, + mock_configuration, + dbsession, + codecov_vcr, + mock_storage, + mock_redis, + celery_app, + ): + mocked_app = mocker.patch.object( + TestResultsFinisherTask, + "app", + tasks={"app.tasks.notify.Notify": mocker.MagicMock()}, + ) + + commit = CommitFactory.create( + message="hello world", + commitid="cd76b0821854a780b60012aed85af0a8263004ad", + repository__owner__unencrypted_oauth_token="test7lk5ndmtqzxlx06rip65nac9c7epqopclnoy", + repository__owner__username="joseph-sentry", + repository__owner__service="github", + repository__name="codecov-demo", + ) + dbsession.add(commit) + dbsession.flush() + + upload1 = UploadFactory.create() + dbsession.add(upload1) + dbsession.flush() + + upload2 = UploadFactory.create() + upload2.created_at = upload2.created_at + datetime.timedelta(0, 3) + dbsession.add(upload2) + dbsession.flush() + + current_report_row = CommitReport( + commit_id=commit.id_, report_type=ReportType.TEST_RESULTS.value + ) + dbsession.add(current_report_row) + dbsession.flush() + upload1.report = current_report_row + upload2.report = current_report_row + dbsession.flush() + + pull = PullFactory.create(repository=commit.repository, head=commit.commitid) + + _ = mocker.patch( + "services.test_results.fetch_and_update_pull_request_information_from_commit", + return_value=EnrichedPull( + database_pull=pull, + provider_pull={}, + ), + ) + + mocker.patch.object(TestResultsFinisherTask, "hard_time_limit_task", 0) + + m = mocker.MagicMock( + edit_comment=AsyncMock(return_value=True), + post_comment=AsyncMock(side_effect=TorngitClientError), + ) + + mocked_repo_provider = mocker.patch( + "services.test_results.get_repo_provider_service", + return_value=m, + ) + + repoid = upload1.report.commit.repoid + upload2.report.commit.repoid = repoid + dbsession.flush() + + flag1 = RepositoryFlag(repository_id=repoid, flag_name="a") + flag2 = RepositoryFlag(repository_id=repoid, flag_name="b") + dbsession.flush() + + upload1.flags = [flag1] + upload2.flags = [flag2] + dbsession.flush() + + upload_id1 = upload1.id + upload_id2 = upload2.id + + test_id1 = generate_test_id(repoid, "test_name", "test_testsuite", "a") + test_id2 = generate_test_id(repoid, "test_name", "test_testsuite", "b") + + test1 = Test( + id_=test_id1, + repoid=repoid, + name="test_name", + testsuite="test_testsuite", + flags_hash="a", + ) + dbsession.add(test1) + dbsession.flush() + test2 = Test( + id_=test_id2, + repoid=repoid, + name="test_name", + testsuite="test_testsuite", + flags_hash="b", + ) + dbsession.add(test2) + dbsession.flush() + + test_instance1 = TestInstance( + test_id=test_id1, + outcome=str(Outcome.Failure), + failure_message="bad", + duration_seconds=1, + upload_id=upload_id1, + ) + dbsession.add(test_instance1) + dbsession.flush() + + test_instance2 = TestInstance( + test_id=test_id1, + outcome=str(Outcome.Failure), + failure_message="not that bad", + duration_seconds=1, + upload_id=upload_id2, + ) + dbsession.add(test_instance2) + dbsession.flush() + + test_instance3 = TestInstance( + test_id=test_id2, + outcome=str(Outcome.Failure), + failure_message="okay i guess", + duration_seconds=2, + upload_id=upload_id1, + ) + + dbsession.add(test_instance3) + dbsession.flush() + + result = await TestResultsFinisherTask().run_async( + dbsession, + [ + [{"successful": True}], + ], + repoid=repoid, + commitid=commit.commitid, + commit_yaml={"codecov": {"max_report_age": False}}, + ) + + expected_result = {"notify_attempted": True, "notify_succeeded": False} + + assert expected_result == result diff --git a/tasks/tests/unit/test_upload_task.py b/tasks/tests/unit/test_upload_task.py index 74119c530..0ee20dedb 100644 --- a/tasks/tests/unit/test_upload_task.py +++ b/tasks/tests/unit/test_upload_task.py @@ -28,6 +28,7 @@ from services.report import NotReadyToBuildReportYetError, ReportService from tasks.bundle_analysis_notify import bundle_analysis_notify_task from tasks.bundle_analysis_processor import bundle_analysis_processor_task +from tasks.test_results_finisher import test_results_finisher_task from tasks.test_results_processor import test_results_processor_task from tasks.upload import UploadContext, UploadTask from tasks.upload_finisher import upload_finisher_task @@ -277,7 +278,7 @@ async def test_upload_task_call_test_results( mock_redis, celery_app, ): - group = mocker.patch("tasks.upload.group") + chord = mocker.patch("tasks.upload.chord") storage_path = "v4/raw/2019-05-22/C3C4715CA57C910D11D5EB899FC86A7E/4c4e4654ac25037ae869caeb3619d485970b6304/a84d445c-9c1e-434f-8275-f18f1f320f81.txt" redis_queue = [{"url": storage_path, "build_code": "some_random_build"}] jsonified_redis_queue = [json.dumps(x) for x in redis_queue] @@ -327,7 +328,15 @@ async def test_upload_task_call_test_results( ), ) - group.assert_called_with([processor_sig]) + notify_sig = test_results_finisher_task.signature( + kwargs=dict( + repoid=commit.repoid, + commitid=commit.commitid, + commit_yaml={"codecov": {"max_report_age": "1y ago"}}, + ) + ) + + chord.assert_called_with([processor_sig], notify_sig) @pytest.mark.asyncio async def test_upload_task_call_no_jobs( diff --git a/tasks/upload.py b/tasks/upload.py index f169ff95a..6028e249f 100644 --- a/tasks/upload.py +++ b/tasks/upload.py @@ -5,7 +5,7 @@ from json import loads from typing import Any, List, Mapping, Optional -from celery import chain, group +from celery import chain, chord from redis import Redis from redis.exceptions import LockError from shared.celery_config import upload_task_name @@ -43,6 +43,7 @@ from tasks.base import BaseCodecovTask from tasks.bundle_analysis_notify import bundle_analysis_notify_task from tasks.bundle_analysis_processor import bundle_analysis_processor_task +from tasks.test_results_finisher import test_results_finisher_task from tasks.test_results_processor import test_results_processor_task from tasks.upload_finisher import upload_finisher_task from tasks.upload_processor import UPLOAD_PROCESSING_LOCK_NAME, upload_processor_task @@ -716,8 +717,16 @@ def _schedule_test_results_processing_task( checkpoints.log(UploadFlow.INITIAL_PROCESSING_COMPLETE) checkpoint_data = checkpoints.data - res = group( + res = chord( processor_task_group, + test_results_finisher_task.signature( + args=(), + kwargs=dict( + repoid=commit.repoid, + commitid=commit.commitid, + commit_yaml=commit_yaml, + ), + ), ).apply_async() log.info(