From b07ef5c324247eb76d9d95b7ebd7eed32484ab96 Mon Sep 17 00:00:00 2001 From: Arpad Borsos Date: Fri, 20 Sep 2024 15:12:56 +0200 Subject: [PATCH] Make `ComparisonProxy` sync As calling `async_to_sync` within an async context, this necessitated also turning all the various `notification` related functionality sync. --- helpers/notifier.py | 23 +- services/bundle_analysis/notify/__init__.py | 14 +- .../notify/messages/__init__.py | 2 +- .../notify/messages/comment.py | 16 +- .../notify/messages/commit_status.py | 20 +- .../notify/messages/tests/test_comment.py | 24 +- .../messages/tests/test_commit_status.py | 27 +- .../notify/tests/test_notify_service.py | 6 +- services/comparison/__init__.py | 322 +++++------ services/comparison/overlays/critical_path.py | 15 +- .../tests/unit/overlay/test_critical_path.py | 40 +- .../comparison/tests/unit/test_behind_by.py | 21 +- .../tests/unit/test_comparison_proxy.py | 25 +- services/comparison/types.py | 6 +- services/notification/__init__.py | 47 +- services/notification/notifiers/base.py | 6 +- .../notification/notifiers/checks/base.py | 58 +- .../notification/notifiers/checks/changes.py | 4 +- .../notifiers/checks/checks_with_fallback.py | 8 +- .../notification/notifiers/checks/patch.py | 74 ++- .../notification/notifiers/checks/project.py | 79 ++- .../notifiers/codecov_slack_app.py | 2 +- .../notifiers/comment/__init__.py | 70 +-- .../notifiers/comment/conditions.py | 65 +-- services/notification/notifiers/generics.py | 14 +- .../notifiers/mixins/message/__init__.py | 24 +- .../notifiers/mixins/message/sections.py | 48 +- .../notification/notifiers/mixins/status.py | 109 ++-- .../notification/notifiers/status/base.py | 157 +++--- .../notification/notifiers/status/changes.py | 5 +- .../notification/notifiers/status/patch.py | 4 +- .../notification/notifiers/status/project.py | 4 +- .../tests/integration/test_comment.py | 35 +- .../notifiers/tests/unit/test_checks.py | 266 ++++----- .../tests/unit/test_codecov_slack_app.py | 17 +- .../notifiers/tests/unit/test_comment.py | 511 +++++++----------- .../tests/unit/test_comment_conditions.py | 35 +- .../notifiers/tests/unit/test_generics.py | 20 +- .../notifiers/tests/unit/test_status.py | 300 ++++------ .../tests/unit/test_comparison.py | 11 +- .../tests/unit/test_notification_service.py | 76 ++- services/repository.py | 16 +- services/test_results.py | 12 +- services/tests/test_test_results.py | 77 ++- tasks/compute_comparison.py | 13 +- tasks/notify.py | 26 +- tasks/notify_error.py | 3 +- tasks/save_report_results.py | 2 +- tasks/status_set_error.py | 15 +- tasks/status_set_pending.py | 19 +- tasks/test_results_finisher.py | 6 +- tasks/tests/unit/test_compute_comparison.py | 5 +- tasks/tests/unit/test_notify_task.py | 7 +- tasks/tests/unit/test_status_set_error.py | 20 +- tasks/tests/unit/test_status_set_pending.py | 10 +- 55 files changed, 1156 insertions(+), 1685 deletions(-) diff --git a/helpers/notifier.py b/helpers/notifier.py index c820094c2..235768435 100644 --- a/helpers/notifier.py +++ b/helpers/notifier.py @@ -2,6 +2,7 @@ from dataclasses import dataclass from enum import Enum +from asgiref.sync import async_to_sync from shared.torngit.base import TorngitBaseAdapter from shared.torngit.exceptions import TorngitClientError @@ -29,23 +30,23 @@ class BaseNotifier: _pull: EnrichedPull | None = None _repo_service: TorngitBaseAdapter | None = None - async def get_pull(self): + def get_pull(self): repo_service = self.get_repo_service() if self._pull is None: - self._pull = await fetch_and_update_pull_request_information_from_commit( - repo_service, self.commit, self.commit_yaml - ) + self._pull = async_to_sync( + fetch_and_update_pull_request_information_from_commit + )(repo_service, self.commit, self.commit_yaml) return self._pull - def get_repo_service(self): + def get_repo_service(self) -> TorngitBaseAdapter: if self._repo_service is None: self._repo_service = get_repo_provider_service(self.commit.repository) return self._repo_service - async def send_to_provider(self, pull, message): + def send_to_provider(self, pull, message): repo_service = self.get_repo_service() assert repo_service @@ -53,9 +54,9 @@ async def send_to_provider(self, pull, message): try: comment_id = pull.database_pull.commentid if comment_id: - await repo_service.edit_comment(pullid, comment_id, message) + async_to_sync(repo_service.edit_comment)(pullid, comment_id, message) else: - res = await repo_service.post_comment(pullid, message) + res = async_to_sync(repo_service.post_comment)(pullid, message) pull.database_pull.commentid = res["id"] return True except TorngitClientError: @@ -71,10 +72,10 @@ async def send_to_provider(self, pull, message): def build_message(self) -> str: raise NotImplementedError - async def notify( + def notify( self, ) -> NotifierResult: - pull = await self.get_pull() + pull = self.get_pull() if pull is None: log.info( "Not notifying since there is no pull request associated with this commit", @@ -86,7 +87,7 @@ async def notify( message = self.build_message() - sent_to_provider = await self.send_to_provider(pull, message) + sent_to_provider = self.send_to_provider(pull, message) if sent_to_provider == False: return NotifierResult.TORNGIT_ERROR diff --git a/services/bundle_analysis/notify/__init__.py b/services/bundle_analysis/notify/__init__.py index b85d730f3..c85ad722f 100644 --- a/services/bundle_analysis/notify/__init__.py +++ b/services/bundle_analysis/notify/__init__.py @@ -2,7 +2,6 @@ from typing import NamedTuple import sentry_sdk -from asgiref.sync import async_to_sync from shared.yaml import UserYaml from database.models.core import GITHUB_APP_INSTALLATION_DEFAULT_NAME, Commit, Owner @@ -17,9 +16,7 @@ from services.bundle_analysis.notify.contexts.commit_status import ( CommitStatusNotificationContextBuilder, ) -from services.bundle_analysis.notify.helpers import ( - get_notification_types_configured, -) +from services.bundle_analysis.notify.helpers import get_notification_types_configured from services.bundle_analysis.notify.messages import MessageStrategyInterface from services.bundle_analysis.notify.messages.comment import ( BundleAnalysisCommentMarkdownStrategy, @@ -27,10 +24,7 @@ from services.bundle_analysis.notify.messages.commit_status import ( CommitStatusMessageStrategy, ) -from services.bundle_analysis.notify.types import ( - NotificationSuccess, - NotificationType, -) +from services.bundle_analysis.notify.types import NotificationSuccess, NotificationType log = logging.getLogger(__name__) @@ -190,9 +184,7 @@ def notify(self) -> BundleAnalysisNotifyReturn: notifications_successful = [] for notification_context, message_strategy in notification_full_contexts: message = message_strategy.build_message(notification_context) - result = async_to_sync(message_strategy.send_message)( - notification_context, message - ) + result = message_strategy.send_message(notification_context, message) if result.notification_attempted: notifications_sent.append(notification_context.notification_type) if result.notification_successful: diff --git a/services/bundle_analysis/notify/messages/__init__.py b/services/bundle_analysis/notify/messages/__init__.py index 6496596ba..4678f43c6 100644 --- a/services/bundle_analysis/notify/messages/__init__.py +++ b/services/bundle_analysis/notify/messages/__init__.py @@ -15,7 +15,7 @@ def build_message( pass @abstractmethod - async def send_message( + def send_message( self, context: BaseBundleAnalysisNotificationContext, message: str | bytes ) -> NotificationResult: pass diff --git a/services/bundle_analysis/notify/messages/comment.py b/services/bundle_analysis/notify/messages/comment.py index 72ca89398..b759edaea 100644 --- a/services/bundle_analysis/notify/messages/comment.py +++ b/services/bundle_analysis/notify/messages/comment.py @@ -2,11 +2,9 @@ from typing import Literal, TypedDict import sentry_sdk +from asgiref.sync import async_to_sync from django.template import loader -from shared.bundle_analysis import ( - BundleAnalysisComparison, - BundleChange, -) +from shared.bundle_analysis import BundleAnalysisComparison, BundleChange from shared.torngit.exceptions import TorngitClientError from shared.validation.types import BundleThreshold @@ -104,7 +102,7 @@ def build_upgrade_message( return template.render(context) @sentry_sdk.trace - async def send_message( + def send_message( self, context: BundleAnalysisPRCommentNotificationContext, message: str ) -> NotificationResult: pull = context.pull.database_pull @@ -112,9 +110,13 @@ async def send_message( try: comment_id = pull.bundle_analysis_commentid if comment_id: - await repository_service.edit_comment(pull.pullid, comment_id, message) + async_to_sync(repository_service.edit_comment)( + pull.pullid, comment_id, message + ) else: - res = await repository_service.post_comment(pull.pullid, message) + res = async_to_sync(repository_service.post_comment)( + pull.pullid, message + ) pull.bundle_analysis_commentid = res["id"] return NotificationResult( notification_attempted=True, diff --git a/services/bundle_analysis/notify/messages/commit_status.py b/services/bundle_analysis/notify/messages/commit_status.py index 5407a172c..cf304c522 100644 --- a/services/bundle_analysis/notify/messages/commit_status.py +++ b/services/bundle_analysis/notify/messages/commit_status.py @@ -2,6 +2,7 @@ from typing import TypedDict import sentry_sdk +from asgiref.sync import async_to_sync from django.template import loader from shared.helpers.cache import make_hash_sha256 from shared.torngit.exceptions import TorngitClientError @@ -11,10 +12,7 @@ CommitStatusLevel, CommitStatusNotificationContext, ) -from services.bundle_analysis.notify.helpers import ( - bytes_readable, - get_github_app_used, -) +from services.bundle_analysis.notify.helpers import bytes_readable, get_github_app_used from services.bundle_analysis.notify.messages import MessageStrategyInterface from services.notification.notifiers.base import NotificationResult @@ -89,7 +87,7 @@ def _cache_key(self, context: CommitStatusNotificationContext) -> str: ) @sentry_sdk.trace - async def send_message( + def send_message( self, context: CommitStatusNotificationContext, message: str | bytes ) -> NotificationResult: repository_service = context.repository_service @@ -102,12 +100,12 @@ async def send_message( explanation="payload_unchanged", ) try: - await repository_service.set_commit_status( - commit=context.commit.commitid, - status=context.commit_status_level.to_str(), - context="codecov/bundles", - description=message, - url=context.commit_status_url, + async_to_sync(repository_service.set_commit_status)( + context.commit.commitid, + context.commit_status_level.to_str(), + "codecov/bundles", + message, + context.commit_status_url, ) # Update the recently-sent messages cache cache.get_backend().set( diff --git a/services/bundle_analysis/notify/messages/tests/test_comment.py b/services/bundle_analysis/notify/messages/tests/test_comment.py index a07aea964..8474b2ed0 100644 --- a/services/bundle_analysis/notify/messages/tests/test_comment.py +++ b/services/bundle_analysis/notify/messages/tests/test_comment.py @@ -1,7 +1,8 @@ from textwrap import dedent -from unittest.mock import AsyncMock, MagicMock +from unittest.mock import MagicMock import pytest +from mock import AsyncMock from shared.torngit.exceptions import TorngitClientError from shared.typings.torngit import TorngitInstanceData from shared.validation.types import BundleThreshold @@ -76,9 +77,11 @@ def test_build_message_from_samples(self, dbsession, mocker, mock_storage): def _setup_send_message_tests( self, dbsession, mocker, torngit_ghapp_data, bundle_analysis_commentid ): - fake_repo_provider = AsyncMock( + fake_repo_provider = MagicMock( name="fake_repo_provider", data=TorngitInstanceData(installation=torngit_ghapp_data), + post_comment=AsyncMock(), + edit_comment=AsyncMock(), ) fake_repo_provider.post_comment.return_value = {"id": 1000} fake_repo_provider.edit_comment.return_value = {"id": 1000} @@ -125,8 +128,7 @@ def _setup_send_message_tests( ), ], ) - @pytest.mark.asyncio - async def test_send_message_no_exising_comment( + def test_send_message_no_exising_comment( self, dbsession, mocker, torngit_ghapp_data ): fake_repo_provider, mock_pull, context, message = ( @@ -135,7 +137,7 @@ async def test_send_message_no_exising_comment( ) ) strategy = BundleAnalysisCommentMarkdownStrategy() - result = await strategy.send_message(context, message) + result = strategy.send_message(context, message) expected_app = torngit_ghapp_data.get("id") if torngit_ghapp_data else None assert result == NotificationResult( notification_attempted=True, @@ -161,15 +163,12 @@ async def test_send_message_no_exising_comment( ), ], ) - @pytest.mark.asyncio - async def test_send_message_exising_comment( - self, dbsession, mocker, torngit_ghapp_data - ): + def test_send_message_exising_comment(self, dbsession, mocker, torngit_ghapp_data): fake_repo_provider, _, context, message = self._setup_send_message_tests( dbsession, mocker, torngit_ghapp_data, bundle_analysis_commentid=1000 ) strategy = BundleAnalysisCommentMarkdownStrategy() - result = await strategy.send_message(context, message) + result = strategy.send_message(context, message) expected_app = torngit_ghapp_data.get("id") if torngit_ghapp_data else None assert result == NotificationResult( notification_attempted=True, @@ -179,8 +178,7 @@ async def test_send_message_exising_comment( fake_repo_provider.edit_comment.assert_called_with(12, 1000, message) fake_repo_provider.post_comment.assert_not_called() - @pytest.mark.asyncio - async def test_send_message_fail(self, dbsession, mocker): + def test_send_message_fail(self, dbsession, mocker): fake_repo_provider, _, context, message = self._setup_send_message_tests( dbsession, mocker, None, bundle_analysis_commentid=None ) @@ -189,7 +187,7 @@ async def test_send_message_fail(self, dbsession, mocker): name="fake_commit_report", external_id="some_UUID4" ) strategy = BundleAnalysisCommentMarkdownStrategy() - result = await strategy.send_message(context, message) + result = strategy.send_message(context, message) assert result == NotificationResult( notification_attempted=True, notification_successful=False, diff --git a/services/bundle_analysis/notify/messages/tests/test_commit_status.py b/services/bundle_analysis/notify/messages/tests/test_commit_status.py index 809bb7540..f48129ecc 100644 --- a/services/bundle_analysis/notify/messages/tests/test_commit_status.py +++ b/services/bundle_analysis/notify/messages/tests/test_commit_status.py @@ -1,7 +1,7 @@ -from unittest.mock import AsyncMock, MagicMock +from unittest.mock import MagicMock import pytest -from asgiref.sync import async_to_sync +from mock import AsyncMock from shared.torngit.exceptions import TorngitClientError from shared.typings.torngit import TorngitInstanceData from shared.yaml import UserYaml @@ -153,9 +153,10 @@ def test_build_message_from_samples( def _setup_send_message_tests( self, dbsession, mocker, torngit_ghapp_data, mock_storage ): - fake_repo_provider = AsyncMock( + fake_repo_provider = MagicMock( name="fake_repo_provider", data=TorngitInstanceData(installation=torngit_ghapp_data), + set_commit_status=AsyncMock(), ) fake_repo_provider.set_commit_status.return_value = {"id": 1000} mocker.patch( @@ -222,13 +223,13 @@ def test_send_message_success( dbsession, mocker, torngit_ghapp_data, mock_storage ) strategy = CommitStatusMessageStrategy() - result = async_to_sync(strategy.send_message)(context, message) + result = strategy.send_message(context, message) fake_repo_provider.set_commit_status.assert_called_with( - commit=context.commit.commitid, - status="success", - context="codecov/bundles", - description=message, - url=context.commit_status_url, + context.commit.commitid, + "success", + "codecov/bundles", + message, + context.commit_status_url, ) expected_app = torngit_ghapp_data.get("id") if torngit_ghapp_data else None assert result == NotificationResult( @@ -245,7 +246,7 @@ def test_send_message_fail(self, dbsession, mocker, mock_storage): ) fake_repo_provider.set_commit_status.side_effect = TorngitClientError() strategy = CommitStatusMessageStrategy() - result = async_to_sync(strategy.send_message)(context, message) + result = strategy.send_message(context, message) assert result == NotificationResult( notification_attempted=True, notification_successful=False, @@ -258,7 +259,7 @@ def test_skip_payload_unchanged(self, dbsession, mocker, mock_storage, mock_cach ) strategy = CommitStatusMessageStrategy() mock_cache.get_backend().set(strategy._cache_key(context), 600, message) - result = async_to_sync(strategy.send_message)(context, message) + result = strategy.send_message(context, message) fake_repo_provider.set_commit_status.assert_not_called() assert result == NotificationResult( notification_attempted=False, @@ -266,9 +267,7 @@ def test_skip_payload_unchanged(self, dbsession, mocker, mock_storage, mock_cach explanation="payload_unchanged", ) # Side effect of sending message is updating the cache - assert async_to_sync(strategy.send_message)( - context, message - ) == NotificationResult( + assert strategy.send_message(context, message) == NotificationResult( notification_attempted=False, notification_successful=False, explanation="payload_unchanged", diff --git a/services/bundle_analysis/notify/tests/test_notify_service.py b/services/bundle_analysis/notify/tests/test_notify_service.py index 5a030852b..c41385bcf 100644 --- a/services/bundle_analysis/notify/tests/test_notify_service.py +++ b/services/bundle_analysis/notify/tests/test_notify_service.py @@ -1,4 +1,4 @@ -from unittest.mock import AsyncMock, MagicMock +from unittest.mock import MagicMock import pytest from shared.yaml import UserYaml @@ -39,7 +39,7 @@ def override_comment_builder_and_message_strategy(mocker): "services.bundle_analysis.notify.BundleAnalysisPRCommentContextBuilder", return_value=mock_comment_builder, ) - mock_markdown_strategy = AsyncMock(name="fake_markdown_strategy") + mock_markdown_strategy = MagicMock(name="fake_markdown_strategy") mock_markdown_strategy = mocker.patch( "services.bundle_analysis.notify.BundleAnalysisCommentMarkdownStrategy", return_value=mock_markdown_strategy, @@ -67,7 +67,7 @@ def override_commit_status_builder_and_message_strategy(mocker): "services.bundle_analysis.notify.CommitStatusNotificationContextBuilder", return_value=mock_commit_status_builder, ) - commit_status_message_strategy = AsyncMock(name="fake_markdown_strategy") + commit_status_message_strategy = MagicMock(name="fake_markdown_strategy") commit_status_message_strategy = mocker.patch( "services.bundle_analysis.notify.CommitStatusMessageStrategy", return_value=commit_status_message_strategy, diff --git a/services/comparison/__init__.py b/services/comparison/__init__.py index c18300077..d43b02ece 100644 --- a/services/comparison/__init__.py +++ b/services/comparison/__init__.py @@ -1,19 +1,16 @@ import asyncio import logging from dataclasses import dataclass -from typing import Dict, List, Optional import sentry_sdk +from asgiref.sync import async_to_sync from shared.reports.changes import get_changes_using_rust, run_comparison_using_rust from shared.reports.types import Change, ReportTotals -from shared.torngit.exceptions import ( - TorngitClientGeneralError, -) +from shared.torngit.exceptions import TorngitClientGeneralError from shared.utils.sessions import SessionType from database.enums import CompareCommitState, TestResultsProcessingError from database.models import CompareCommit -from helpers.metrics import metrics from services.archive import ArchiveService from services.comparison.changes import get_changes from services.comparison.overlays import get_overlay @@ -58,7 +55,7 @@ class ComparisonProxy(object): """ def __init__( - self, comparison: Comparison, context: Optional[ComparisonContext] = None + self, comparison: Comparison, context: ComparisonContext | None = None ): self.comparison = comparison self._repository_service = None @@ -69,14 +66,11 @@ def __init__( self._existing_statuses = None self._behind_by = None self._branch = None - self._diff_lock = asyncio.Lock() - self._changes_lock = asyncio.Lock() - self._existing_statuses_lock = asyncio.Lock() self._behind_by_lock = asyncio.Lock() self._archive_service = None self._overlays = {} self.context = context or ComparisonContext() - self._cached_reports_uploaded_per_flag: List[ReportUploadedCount] | None = None + self._cached_reports_uploaded_per_flag: list[ReportUploadedCount] | None = None def get_archive_service(self): if self._archive_service is None: @@ -85,7 +79,6 @@ def get_archive_service(self): ) return self._archive_service - @metrics.timer("internal.services.comparison.get_filtered_comparison") def get_filtered_comparison(self, flags, path_patterns): if not flags and not path_patterns: return self @@ -122,156 +115,144 @@ def enriched_pull(self): def pull(self): return self.comparison.pull - async def get_diff(self, use_original_base=False): - async with self._diff_lock: - head = self.comparison.head.commit - base = self.comparison.project_coverage_base.commit - patch_coverage_base_commitid = self.comparison.patch_coverage_base_commitid + def get_diff(self, use_original_base=False): + head = self.comparison.head.commit + base = self.comparison.project_coverage_base.commit + patch_coverage_base_commitid = self.comparison.patch_coverage_base_commitid - # If the original and adjusted bases are the same commit, then if we - # already fetched the diff for one we can return it for the other. - bases_match = patch_coverage_base_commitid == ( - base.commitid if base else "" - ) + # If the original and adjusted bases are the same commit, then if we + # already fetched the diff for one we can return it for the other. + bases_match = patch_coverage_base_commitid == (base.commitid if base else "") - populate_original_base_diff = use_original_base and ( - not self._original_base_diff - ) - populate_adjusted_base_diff = (not use_original_base) and ( - not self._adjusted_base_diff + populate_original_base_diff = use_original_base and ( + not self._original_base_diff + ) + populate_adjusted_base_diff = (not use_original_base) and ( + not self._adjusted_base_diff + ) + if populate_original_base_diff: + if bases_match and self._adjusted_base_diff: + self._original_base_diff = self._adjusted_base_diff + elif patch_coverage_base_commitid is not None: + pull_diff = async_to_sync(self.repository_service.get_compare)( + patch_coverage_base_commitid, head.commitid, with_commits=False + ) + self._original_base_diff = pull_diff["diff"] + else: + return None + elif populate_adjusted_base_diff: + if bases_match and self._original_base_diff: + self._adjusted_base_diff = self._original_base_diff + elif base is not None: + pull_diff = async_to_sync(self.repository_service.get_compare)( + base.commitid, head.commitid, with_commits=False + ) + self._adjusted_base_diff = pull_diff["diff"] + else: + return None + + if use_original_base: + return self._original_base_diff + else: + return self._adjusted_base_diff + + def get_changes(self) -> list[Change] | None: + if self._changes is None: + diff = self.get_diff() + self._changes = get_changes( + self.comparison.project_coverage_base.report, + self.comparison.head.report, + diff, ) - if populate_original_base_diff: - if bases_match and self._adjusted_base_diff: - self._original_base_diff = self._adjusted_base_diff - elif patch_coverage_base_commitid is not None: - pull_diff = await self.repository_service.get_compare( - patch_coverage_base_commitid, head.commitid, with_commits=False - ) - self._original_base_diff = pull_diff["diff"] - else: - return None - elif populate_adjusted_base_diff: - if bases_match and self._original_base_diff: - self._adjusted_base_diff = self._original_base_diff - elif base is not None: - pull_diff = await self.repository_service.get_compare( - base.commitid, head.commitid, with_commits=False + if ( + self.comparison.project_coverage_base.report is not None + and self.comparison.head.report is not None + and self.comparison.project_coverage_base.report.rust_report is not None + and self.comparison.head.report.rust_report is not None + ): + rust_changes = get_changes_using_rust( + self.comparison.project_coverage_base.report, + self.comparison.head.report, + diff, + ) + original_paths = set([c.path for c in self._changes]) + new_paths = set([c.path for c in rust_changes]) + if original_paths != new_paths: + only_on_new = sorted(new_paths - original_paths) + only_on_original = sorted(original_paths - new_paths) + log.info( + "There are differences between python changes and rust changes", + extra=dict( + only_on_new=only_on_new[:100], + only_on_original=only_on_original[:100], + repoid=self.head.commit.repoid, + ), ) - self._adjusted_base_diff = pull_diff["diff"] - else: - return None - if use_original_base: - return self._original_base_diff - else: - return self._adjusted_base_diff - - async def get_changes(self) -> Optional[List[Change]]: - # Just make sure to not cause a deadlock between this and get_diff - async with self._changes_lock: - if self._changes is None: - diff = await self.get_diff() - with metrics.timer( - "internal.worker.services.comparison.changes.get_changes_python" - ): - self._changes = get_changes( - self.comparison.project_coverage_base.report, - self.comparison.head.report, - diff, - ) - if ( - self.comparison.project_coverage_base.report is not None - and self.comparison.head.report is not None - and self.comparison.project_coverage_base.report.rust_report - is not None - and self.comparison.head.report.rust_report is not None - ): - with metrics.timer( - "internal.worker.services.comparison.changes.get_changes_rust" - ): - rust_changes = get_changes_using_rust( - self.comparison.project_coverage_base.report, - self.comparison.head.report, - diff, - ) - original_paths = set([c.path for c in self._changes]) - new_paths = set([c.path for c in rust_changes]) - if original_paths != new_paths: - only_on_new = sorted(new_paths - original_paths) - only_on_original = sorted(original_paths - new_paths) - log.info( - "There are differences between python changes and rust changes", - extra=dict( - only_on_new=only_on_new[:100], - only_on_original=only_on_original[:100], - repoid=self.head.commit.repoid, - ), - ) - return self._changes + return self._changes @sentry_sdk.trace - async def get_patch_totals(self) -> ReportTotals | None: + def get_patch_totals(self) -> ReportTotals | None: """Returns the patch coverage for the comparison. Patch coverage refers to looking at the coverage in HEAD report filtered by the git diff HEAD..BASE. """ if self._patch_totals: return self._patch_totals - diff = await self.get_diff(use_original_base=True) + diff = self.get_diff(use_original_base=True) self._patch_totals = self.head.report.apply_diff(diff) return self._patch_totals - async def get_behind_by(self): - async with self._behind_by_lock: - if self._behind_by is None: - if not getattr( - self.comparison.project_coverage_base.commit, "commitid", None - ): - log.info( - "Comparison base commit does not have commitid, unable to get behind_by" + def get_behind_by(self): + if self._behind_by is None: + if not getattr( + self.comparison.project_coverage_base.commit, "commitid", None + ): + log.info( + "Comparison base commit does not have commitid, unable to get behind_by" + ) + return None + + provider_pull = self.comparison.enriched_pull.provider_pull + if provider_pull is None: + log.info( + "Comparison does not have provider pull request information, unable to get behind_by" + ) + return None + branch_to_get = provider_pull["base"]["branch"] + if self._branch is None: + try: + branch_response = async_to_sync(self.repository_service.get_branch)( + branch_to_get + ) + except TorngitClientGeneralError: + log.warning( + "Unable to fetch base branch from Git provider", + extra=dict( + branch=branch_to_get, + ), ) return None - - provider_pull = self.comparison.enriched_pull.provider_pull - if provider_pull is None: - log.info( - "Comparison does not have provider pull request information, unable to get behind_by" + except KeyError: + log.warning( + "Error fetching base branch from Git provider", + extra=dict( + branch=branch_to_get, + ), ) return None - branch_to_get = provider_pull["base"]["branch"] - if self._branch is None: - try: - branch_response = await self.repository_service.get_branch( - branch_to_get - ) - except TorngitClientGeneralError: - log.warning( - "Unable to fetch base branch from Git provider", - extra=dict( - branch=branch_to_get, - ), - ) - return None - except KeyError: - log.warning( - "Error fetching base branch from Git provider", - extra=dict( - branch=branch_to_get, - ), - ) - return None - self._branch = branch_response - - distance = await self.repository_service.get_distance_in_commits( - self._branch["sha"], - self.comparison.project_coverage_base.commit.commitid, - with_commits=False, - ) - self._behind_by = distance["behind_by"] - self.enriched_pull.database_pull.behind_by = distance["behind_by"] - self.enriched_pull.database_pull.behind_by_commit = distance[ - "behind_by_commit" - ] + self._branch = branch_response + + distance = async_to_sync(self.repository_service.get_distance_in_commits)( + self._branch["sha"], + self.comparison.project_coverage_base.commit.commitid, + with_commits=False, + ) + self._behind_by = distance["behind_by"] + self.enriched_pull.database_pull.behind_by = distance["behind_by"] + self.enriched_pull.database_pull.behind_by_commit = distance[ + "behind_by_commit" + ] return self._behind_by def all_tests_passed(self): @@ -280,30 +261,28 @@ def all_tests_passed(self): def test_results_error(self): return self.context is not None and self.context.test_results_error - async def get_existing_statuses(self): - async with self._existing_statuses_lock: - if self._existing_statuses is None: - self._existing_statuses = ( - await self.repository_service.get_commit_statuses( - self.head.commit.commitid - ) - ) - return self._existing_statuses + def get_existing_statuses(self): + if self._existing_statuses is None: + self._existing_statuses = async_to_sync( + self.repository_service.get_commit_statuses + )(self.head.commit.commitid) + return self._existing_statuses def get_overlay(self, overlay_type, **kwargs): if overlay_type not in self._overlays: self._overlays[overlay_type] = get_overlay(overlay_type, self, **kwargs) return self._overlays[overlay_type] - async def get_impacted_files(self): - files_in_diff = await self.get_diff() + @sentry_sdk.trace + def get_impacted_files(self) -> dict: + files_in_diff = self.get_diff() return run_comparison_using_rust( self.comparison.project_coverage_base.report, self.comparison.head.report, files_in_diff, ) - def get_reports_uploaded_count_per_flag(self) -> List[ReportUploadedCount]: + def get_reports_uploaded_count_per_flag(self) -> list[ReportUploadedCount]: """This function counts how many reports (by flag) the BASE and HEAD commit have.""" if self._cached_reports_uploaded_per_flag: # Reports may have many sessions, so it's useful to memoize this function @@ -317,7 +296,7 @@ def get_reports_uploaded_count_per_flag(self) -> List[ReportUploadedCount]: ), ) return [] - per_flag_dict: Dict[str, ReportUploadedCount] = dict() + per_flag_dict: dict[str, ReportUploadedCount] = dict() base_report = self.comparison.project_coverage_base.report head_report = self.comparison.head.report ops = [(base_report, "base_count"), (head_report, "head_count")] @@ -341,7 +320,7 @@ def get_reports_uploaded_count_per_flag(self) -> List[ReportUploadedCount]: self._cached_reports_uploaded_per_flag = list(per_flag_dict.values()) return self._cached_reports_uploaded_per_flag - def get_reports_uploaded_count_per_flag_diff(self) -> List[ReportUploadedCount]: + def get_reports_uploaded_count_per_flag_diff(self) -> list[ReportUploadedCount]: """ Returns the difference, per flag, or reports uploaded in BASE and HEAD @@ -395,28 +374,27 @@ def __init__(self, real_comparison: ComparisonProxy, *, flags, path_patterns): commit=real_comparison.head.commit, report=real_comparison.head.report.filter(flags=flags, paths=path_patterns), ) - self._changes_lock = asyncio.Lock() - async def get_impacted_files(self): - return await self.real_comparison.get_impacted_files() + def get_impacted_files(self) -> dict: + return self.real_comparison.get_impacted_files() - async def get_diff(self, use_original_base=False): - return await self.real_comparison.get_diff(use_original_base=use_original_base) + def get_diff(self, use_original_base=False): + return self.real_comparison.get_diff(use_original_base=use_original_base) @sentry_sdk.trace - async def get_patch_totals(self) -> ReportTotals | None: + def get_patch_totals(self) -> ReportTotals | None: """Returns the patch coverage for the comparison. Patch coverage refers to looking at the coverage in HEAD report filtered by the git diff HEAD..BASE. """ if self._patch_totals: return self._patch_totals - diff = await self.get_diff(use_original_base=True) + diff = self.get_diff(use_original_base=True) self._patch_totals = self.head.report.apply_diff(diff) return self._patch_totals - async def get_existing_statuses(self): - return await self.real_comparison.get_existing_statuses() + def get_existing_statuses(self): + return self.real_comparison.get_existing_statuses() def has_project_coverage_base_report(self): return self.real_comparison.has_project_coverage_base_report() @@ -425,15 +403,13 @@ def has_project_coverage_base_report(self): def enriched_pull(self): return self.real_comparison.enriched_pull - async def get_changes(self) -> Optional[List[Change]]: - # Just make sure to not cause a deadlock between this and get_diff - async with self._changes_lock: - if self._changes is None: - diff = await self.get_diff() - self._changes = get_changes( - self.project_coverage_base.report, self.head.report, diff - ) - return self._changes + def get_changes(self) -> list[Change] | None: + if self._changes is None: + diff = self.get_diff() + self._changes = get_changes( + self.project_coverage_base.report, self.head.report, diff + ) + return self._changes @property def pull(self): diff --git a/services/comparison/overlays/critical_path.py b/services/comparison/overlays/critical_path.py index dcf1d6358..ffc6cda1d 100644 --- a/services/comparison/overlays/critical_path.py +++ b/services/comparison/overlays/critical_path.py @@ -2,6 +2,7 @@ import re from typing import Sequence +from asgiref.sync import async_to_sync from cc_rustyribs import rustify_diff from shared.profiling import ProfilingDataFullAnalyzer, ProfilingSummaryDataAnalyzer from shared.storage.exceptions import FileNotInStorageError @@ -76,7 +77,7 @@ def full_analyzer(self): self._profiling_analyzer = _load_full_profiling_analyzer(self._comparison) return self._profiling_analyzer - async def _get_critical_files_from_yaml(self, filenames_to_search: Sequence[str]): + def _get_critical_files_from_yaml(self, filenames_to_search: Sequence[str]): """ Get list of files in filenames_to_search that match the list of critical_file paths defined by the user in the YAML (under profiling.critical_files_paths) """ @@ -87,7 +88,7 @@ async def _get_critical_files_from_yaml(self, filenames_to_search: Sequence[str] repo_provider = get_repo_provider_service( repo, installation_name_to_use=gh_app_installation_name ) - current_yaml = await get_current_yaml( + current_yaml = async_to_sync(get_current_yaml)( self._comparison.head.commit, repo_provider ) if not current_yaml.get("profiling") or not current_yaml["profiling"].get( @@ -103,9 +104,7 @@ async def _get_critical_files_from_yaml(self, filenames_to_search: Sequence[str] ] return user_defined_critical_files - async def search_files_for_critical_changes( - self, filenames_to_search: Sequence[str] - ): + def search_files_for_critical_changes(self, filenames_to_search: Sequence[str]): """ Returns list of files considered critical in filenames_to_search. Critical files comes from 2 sources: @@ -118,15 +117,15 @@ async def search_files_for_critical_changes( self._critical_path_report.get_critical_files_filenames() ) critical_files_from_yaml = set( - await self._get_critical_files_from_yaml(filenames_to_search) + self._get_critical_files_from_yaml(filenames_to_search) ) return list(critical_files_from_profiling | critical_files_from_yaml) - async def find_impacted_endpoints(self): + def find_impacted_endpoints(self): analyzer = self.full_analyzer if analyzer is None: return None - diff = rustify_diff(await self._comparison.get_diff()) + diff = rustify_diff(self._comparison.get_diff()) return self.full_analyzer.find_impacted_endpoints( self._comparison.project_coverage_base.report.rust_report.get_report(), self._comparison.head.report.rust_report.get_report(), diff --git a/services/comparison/tests/unit/overlay/test_critical_path.py b/services/comparison/tests/unit/overlay/test_critical_path.py index 5ace911d2..b78fb26c5 100644 --- a/services/comparison/tests/unit/overlay/test_critical_path.py +++ b/services/comparison/tests/unit/overlay/test_critical_path.py @@ -143,22 +143,20 @@ def test_load_critical_path_report_yes_commit_no_storage( assert _load_full_profiling_analyzer(sample_comparison) is None -@pytest.mark.asyncio -async def test_critical_files_from_yaml_no_paths(mocker, sample_comparison): +def test_critical_files_from_yaml_no_paths(mocker, sample_comparison): sample_comparison.comparison.current_yaml = dict() mocked_get_yaml = mocker.patch( "services.comparison.overlays.critical_path.get_current_yaml" ) overlay = CriticalPathOverlay(sample_comparison, None) - critical_paths_from_yaml = await overlay._get_critical_files_from_yaml( + critical_paths_from_yaml = overlay._get_critical_files_from_yaml( ["batata.txt", "a.py"] ) assert critical_paths_from_yaml == [] mocked_get_yaml.assert_not_called() -@pytest.mark.asyncio -async def test_critical_files_from_yaml_with_paths(mocker, sample_comparison): +def test_critical_files_from_yaml_with_paths(mocker, sample_comparison): sample_comparison.comparison.current_yaml = { "profiling": { "critical_files_paths": ["src/critical", "important.txt"], @@ -168,15 +166,14 @@ async def test_critical_files_from_yaml_with_paths(mocker, sample_comparison): "services.comparison.overlays.critical_path.get_current_yaml" ) overlay = CriticalPathOverlay(sample_comparison, None) - critical_paths_from_yaml = await overlay._get_critical_files_from_yaml( + critical_paths_from_yaml = overlay._get_critical_files_from_yaml( ["batata.txt", "src/critical/a.py"] ) assert critical_paths_from_yaml == ["src/critical/a.py"] mocked_get_yaml.assert_not_called() -@pytest.mark.asyncio -async def test_critical_files_from_yaml_with_paths_get_yaml_from_provider( +def test_critical_files_from_yaml_with_paths_get_yaml_from_provider( mocker, sample_comparison ): mocked_get_yaml = mocker.patch( @@ -188,7 +185,7 @@ async def test_critical_files_from_yaml_with_paths_get_yaml_from_provider( }, ) overlay = CriticalPathOverlay(sample_comparison, None) - critical_paths_from_yaml = await overlay._get_critical_files_from_yaml( + critical_paths_from_yaml = overlay._get_critical_files_from_yaml( ["batata.txt", "src/critical/a.py"] ) assert critical_paths_from_yaml == ["src/critical/a.py"] @@ -196,19 +193,12 @@ async def test_critical_files_from_yaml_with_paths_get_yaml_from_provider( class TestCriticalPathOverlay(object): - @pytest.mark.asyncio - async def test_search_files_for_critical_changes_none_report( - self, sample_comparison - ): + def test_search_files_for_critical_changes_none_report(self, sample_comparison): sample_comparison.comparison.current_yaml = dict() a = CriticalPathOverlay(sample_comparison, None) - assert ( - await a.search_files_for_critical_changes(["filenames", "to", "search"]) - == [] - ) + assert a.search_files_for_critical_changes(["filenames", "to", "search"]) == [] - @pytest.mark.asyncio - async def test_search_files_for_critical_changes_none_report_with_yaml_path( + def test_search_files_for_critical_changes_none_report_with_yaml_path( self, sample_comparison, mocker ): sample_comparison.comparison.current_yaml = { @@ -217,18 +207,16 @@ async def test_search_files_for_critical_changes_none_report_with_yaml_path( } } a = CriticalPathOverlay(sample_comparison, None) - assert await a.search_files_for_critical_changes( + assert a.search_files_for_critical_changes( ["filenames", "to", "search", "important.txt"] ) == ["important.txt"] - @pytest.mark.asyncio - async def test_find_impacted_endpoints_no_analyzer(self, sample_comparison): + def test_find_impacted_endpoints_no_analyzer(self, sample_comparison): a = CriticalPathOverlay(sample_comparison, None) a._profiling_analyzer = None - await a.find_impacted_endpoints() is None + a.find_impacted_endpoints() is None - @pytest.mark.asyncio - async def test_find_impacted_endpoints( + def test_find_impacted_endpoints( self, dbsession, sample_comparison, @@ -288,7 +276,7 @@ async def test_find_impacted_endpoints( a = CriticalPathOverlay(sample_comparison, None) print(sample_comparison.head.report.files) print(sample_comparison.head.report.files) - res = await a.find_impacted_endpoints() + res = a.find_impacted_endpoints() assert res == [ { "files": [{"filename": "file_1.go", "impacted_base_lines": [5]}], diff --git a/services/comparison/tests/unit/test_behind_by.py b/services/comparison/tests/unit/test_behind_by.py index d3cbd4e9c..d910da388 100644 --- a/services/comparison/tests/unit/test_behind_by.py +++ b/services/comparison/tests/unit/test_behind_by.py @@ -1,12 +1,10 @@ -import pytest from shared.torngit.exceptions import TorngitClientGeneralError from services.comparison import ComparisonProxy class TestGetBehindBy(object): - @pytest.mark.asyncio - async def test_get_behind_by(self, mocker, mock_repo_provider): + def test_get_behind_by(self, mocker, mock_repo_provider): comparison = ComparisonProxy(mocker.MagicMock()) comparison.comparison.enriched_pull.provider_pull = {"base": {"branch": "a"}} mock_repo_provider.get_branches.return_value = [("a", "1")] @@ -18,25 +16,22 @@ async def test_get_behind_by(self, mocker, mock_repo_provider): "services.comparison.get_repo_provider_service", return_value=mock_repo_provider, ) - res = await comparison.get_behind_by() + res = comparison.get_behind_by() assert res == 3 - @pytest.mark.asyncio - async def test_get_behind_by_no_base_commit(self, mocker): + def test_get_behind_by_no_base_commit(self, mocker): comparison = ComparisonProxy(mocker.MagicMock()) del comparison.comparison.project_coverage_base.commit.commitid - res = await comparison.get_behind_by() + res = comparison.get_behind_by() assert res is None - @pytest.mark.asyncio - async def test_get_behind_by_no_provider_pull(self, mocker): + def test_get_behind_by_no_provider_pull(self, mocker): comparison = ComparisonProxy(mocker.MagicMock()) comparison.comparison.enriched_pull.provider_pull = None - res = await comparison.get_behind_by() + res = comparison.get_behind_by() assert res is None - @pytest.mark.asyncio - async def test_get_behind_by_no_matching_branches(self, mocker, mock_repo_provider): + def test_get_behind_by_no_matching_branches(self, mocker, mock_repo_provider): mock_repo_provider.get_branch.side_effect = TorngitClientGeneralError( 404, None, @@ -47,5 +42,5 @@ async def test_get_behind_by_no_matching_branches(self, mocker, mock_repo_provid return_value=mock_repo_provider, ) comparison = ComparisonProxy(mocker.MagicMock()) - res = await comparison.get_behind_by() + res = comparison.get_behind_by() assert res is None diff --git a/services/comparison/tests/unit/test_comparison_proxy.py b/services/comparison/tests/unit/test_comparison_proxy.py index d4ab458aa..399d0bade 100644 --- a/services/comparison/tests/unit/test_comparison_proxy.py +++ b/services/comparison/tests/unit/test_comparison_proxy.py @@ -1,4 +1,3 @@ -import pytest from mock import call, patch from database.tests.factories import CommitFactory, PullFactory, RepositoryFactory @@ -44,12 +43,11 @@ def make_sample_comparison(adjusted_base=False): class TestComparisonProxy(object): compare_url = "https://api.github.com/repos/{}/compare/{}...{}" - @pytest.mark.asyncio @patch("shared.torngit.github.Github.get_compare") - async def test_get_diff_adjusted_base(self, mock_get_compare): + def test_get_diff_adjusted_base(self, mock_get_compare): comparison = make_sample_comparison(adjusted_base=True) mock_get_compare.return_value = {"diff": "magic string"} - result = await comparison.get_diff(use_original_base=False) + result = comparison.get_diff(use_original_base=False) assert result == "magic string" assert comparison._adjusted_base_diff == "magic string" @@ -67,12 +65,11 @@ async def test_get_diff_adjusted_base(self, mock_get_compare): ), ] - @pytest.mark.asyncio @patch("shared.torngit.github.Github.get_compare") - async def test_get_diff_original_base(self, mock_get_compare): + def test_get_diff_original_base(self, mock_get_compare): comparison = make_sample_comparison(adjusted_base=True) mock_get_compare.return_value = {"diff": "magic string"} - result = await comparison.get_diff(use_original_base=True) + result = comparison.get_diff(use_original_base=True) assert result == "magic string" assert comparison._original_base_diff == "magic string" @@ -90,12 +87,11 @@ async def test_get_diff_original_base(self, mock_get_compare): ), ] - @pytest.mark.asyncio @patch("shared.torngit.github.Github.get_compare") - async def test_get_diff_bases_match_original_base(self, mock_get_compare): + def test_get_diff_bases_match_original_base(self, mock_get_compare): comparison = make_sample_comparison(adjusted_base=False) mock_get_compare.return_value = {"diff": "magic string"} - result = await comparison.get_diff(use_original_base=True) + result = comparison.get_diff(use_original_base=True) assert result == "magic string" assert comparison._original_base_diff == "magic string" @@ -106,7 +102,7 @@ async def test_get_diff_bases_match_original_base(self, mock_get_compare): # In this test case, the adjusted and original base commits are the # same. If we get one, we should set the cache for the other. - adjusted_base_result = await comparison.get_diff(use_original_base=False) + adjusted_base_result = comparison.get_diff(use_original_base=False) assert comparison._adjusted_base_diff == "magic string" # Make sure we only called the Git provider API once @@ -118,12 +114,11 @@ async def test_get_diff_bases_match_original_base(self, mock_get_compare): ), ] - @pytest.mark.asyncio @patch("shared.torngit.github.Github.get_compare") - async def test_get_diff_bases_match_adjusted_base(self, mock_get_compare): + def test_get_diff_bases_match_adjusted_base(self, mock_get_compare): comparison = make_sample_comparison(adjusted_base=False) mock_get_compare.return_value = {"diff": "magic string"} - result = await comparison.get_diff(use_original_base=False) + result = comparison.get_diff(use_original_base=False) assert result == "magic string" assert comparison._adjusted_base_diff == "magic string" @@ -134,7 +129,7 @@ async def test_get_diff_bases_match_adjusted_base(self, mock_get_compare): # In this test case, the adjusted and original base commits are the # same. If we get one, we should set the cache for the other. - adjusted_base_result = await comparison.get_diff(use_original_base=True) + adjusted_base_result = comparison.get_diff(use_original_base=True) assert comparison._adjusted_base_diff == "magic string" # Make sure we only called the Git provider API once diff --git a/services/comparison/types.py b/services/comparison/types.py index 603dab364..0229e8bca 100644 --- a/services/comparison/types.py +++ b/services/comparison/types.py @@ -15,9 +15,9 @@ class FullCommit(object): class ReportUploadedCount(TypedDict): - flag: str = "" - base_count: int = 0 - head_count: int = 0 + flag: str + base_count: int + head_count: int @dataclass diff --git a/services/notification/__init__.py b/services/notification/__init__.py index 9fad78dbf..0a3fb2bc1 100644 --- a/services/notification/__init__.py +++ b/services/notification/__init__.py @@ -5,7 +5,6 @@ """ -import asyncio import logging from typing import Iterator, List, TypedDict @@ -16,7 +15,6 @@ from shared.yaml import UserYaml from database.models.core import GITHUB_APP_INSTALLATION_DEFAULT_NAME, Owner, Repository -from helpers.metrics import metrics from services.comparison import ComparisonProxy from services.decoration import Decoration from services.license import is_properly_licensed @@ -233,7 +231,7 @@ def get_statuses(self, current_flags: List[str]): for component_status in self._get_component_statuses(current_flags): yield component_status - async def notify(self, comparison: ComparisonProxy) -> List[NotificationResult]: + def notify(self, comparison: ComparisonProxy) -> list[NotificationResult]: if not is_properly_licensed(comparison.head.commit.get_db_session()): log.warning( "Not sending notifications because the system is not properly licensed" @@ -256,18 +254,13 @@ async def notify(self, comparison: ComparisonProxy) -> List[NotificationResult]: for notifier in self.get_notifiers_instances() if notifier.is_enabled() ] - results = [] - chunk_size = 3 - for i in range(0, len(notification_instances), chunk_size): - notification_instances_chunk = notification_instances[i : i + chunk_size] - task_chunk = [ - self.notify_individual_notifier(notifier, comparison) - for notifier in notification_instances_chunk - ] - results.extend(await asyncio.gather(*task_chunk)) + results = [ + self.notify_individual_notifier(notifier, comparison) + for notifier in notification_instances + ] return results - async def notify_individual_notifier( + def notify_individual_notifier( self, notifier: AbstractBaseNotifier, comparison: ComparisonProxy ) -> NotificationResult: commit = comparison.head.commit @@ -289,17 +282,13 @@ async def notify_individual_notifier( notifier=notifier.name, title=notifier.title, result=None ) try: - with metrics.timer( - f"worker.services.notifications.notifiers.{notifier.name}" - ) as notify_timer: - res = await notifier.notify(comparison) + res = notifier.notify(comparison) individual_result["result"] = res notifier.store_results(comparison, res) log.info( "Individual notification done", extra=dict( - timing_ms=notify_timer.ms, individual_result=individual_result, commit=commit.commitid, base_commit=( @@ -311,28 +300,6 @@ async def notify_individual_notifier( return individual_result except (CeleryError, SoftTimeLimitExceeded): raise - except asyncio.TimeoutError: - log.warning( - "Individual notifier timed out", - extra=dict( - repoid=commit.repoid, - commit=commit.commitid, - individual_result=individual_result, - base_commit=( - base_commit.commitid if base_commit is not None else "NO_BASE" - ), - ), - ) - return individual_result - except asyncio.CancelledError as e: - log.warning( - "Individual notifier cancelled", - extra=dict( - repoid=commit.repoid, commit=commit.commitid, exception=str(e) - ), - exc_info=True, - ) - raise except Exception: log.exception( "Individual notifier failed", diff --git a/services/notification/notifiers/base.py b/services/notification/notifiers/base.py index 61e1ae7f2..39c4737bf 100644 --- a/services/notification/notifiers/base.py +++ b/services/notification/notifiers/base.py @@ -14,7 +14,7 @@ class NotificationResult(object): notification_attempted: bool = False notification_successful: bool = False - explanation: str = None + explanation: str | None = None data_sent: Mapping[str, Any] | None = None data_received: Mapping[str, Any] | None = None github_app_used: int | None = None @@ -53,7 +53,7 @@ def __init__( notifier_yaml_settings: Mapping[str, Any], notifier_site_settings: Mapping[str, Any], current_yaml: Mapping[str, Any], - decoration_type: Decoration = None, + decoration_type: Decoration | None = None, gh_installation_name_to_use: str = GITHUB_APP_INSTALLATION_DEFAULT_NAME, ): """ @@ -78,7 +78,7 @@ def __init__( def name(self) -> str: raise NotImplementedError() - async def notify(self, comparison: Comparison, **extra_data) -> NotificationResult: + def notify(self, comparison: Comparison, **extra_data) -> NotificationResult: raise NotImplementedError() def is_enabled(self) -> bool: diff --git a/services/notification/notifiers/checks/base.py b/services/notification/notifiers/checks/base.py index c575bf234..1279f1cf2 100644 --- a/services/notification/notifiers/checks/base.py +++ b/services/notification/notifiers/checks/base.py @@ -2,9 +2,9 @@ from contextlib import nullcontext from typing import Dict +from asgiref.sync import async_to_sync from shared.torngit.exceptions import TorngitClientError, TorngitError -from helpers.metrics import metrics from services.notification.notifiers.base import Comparison, NotificationResult from services.notification.notifiers.status.base import StatusNotifier from services.urls import ( @@ -64,14 +64,14 @@ def paginate_annotations(self, annotations): for i in range(0, len(annotations), self.ANNOTATIONS_PER_REQUEST): yield annotations[i : i + self.ANNOTATIONS_PER_REQUEST] - async def build_payload(self, comparison) -> Dict[str, str]: + def build_payload(self, comparison) -> Dict[str, str]: raise NotImplementedError() def get_status_external_name(self) -> str: status_piece = f"/{self.title}" if self.title != "default" else "" return f"codecov/{self.context}{status_piece}" - async def notify(self, comparison: Comparison): + def notify(self, comparison: Comparison): if comparison.pull is None or (): log.debug( "Falling back to commit_status: Not a pull request", @@ -152,7 +152,7 @@ async def notify(self, comparison: Comparison): ) ) if not comparison.has_head_report(): - payload = await self.build_payload(comparison) + payload = self.build_payload(comparison) elif ( flag_coverage_not_uploaded_behavior == "exclude" and not self.flag_coverage_was_uploaded(comparison) @@ -171,7 +171,7 @@ async def notify(self, comparison: Comparison): filtered_comparison = comparison.get_filtered_comparison( **self.get_notifier_filters() ) - payload = await self.build_payload(filtered_comparison) + payload = self.build_payload(filtered_comparison) payload["state"] = "success" payload["output"]["summary"] = ( payload.get("output", {}).get("summary", "") @@ -181,12 +181,12 @@ async def notify(self, comparison: Comparison): filtered_comparison = comparison.get_filtered_comparison( **self.get_notifier_filters() ) - payload = await self.build_payload(filtered_comparison) + payload = self.build_payload(filtered_comparison) if comparison.pull: payload["url"] = get_pull_url(comparison.pull) else: payload["url"] = get_commit_url(comparison.head.commit) - return await self.maybe_send_notification(comparison, payload) + return self.maybe_send_notification(comparison, payload) except TorngitClientError as e: if e.code == 403: raise e @@ -316,7 +316,6 @@ def get_lines_to_annotate(self, comparison, files_with_change): previous_line = line return line_headers - @metrics.timer("worker.services.notifications.notifiers.checks.create_annotations") def create_annotations(self, comparison, diff): files_with_change = [ {"type": _diff["type"], "path": path, "segments": _diff["segments"]} @@ -343,7 +342,7 @@ def create_annotations(self, comparison, diff): annotations.append(annotation) return annotations - async def send_notification(self, comparison: Comparison, payload): + def send_notification(self, comparison: Comparison, payload): title = self.get_status_external_name() head = comparison.head.commit repository_service = self.repository_service(head) @@ -382,12 +381,9 @@ async def send_notification(self, comparison: Comparison, payload): ) # We need to first create the check run, get that id and update the status - with metrics.timer( - "worker.services.notifications.notifiers.checks.create_check_run" - ): - check_id = await repository_service.create_check_run( - check_name=title, head_sha=head.commitid - ) + check_id = async_to_sync(repository_service.create_check_run)( + check_name=title, head_sha=head.commitid + ) if len(output.get("annotations", [])) > self.ANNOTATIONS_PER_REQUEST: annotation_pages = list( @@ -401,27 +397,21 @@ async def send_notification(self, comparison: Comparison, payload): ), ) for annotation_page in annotation_pages: - with metrics.timer( - "worker.services.notifications.notifiers.checks.update_check_run" - ): - await repository_service.update_check_run( - check_id, - state, - output={ - "title": output.get("title"), - "summary": output.get("summary"), - "annotations": annotation_page, - }, - url=payload.get("url"), - ) + async_to_sync(repository_service.update_check_run)( + check_id, + state, + output={ + "title": output.get("title"), + "summary": output.get("summary"), + "annotations": annotation_page, + }, + url=payload.get("url"), + ) else: - with metrics.timer( - "worker.services.notifications.notifiers.checks.update_check_run" - ): - await repository_service.update_check_run( - check_id, state, output=output, url=payload.get("url") - ) + async_to_sync(repository_service.update_check_run)( + check_id, state, output=output, url=payload.get("url") + ) return NotificationResult( notification_attempted=True, diff --git a/services/notification/notifiers/checks/changes.py b/services/notification/notifiers/checks/changes.py index 58ed9d4b1..09873f230 100644 --- a/services/notification/notifiers/checks/changes.py +++ b/services/notification/notifiers/checks/changes.py @@ -12,7 +12,7 @@ class ChangesChecksNotifier(StatusChangesMixin, ChecksNotifier): def notification_type(self) -> Notification: return Notification.checks_changes - async def build_payload(self, comparison) -> Dict[str, str]: + def build_payload(self, comparison) -> Dict[str, str]: if self.is_empty_upload(): state, message = self.get_status_check_for_empty_upload() return { @@ -22,7 +22,7 @@ async def build_payload(self, comparison) -> Dict[str, str]: "summary": message, }, } - state, message = await self.get_changes_status(comparison) + state, message = self.get_changes_status(comparison) codecov_link = self.get_codecov_pr_link(comparison) title = message diff --git a/services/notification/notifiers/checks/checks_with_fallback.py b/services/notification/notifiers/checks/checks_with_fallback.py index 3d9272e0c..ea57e2bd8 100644 --- a/services/notification/notifiers/checks/checks_with_fallback.py +++ b/services/notification/notifiers/checks/checks_with_fallback.py @@ -45,9 +45,9 @@ def decoration_type(self): def store_results(self, comparison, res): pass - async def notify(self, comparison): + def notify(self, comparison): try: - res = await self._checks_notifier.notify(comparison) + res = self._checks_notifier.notify(comparison) if not res.notification_successful and ( res.explanation == "no_pull_request" or res.explanation == "pull_request_not_in_provider" @@ -63,7 +63,7 @@ async def notify(self, comparison): explanation=res.explanation, ), ) - res = await self._status_notifier.notify(comparison) + res = self._status_notifier.notify(comparison) return res except TorngitClientError as e: if e.code == 403: @@ -76,5 +76,5 @@ async def notify(self, comparison): commit=comparison.head.commit, ), ) - return await self._status_notifier.notify(comparison) + return self._status_notifier.notify(comparison) raise e diff --git a/services/notification/notifiers/checks/patch.py b/services/notification/notifiers/checks/patch.py index 81947626c..feadb1a77 100644 --- a/services/notification/notifiers/checks/patch.py +++ b/services/notification/notifiers/checks/patch.py @@ -1,5 +1,4 @@ from database.enums import Notification -from helpers.metrics import metrics from services.notification.notifiers.base import Comparison from services.notification.notifiers.checks.base import ChecksNotifier from services.notification.notifiers.mixins.status import StatusPatchMixin @@ -13,7 +12,7 @@ class PatchChecksNotifier(StatusPatchMixin, ChecksNotifier): def notification_type(self) -> Notification: return Notification.checks_patch - async def build_payload(self, comparison: Comparison): + def build_payload(self, comparison: Comparison): """ This method build the paylod of the patch github checks. @@ -29,52 +28,49 @@ async def build_payload(self, comparison: Comparison): "summary": message, }, } - with metrics.timer( - "worker.services.notifications.notifiers.checks.patch.build_payload" - ): - state, message = await self.get_patch_status(comparison) - codecov_link = self.get_codecov_pr_link(comparison) - - title = message + state, message = self.get_patch_status(comparison) + codecov_link = self.get_codecov_pr_link(comparison) - should_use_upgrade = self.should_use_upgrade_decoration() - if should_use_upgrade: - message = self.get_upgrade_message(comparison) - title = "Codecov Report" + title = message - checks_yaml_field = read_yaml_field(self.current_yaml, ("github_checks",)) + should_use_upgrade = self.should_use_upgrade_decoration() + if should_use_upgrade: + message = self.get_upgrade_message(comparison) + title = "Codecov Report" - should_annotate = ( - checks_yaml_field.get("annotations", False) - if checks_yaml_field is not None - else True - ) + checks_yaml_field = read_yaml_field(self.current_yaml, ("github_checks",)) - flags = self.notifier_yaml_settings.get("flags") - paths = self.notifier_yaml_settings.get("paths") - if ( - flags is not None - or paths is not None - or should_use_upgrade - or should_annotate is False - ): - return { - "state": state, - "output": { - "title": f"{title}", - "summary": "\n\n".join([codecov_link, message]), - }, - } - diff = await comparison.get_diff(use_original_base=True) - # TODO: Look into why the apply diff in get_patch_status is not saving state at this point - comparison.head.report.apply_diff(diff) - annotations = self.create_annotations(comparison, diff) + should_annotate = ( + checks_yaml_field.get("annotations", False) + if checks_yaml_field is not None + else True + ) + flags = self.notifier_yaml_settings.get("flags") + paths = self.notifier_yaml_settings.get("paths") + if ( + flags is not None + or paths is not None + or should_use_upgrade + or should_annotate is False + ): return { "state": state, "output": { "title": f"{title}", "summary": "\n\n".join([codecov_link, message]), - "annotations": annotations, }, } + diff = comparison.get_diff(use_original_base=True) + # TODO: Look into why the apply diff in get_patch_status is not saving state at this point + comparison.head.report.apply_diff(diff) + annotations = self.create_annotations(comparison, diff) + + return { + "state": state, + "output": { + "title": f"{title}", + "summary": "\n\n".join([codecov_link, message]), + "annotations": annotations, + }, + } diff --git a/services/notification/notifiers/checks/project.py b/services/notification/notifiers/checks/project.py index 34f00222f..cb47b37f1 100644 --- a/services/notification/notifiers/checks/project.py +++ b/services/notification/notifiers/checks/project.py @@ -1,5 +1,4 @@ from database.enums import Notification -from helpers.metrics import metrics from services.notification.notifiers.base import Comparison from services.notification.notifiers.checks.base import ChecksNotifier from services.notification.notifiers.mixins.message import MessageMixin @@ -14,11 +13,11 @@ class ProjectChecksNotifier(MessageMixin, StatusProjectMixin, ChecksNotifier): def notification_type(self) -> Notification: return Notification.checks_project - async def get_message(self, comparison: Comparison, yaml_comment_settings): + def get_message(self, comparison: Comparison, yaml_comment_settings): pull_dict = comparison.enriched_pull.provider_pull - return await self.create_message(comparison, pull_dict, yaml_comment_settings) + return self.create_message(comparison, pull_dict, yaml_comment_settings) - async def build_payload(self, comparison: Comparison): + def build_payload(self, comparison: Comparison): """ This method build the paylod of the project github checks. @@ -34,52 +33,48 @@ async def build_payload(self, comparison: Comparison): "summary": message, }, } - with metrics.timer( - "worker.services.notifications.notifiers.checks.project.build_payload" - ): - state, summary = await self.get_project_status(comparison) - codecov_link = self.get_codecov_pr_link(comparison) - title = summary + state, summary = self.get_project_status(comparison) + codecov_link = self.get_codecov_pr_link(comparison) - should_use_upgrade = self.should_use_upgrade_decoration() - if should_use_upgrade: - summary = self.get_upgrade_message(comparison) - title = "Codecov Report" - flags = self.notifier_yaml_settings.get("flags") - paths = self.notifier_yaml_settings.get("paths") - yaml_comment_settings = ( - read_yaml_field(self.current_yaml, ("comment",)) or {} - ) - if yaml_comment_settings is True: - yaml_comment_settings = self.site_settings.get("comment", {}) - # copying to a new variable because we will be modifying that - settings_to_be_used = dict(yaml_comment_settings) - if "flag" in settings_to_be_used.get("layout", ""): - old_flags_list = settings_to_be_used.get("layout", "").split(",") - new_flags_list = [x for x in old_flags_list if "flag" not in x] - settings_to_be_used["layout"] = ",".join(new_flags_list) + title = summary - if ( - flags is not None - or paths is not None - or should_use_upgrade - or not settings_to_be_used - ): - return { - "state": state, - "output": { - "title": f"{title}", - "summary": "\n\n".join([codecov_link, summary]), - }, - } + should_use_upgrade = self.should_use_upgrade_decoration() + if should_use_upgrade: + summary = self.get_upgrade_message(comparison) + title = "Codecov Report" + flags = self.notifier_yaml_settings.get("flags") + paths = self.notifier_yaml_settings.get("paths") + yaml_comment_settings = read_yaml_field(self.current_yaml, ("comment",)) or {} + if yaml_comment_settings is True: + yaml_comment_settings = self.site_settings.get("comment", {}) + # copying to a new variable because we will be modifying that + settings_to_be_used = dict(yaml_comment_settings) + if "flag" in settings_to_be_used.get("layout", ""): + old_flags_list = settings_to_be_used.get("layout", "").split(",") + new_flags_list = [x for x in old_flags_list if "flag" not in x] + settings_to_be_used["layout"] = ",".join(new_flags_list) - message = await self.get_message(comparison, settings_to_be_used) + if ( + flags is not None + or paths is not None + or should_use_upgrade + or not settings_to_be_used + ): return { "state": state, "output": { "title": f"{title}", "summary": "\n\n".join([codecov_link, summary]), - "text": "\n".join(message), }, } + + message = self.get_message(comparison, settings_to_be_used) + return { + "state": state, + "output": { + "title": f"{title}", + "summary": "\n\n".join([codecov_link, summary]), + "text": "\n".join(message), + }, + } diff --git a/services/notification/notifiers/codecov_slack_app.py b/services/notification/notifiers/codecov_slack_app.py index ce9010400..cd99bbd35 100644 --- a/services/notification/notifiers/codecov_slack_app.py +++ b/services/notification/notifiers/codecov_slack_app.py @@ -101,7 +101,7 @@ def build_payload(self, comparison: Comparison): "head_totals_c": str(comparison.head.report.totals.coverage), } - async def notify(self, comparison: Comparison, **extra_data) -> NotificationResult: + def notify(self, comparison: Comparison, **extra_data) -> NotificationResult: request_url = f"{CODECOV_SLACK_APP_URL}/notify" headers = { diff --git a/services/notification/notifiers/comment/__init__.py b/services/notification/notifiers/comment/__init__.py index 79c1a9bc6..a5874dab2 100644 --- a/services/notification/notifiers/comment/__init__.py +++ b/services/notification/notifiers/comment/__init__.py @@ -3,6 +3,7 @@ from typing import Any, List, Mapping import sentry_sdk +from asgiref.sync import async_to_sync from shared.torngit.base import TorngitBaseAdapter from shared.torngit.exceptions import ( TorngitClientError, @@ -11,7 +12,6 @@ ) from database.enums import Notification -from helpers.metrics import metrics from services.billing import BillingPlan from services.comparison import ComparisonProxy from services.comparison.types import Comparison @@ -75,12 +75,10 @@ def name(self) -> str: def notification_type(self) -> Notification: return Notification.comment - async def get_diff(self, comparison: Comparison): - return await comparison.get_diff() + def get_diff(self, comparison: Comparison): + return comparison.get_diff() - async def notify( - self, comparison: ComparisonProxy, **extra_data - ) -> NotificationResult: + def notify(self, comparison: ComparisonProxy, **extra_data) -> NotificationResult: # TODO: remove this when we don't need it anymore # this line is measuring how often we try to comment on a PR that is closed if comparison.pull is not None and comparison.pull.state != "open": @@ -94,17 +92,11 @@ async def notify( ) for condition in self.notify_conditions: - condition_result = ( - await condition.check_condition(notifier=self, comparison=comparison) - if condition.is_async_condition - else condition.check_condition(notifier=self, comparison=comparison) + condition_result = condition.check_condition( + notifier=self, comparison=comparison ) if condition_result == False: - side_effect_result = ( - condition.on_failure_side_effect(self, comparison) - if condition.is_async_condition is False - else (await condition.on_failure_side_effect(self, comparison)) - ) + side_effect_result = condition.on_failure_side_effect(self, comparison) default_result = NotificationResult( notification_attempted=False, explanation=condition.failure_explanation, @@ -114,10 +106,7 @@ async def notify( return default_result.merge(side_effect_result) pull = comparison.pull try: - with metrics.timer( - "worker.services.notifications.notifiers.comment.build_message" - ): - message = await self.build_message(comparison) + message = self.build_message(comparison) except TorngitClientError: log.warning( "Unable to fetch enough information to build message for comment", @@ -135,10 +124,7 @@ async def notify( ) data = {"message": message, "commentid": pull.commentid, "pullid": pull.pullid} try: - with metrics.timer( - "worker.services.notifications.notifiers.comment.send_notifications" - ): - return await self.send_actual_notification(data) + return self.send_actual_notification(data) except TorngitServerFailureError: log.warning( "Unable to send comments because the provider server was not reachable or errored", @@ -153,7 +139,7 @@ async def notify( data_received=None, ) - async def send_actual_notification(self, data: Mapping[str, Any]): + def send_actual_notification(self, data: Mapping[str, Any]): message = "\n".join(data["message"]) # Append tracking parameters to any codecov urls in the message @@ -166,19 +152,19 @@ async def send_actual_notification(self, data: Mapping[str, Any]): behavior = self.notifier_yaml_settings.get("behavior", "default") if behavior == "default": - res = await self.send_comment_default_behavior( + res = self.send_comment_default_behavior( data["pullid"], data["commentid"], message ) elif behavior == "once": - res = await self.send_comment_once_behavior( + res = self.send_comment_once_behavior( data["pullid"], data["commentid"], message ) elif behavior == "new": - res = await self.send_comment_new_behavior( + res = self.send_comment_new_behavior( data["pullid"], data["commentid"], message ) elif behavior == "spammy": - res = await self.send_comment_spammy_behavior( + res = self.send_comment_spammy_behavior( data["pullid"], data["commentid"], message ) return NotificationResult( @@ -189,10 +175,10 @@ async def send_actual_notification(self, data: Mapping[str, Any]): data_received=res["data_received"], ) - async def send_comment_default_behavior(self, pullid, commentid, message): + def send_comment_default_behavior(self, pullid, commentid, message): if commentid: try: - res = await self.repository_service.edit_comment( + res = async_to_sync(self.repository_service.edit_comment)( pullid, commentid, message ) return { @@ -210,7 +196,7 @@ async def send_comment_default_behavior(self, pullid, commentid, message): extra=dict(pullid=pullid, commentid=commentid), ) try: - res = await self.repository_service.post_comment(pullid, message) + res = async_to_sync(self.repository_service.post_comment)(pullid, message) return { "notification_attempted": True, "notification_successful": True, @@ -230,10 +216,10 @@ async def send_comment_default_behavior(self, pullid, commentid, message): "data_received": None, } - async def send_comment_once_behavior(self, pullid, commentid, message): + def send_comment_once_behavior(self, pullid, commentid, message): if commentid: try: - res = await self.repository_service.edit_comment( + res = async_to_sync(self.repository_service.edit_comment)( pullid, commentid, message ) return { @@ -261,7 +247,7 @@ async def send_comment_once_behavior(self, pullid, commentid, message): "explanation": "no_permissions", "data_received": None, } - res = await self.repository_service.post_comment(pullid, message) + res = async_to_sync(self.repository_service.post_comment)(pullid, message) return { "notification_attempted": True, "notification_successful": True, @@ -269,10 +255,10 @@ async def send_comment_once_behavior(self, pullid, commentid, message): "data_received": {"id": res["id"]}, } - async def send_comment_new_behavior(self, pullid, commentid, message): + def send_comment_new_behavior(self, pullid, commentid, message): if commentid: try: - await self.repository_service.delete_comment(pullid, commentid) + async_to_sync(self.repository_service.delete_comment)(pullid, commentid) except TorngitObjectNotFoundError: log.info("Comment was already deleted") except TorngitClientError: @@ -292,7 +278,7 @@ async def send_comment_new_behavior(self, pullid, commentid, message): "data_received": None, } try: - res = await self.repository_service.post_comment(pullid, message) + res = async_to_sync(self.repository_service.post_comment)(pullid, message) return { "notification_attempted": True, "notification_successful": True, @@ -314,8 +300,8 @@ async def send_comment_new_behavior(self, pullid, commentid, message): "data_received": None, } - async def send_comment_spammy_behavior(self, pullid, commentid, message): - res = await self.repository_service.post_comment(pullid, message) + def send_comment_spammy_behavior(self, pullid, commentid, message): + res = async_to_sync(self.repository_service.post_comment)(pullid, message) return { "notification_attempted": True, "notification_successful": True, @@ -328,7 +314,7 @@ def is_enabled(self) -> bool: self.notifier_yaml_settings, dict ) - async def build_message(self, comparison: Comparison) -> List[str]: + def build_message(self, comparison: Comparison) -> list[str]: if self.should_use_upgrade_decoration(): return self._create_upgrade_message(comparison) if self.is_processing_upload(): @@ -340,9 +326,7 @@ async def build_message(self, comparison: Comparison) -> List[str]: if comparison.pull.is_first_coverage_pull: return self._create_welcome_message() pull_dict = comparison.enriched_pull.provider_pull - return await self.create_message( - comparison, pull_dict, self.notifier_yaml_settings - ) + return self.create_message(comparison, pull_dict, self.notifier_yaml_settings) def should_see_project_coverage_cta(self): """ diff --git a/services/notification/notifiers/comment/conditions.py b/services/notification/notifiers/comment/conditions.py index df9855863..2d44e8190 100644 --- a/services/notification/notifiers/comment/conditions.py +++ b/services/notification/notifiers/comment/conditions.py @@ -25,7 +25,6 @@ class NotifyCondition(ABC): NotifyCondition can have a side effect that is called when the condition fails. """ - is_async_condition: bool = False failure_explanation: str @abstractmethod @@ -40,23 +39,6 @@ def on_failure_side_effect( return NotificationResult() -class AsyncNotifyCondition(NotifyCondition): - """Async version of NotifyCondition""" - - is_async_condition: bool = True - - @abstractmethod - async def check_condition( - notifier: AbstractBaseNotifier, comparison: ComparisonProxy - ) -> bool: - return True - - async def on_failure_side_effect( - notifier: AbstractBaseNotifier, comparison: ComparisonProxy - ) -> NotificationResult: - return NotificationResult() - - class ComparisonHasPull(NotifyCondition): failure_explanation = "no_pull_request" @@ -107,34 +89,27 @@ def check_condition( return present_builds >= expected_builds -class HasEnoughRequiredChanges(AsyncNotifyCondition): +class HasEnoughRequiredChanges(NotifyCondition): failure_explanation = "changes_required" - async def _check_unexpected_changes(comparison: ComparisonProxy) -> bool: + def _check_unexpected_changes(comparison: ComparisonProxy) -> bool: """Returns a bool that indicates wether there are unexpected changes""" - changes = await comparison.get_changes() - if changes: - return True - return False + return bool(comparison.get_changes()) - async def _check_coverage_change(comparison: ComparisonProxy) -> bool: + def _check_coverage_change(comparison: ComparisonProxy) -> bool: """Returns a bool that indicates wether there is any change in coverage""" - diff = await comparison.get_diff() + diff = comparison.get_diff() res = comparison.head.report.calculate_diff(diff) - if res is not None and res["general"].lines > 0: - return True - return False + return res is not None and res["general"].lines > 0 - async def _check_any_change(comparison: ComparisonProxy) -> bool: - unexpected_changes = await HasEnoughRequiredChanges._check_unexpected_changes( - comparison - ) - coverage_changes = await HasEnoughRequiredChanges._check_coverage_change( + def _check_any_change(comparison: ComparisonProxy) -> bool: + unexpected_changes = HasEnoughRequiredChanges._check_unexpected_changes( comparison ) + coverage_changes = HasEnoughRequiredChanges._check_coverage_change(comparison) return unexpected_changes or coverage_changes - async def _check_coverage_drop(comparison: ComparisonProxy) -> bool: + def _check_coverage_drop(comparison: ComparisonProxy) -> bool: no_head_coverage = comparison.head.report.totals.coverage is None no_base_report = comparison.project_coverage_base.report is None no_base_coverage = ( @@ -162,8 +137,8 @@ async def _check_coverage_drop(comparison: ComparisonProxy) -> bool: # Need to take the project threshold into consideration return diff < 0 and abs(diff) >= (threshold + Decimal(0.01)) - async def _check_uncovered_patch(comparison: ComparisonProxy): - diff = await comparison.get_diff(use_original_base=True) + def _check_uncovered_patch(comparison: ComparisonProxy) -> bool: + diff = comparison.get_diff(use_original_base=True) totals = comparison.head.report.apply_diff(diff) coverage_not_affected_by_patch = totals and totals.lines == 0 if totals is None or coverage_not_affected_by_patch: @@ -175,7 +150,7 @@ async def _check_uncovered_patch(comparison: ComparisonProxy): 0.01 ) - async def check_condition_OR_group( + def check_condition_OR_group( condition_group: CoverageCommentRequiredChangesORGroup, comparison: ComparisonProxy, ) -> bool: @@ -195,13 +170,11 @@ async def check_condition_OR_group( if condition_group & individual_condition.value: if cache_results[individual_condition] is None: function_to_call = functions_lookup[individual_condition] - cache_results[individual_condition] = await function_to_call( - comparison - ) + cache_results[individual_condition] = function_to_call(comparison) final_result |= cache_results[individual_condition] return final_result - async def check_condition( + def check_condition( notifier: AbstractBaseNotifier, comparison: ComparisonProxy ) -> bool: if comparison.pull and comparison.pull.commentid: @@ -219,10 +192,6 @@ async def check_condition( # False --> 0 (no_requirements) required_changes = [int(required_changes)] return all( - [ - await HasEnoughRequiredChanges.check_condition_OR_group( - or_group, comparison - ) - for or_group in required_changes - ] + HasEnoughRequiredChanges.check_condition_OR_group(or_group, comparison) + for or_group in required_changes ) diff --git a/services/notification/notifiers/generics.py b/services/notification/notifiers/generics.py index 3e75f8dbf..df4df998d 100644 --- a/services/notification/notifiers/generics.py +++ b/services/notification/notifiers/generics.py @@ -90,14 +90,14 @@ def should_notify_comparison(self, comparison: Comparison) -> bool: return False return True - async def notify(self, comparison: Comparison, **extra_data) -> NotificationResult: + def notify(self, comparison: Comparison, **extra_data) -> NotificationResult: filtered_comparison = comparison.get_filtered_comparison( **self.get_notifier_filters() ) with nullcontext(): with nullcontext(): if self.should_notify_comparison(filtered_comparison): - result = await self.do_notify(filtered_comparison, **extra_data) + result = self.do_notify(filtered_comparison, **extra_data) else: result = NotificationResult( notification_attempted=False, @@ -117,9 +117,9 @@ def get_notifier_filters(self) -> dict: flags=flag_list, ) - async def do_notify(self, comparison) -> NotificationResult: + def do_notify(self, comparison) -> NotificationResult: data = self.build_payload(comparison) - result = await self.send_actual_notification(data) + result = self.send_actual_notification(data) return NotificationResult( notification_attempted=result["notification_attempted"], notification_successful=result["notification_successful"], @@ -241,15 +241,15 @@ class RequestsYamlBasedNotifier(StandardNotifier): "User-Agent": "Codecov", } - async def send_actual_notification(self, data: Mapping[str, Any]): + def send_actual_notification(self, data: Mapping[str, Any]): _timeouts = get_config("setup", "http", "timeouts", "external", default=10) kwargs = dict(timeout=_timeouts, headers=self.json_headers) try: with metrics.timer( f"worker.services.notifications.notifiers.{self.name}.actual_connection" ): - async with httpx.AsyncClient() as client: - res = await client.post( + with httpx.Client() as client: + res = client.post( url=self.notifier_yaml_settings["url"], data=json.dumps(data, cls=EnhancedJSONEncoder), **kwargs, diff --git a/services/notification/notifiers/mixins/message/__init__.py b/services/notification/notifiers/mixins/message/__init__.py index 2e5d54b07..3851c9c78 100644 --- a/services/notification/notifiers/mixins/message/__init__.py +++ b/services/notification/notifiers/mixins/message/__init__.py @@ -25,9 +25,7 @@ class MessageMixin(object): - async def create_message( - self, comparison: ComparisonProxy, pull_dict, yaml_settings - ): + def create_message(self, comparison: ComparisonProxy, pull_dict, yaml_settings): """ Assemble the various components of the PR comments message in accordance with their YAML configuration. See https://docs.codecov.io/docs/pull-request-comments for more context on the different parts of a PR comment. @@ -41,9 +39,9 @@ async def create_message( Thus, the comment block of the codecov YAML is passed as the "yaml_settings" parameter for these Notifiers. """ - changes = await comparison.get_changes() - diff = await comparison.get_diff(use_original_base=True) - behind_by = await comparison.get_behind_by() + changes = comparison.get_changes() + diff = comparison.get_diff(use_original_base=True) + behind_by = comparison.get_behind_by() base_report = comparison.project_coverage_base.report head_report = comparison.head.report pull = comparison.pull @@ -75,7 +73,7 @@ async def create_message( # note: since we're using append, calling write("") will add a newline to the message write = message.append - await self._possibly_write_install_app(comparison, write) + self._possibly_write_install_app(comparison, write) # Write Header write(f'## [Codecov]({links["pull"]}?dropdown=coverage&src=pr&el=h1) Report') @@ -112,7 +110,7 @@ async def create_message( current_yaml, ) - await self.write_section_to_msg( + self.write_section_to_msg( comparison, changes, diff, links, write, section_writer, behind_by ) @@ -145,7 +143,7 @@ async def create_message( self.repository, layout, show_complexity, settings, current_yaml ) - await self.write_section_to_msg( + self.write_section_to_msg( comparison, changes, diff, @@ -170,7 +168,7 @@ async def create_message( settings, current_yaml, ) - await self.write_section_to_msg( + self.write_section_to_msg( comparison, changes, diff, @@ -181,7 +179,7 @@ async def create_message( return [m for m in message if m is not None] - async def _possibly_write_install_app( + def _possibly_write_install_app( self, comparison: ComparisonProxy, write: Callable ) -> None: """Write a message if the user does not have any GH installations @@ -233,14 +231,14 @@ def _team_plan_notification( return message - async def write_section_to_msg( + def write_section_to_msg( self, comparison, changes, diff, links, write, section_writer, behind_by=None ): wrote_something: bool = False with metrics.timer( f"worker.services.notifications.notifiers.comment.section.{section_writer.name}" ): - for line in await section_writer.write_section( + for line in section_writer.write_section( comparison, diff, changes, links, behind_by=behind_by ): wrote_something |= line is not None diff --git a/services/notification/notifiers/mixins/message/sections.py b/services/notification/notifiers/mixins/message/sections.py index 718a02017..0efaeb7ab 100644 --- a/services/notification/notifiers/mixins/message/sections.py +++ b/services/notification/notifiers/mixins/message/sections.py @@ -4,7 +4,6 @@ from decimal import Decimal from enum import Enum, auto from itertools import starmap -from typing import List from urllib.parse import urlencode from shared.helpers.yaml import walk @@ -71,17 +70,17 @@ def __init__(self, repository, layout, show_complexity, settings, current_yaml): def name(self): return self.__class__.__name__ - async def write_section(self, *args, **kwargs): - return [i async for i in self.do_write_section(*args, **kwargs)] + def write_section(self, *args, **kwargs): + return [i for i in self.do_write_section(*args, **kwargs)] class NullSectionWriter(BaseSectionWriter): - async def write_section(*args, **kwargs): + def write_section(*args, **kwargs): return [] class NewFooterSectionWriter(BaseSectionWriter): - async def do_write_section(self, comparison, diff, changes, links, behind_by=None): + def do_write_section(self, comparison, diff, changes, links, behind_by=None): hide_project_coverage = self.settings.get("hide_project_coverage", False) if hide_project_coverage: yield ("") @@ -118,12 +117,11 @@ def _possibly_include_test_result_setup_confirmation(self, comparison): yield "" yield (":white_check_mark: All tests successful. No failed tests found.") - async def do_write_section(self, comparison, diff, changes, links, behind_by=None): + def do_write_section(self, comparison, diff, changes, links, behind_by=None): yaml = self.current_yaml base_report = comparison.project_coverage_base.report head_report = comparison.head.report pull_dict = comparison.enriched_pull.provider_pull - repo_service = comparison.repository_service.service diff_totals = head_report.apply_diff(diff) if diff_totals: @@ -206,7 +204,7 @@ async def do_write_section(self, comparison, diff, changes, links, behind_by=Non ) | set(c.path for c in changes or []) overlay = comparison.get_overlay(OverlayType.line_execution_count) files_in_critical = set( - await overlay.search_files_for_critical_changes( + overlay.search_files_for_critical_changes( all_potentially_affected_critical_files ) ) @@ -229,7 +227,7 @@ class AnnouncementSectionWriter(BaseSectionWriter): # "Codecov can now indicate which changes are the most critical in Pull Requests. [Learn more](https://about.codecov.io/product/feature/runtime-insights/)" # This is disabled as of CODE-1885. But we might bring it back later. ] - async def do_write_section(self, comparison: ComparisonProxy, *args, **kwargs): + def do_write_section(self, comparison: ComparisonProxy, *args, **kwargs): if self._potential_ats_user(comparison): message_to_display = AnnouncementSectionWriter.ats_message else: @@ -271,9 +269,9 @@ def _potential_ats_user(self, comparison: ComparisonProxy) -> bool: class ImpactedEntrypointsSectionWriter(BaseSectionWriter): - async def do_write_section(self, comparison, diff, changes, links, behind_by=None): + def do_write_section(self, comparison, diff, changes, links, behind_by=None): overlay = comparison.get_overlay(OverlayType.line_execution_count) - impacted_endpoints = await overlay.find_impacted_endpoints() + impacted_endpoints = overlay.find_impacted_endpoints() if impacted_endpoints: yield "| Related Entrypoints |" yield "|---|" @@ -284,7 +282,7 @@ async def do_write_section(self, comparison, diff, changes, links, behind_by=Non class FooterSectionWriter(BaseSectionWriter): - async def do_write_section(self, comparison, diff, changes, links, behind_by=None): + def do_write_section(self, comparison, diff, changes, links, behind_by=None): pull_dict = comparison.enriched_pull.provider_pull yield ("------") yield ("") @@ -310,7 +308,7 @@ async def do_write_section(self, comparison, diff, changes, links, behind_by=Non class ReachSectionWriter(BaseSectionWriter): - async def do_write_section(self, comparison, diff, changes, links, behind_by=None): + def do_write_section(self, comparison, diff, changes, links, behind_by=None): pull = comparison.enriched_pull.database_pull yield ( "[![Impacted file tree graph]({})]({}?src=pr&el=tree)".format( @@ -328,7 +326,7 @@ async def do_write_section(self, comparison, diff, changes, links, behind_by=Non class DiffSectionWriter(BaseSectionWriter): - async def do_write_section(self, comparison, diff, changes, links, behind_by=None): + def do_write_section(self, comparison, diff, changes, links, behind_by=None): base_report = comparison.project_coverage_base.report head_report = comparison.head.report if base_report is None: @@ -361,7 +359,7 @@ def _get_tree_cell(typ, path, metrics, compare, is_critical): class NewFilesSectionWriter(BaseSectionWriter): - async def do_write_section(self, comparison, diff, changes, links, behind_by=None): + def do_write_section(self, comparison, diff, changes, links, behind_by=None): # create list of files changed in diff base_report = comparison.project_coverage_base.report head_report = comparison.head.report @@ -399,7 +397,7 @@ async def do_write_section(self, comparison, diff, changes, links, behind_by=Non if self.settings.get("show_critical_paths", False): overlay = comparison.get_overlay(OverlayType.line_execution_count) files_in_critical = set( - await overlay.search_files_for_critical_changes(all_files) + overlay.search_files_for_critical_changes(all_files) ) def tree_cell(typ, path, metrics, _=None): @@ -442,7 +440,7 @@ def tree_cell(typ, path, metrics, _=None): class FileSectionWriter(BaseSectionWriter): - async def do_write_section(self, comparison, diff, changes, links, behind_by=None): + def do_write_section(self, comparison, diff, changes, links, behind_by=None): # create list of files changed in diff base_report = comparison.project_coverage_base.report head_report = comparison.head.report @@ -480,7 +478,7 @@ async def do_write_section(self, comparison, diff, changes, links, behind_by=Non if self.settings.get("show_critical_paths", False): overlay = comparison.get_overlay(OverlayType.line_execution_count) files_in_critical = set( - await overlay.search_files_for_critical_changes(all_files) + overlay.search_files_for_critical_changes(all_files) ) def tree_cell(typ, path, metrics, _=None): @@ -536,7 +534,7 @@ def tree_cell(typ, path, metrics, _=None): class FlagSectionWriter(BaseSectionWriter): - async def do_write_section(self, comparison, diff, changes, links, behind_by=None): + def do_write_section(self, comparison, diff, changes, links, behind_by=None): # flags base_report = comparison.project_coverage_base.report head_report = comparison.head.report @@ -666,16 +664,16 @@ async def do_write_section(self, comparison, diff, changes, links, behind_by=Non class ComponentsSectionWriter(BaseSectionWriter): - async def _get_table_data_for_components( + def _get_table_data_for_components( self, all_components, comparison: ComparisonProxy - ) -> List[dict]: + ) -> list[dict]: component_data = [] for component in all_components: flags = component.get_matching_flags(comparison.head.report.flags.keys()) filtered_comparison = comparison.get_filtered_comparison( flags, component.paths ) - diff = await filtered_comparison.get_diff() + diff = filtered_comparison.get_diff() component_data.append( { "name": component.get_display_name(), @@ -692,14 +690,14 @@ async def _get_table_data_for_components( ) return component_data - async def do_write_section( + def do_write_section( self, comparison: ComparisonProxy, diff, changes, links, behind_by=None ): all_components = get_components_from_yaml(self.current_yaml) if all_components == []: return # fast return if there's noting to process - component_data_to_show = await self._get_table_data_for_components( + component_data_to_show = self._get_table_data_for_components( all_components, comparison ) @@ -790,7 +788,7 @@ def _write_install_github_app_warning(self, comparison: ComparisonProxy) -> str: return ":exclamation: Your organization needs to install the [Codecov GitHub app](https://github.com/apps/codecov/installations/select_target) to enable full functionality." return "" - async def do_write_section(self, comparison: ComparisonProxy, *args, **kwargs): + def do_write_section(self, comparison: ComparisonProxy, *args, **kwargs): messages_ordering = [ self.Messages.INSTALL_GITHUB_APP_WARNING, self.Messages.DIFFERENT_UPLOAD_COUNT_WARNING, diff --git a/services/notification/notifiers/mixins/status.py b/services/notification/notifiers/mixins/status.py index cb22ca06f..999c1b434 100644 --- a/services/notification/notifiers/mixins/status.py +++ b/services/notification/notifiers/mixins/status.py @@ -1,17 +1,17 @@ import logging from decimal import Decimal, InvalidOperation -from typing import Any, Callable, List, Optional, Tuple, Union from services.comparison import ComparisonProxy, FilteredComparison +from services.comparison.types import Comparison from services.yaml.reader import round_number log = logging.getLogger(__name__) class StatusPatchMixin(object): - async def get_patch_status( + def get_patch_status( self, comparison: ComparisonProxy | FilteredComparison - ) -> Tuple[str, str]: + ) -> tuple[str, str]: threshold = self.notifier_yaml_settings.get("threshold", "0.0") # check if user has erroneously added a % to this input and fix @@ -23,7 +23,8 @@ async def get_patch_status( except (InvalidOperation, TypeError): threshold = Decimal("0.0") - totals = await comparison.get_patch_totals() + target_coverage: Decimal | None + totals = comparison.get_patch_totals() if self.notifier_yaml_settings.get("target") not in ("auto", None): target_coverage = Decimal( str(self.notifier_yaml_settings.get("target")).replace("%", "") @@ -78,7 +79,7 @@ def is_a_change_worth_noting(self, change) -> bool: return (t.misses + t.partials) > 0 return False - async def get_changes_status(self, comparison) -> Tuple[str, str]: + def get_changes_status(self, comparison: Comparison) -> tuple[str, str]: pull = comparison.pull if self.notifier_yaml_settings.get("base") in ("auto", None, "pr") and pull: if not comparison.has_project_coverage_base_report(): @@ -89,7 +90,7 @@ async def get_changes_status(self, comparison) -> Tuple[str, str]: return (state, description) # filter changes - changes = await comparison.get_changes() + changes = comparison.get_changes() if changes: changes = list(filter(self.is_a_change_worth_noting, changes)) @@ -114,9 +115,9 @@ async def get_changes_status(self, comparison) -> Tuple[str, str]: class StatusProjectMixin(object): DEFAULT_REMOVED_CODE_BEHAVIOR = "adjust_base" - async def _apply_removals_only_behavior( - self, comparison: Union[ComparisonProxy, FilteredComparison] - ) -> Optional[Tuple[str, str]]: + def _apply_removals_only_behavior( + self, comparison: ComparisonProxy | FilteredComparison + ) -> tuple[str, str] | None: """ Rule for passing project status on removals_only behavior: Pass if code was _only removed_ (i.e. no addition, no unexpected changes) @@ -125,30 +126,22 @@ async def _apply_removals_only_behavior( "Applying removals_only behavior to project status", extra=dict(commit=comparison.head.commit.commitid), ) - impacted_files_dict = await comparison.get_impacted_files() - impacted_files = impacted_files_dict.get("files", []) + impacted_files = comparison.get_impacted_files().get("files", []) + no_added_no_unexpected_change = all( - map( - lambda file_dict: ( - file_dict.get("added_diff_coverage", []) == [] - and file_dict.get("unexpected_line_changes") == [] - ), - impacted_files, - ) - ) - some_removed = any( - map( - lambda file_dict: (file_dict.get("removed_diff_coverage", []) != []), - impacted_files, - ) + not file.get("added_diff_coverage") + and not file.get("unexpected_line_changes") + for file in impacted_files ) + some_removed = any(file.get("removed_diff_coverage") for file in impacted_files) + if no_added_no_unexpected_change and some_removed: return ("success", ", passed because this change only removed code") return None - async def _apply_adjust_base_behavior( - self, comparison: ComparisonProxy - ) -> Optional[Tuple[str, str]]: + def _apply_adjust_base_behavior( + self, comparison: ComparisonProxy | FilteredComparison + ) -> tuple[str, str] | None: """ Rule for passing project status on adjust_base behavior: We adjust the BASE of the comparison by removing from it lines that were removed in HEAD @@ -168,28 +161,23 @@ async def _apply_adjust_base_behavior( ) return None - impacted_files_dict = await comparison.get_impacted_files() - impacted_files = impacted_files_dict.get("files", []) - - def get_sum_from_lists( - coverage_diff_info: List[Any], comparison_fn: Callable[[Any], int] - ): - return sum(map(comparison_fn, coverage_diff_info)) + impacted_files = comparison.get_impacted_files().get("files", []) hits_removed = 0 misses_removed = 0 partials_removed = 0 + for file_dict in impacted_files: - removed_diff_coverage_list = file_dict.get("removed_diff_coverage", []) - if removed_diff_coverage_list is not None: - hits_removed += get_sum_from_lists( - removed_diff_coverage_list, lambda item: 1 if item[1] == "h" else 0 + removed_diff_coverage_list = file_dict.get("removed_diff_coverage") + if removed_diff_coverage_list: + hits_removed += sum( + 1 if item[1] == "h" else 0 for item in removed_diff_coverage_list ) - misses_removed += get_sum_from_lists( - removed_diff_coverage_list, lambda item: 1 if item[1] == "m" else 0 + misses_removed += sum( + 1 if item[1] == "m" else 0 for item in removed_diff_coverage_list ) - partials_removed += get_sum_from_lists( - removed_diff_coverage_list, lambda item: 1 if item[1] == "p" else 0 + partials_removed += sum( + 1 if item[1] == "p" else 0 for item in removed_diff_coverage_list ) base_totals = comparison.project_coverage_base.report.totals @@ -235,9 +223,9 @@ def get_sum_from_lists( ) return None - async def _apply_fully_covered_patch_behavior( - self, comparison: ComparisonProxy - ) -> Optional[Tuple[str, str]]: + def _apply_fully_covered_patch_behavior( + self, comparison: ComparisonProxy | FilteredComparison + ) -> tuple[str, str] | None: """ Rule for passing project status on fully_covered_patch behavior: Pass if patch coverage is 100% and there are no unexpected changes @@ -246,21 +234,20 @@ async def _apply_fully_covered_patch_behavior( "Applying fully_covered_patch behavior to project status", extra=dict(commit=comparison.head.commit.commitid), ) - impacted_files_dict = await comparison.get_impacted_files() - impacted_files = impacted_files_dict.get("files", []) + impacted_files = comparison.get_impacted_files().get("files", []) + no_unexpected_changes = all( - map( - lambda file_dict: file_dict.get("unexpected_line_changes") == [], - impacted_files, - ) + not file.get("unexpected_line_changes") for file in impacted_files ) + if not no_unexpected_changes: log.info( "Unexpected changes when applying patch_100 behavior", extra=dict(commit=comparison.head.commit.commitid), ) return None - diff = await comparison.get_diff(use_original_base=True) + + diff = comparison.get_diff(use_original_base=True) patch_totals = comparison.head.report.apply_diff(diff) if patch_totals is None or patch_totals.lines == 0: # Coverage was not changed by patch @@ -273,9 +260,9 @@ async def _apply_fully_covered_patch_behavior( ) return None - async def get_project_status( - self, comparison: Union[ComparisonProxy, FilteredComparison] - ) -> Tuple[str, str]: + def get_project_status( + self, comparison: ComparisonProxy | FilteredComparison + ) -> tuple[str, str]: state, message = self._get_project_status(comparison) if state == "success": return (state, message) @@ -289,13 +276,11 @@ async def get_project_status( # Apply removed_code_behavior removed_code_result = None if removed_code_behavior == "removals_only": - removed_code_result = await self._apply_removals_only_behavior( - comparison - ) + removed_code_result = self._apply_removals_only_behavior(comparison) elif removed_code_behavior == "adjust_base": - removed_code_result = await self._apply_adjust_base_behavior(comparison) + removed_code_result = self._apply_adjust_base_behavior(comparison) elif removed_code_behavior == "fully_covered_patch": - removed_code_result = await self._apply_fully_covered_patch_behavior( + removed_code_result = self._apply_fully_covered_patch_behavior( comparison ) else: @@ -313,7 +298,9 @@ async def get_project_status( return (removed_code_state, message + removed_code_message) return (state, message) - def _get_project_status(self, comparison) -> Tuple[str, str]: + def _get_project_status( + self, comparison: ComparisonProxy | FilteredComparison + ) -> tuple[str, str]: if comparison.head.report.totals.coverage is None: state = self.notifier_yaml_settings.get("if_not_found", "success") message = "No coverage information found on head" diff --git a/services/notification/notifiers/status/base.py b/services/notification/notifiers/status/base.py index 9597f42c4..fba5ae220 100644 --- a/services/notification/notifiers/status/base.py +++ b/services/notification/notifiers/status/base.py @@ -1,7 +1,7 @@ -import asyncio import logging from typing import Dict +from asgiref.sync import async_to_sync from shared.config import get_config from shared.helpers.cache import NO_VALUE, make_hash_sha256 from shared.torngit.base import TorngitBaseAdapter @@ -10,7 +10,6 @@ from database.models.core import Commit from helpers.cache import cache from helpers.match import match -from helpers.metrics import metrics from services.comparison import ComparisonProxy from services.notification.notifiers.base import ( AbstractBaseNotifier, @@ -40,7 +39,7 @@ def store_results(self, comparison: Comparison, result: NotificationResult) -> b def name(self): return f"status-{self.context}" - async def build_payload(self, comparison) -> Dict[str, str]: + def build_payload(self, comparison) -> Dict[str, str]: raise NotImplementedError() def get_upgrade_message(self) -> str: @@ -159,7 +158,7 @@ def get_github_app_used(self) -> int | None: ) return selected_installation_id - async def notify(self, comparison: ComparisonProxy): + def notify(self, comparison: ComparisonProxy): payload = None if not self.can_we_set_this_status(comparison): return NotificationResult( @@ -185,7 +184,7 @@ async def notify(self, comparison: ComparisonProxy): ) ) if not comparison.has_head_report(): - payload = await self.build_payload(comparison) + payload = self.build_payload(comparison) elif ( flag_coverage_not_uploaded_behavior == "exclude" and not self.flag_coverage_was_uploaded(comparison) @@ -204,7 +203,7 @@ async def notify(self, comparison: ComparisonProxy): filtered_comparison = comparison.get_filtered_comparison( **self.get_notifier_filters() ) - payload = await self.build_payload(filtered_comparison) + payload = self.build_payload(filtered_comparison) payload["state"] = "success" payload["message"] = ( payload["message"] @@ -214,13 +213,13 @@ async def notify(self, comparison: ComparisonProxy): filtered_comparison = comparison.get_filtered_comparison( **self.get_notifier_filters() ) - payload = await self.build_payload(filtered_comparison) + payload = self.build_payload(filtered_comparison) if comparison.pull: payload["url"] = get_pull_url(comparison.pull) else: payload["url"] = get_commit_url(comparison.head.commit) - return await self.maybe_send_notification(comparison, payload) + return self.maybe_send_notification(comparison, payload) except TorngitClientError: log.warning( "Unable to send status notification to user due to a client-side error", @@ -254,10 +253,10 @@ async def notify(self, comparison: ComparisonProxy): data_sent=payload, ) - async def status_already_exists( + def status_already_exists( self, comparison: ComparisonProxy, title, state, description ) -> bool: - statuses = await comparison.get_existing_statuses() + statuses = comparison.get_existing_statuses() if statuses: exists = statuses.get(title) return ( @@ -271,7 +270,7 @@ def get_status_external_name(self) -> str: status_piece = f"/{self.title}" if self.title != "default" else "" return f"codecov/{self.context}{status_piece}" - async def maybe_send_notification( + def maybe_send_notification( self, comparison: ComparisonProxy, payload: dict ) -> NotificationResult: base_commit = ( @@ -298,7 +297,7 @@ async def maybe_send_notification( get_config("setup", "cache", "send_status_notification", default=600) ) # 10 min default cache.get_backend().set(cache_key, ttl, payload) - return await self.send_notification(comparison, payload) + return self.send_notification(comparison, payload) else: log.info( "Notification payload unchanged. Skipping notification.", @@ -317,29 +316,7 @@ async def maybe_send_notification( data_sent=None, ) - async def _send_notification_with_metrics( - self, - repo_service: TorngitBaseAdapter, - commit_sha: str, - state: str, - title: str, - coverage: float, - description: str, - url: str, - ): - with metrics.timer( - "worker.services.notifications.notifiers.status.set_commit_status" - ): - return await repo_service.set_commit_status( - commit=commit_sha, - status=state, - context=title, - coverage=coverage, - description=description, - url=url, - ) - - async def send_notification(self, comparison: ComparisonProxy, payload): + def send_notification(self, comparison: ComparisonProxy, payload): title = self.get_status_external_name() head_commit_sha = comparison.head.commit.commitid repository_service = self.repository_service(comparison.head.commit) @@ -347,73 +324,71 @@ async def send_notification(self, comparison: ComparisonProxy, payload): state = payload["state"] message = payload["message"] url = payload["url"] - if not await self.status_already_exists(comparison, title, state, message): - state = ( - "success" if self.notifier_yaml_settings.get("informational") else state + if self.status_already_exists(comparison, title, state, message): + log.info( + "Status already set", + extra=dict(context=title, description=message, state=state), + ) + return NotificationResult( + notification_attempted=False, + notification_successful=None, + explanation="already_done", + data_sent={"title": title, "state": state, "message": message}, ) - notification_result_data_sent = { - "title": title, - "state": state, - "message": message, - } + state = "success" if self.notifier_yaml_settings.get("informational") else state - all_shas_to_notify = [head_commit_sha] + list( - comparison.context.gitlab_extra_shas or set() + notification_result_data_sent = { + "title": title, + "state": state, + "message": message, + } + + all_shas_to_notify = [head_commit_sha] + list( + comparison.context.gitlab_extra_shas or set() + ) + if len(all_shas_to_notify) > 1: + log.info( + "Notifying multiple SHAs", + extra=dict(all_shas=all_shas_to_notify, commit=head_commit_sha), ) - if len(all_shas_to_notify) > 1: - log.info( - "Notifying multiple SHAs", - extra=dict(all_shas=all_shas_to_notify, commit=head_commit_sha), - ) - all_notify_tasks = [ - self._send_notification_with_metrics( - repo_service=repository_service, - commit_sha=commitid, - state=state, - title=title, - coverage=(float(head_report.totals.coverage) if head_report else 0), + + try: + _set_commit_status = async_to_sync(repository_service.set_commit_status) + all_results = [ + _set_commit_status( + commitid, + state, + title, description=message, url=url, + coverage=(float(head_report.totals.coverage) if head_report else 0), ) for commitid in all_shas_to_notify ] - try: - all_results = await asyncio.gather(*all_notify_tasks) - res = all_results[0] - - except TorngitClientError: - log.warning( - "Status not posted because this user can see but not set statuses on this repo", - extra=dict( - data_sent=notification_result_data_sent, - commit=head_commit_sha, - repoid=comparison.head.commit.repoid, - ), - ) - return NotificationResult( - notification_attempted=True, - notification_successful=False, - explanation="no_write_permission", + res = all_results[0] + + except TorngitClientError: + log.warning( + "Status not posted because this user can see but not set statuses on this repo", + extra=dict( data_sent=notification_result_data_sent, - data_received=None, - ) + commit=head_commit_sha, + repoid=comparison.head.commit.repoid, + ), + ) return NotificationResult( notification_attempted=True, - notification_successful=True, - explanation=None, + notification_successful=False, + explanation="no_write_permission", data_sent=notification_result_data_sent, - data_received={"id": res.get("id", "NO_ID")}, - github_app_used=self.get_github_app_used(), - ) - else: - log.info( - "Status already set", - extra=dict(context=title, description=message, state=state), - ) - return NotificationResult( - notification_attempted=False, - notification_successful=None, - explanation="already_done", - data_sent={"title": title, "state": state, "message": message}, + data_received=None, ) + return NotificationResult( + notification_attempted=True, + notification_successful=True, + explanation=None, + data_sent=notification_result_data_sent, + data_received={"id": res.get("id", "NO_ID")}, + github_app_used=self.get_github_app_used(), + ) diff --git a/services/notification/notifiers/status/changes.py b/services/notification/notifiers/status/changes.py index 1e7c8be03..8899d12fd 100644 --- a/services/notification/notifiers/status/changes.py +++ b/services/notification/notifiers/status/changes.py @@ -1,5 +1,4 @@ import logging -from typing import Dict from database.enums import Notification from services.notification.notifiers.mixins.status import StatusChangesMixin @@ -27,11 +26,11 @@ class ChangesStatusNotifier(StatusChangesMixin, StatusNotifier): def notification_type(self) -> Notification: return Notification.status_changes - async def build_payload(self, comparison) -> Dict[str, str]: + def build_payload(self, comparison) -> dict[str, str]: if self.is_empty_upload(): state, message = self.get_status_check_for_empty_upload() return {"state": state, "message": message} - state, message = await self.get_changes_status(comparison) + state, message = self.get_changes_status(comparison) if self.should_use_upgrade_decoration(): message = self.get_upgrade_message() diff --git a/services/notification/notifiers/status/patch.py b/services/notification/notifiers/status/patch.py index 47e50cdea..1c127fa43 100644 --- a/services/notification/notifiers/status/patch.py +++ b/services/notification/notifiers/status/patch.py @@ -22,12 +22,12 @@ class PatchStatusNotifier(StatusPatchMixin, StatusNotifier): def notification_type(self) -> Notification: return Notification.status_patch - async def build_payload(self, comparison: Comparison): + def build_payload(self, comparison: Comparison): if self.is_empty_upload(): state, message = self.get_status_check_for_empty_upload() return {"state": state, "message": message} - state, message = await self.get_patch_status(comparison) + state, message = self.get_patch_status(comparison) if self.should_use_upgrade_decoration(): message = self.get_upgrade_message() diff --git a/services/notification/notifiers/status/project.py b/services/notification/notifiers/status/project.py index 0b10e0333..9f9c54f45 100644 --- a/services/notification/notifiers/status/project.py +++ b/services/notification/notifiers/status/project.py @@ -30,12 +30,12 @@ class ProjectStatusNotifier(StatusProjectMixin, StatusNotifier): def notification_type(self) -> Notification: return Notification.status_project - async def build_payload(self, comparison: Comparison): + def build_payload(self, comparison: Comparison): if self.is_empty_upload(): state, message = self.get_status_check_for_empty_upload() return {"state": state, "message": message} - state, message = await self.get_project_status(comparison) + state, message = self.get_project_status(comparison) if self.should_use_upgrade_decoration(): message = self.get_upgrade_message() return {"state": state, "message": message} diff --git a/services/notification/notifiers/tests/integration/test_comment.py b/services/notification/notifiers/tests/integration/test_comment.py index 2d630d566..3035aca46 100644 --- a/services/notification/notifiers/tests/integration/test_comment.py +++ b/services/notification/notifiers/tests/integration/test_comment.py @@ -336,8 +336,7 @@ def sample_comparison_for_limited_upload( @pytest.mark.usefixtures("is_not_first_pull") class TestCommentNotifierIntegration(object): - @pytest.mark.asyncio - async def test_notify(self, sample_comparison, codecov_vcr, mock_configuration): + def test_notify(self, sample_comparison, codecov_vcr, mock_configuration): sample_comparison.context = ComparisonContext( all_tests_passed=True, test_results_error=None ) @@ -353,7 +352,7 @@ async def test_notify(self, sample_comparison, codecov_vcr, mock_configuration): notifier_site_settings=True, current_yaml={}, ) - result = await notifier.notify(comparison) + result = notifier.notify(comparison) assert result.notification_attempted assert result.notification_successful assert result.explanation is None @@ -408,8 +407,7 @@ async def test_notify(self, sample_comparison, codecov_vcr, mock_configuration): assert result.data_sent == {"commentid": None, "message": message, "pullid": 9} assert result.data_received == {"id": 1699669247} - @pytest.mark.asyncio - async def test_notify_test_results_error( + def test_notify_test_results_error( self, sample_comparison, codecov_vcr, mock_configuration ): sample_comparison.context = ComparisonContext( @@ -428,7 +426,7 @@ async def test_notify_test_results_error( notifier_site_settings=True, current_yaml={}, ) - result = await notifier.notify(comparison) + result = notifier.notify(comparison) assert result.notification_attempted assert result.notification_successful assert result.explanation is None @@ -483,8 +481,7 @@ async def test_notify_test_results_error( assert result.data_sent == {"commentid": None, "message": message, "pullid": 9} assert result.data_received == {"id": 1699669247} - @pytest.mark.asyncio - async def test_notify_upgrade( + def test_notify_upgrade( self, dbsession, sample_comparison_for_upgrade, codecov_vcr, mock_configuration ): mock_configuration._params["setup"] = {"codecov_dashboard_url": None} @@ -497,7 +494,7 @@ async def test_notify_upgrade( current_yaml={}, decoration_type=Decoration.upgrade, ) - result = await notifier.notify(comparison) + result = notifier.notify(comparison) assert result.notification_attempted assert result.notification_successful assert result.explanation is None @@ -517,8 +514,7 @@ async def test_notify_upgrade( } assert result.data_received == {"id": 1361234119} - @pytest.mark.asyncio - async def test_notify_upload_limited( + def test_notify_upload_limited( self, dbsession, sample_comparison_for_limited_upload, @@ -538,7 +534,7 @@ async def test_notify_upload_limited( current_yaml={}, decoration_type=Decoration.upload_limit, ) - result = await notifier.notify(comparison) + result = notifier.notify(comparison) assert result.notification_attempted assert result.notification_successful assert result.explanation is None @@ -560,8 +556,7 @@ async def test_notify_upload_limited( } assert result.data_received == {"id": 1111984446} - @pytest.mark.asyncio - async def test_notify_gitlab( + def test_notify_gitlab( self, sample_comparison_gitlab, codecov_vcr, mock_configuration ): mock_configuration._params["setup"] = { @@ -576,7 +571,7 @@ async def test_notify_gitlab( notifier_site_settings=True, current_yaml={}, ) - result = await notifier.notify(comparison) + result = notifier.notify(comparison) assert result.notification_attempted assert result.notification_successful assert result.explanation is None @@ -626,8 +621,7 @@ async def test_notify_gitlab( assert result.data_sent == {"commentid": None, "message": message, "pullid": 1} assert result.data_received == {"id": 1457135397} - @pytest.mark.asyncio - async def test_notify_new_layout( + def test_notify_new_layout( self, sample_comparison, codecov_vcr, mock_configuration ): mock_configuration._params["setup"] = {"codecov_dashboard_url": None} @@ -642,7 +636,7 @@ async def test_notify_new_layout( notifier_site_settings=True, current_yaml={}, ) - result = await notifier.notify(comparison) + result = notifier.notify(comparison) assert result.notification_attempted assert result.notification_successful assert result.explanation is None @@ -696,8 +690,7 @@ async def test_notify_new_layout( assert result.data_sent == {"commentid": None, "message": message, "pullid": 9} assert result.data_received == {"id": 1699669290} - @pytest.mark.asyncio - async def test_notify_with_components( + def test_notify_with_components( self, sample_comparison, codecov_vcr, mock_configuration ): mock_configuration._params["setup"] = {"codecov_dashboard_url": None} @@ -718,7 +711,7 @@ async def test_notify_with_components( } }, ) - result = await notifier.notify(comparison) + result = notifier.notify(comparison) assert result.notification_attempted assert result.notification_successful assert result.explanation is None diff --git a/services/notification/notifiers/tests/unit/test_checks.py b/services/notification/notifiers/tests/unit/test_checks.py index 5496a3d0f..f208ebd35 100644 --- a/services/notification/notifiers/tests/unit/test_checks.py +++ b/services/notification/notifiers/tests/unit/test_checks.py @@ -263,10 +263,7 @@ def multiple_diff_changes(): class TestChecksWithFallback(object): - @pytest.mark.asyncio - async def test_checks_403_failure( - self, sample_comparison, mocker, mock_repo_provider - ): + def test_checks_403_failure(self, sample_comparison, mocker, mock_repo_provider): mock_repo_provider.create_check_run = Mock( side_effect=TorngitClientGeneralError( 403, response_data="No Access", message="No Access" @@ -299,7 +296,7 @@ async def test_checks_403_failure( assert fallback_notifier.notification_type.value == "checks_patch" assert fallback_notifier.decoration_type is None - res = await fallback_notifier.notify(sample_comparison) + res = fallback_notifier.notify(sample_comparison) fallback_notifier.store_results(sample_comparison, res) assert status_notifier.notify.call_count == 1 assert fallback_notifier.name == "checks-patch-with-fallback" @@ -309,8 +306,7 @@ async def test_checks_403_failure( assert fallback_notifier.decoration_type is None assert res == "success" - @pytest.mark.asyncio - async def test_checks_failure(self, sample_comparison, mocker, mock_repo_provider): + def test_checks_failure(self, sample_comparison, mocker, mock_repo_provider): mock_repo_provider.create_check_run = Mock( side_effect=TorngitClientGeneralError( 409, response_data="No Access", message="No Access" @@ -343,24 +339,23 @@ async def test_checks_failure(self, sample_comparison, mocker, mock_repo_provide assert fallback_notifier.notification_type.value == "checks_patch" assert fallback_notifier.decoration_type is None - res = await fallback_notifier.notify(sample_comparison) + res = fallback_notifier.notify(sample_comparison) assert res.notification_successful == False assert res.explanation == "client_side_error_provider" mock_repo_provider.create_check_run = Mock(side_effect=TorngitError()) - res = await fallback_notifier.notify(sample_comparison) + res = fallback_notifier.notify(sample_comparison) assert res.notification_successful == False assert res.explanation == "server_side_error_provider" mock_repo_provider.create_check_run.return_value = 1234 mock_repo_provider.update_check_run = Mock(side_effect=TorngitError()) - res = await fallback_notifier.notify(sample_comparison) + res = fallback_notifier.notify(sample_comparison) assert res.notification_successful == False assert res.explanation == "server_side_error_provider" - @pytest.mark.asyncio - async def test_checks_no_pull(self, sample_comparison_without_pull, mocker): + def test_checks_no_pull(self, sample_comparison_without_pull, mocker): comparison = sample_comparison_without_pull checks_notifier = PatchChecksNotifier( repository=comparison.head.commit.repository, @@ -382,12 +377,11 @@ async def test_checks_no_pull(self, sample_comparison_without_pull, mocker): fallback_notifier = ChecksWithFallback( checks_notifier=checks_notifier, status_notifier=status_notifier ) - result = await fallback_notifier.notify(sample_comparison_without_pull) + result = fallback_notifier.notify(sample_comparison_without_pull) assert result == "success" assert status_notifier.notify.call_count == 1 - @pytest.mark.asyncio - async def test_notify_pull_request_not_in_provider( + def test_notify_pull_request_not_in_provider( self, dbsession, sample_comparison_database_pull_without_provider, mocker ): comparison = sample_comparison_database_pull_without_provider @@ -411,14 +405,11 @@ async def test_notify_pull_request_not_in_provider( fallback_notifier = ChecksWithFallback( checks_notifier=checks_notifier, status_notifier=status_notifier ) - result = await fallback_notifier.notify(comparison) + result = fallback_notifier.notify(comparison) assert result == "success" assert status_notifier.notify.call_count == 1 - @pytest.mark.asyncio - async def test_notify_closed_pull_request( - self, dbsession, sample_comparison, mocker - ): + def test_notify_closed_pull_request(self, dbsession, sample_comparison, mocker): sample_comparison.pull.state = "closed" checks_notifier = PatchChecksNotifier( @@ -441,7 +432,7 @@ async def test_notify_closed_pull_request( fallback_notifier = ChecksWithFallback( checks_notifier=checks_notifier, status_notifier=status_notifier ) - result = await fallback_notifier.notify(sample_comparison) + result = fallback_notifier.notify(sample_comparison) assert result == "success" assert status_notifier.notify.call_count == 1 @@ -678,8 +669,7 @@ def test_paginate_annotations( result = list(notifier.paginate_annotations(sample_array)) assert expected_result == result - @pytest.mark.asyncio - async def test_build_flag_payload( + def test_build_flag_payload( self, sample_comparison, mock_repo_provider, mock_configuration ): mock_configuration.params["setup"]["codecov_dashboard_url"] = "test.example.br" @@ -697,11 +687,10 @@ async def test_build_flag_payload( "summary": f"[View this Pull Request on Codecov](test.example.br/gh/test_build_flag_payload/{sample_comparison.head.commit.repository.name}/pull/{sample_comparison.pull.pullid}?dropdown=coverage&src=pr&el=h1)\n\n66.67% of diff hit (target 50.00%)", }, } - result = await notifier.build_payload(sample_comparison) + result = notifier.build_payload(sample_comparison) assert expected_result == result - @pytest.mark.asyncio - async def test_build_upgrade_payload( + def test_build_upgrade_payload( self, sample_comparison, mock_repo_provider, mock_configuration ): mock_configuration.params["setup"] = { @@ -723,11 +712,10 @@ async def test_build_upgrade_payload( "summary": f"[View this Pull Request on Codecov](test.example.br/gh/test_build_upgrade_payload/{sample_comparison.head.commit.repository.name}/pull/{sample_comparison.pull.pullid}?dropdown=coverage&src=pr&el=h1)\n\nThe author of this PR, codecov-test-user, is not an activated member of this organization on Codecov.\nPlease [activate this user on Codecov](test.example.br/members/gh/test_build_upgrade_payload) to display a detailed status check.\nCoverage data is still being uploaded to Codecov.io for purposes of overall coverage calculations.\nPlease don't hesitate to email us at support@codecov.io with any questions.", }, } - result = await notifier.build_payload(sample_comparison) + result = notifier.build_payload(sample_comparison) assert expected_result == result - @pytest.mark.asyncio - async def test_build_default_payload( + def test_build_default_payload( self, sample_comparison, mock_repo_provider, mock_configuration ): mock_configuration.params["setup"]["codecov_dashboard_url"] = "test.example.br" @@ -754,12 +742,11 @@ async def test_build_default_payload( ], }, } - result = await notifier.build_payload(sample_comparison) + result = notifier.build_payload(sample_comparison) assert expected_result["output"]["summary"] == result["output"]["summary"] assert expected_result == result - @pytest.mark.asyncio - async def test_build_payload_target_coverage_failure( + def test_build_payload_target_coverage_failure( self, sample_comparison, mock_repo_provider, mock_configuration ): mock_configuration.params["setup"]["codecov_dashboard_url"] = "test.example.br" @@ -777,11 +764,10 @@ async def test_build_payload_target_coverage_failure( "summary": f"[View this Pull Request on Codecov](test.example.br/gh/test_build_payload_target_coverage_failure/{sample_comparison.head.commit.repository.name}/pull/{sample_comparison.pull.pullid}?dropdown=coverage&src=pr&el=h1)\n\n66.67% of diff hit (target 70.00%)", }, } - result = await notifier.build_payload(sample_comparison) + result = notifier.build_payload(sample_comparison) assert expected_result == result - @pytest.mark.asyncio - async def test_build_payload_without_base_report( + def test_build_payload_without_base_report( self, sample_comparison_without_base_report, mock_repo_provider, @@ -812,11 +798,10 @@ async def test_build_payload_without_base_report( ], }, } - result = await notifier.build_payload(comparison) + result = notifier.build_payload(comparison) assert expected_result == result - @pytest.mark.asyncio - async def test_build_payload_target_coverage_failure_witinh_threshold( + def test_build_payload_target_coverage_failure_witinh_threshold( self, sample_comparison, mock_repo_provider, mock_configuration ): mock_configuration.params["setup"]["codecov_dashboard_url"] = "test.example.br" @@ -849,14 +834,13 @@ async def test_build_payload_target_coverage_failure_witinh_threshold( ], }, } - result = await notifier.build_payload(sample_comparison) + result = notifier.build_payload(sample_comparison) assert expected_result["state"] == result["state"] assert expected_result["output"]["summary"] == result["output"]["summary"] assert expected_result["output"] == result["output"] assert expected_result == result - @pytest.mark.asyncio - async def test_build_payload_with_multiple_changes( + def test_build_payload_with_multiple_changes( self, comparison_with_multiple_changes, mock_repo_provider, @@ -891,7 +875,7 @@ async def test_build_payload_with_multiple_changes( ], }, } - result = await notifier.build_payload(comparison_with_multiple_changes) + result = notifier.build_payload(comparison_with_multiple_changes) assert expected_result["state"] == result["state"] assert expected_result["output"] == result["output"] assert expected_result == result @@ -901,8 +885,7 @@ async def test_build_payload_with_multiple_changes( "segments" ) == multiple_diff_changes["files"][filename].get("segments") - @pytest.mark.asyncio - async def test_build_payload_no_diff( + def test_build_payload_no_diff( self, sample_comparison, mock_repo_provider, mock_configuration ): mock_repo_provider.get_compare.return_value = { @@ -953,14 +936,11 @@ async def test_build_payload_no_diff( "summary": f"[View this Pull Request on Codecov](test.example.br/gh/test_build_payload_no_diff/{sample_comparison.head.commit.repository.name}/pull/{sample_comparison.pull.pullid}?dropdown=coverage&src=pr&el=h1)\n\nCoverage not affected when comparing {base_commit.commitid[:7]}...{head_commit.commitid[:7]}", }, } - result = await notifier.build_payload(sample_comparison) + result = notifier.build_payload(sample_comparison) assert notifier.notification_type.value == "checks_patch" assert expected_result == result - @pytest.mark.asyncio - async def test_send_notification( - self, sample_comparison, mocker, mock_repo_provider - ): + def test_send_notification(self, sample_comparison, mocker, mock_repo_provider): comparison = sample_comparison payload = { "state": "success", @@ -976,7 +956,7 @@ async def test_send_notification( notifier_site_settings=True, current_yaml=UserYaml({}), ) - result = await notifier.send_notification(sample_comparison, payload) + result = notifier.send_notification(sample_comparison, payload) assert result.notification_successful == True assert result.explanation is None assert result.data_sent == { @@ -985,8 +965,7 @@ async def test_send_notification( "url": "https://app.codecov.io/gh/codecov/worker/compare/100?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=checks&utm_campaign=pr+comments&utm_term=codecov", } - @pytest.mark.asyncio - async def test_send_notification_annotations_paginations( + def test_send_notification_annotations_paginations( self, sample_comparison, mocker, mock_repo_provider ): comparison = sample_comparison @@ -1025,7 +1004,7 @@ async def test_send_notification_annotations_paginations( "url": None, }, ] - result = await notifier.send_notification(sample_comparison, payload) + result = notifier.send_notification(sample_comparison, payload) assert result.notification_successful == True assert result.explanation is None calls = [call[1] for call in mock_repo_provider.update_check_run.call_args_list] @@ -1040,8 +1019,7 @@ async def test_send_notification_annotations_paginations( }, } - @pytest.mark.asyncio - async def test_notify( + def test_notify( self, sample_comparison, mocker, mock_repo_provider, mock_configuration ): mock_configuration.params["setup"]["codecov_dashboard_url"] = "test.example.br" @@ -1061,7 +1039,7 @@ async def test_notify( ) base_commit = sample_comparison.project_coverage_base.commit head_commit = sample_comparison.head.commit - result = await notifier.notify(sample_comparison) + result = notifier.notify(sample_comparison) assert result.notification_successful == True assert result.explanation is None assert result.data_sent == { @@ -1073,8 +1051,7 @@ async def test_notify( "url": f"test.example.br/gh/test_notify/{sample_comparison.head.commit.repository.name}/pull/{comparison.pull.pullid}", } - @pytest.mark.asyncio - async def test_notify_passing_empty_upload( + def test_notify_passing_empty_upload( self, sample_comparison, mocker, mock_repo_provider, mock_configuration ): mock_configuration.params["setup"]["codecov_dashboard_url"] = "test.example.br" @@ -1089,7 +1066,7 @@ async def test_notify_passing_empty_upload( current_yaml=UserYaml({}), decoration_type=Decoration.passing_empty_upload, ) - result = await notifier.notify(sample_comparison) + result = notifier.notify(sample_comparison) assert result.notification_successful == True assert result.explanation is None assert result.data_sent == { @@ -1101,8 +1078,7 @@ async def test_notify_passing_empty_upload( "url": f"test.example.br/gh/test_notify_passing_empty_upload/{sample_comparison.head.commit.repository.name}/pull/{comparison.pull.pullid}", } - @pytest.mark.asyncio - async def test_notify_failing_empty_upload( + def test_notify_failing_empty_upload( self, sample_comparison, mocker, mock_repo_provider, mock_configuration ): mock_configuration.params["setup"]["codecov_dashboard_url"] = "test.example.br" @@ -1117,7 +1093,7 @@ async def test_notify_failing_empty_upload( current_yaml=UserYaml({}), decoration_type=Decoration.failing_empty_upload, ) - result = await notifier.notify(sample_comparison) + result = notifier.notify(sample_comparison) assert result.notification_successful == True assert result.explanation is None assert result.data_sent == { @@ -1129,8 +1105,7 @@ async def test_notify_failing_empty_upload( "url": f"test.example.br/gh/test_notify_failing_empty_upload/{sample_comparison.head.commit.repository.name}/pull/{comparison.pull.pullid}", } - @pytest.mark.asyncio - async def test_notification_exception( + def test_notification_exception( self, sample_comparison, mock_repo_provider, mock_configuration ): mock_configuration.params["setup"]["codecov_dashboard_url"] = "test.example.br" @@ -1148,20 +1123,19 @@ async def test_notification_exception( 400, response_data="Error", message="Error" ) ) - result = await notifier.notify(sample_comparison) + result = notifier.notify(sample_comparison) assert result.notification_successful == False assert result.explanation == "client_side_error_provider" assert result.data_sent is None # Test exception handling when there's a TorngitError mock_repo_provider.get_compare = Mock(side_effect=TorngitError()) - result = await notifier.notify(sample_comparison) + result = notifier.notify(sample_comparison) assert result.notification_successful == False assert result.explanation == "server_side_error_provider" assert result.data_sent is None - @pytest.mark.asyncio - async def test_notification_exception_not_fit(self, sample_comparison, mocker): + def test_notification_exception_not_fit(self, sample_comparison, mocker): notifier = ChecksNotifier( repository=sample_comparison.head.commit.repository, title="title", @@ -1172,15 +1146,14 @@ async def test_notification_exception_not_fit(self, sample_comparison, mocker): mocker.patch.object( ChecksNotifier, "can_we_set_this_status", return_value=False ) - result = await notifier.notify(sample_comparison) + result = notifier.notify(sample_comparison) assert not result.notification_attempted assert result.notification_successful is None assert result.explanation == "not_fit_criteria" assert result.data_sent is None assert result.data_received is None - @pytest.mark.asyncio - async def test_checks_with_after_n_builds(self, sample_comparison, mocker): + def test_checks_with_after_n_builds(self, sample_comparison, mocker): notifier = ChecksNotifier( repository=sample_comparison.head.commit.repository, title="title", @@ -1206,7 +1179,7 @@ async def test_checks_with_after_n_builds(self, sample_comparison, mocker): ) mocker.patch.object(ChecksNotifier, "can_we_set_this_status", return_value=True) - result = await notifier.notify(sample_comparison) + result = notifier.notify(sample_comparison) assert not result.notification_attempted assert result.notification_successful is None assert result.explanation == "need_more_builds" @@ -1215,8 +1188,7 @@ async def test_checks_with_after_n_builds(self, sample_comparison, mocker): class TestChangesChecksNotifier(object): - @pytest.mark.asyncio - async def test_build_payload( + def test_build_payload( self, sample_comparison, mock_repo_provider, mock_configuration ): mock_configuration.params["setup"]["codecov_dashboard_url"] = "test.example.br" @@ -1234,12 +1206,11 @@ async def test_build_payload( "summary": f"[View this Pull Request on Codecov](test.example.br/gh/test_build_payload/{sample_comparison.head.commit.repository.name}/pull/{sample_comparison.pull.pullid}?dropdown=coverage&src=pr&el=h1)\n\nNo indirect coverage changes found", }, } - result = await notifier.build_payload(sample_comparison) + result = notifier.build_payload(sample_comparison) assert expected_result == result assert notifier.notification_type.value == "checks_changes" - @pytest.mark.asyncio - async def test_build_upgrade_payload( + def test_build_upgrade_payload( self, sample_comparison, mock_repo_provider, mock_configuration ): mock_configuration.params["setup"] = { @@ -1261,11 +1232,10 @@ async def test_build_upgrade_payload( "summary": f"[View this Pull Request on Codecov](test.example.br/gh/test_build_upgrade_payload/{sample_comparison.head.commit.repository.name}/pull/{sample_comparison.pull.pullid}?dropdown=coverage&src=pr&el=h1)\n\nThe author of this PR, codecov-test-user, is not an activated member of this organization on Codecov.\nPlease [activate this user on Codecov](test.example.br/members/gh/test_build_upgrade_payload) to display a detailed status check.\nCoverage data is still being uploaded to Codecov.io for purposes of overall coverage calculations.\nPlease don't hesitate to email us at support@codecov.io with any questions.", }, } - result = await notifier.build_payload(sample_comparison) + result = notifier.build_payload(sample_comparison) assert expected_result == result - @pytest.mark.asyncio - async def test_build_payload_with_multiple_changes( + def test_build_payload_with_multiple_changes( self, comparison_with_multiple_changes, mock_repo_provider, @@ -1290,11 +1260,10 @@ async def test_build_payload_with_multiple_changes( "summary": f"[View this Pull Request on Codecov](test.example.br/gh/test_build_payload_with_multiple_changes/{comparison_with_multiple_changes.head.commit.repository.name}/pull/{comparison_with_multiple_changes.pull.pullid}?dropdown=coverage&src=pr&el=h1)\n\n3 files have indirect coverage changes not visible in diff", }, } - result = await notifier.build_payload(comparison_with_multiple_changes) + result = notifier.build_payload(comparison_with_multiple_changes) assert expected_result == result - @pytest.mark.asyncio - async def test_build_payload_without_base_report( + def test_build_payload_without_base_report( self, sample_comparison_without_base_report, mock_repo_provider, @@ -1316,11 +1285,10 @@ async def test_build_payload_without_base_report( "summary": f"[View this Pull Request on Codecov](test.example.br/gh/test_build_payload_without_base_report/{sample_comparison_without_base_report.head.commit.repository.name}/pull/{sample_comparison_without_base_report.pull.pullid}?dropdown=coverage&src=pr&el=h1)\n\nUnable to determine changes, no report found at pull request base", }, } - result = await notifier.build_payload(comparison) + result = notifier.build_payload(comparison) assert expected_result == result - @pytest.mark.asyncio - async def test_build_failing_empty_upload_payload( + def test_build_failing_empty_upload_payload( self, sample_comparison, mock_repo_provider, mock_configuration ): mock_configuration.params["setup"] = { @@ -1342,13 +1310,12 @@ async def test_build_failing_empty_upload_payload( "summary": "Testable files changed", }, } - result = await notifier.build_payload(sample_comparison) + result = notifier.build_payload(sample_comparison) assert expected_result == result class TestProjectChecksNotifier(object): - @pytest.mark.asyncio - async def test_analytics_url( + def test_analytics_url( self, sample_comparison, mock_repo_provider, mock_configuration ): mock_configuration.params["setup"]["codecov_dashboard_url"] = "codecov.io" @@ -1382,7 +1349,7 @@ async def test_analytics_url( notifier_site_settings=True, current_yaml={"comment": {"layout": "files"}}, ) - result = await notifier.send_notification(sample_comparison, payload) + result = notifier.send_notification(sample_comparison, payload) expected_result = { "state": "success", "output": { @@ -1408,8 +1375,7 @@ async def test_analytics_url( ]["text"].split("\n") assert expected_result == result.data_sent - @pytest.mark.asyncio - async def test_build_flag_payload( + def test_build_flag_payload( self, sample_comparison, mock_repo_provider, mock_configuration ): mock_configuration.params["setup"]["codecov_dashboard_url"] = "test.example.br" @@ -1420,7 +1386,7 @@ async def test_build_flag_payload( notifier_site_settings=True, current_yaml=UserYaml({}), ) - result = await notifier.build_payload(sample_comparison) + result = notifier.build_payload(sample_comparison) base_commit = sample_comparison.project_coverage_base.commit expected_result = { "state": "success", @@ -1432,8 +1398,7 @@ async def test_build_flag_payload( assert result == expected_result assert notifier.notification_type.value == "checks_project" - @pytest.mark.asyncio - async def test_build_upgrade_payload( + def test_build_upgrade_payload( self, sample_comparison, mock_repo_provider, mock_configuration ): mock_configuration.params["setup"] = { @@ -1455,11 +1420,10 @@ async def test_build_upgrade_payload( "summary": f"[View this Pull Request on Codecov](test.example.br/gh/test_build_upgrade_payload/{sample_comparison.head.commit.repository.name}/pull/{sample_comparison.pull.pullid}?dropdown=coverage&src=pr&el=h1)\n\nThe author of this PR, codecov-test-user, is not an activated member of this organization on Codecov.\nPlease [activate this user on Codecov](test.example.br/members/gh/test_build_upgrade_payload) to display a detailed status check.\nCoverage data is still being uploaded to Codecov.io for purposes of overall coverage calculations.\nPlease don't hesitate to email us at support@codecov.io with any questions.", }, } - result = await notifier.build_payload(sample_comparison) + result = notifier.build_payload(sample_comparison) assert expected_result == result - @pytest.mark.asyncio - async def test_build_passing_empty_upload_payload( + def test_build_passing_empty_upload_payload( self, sample_comparison, mock_repo_provider, mock_configuration ): mock_configuration.params["setup"] = { @@ -1481,11 +1445,10 @@ async def test_build_passing_empty_upload_payload( "summary": "Non-testable files changed.", }, } - result = await notifier.build_payload(sample_comparison) + result = notifier.build_payload(sample_comparison) assert expected_result == result - @pytest.mark.asyncio - async def test_build_default_payload( + def test_build_default_payload( self, sample_comparison, mock_repo_provider, mock_configuration ): mock_configuration.params["setup"]["codecov_dashboard_url"] = "test.example.br" @@ -1496,7 +1459,7 @@ async def test_build_default_payload( notifier_site_settings=True, current_yaml={"comment": {"layout": "files"}}, ) - result = await notifier.build_payload(sample_comparison) + result = notifier.build_payload(sample_comparison) repo = sample_comparison.head.commit.repository base_commit = sample_comparison.project_coverage_base.commit head_commit = sample_comparison.head.commit @@ -1531,8 +1494,7 @@ async def test_build_default_payload( ].split("\n") assert expected_result == result - @pytest.mark.asyncio - async def test_build_default_payload_with_flags( + def test_build_default_payload_with_flags( self, sample_comparison, mock_repo_provider, mock_configuration ): mock_configuration.params["setup"]["codecov_dashboard_url"] = "test.example.br" @@ -1543,7 +1505,7 @@ async def test_build_default_payload_with_flags( notifier_site_settings=True, current_yaml={"comment": {"layout": "files, flags"}}, ) - result = await notifier.build_payload(sample_comparison) + result = notifier.build_payload(sample_comparison) repo = sample_comparison.head.commit.repository base_commit = sample_comparison.project_coverage_base.commit head_commit = sample_comparison.head.commit @@ -1578,8 +1540,7 @@ async def test_build_default_payload_with_flags( ].split("\n") assert expected_result == result - @pytest.mark.asyncio - async def test_build_default_payload_with_flags_and_footer( + def test_build_default_payload_with_flags_and_footer( self, sample_comparison, mock_repo_provider, mock_configuration ): test_name = "test_build_default_payload_with_flags_and_footer" @@ -1591,7 +1552,7 @@ async def test_build_default_payload_with_flags_and_footer( notifier_site_settings=True, current_yaml={"comment": {"layout": "files, flags, footer"}}, ) - result = await notifier.build_payload(sample_comparison) + result = notifier.build_payload(sample_comparison) repo = sample_comparison.head.commit.repository base_commit = sample_comparison.project_coverage_base.commit head_commit = sample_comparison.head.commit @@ -1632,8 +1593,7 @@ async def test_build_default_payload_with_flags_and_footer( ].split("\n") assert expected_result == result - @pytest.mark.asyncio - async def test_build_default_payload_comment_off( + def test_build_default_payload_comment_off( self, sample_comparison, mock_repo_provider, mock_configuration ): mock_configuration.params["setup"]["codecov_dashboard_url"] = "test.example.br" @@ -1644,7 +1604,7 @@ async def test_build_default_payload_comment_off( notifier_site_settings=True, current_yaml={"comment": False}, ) - result = await notifier.build_payload(sample_comparison) + result = notifier.build_payload(sample_comparison) repo = sample_comparison.head.commit.repository base_commit = sample_comparison.project_coverage_base.commit expected_result = { @@ -1656,8 +1616,7 @@ async def test_build_default_payload_comment_off( } assert expected_result == result - @pytest.mark.asyncio - async def test_build_default_payload_negative_change_comment_off( + def test_build_default_payload_negative_change_comment_off( self, sample_comparison_negative_change, mock_repo_provider, mock_configuration ): mock_configuration.params["setup"]["codecov_dashboard_url"] = "test.example.br" @@ -1668,7 +1627,7 @@ async def test_build_default_payload_negative_change_comment_off( notifier_site_settings=True, current_yaml={"comment": False}, ) - result = await notifier.build_payload(sample_comparison_negative_change) + result = notifier.build_payload(sample_comparison_negative_change) repo = sample_comparison_negative_change.head.commit.repository base_commit = sample_comparison_negative_change.project_coverage_base.commit expected_result = { @@ -1680,8 +1639,7 @@ async def test_build_default_payload_negative_change_comment_off( } assert expected_result == result - @pytest.mark.asyncio - async def test_build_payload_not_auto( + def test_build_payload_not_auto( self, sample_comparison, mock_repo_provider, mock_configuration ): mock_configuration.params["setup"]["codecov_dashboard_url"] = "test.example.br" @@ -1700,11 +1658,10 @@ async def test_build_payload_not_auto( "summary": f"[View this Pull Request on Codecov](test.example.br/gh/test_build_payload_not_auto/{repo.name}/pull/{sample_comparison.pull.pullid}?dropdown=coverage&src=pr&el=h1)\n\n60.00% (target 57.00%)", }, } - result = await notifier.build_payload(sample_comparison) + result = notifier.build_payload(sample_comparison) assert expected_result == result - @pytest.mark.asyncio - async def test_build_payload_no_base_report( + def test_build_payload_no_base_report( self, sample_comparison_without_base_report, mock_repo_provider, @@ -1727,11 +1684,10 @@ async def test_build_payload_no_base_report( "summary": f"[View this Pull Request on Codecov](test.example.br/gh/test_build_payload_no_base_report/{repo.name}/pull/{sample_comparison_without_base_report.pull.pullid}?dropdown=coverage&src=pr&el=h1)\n\nNo report found to compare against", }, } - result = await notifier.build_payload(comparison) + result = notifier.build_payload(comparison) assert expected_result == result - @pytest.mark.asyncio - async def test_check_notify_no_path_match( + def test_check_notify_no_path_match( self, sample_comparison, mocker, mock_repo_provider, mock_configuration ): mock_configuration.params["setup"]["codecov_dashboard_url"] = "test.example.br" @@ -1752,7 +1708,7 @@ async def test_check_notify_no_path_match( ) base_commit = sample_comparison.project_coverage_base.commit head_commit = sample_comparison.head.commit - result = await notifier.notify(sample_comparison) + result = notifier.notify(sample_comparison) assert result.notification_successful == True assert result.explanation is None assert result.data_sent == { @@ -1764,8 +1720,7 @@ async def test_check_notify_no_path_match( "url": f"test.example.br/gh/test_check_notify_no_path_match/{sample_comparison.head.commit.repository.name}/pull/{sample_comparison.pull.pullid}", } - @pytest.mark.asyncio - async def test_check_notify_single_path_match( + def test_check_notify_single_path_match( self, sample_comparison, mocker, mock_repo_provider, mock_configuration ): mock_configuration.params["setup"]["codecov_dashboard_url"] = "test.example.br" @@ -1787,7 +1742,7 @@ async def test_check_notify_single_path_match( base_commit = sample_comparison.project_coverage_base.commit head_commit = sample_comparison.head.commit - result = await notifier.notify(sample_comparison) + result = notifier.notify(sample_comparison) assert result.notification_successful is True assert result.explanation is None expected_result = { @@ -1805,8 +1760,7 @@ async def test_check_notify_single_path_match( ) assert result.data_sent["output"] == expected_result["output"] - @pytest.mark.asyncio - async def test_check_notify_multiple_path_match( + def test_check_notify_multiple_path_match( self, sample_comparison, mocker, mock_repo_provider, mock_configuration ): mock_configuration.params["setup"]["codecov_dashboard_url"] = "test.example.br" @@ -1828,7 +1782,7 @@ async def test_check_notify_multiple_path_match( base_commit = sample_comparison.project_coverage_base.commit head_commit = sample_comparison.head.commit - result = await notifier.notify(sample_comparison) + result = notifier.notify(sample_comparison) assert result.notification_successful == True assert result.explanation is None assert result.data_sent == { @@ -1840,8 +1794,7 @@ async def test_check_notify_multiple_path_match( "url": f"test.example.br/gh/test_check_notify_multiple_path_match/{sample_comparison.head.commit.repository.name}/pull/{sample_comparison.pull.pullid}", } - @pytest.mark.asyncio - async def test_check_notify_with_paths( + def test_check_notify_with_paths( self, sample_comparison, mocker, mock_repo_provider, mock_configuration ): mock_configuration.params["setup"]["codecov_dashboard_url"] = "test.example.br" @@ -1862,7 +1815,7 @@ async def test_check_notify_with_paths( ) base_commit = sample_comparison.project_coverage_base.commit head_commit = sample_comparison.head.commit - result = await notifier.notify(sample_comparison) + result = notifier.notify(sample_comparison) assert result.notification_successful == True assert result.explanation is None assert result.data_sent == { @@ -1874,8 +1827,7 @@ async def test_check_notify_with_paths( "url": f"test.example.br/gh/test_check_notify_with_paths/{sample_comparison.head.commit.repository.name}/pull/{sample_comparison.pull.pullid}", } - @pytest.mark.asyncio - async def test_notify_pass_behavior_when_coverage_not_uploaded( + def test_notify_pass_behavior_when_coverage_not_uploaded( self, sample_comparison_coverage_carriedforward, mock_repo_provider, @@ -1912,7 +1864,7 @@ async def test_notify_pass_behavior_when_coverage_not_uploaded( "url": f"test.example.br/gh/{head_commit.repository.owner.username}/{head_commit.repository.name}/pull/{sample_comparison_coverage_carriedforward.pull.pullid}", }, ) - result = await notifier.notify(sample_comparison_coverage_carriedforward) + result = notifier.notify(sample_comparison_coverage_carriedforward) assert ( expected_result.data_sent["output"]["summary"] == result.data_sent["output"]["summary"] @@ -1920,8 +1872,7 @@ async def test_notify_pass_behavior_when_coverage_not_uploaded( assert expected_result.data_sent["output"] == result.data_sent["output"] assert expected_result == result - @pytest.mark.asyncio - async def test_notify_pass_behavior_when_coverage_uploaded( + def test_notify_pass_behavior_when_coverage_uploaded( self, sample_comparison_coverage_carriedforward, mock_repo_provider, @@ -1957,11 +1908,10 @@ async def test_notify_pass_behavior_when_coverage_uploaded( "url": f"test.example.br/gh/{head_commit.repository.owner.username}/{head_commit.repository.name}/pull/{sample_comparison_coverage_carriedforward.pull.pullid}", }, ) - result = await notifier.notify(sample_comparison_coverage_carriedforward) + result = notifier.notify(sample_comparison_coverage_carriedforward) assert expected_result == result - @pytest.mark.asyncio - async def test_notify_include_behavior_when_coverage_not_uploaded( + def test_notify_include_behavior_when_coverage_not_uploaded( self, sample_comparison_coverage_carriedforward, mock_repo_provider, @@ -1998,11 +1948,10 @@ async def test_notify_include_behavior_when_coverage_not_uploaded( "url": f"test.example.br/gh/{head_commit.repository.owner.username}/{head_commit.repository.name}/pull/{sample_comparison_coverage_carriedforward.pull.pullid}", }, ) - result = await notifier.notify(sample_comparison_coverage_carriedforward) + result = notifier.notify(sample_comparison_coverage_carriedforward) assert expected_result == result - @pytest.mark.asyncio - async def test_notify_exclude_behavior_when_coverage_not_uploaded( + def test_notify_exclude_behavior_when_coverage_not_uploaded( self, sample_comparison_coverage_carriedforward, mock_repo_provider, @@ -2028,11 +1977,10 @@ async def test_notify_exclude_behavior_when_coverage_not_uploaded( data_sent=None, data_received=None, ) - result = await notifier.notify(sample_comparison_coverage_carriedforward) + result = notifier.notify(sample_comparison_coverage_carriedforward) assert expected_result == result - @pytest.mark.asyncio - async def test_notify_exclude_behavior_when_coverage_uploaded( + def test_notify_exclude_behavior_when_coverage_uploaded( self, sample_comparison_coverage_carriedforward, mock_repo_provider, @@ -2069,11 +2017,10 @@ async def test_notify_exclude_behavior_when_coverage_uploaded( "url": f"test.example.br/gh/{head_commit.repository.owner.username}/{head_commit.repository.name}/pull/{sample_comparison_coverage_carriedforward.pull.pullid}", }, ) - result = await notifier.notify(sample_comparison_coverage_carriedforward) + result = notifier.notify(sample_comparison_coverage_carriedforward) assert expected_result == result - @pytest.mark.asyncio - async def test_notify_exclude_behavior_when_some_coverage_uploaded( + def test_notify_exclude_behavior_when_some_coverage_uploaded( self, sample_comparison_coverage_carriedforward, mock_repo_provider, @@ -2114,11 +2061,10 @@ async def test_notify_exclude_behavior_when_some_coverage_uploaded( "url": f"test.example.br/gh/{head_commit.repository.owner.username}/{head_commit.repository.name}/pull/{sample_comparison_coverage_carriedforward.pull.pullid}", }, ) - result = await notifier.notify(sample_comparison_coverage_carriedforward) + result = notifier.notify(sample_comparison_coverage_carriedforward) assert expected_result == result - @pytest.mark.asyncio - async def test_notify_exclude_behavior_no_flags( + def test_notify_exclude_behavior_no_flags( self, sample_comparison_coverage_carriedforward, mock_repo_provider, @@ -2156,7 +2102,7 @@ async def test_notify_exclude_behavior_no_flags( "url": f"test.example.br/gh/{head_commit.repository.owner.username}/{head_commit.repository.name}/pull/{sample_comparison_coverage_carriedforward.pull.pullid}", }, ) - result = await notifier.notify(sample_comparison_coverage_carriedforward) + result = notifier.notify(sample_comparison_coverage_carriedforward) assert ( expected_result.data_sent["output"]["summary"] == result.data_sent["output"]["summary"] @@ -2165,10 +2111,7 @@ async def test_notify_exclude_behavior_no_flags( assert expected_result.data_sent == result.data_sent assert expected_result == result - @pytest.mark.asyncio - async def test_build_payload_comments_true( - self, sample_comparison, mock_configuration - ): + def test_build_payload_comments_true(self, sample_comparison, mock_configuration): base_commit = sample_comparison.project_coverage_base.commit head_commit = sample_comparison.head.commit mock_configuration.params["setup"]["codecov_dashboard_url"] = "test.example.br" @@ -2179,7 +2122,7 @@ async def test_build_payload_comments_true( notifier_site_settings={}, current_yaml={"comment": True}, ) - res = await notifier.build_payload(sample_comparison) + res = notifier.build_payload(sample_comparison) assert res == { "state": "success", "output": { @@ -2188,10 +2131,7 @@ async def test_build_payload_comments_true( }, } - @pytest.mark.asyncio - async def test_build_payload_comments_false( - self, sample_comparison, mock_configuration - ): + def test_build_payload_comments_false(self, sample_comparison, mock_configuration): base_commit = sample_comparison.project_coverage_base.commit head_commit = sample_comparison.head.commit mock_configuration.params["setup"]["codecov_dashboard_url"] = "test.example.br" @@ -2202,7 +2142,7 @@ async def test_build_payload_comments_false( notifier_site_settings={}, current_yaml={"comment": False}, ) - res = await notifier.build_payload(sample_comparison) + res = notifier.build_payload(sample_comparison) assert res == { "state": "success", "output": { diff --git a/services/notification/notifiers/tests/unit/test_codecov_slack_app.py b/services/notification/notifiers/tests/unit/test_codecov_slack_app.py index d63ba2a81..a53c8d2b3 100644 --- a/services/notification/notifiers/tests/unit/test_codecov_slack_app.py +++ b/services/notification/notifiers/tests/unit/test_codecov_slack_app.py @@ -1,7 +1,5 @@ from unittest.mock import patch -import pytest - from database.enums import Notification from services.notification.notifiers.codecov_slack_app import CodecovSlackAppNotifier @@ -39,8 +37,7 @@ def test_notification_type(self, dbsession, mock_configuration, sample_compariso assert notifier.notification_type == Notification.codecov_slack_app @patch("requests.post") - @pytest.mark.asyncio - async def test_notify( + def test_notify( self, mock_requests_post, dbsession, mock_configuration, sample_comparison ): mock_requests_post.return_value.status_code = 200 @@ -51,13 +48,12 @@ async def test_notify( notifier_site_settings=True, current_yaml={"slack_app": {"enabled": True}}, ) - result = await notifier.notify(sample_comparison) + result = notifier.notify(sample_comparison) assert result.notification_successful == True assert result.explanation == "Successfully notified slack app" @patch("requests.post") - @pytest.mark.asyncio - async def test_notify_failure( + def test_notify_failure( self, mock_requests_post, dbsession, mock_configuration, sample_comparison ): mock_requests_post.return_value.status_code = 500 @@ -69,7 +65,7 @@ async def test_notify_failure( notifier_site_settings=True, current_yaml={"slack_app": {"enabled": True}}, ) - result = await notifier.notify(sample_comparison) + result = notifier.notify(sample_comparison) assert result.notification_successful == False assert ( result.explanation @@ -77,8 +73,7 @@ async def test_notify_failure( ) @patch("requests.post") - @pytest.mark.asyncio - async def test_notify_request_being_called( + def test_notify_request_being_called( self, mock_requests_post, dbsession, mock_configuration, sample_comparison ): mock_requests_post.return_value.status_code = 200 @@ -89,7 +84,7 @@ async def test_notify_request_being_called( notifier_site_settings=True, current_yaml={"slack_app": {"enabled": True}}, ) - result = await notifier.notify(sample_comparison) + result = notifier.notify(sample_comparison) assert result.notification_successful == True assert mock_requests_post.call_count == 1 assert mock_requests_post.is_called_with( diff --git a/services/notification/notifiers/tests/unit/test_comment.py b/services/notification/notifiers/tests/unit/test_comment.py index 77c7fe57f..088cf4e91 100644 --- a/services/notification/notifiers/tests/unit/test_comment.py +++ b/services/notification/notifiers/tests/unit/test_comment.py @@ -432,8 +432,7 @@ def test_diff_to_string_case_different_types(self): @pytest.mark.usefixtures("is_not_first_pull") class TestCommentNotifier(object): - @pytest.mark.asyncio - async def test_is_enabled_settings_individual_settings_false(self, dbsession): + def test_is_enabled_settings_individual_settings_false(self, dbsession): repository = RepositoryFactory.create() dbsession.add(repository) dbsession.flush() @@ -446,8 +445,7 @@ async def test_is_enabled_settings_individual_settings_false(self, dbsession): ) assert not notifier.is_enabled() - @pytest.mark.asyncio - async def test_is_enabled_settings_individual_settings_none(self, dbsession): + def test_is_enabled_settings_individual_settings_none(self, dbsession): repository = RepositoryFactory.create() dbsession.add(repository) dbsession.flush() @@ -460,8 +458,7 @@ async def test_is_enabled_settings_individual_settings_none(self, dbsession): ) assert not notifier.is_enabled() - @pytest.mark.asyncio - async def test_is_enabled_settings_individual_settings_true(self, dbsession): + def test_is_enabled_settings_individual_settings_true(self, dbsession): repository = RepositoryFactory.create() dbsession.add(repository) dbsession.flush() @@ -474,8 +471,7 @@ async def test_is_enabled_settings_individual_settings_true(self, dbsession): ) assert not notifier.is_enabled() - @pytest.mark.asyncio - async def test_is_enabled_settings_individual_settings_dict(self, dbsession): + def test_is_enabled_settings_individual_settings_dict(self, dbsession): repository = RepositoryFactory.create() dbsession.add(repository) dbsession.flush() @@ -488,8 +484,7 @@ async def test_is_enabled_settings_individual_settings_dict(self, dbsession): ) assert notifier.is_enabled() - @pytest.mark.asyncio - async def test_create_message_files_section( + def test_create_message_files_section( self, dbsession, mock_configuration, @@ -635,12 +630,11 @@ async def test_create_message_files_section( f"| [file\\_2.py](https://app.codecov.io/gh/{repository.slug}/pull/{pull.pullid}?src=pr&el=tree&filepath=file_2.py#diff-ZmlsZV8yLnB5) | `50.00% <ø> (ø)` | `0.00 <0.00> (ø)` | |", "", ] - res = await notifier.create_message(comparison, pull_dict, {"layout": "files"}) + res = notifier.create_message(comparison, pull_dict, {"layout": "files"}) for expected, res in zip(expected_result, res): assert expected == res - @pytest.mark.asyncio - async def test_create_message_files_section_with_critical_files( + def test_create_message_files_section_with_critical_files( self, dbsession, mock_configuration, @@ -794,7 +788,7 @@ async def test_create_message_files_section_with_critical_files( f"| [file\\_2.py](https://app.codecov.io/gh/{repository.slug}/pull/{pull.pullid}?src=pr&el=tree&filepath=file_2.py#diff-ZmlsZV8yLnB5) **Critical** | `50.00% <ø> (ø)` | `0.00 <0.00> (ø)` | |", "", ] - res = await notifier.build_message(comparison) + res = notifier.build_message(comparison) assert expected_result == res mocked_search_files_for_critical_changes.assert_called_with( {"file_2.py", "file_1.go"} @@ -803,8 +797,7 @@ async def test_create_message_files_section_with_critical_files( mocked_search_files_for_critical_changes.call_count == 3 ) # called 2 times by FilesSectionWriter and 1 time by NewFilesSectionWriter - @pytest.mark.asyncio - async def test_create_message_with_github_app_comment( + def test_create_message_with_github_app_comment( self, dbsession, mock_configuration, @@ -823,14 +816,13 @@ async def test_create_message_with_github_app_comment( notifier_site_settings=True, current_yaml={}, ) - res = await notifier.build_message(comparison) + res = notifier.build_message(comparison) assert ( res[0] == ":warning: Please install the !['codecov app svg image'](https://github.com/codecov/engineering-team/assets/152432831/e90313f4-9d3a-4b63-8b54-cfe14e7ec20d) to ensure uploads and comments are reliably processed by Codecov." ) - @pytest.mark.asyncio - async def test_build_message( + def test_build_message( self, dbsession, mock_configuration, mock_repo_provider, sample_comparison ): mock_configuration.params["setup"]["codecov_dashboard_url"] = "test.example.br" @@ -844,7 +836,7 @@ async def test_build_message( current_yaml={}, ) repository = sample_comparison.head.commit.repository - result = await notifier.build_message(comparison) + result = notifier.build_message(comparison) expected_result = [ f"## [Codecov](test.example.br/gh/{repository.slug}/pull/{pull.pullid}?dropdown=coverage&src=pr&el=h1) Report", "Attention: Patch coverage is `66.66667%` with `1 line` in your changes missing coverage. Please review.", @@ -903,8 +895,7 @@ async def test_build_message( assert exp == res assert result == expected_result - @pytest.mark.asyncio - async def test_build_message_flags_empty_coverage( + def test_build_message_flags_empty_coverage( self, dbsession, mock_configuration, @@ -922,7 +913,7 @@ async def test_build_message_flags_empty_coverage( current_yaml={}, ) repository = comparison.head.commit.repository - result = await notifier.build_message(comparison) + result = notifier.build_message(comparison) expected_result = [ f"## [Codecov](test.example.br/gh/{repository.slug}/pull/{pull.pullid}?dropdown=coverage&src=pr&el=h1) Report", "All modified and coverable lines are covered by tests :white_check_mark:", @@ -948,8 +939,7 @@ async def test_build_message_flags_empty_coverage( assert exp == res assert result == expected_result - @pytest.mark.asyncio - async def test_build_message_more_sections( + def test_build_message_more_sections( self, dbsession, mock_configuration, mock_repo_provider, sample_comparison ): mock_configuration.params["setup"]["codecov_dashboard_url"] = "test.example.br" @@ -973,7 +963,7 @@ async def test_build_message_more_sections( current_yaml={}, ) repository = sample_comparison.head.commit.repository - result = await notifier.build_message(comparison) + result = notifier.build_message(comparison) expected_result = [ f"## [Codecov](test.example.br/gh/{repository.slug}/pull/{pull.pullid}?dropdown=coverage&src=pr&el=h1) Report", "Attention: Patch coverage is `66.66667%` with `1 line` in your changes missing coverage. Please review.", @@ -1032,8 +1022,7 @@ async def test_build_message_more_sections( assert exp == res assert result == expected_result - @pytest.mark.asyncio - async def test_build_upgrade_message( + def test_build_upgrade_message( self, request, dbsession, @@ -1055,7 +1044,7 @@ async def test_build_upgrade_message( current_yaml={}, decoration_type=Decoration.upgrade, ) - result = await notifier.build_message(comparison) + result = notifier.build_message(comparison) provider_pull = comparison.enriched_pull.provider_pull expected_result = [ f"The author of this PR, {provider_pull['author']['username']}, is not an activated member of this organization on Codecov.", @@ -1069,8 +1058,7 @@ async def test_build_upgrade_message( assert exp == res assert result == expected_result - @pytest.mark.asyncio - async def test_build_limited_upload_message( + def test_build_limited_upload_message( self, request, dbsession, @@ -1094,7 +1082,7 @@ async def test_build_limited_upload_message( current_yaml={}, decoration_type=Decoration.upload_limit, ) - result = await notifier.build_message(comparison) + result = notifier.build_message(comparison) expected_result = [ f"## [Codecov](test.example.br/plan/gh/{pull.repository.owner.username}) upload limit reached :warning:", f"This org is currently on the free Basic Plan; which includes 250 free private repo uploads each rolling month.\ @@ -1109,8 +1097,7 @@ async def test_build_limited_upload_message( assert exp == res assert result == expected_result - @pytest.mark.asyncio - async def test_build_passing_empty_upload( + def test_build_passing_empty_upload( self, request, dbsession, @@ -1134,7 +1121,7 @@ async def test_build_passing_empty_upload( current_yaml={}, decoration_type=Decoration.passing_empty_upload, ) - result = await notifier.build_message(comparison) + result = notifier.build_message(comparison) expected_result = [ "## Codecov Report", ":heavy_check_mark: **No coverage data to report**, because files changed do not require tests or are set to [ignore](https://docs.codecov.com/docs/ignoring-paths#:~:text=You%20can%20use%20the%20top,will%20be%20skipped%20during%20processing.) ", @@ -1143,8 +1130,7 @@ async def test_build_passing_empty_upload( assert exp == res assert result == expected_result - @pytest.mark.asyncio - async def test_build_failing_empty_upload( + def test_build_failing_empty_upload( self, request, dbsession, @@ -1168,7 +1154,7 @@ async def test_build_failing_empty_upload( current_yaml={}, decoration_type=Decoration.failing_empty_upload, ) - result = await notifier.build_message(comparison) + result = notifier.build_message(comparison) expected_result = [ "## Codecov Report", "This is an empty upload", @@ -1178,8 +1164,7 @@ async def test_build_failing_empty_upload( assert exp == res assert result == expected_result - @pytest.mark.asyncio - async def test_processing_upload( + def test_processing_upload( self, request, dbsession, @@ -1203,7 +1188,7 @@ async def test_processing_upload( current_yaml={}, decoration_type=Decoration.processing_upload, ) - result = await notifier.build_message(comparison) + result = notifier.build_message(comparison) expected_result = [ "We're currently processing your upload. This comment will be updated when the results are available.", ] @@ -1211,8 +1196,7 @@ async def test_processing_upload( assert exp == res assert result == expected_result - @pytest.mark.asyncio - async def test_build_upgrade_message_enterprise( + def test_build_upgrade_message_enterprise( self, request, dbsession, @@ -1240,7 +1224,7 @@ async def test_build_upgrade_message_enterprise( current_yaml={}, decoration_type=Decoration.upgrade, ) - result = await notifier.build_message(comparison) + result = notifier.build_message(comparison) provider_pull = comparison.enriched_pull.provider_pull expected_result = [ f"The author of this PR, {provider_pull['author']['username']}, is not activated in your Codecov Self-Hosted installation.", @@ -1254,8 +1238,7 @@ async def test_build_upgrade_message_enterprise( assert exp == res assert result == expected_result - @pytest.mark.asyncio - async def test_build_message_hide_complexity( + def test_build_message_hide_complexity( self, dbsession, mock_configuration, mock_repo_provider, sample_comparison ): mock_configuration.params["setup"]["codecov_dashboard_url"] = "test.example.br" @@ -1269,7 +1252,7 @@ async def test_build_message_hide_complexity( current_yaml={"codecov": {"ui": {"hide_complexity": True}}}, ) repository = sample_comparison.head.commit.repository - result = await notifier.build_message(comparison) + result = notifier.build_message(comparison) expected_result = [ f"## [Codecov](test.example.br/gh/{repository.slug}/pull/{pull.pullid}?dropdown=coverage&src=pr&el=h1) Report", "Attention: Patch coverage is `66.66667%` with `1 line` in your changes missing coverage. Please review.", @@ -1328,8 +1311,7 @@ async def test_build_message_hide_complexity( assert exp == res assert result == expected_result - @pytest.mark.asyncio - async def test_build_message_no_base_report( + def test_build_message_no_base_report( self, dbsession, mock_configuration, @@ -1347,7 +1329,7 @@ async def test_build_message_no_base_report( current_yaml={}, ) repository = comparison.head.commit.repository - result = await notifier.build_message(comparison) + result = notifier.build_message(comparison) expected_result = [ f"## [Codecov](test.example.br/gh/{repository.slug}/pull/{pull.pullid}?dropdown=coverage&src=pr&el=h1) Report", "Attention: Patch coverage is `66.66667%` with `1 line` in your changes missing coverage. Please review.", @@ -1403,8 +1385,7 @@ async def test_build_message_no_base_report( assert exp == res assert result == expected_result - @pytest.mark.asyncio - async def test_build_message_no_base_commit( + def test_build_message_no_base_commit( self, dbsession, mock_configuration, @@ -1422,7 +1403,7 @@ async def test_build_message_no_base_commit( current_yaml={}, ) repository = comparison.head.commit.repository - result = await notifier.build_message(comparison) + result = notifier.build_message(comparison) expected_result = [ f"## [Codecov](test.example.br/gh/{repository.slug}/pull/{pull.pullid}?dropdown=coverage&src=pr&el=h1) Report", "Attention: Patch coverage is `66.66667%` with `1 line` in your changes missing coverage. Please review.", @@ -1478,8 +1459,7 @@ async def test_build_message_no_base_commit( assert exp == res assert result == expected_result - @pytest.mark.asyncio - async def test_build_message_no_change( + def test_build_message_no_change( self, dbsession, mock_configuration, @@ -1498,7 +1478,7 @@ async def test_build_message_no_change( current_yaml={}, ) repository = comparison.head.commit.repository - result = await notifier.build_message(comparison) + result = notifier.build_message(comparison) expected_result = [ f"## [Codecov](test.example.br/gh/{repository.slug}/pull/{pull.pullid}?dropdown=coverage&src=pr&el=h1) Report", @@ -1557,8 +1537,7 @@ async def test_build_message_no_change( assert exp == res assert result == expected_result - @pytest.mark.asyncio - async def test_build_message_negative_change( + def test_build_message_negative_change( self, dbsession, mock_configuration, @@ -1576,7 +1555,7 @@ async def test_build_message_negative_change( current_yaml={}, ) repository = comparison.head.commit.repository - result = await notifier.build_message(comparison) + result = notifier.build_message(comparison) expected_result = [ f"## [Codecov](test.example.br/gh/{repository.slug}/pull/{pull.pullid}?dropdown=coverage&src=pr&el=h1) Report", "All modified and coverable lines are covered by tests :white_check_mark:", @@ -1631,8 +1610,7 @@ async def test_build_message_negative_change( assert exp == res assert result == expected_result - @pytest.mark.asyncio - async def test_build_message_negative_change_tricky_rounding( + def test_build_message_negative_change_tricky_rounding( self, dbsession, mock_configuration, @@ -1672,7 +1650,7 @@ async def test_build_message_negative_change_tricky_rounding( new_head_file.append(i, ReportLine.create(coverage=0)) new_head_report.append(new_head_file) comparison.head.report = ReadOnlyReport.create_from_report(new_head_report) - result = await notifier.build_message(comparison) + result = notifier.build_message(comparison) expected_result = [ f"## [Codecov](test.example.br/gh/{repository.slug}/pull/{pull.pullid}?dropdown=coverage&src=pr&el=h1) Report", "All modified and coverable lines are covered by tests :white_check_mark:", @@ -1698,8 +1676,7 @@ async def test_build_message_negative_change_tricky_rounding( assert exp == res assert result == expected_result - @pytest.mark.asyncio - async def test_build_message_negative_change_tricky_rounding_newheader( + def test_build_message_negative_change_tricky_rounding_newheader( self, dbsession, mock_configuration, @@ -1739,7 +1716,7 @@ async def test_build_message_negative_change_tricky_rounding_newheader( new_head_file.append(i, ReportLine.create(coverage=0)) new_head_report.append(new_head_file) comparison.head.report = ReadOnlyReport.create_from_report(new_head_report) - result = await notifier.build_message(comparison) + result = notifier.build_message(comparison) expected_result = [ f"## [Codecov](test.example.br/gh/{repository.slug}/pull/{pull.pullid}?dropdown=coverage&src=pr&el=h1) Report", "All modified and coverable lines are covered by tests :white_check_mark:", @@ -1752,8 +1729,7 @@ async def test_build_message_negative_change_tricky_rounding_newheader( assert exp == res assert result == expected_result - @pytest.mark.asyncio - async def test_build_message_show_carriedforward_flags_no_cf_coverage( + def test_build_message_show_carriedforward_flags_no_cf_coverage( self, dbsession, mock_configuration, mock_repo_provider, sample_comparison ): mock_configuration.params["setup"]["codecov_dashboard_url"] = "test.example.br" @@ -1770,7 +1746,7 @@ async def test_build_message_show_carriedforward_flags_no_cf_coverage( current_yaml={}, ) repository = comparison.head.commit.repository - result = await notifier.build_message(comparison) + result = notifier.build_message(comparison) expected_result = [ f"## [Codecov](test.example.br/gh/{repository.slug}/pull/{pull.pullid}?dropdown=coverage&src=pr&el=h1) Report", "Attention: Patch coverage is `66.66667%` with `1 line` in your changes missing coverage. Please review.", @@ -1827,8 +1803,7 @@ async def test_build_message_show_carriedforward_flags_no_cf_coverage( assert exp == res assert result == expected_result - @pytest.mark.asyncio - async def test_build_message_with_without_flags( + def test_build_message_with_without_flags( self, dbsession, mock_configuration, @@ -1850,7 +1825,7 @@ async def test_build_message_with_without_flags( current_yaml={}, ) repository = comparison.head.commit.repository - result = await notifier.build_message(comparison) + result = notifier.build_message(comparison) expected_result = [ f"## [Codecov](test.example.br/gh/{repository.slug}/pull/{pull.pullid}?dropdown=coverage&src=pr&el=h1) Report", "All modified and coverable lines are covered by tests :white_check_mark:", @@ -1905,7 +1880,7 @@ async def test_build_message_with_without_flags( current_yaml={}, ) repository = comparison.head.commit.repository - result = await notifier.build_message(comparison) + result = notifier.build_message(comparison) expected_result = [ f"## [Codecov](test.example.br/gh/{repository.slug}/pull/{pull.pullid}?dropdown=coverage&src=pr&el=h1) Report", "All modified and coverable lines are covered by tests :white_check_mark:", @@ -1956,8 +1931,7 @@ async def test_build_message_with_without_flags( assert exp == res assert result == expected_result - @pytest.mark.asyncio - async def test_build_message_show_carriedforward_flags_has_cf_coverage( + def test_build_message_show_carriedforward_flags_has_cf_coverage( self, dbsession, mock_configuration, @@ -1978,7 +1952,7 @@ async def test_build_message_show_carriedforward_flags_has_cf_coverage( current_yaml={}, ) repository = comparison.head.commit.repository - result = await notifier.build_message(comparison) + result = notifier.build_message(comparison) expected_result = [ f"## [Codecov](test.example.br/gh/{repository.slug}/pull/{pull.pullid}?dropdown=coverage&src=pr&el=h1) Report", "All modified and coverable lines are covered by tests :white_check_mark:", @@ -2029,8 +2003,7 @@ async def test_build_message_show_carriedforward_flags_has_cf_coverage( assert exp == res assert result == expected_result - @pytest.mark.asyncio - async def test_build_message_hide_carriedforward_flags_has_cf_coverage( + def test_build_message_hide_carriedforward_flags_has_cf_coverage( self, dbsession, mock_configuration, @@ -2051,7 +2024,7 @@ async def test_build_message_hide_carriedforward_flags_has_cf_coverage( current_yaml={}, ) repository = comparison.head.commit.repository - result = await notifier.build_message(comparison) + result = notifier.build_message(comparison) expected_result = [ f"## [Codecov](test.example.br/gh/{repository.slug}/pull/{pull.pullid}?dropdown=coverage&src=pr&el=h1) Report", "All modified and coverable lines are covered by tests :white_check_mark:", @@ -2100,8 +2073,7 @@ async def test_build_message_hide_carriedforward_flags_has_cf_coverage( assert exp == res assert result == expected_result - @pytest.mark.asyncio - async def test_send_actual_notification_spammy( + def test_send_actual_notification_spammy( self, dbsession, mock_configuration, mock_repo_provider, sample_comparison ): notifier = CommentNotifier( @@ -2116,7 +2088,7 @@ async def test_send_actual_notification_spammy( ) data = {"message": ["message"], "commentid": "12345", "pullid": 98} mock_repo_provider.post_comment.return_value = {"id": 9865} - result = await notifier.send_actual_notification(data) + result = notifier.send_actual_notification(data) assert result.notification_attempted assert result.notification_successful assert result.explanation is None @@ -2126,8 +2098,7 @@ async def test_send_actual_notification_spammy( assert not mock_repo_provider.edit_comment.called assert not mock_repo_provider.delete_comment.called - @pytest.mark.asyncio - async def test_build_message_no_flags( + def test_build_message_no_flags( self, dbsession, mock_configuration, @@ -2149,7 +2120,7 @@ async def test_build_message_no_flags( ) comparison = sample_comparison repository = sample_comparison.head.commit.repository - result = await notifier.build_message(sample_comparison) + result = notifier.build_message(sample_comparison) expected_result = [ f"## [Codecov](test.example.br/gh/{repository.slug}/pull/{pull.pullid}?dropdown=coverage&src=pr&el=h1) Report", "Attention: Patch coverage is `66.66667%` with `1 line` in your changes missing coverage. Please review.", @@ -2207,8 +2178,7 @@ async def test_build_message_no_flags( assert exp == res assert result == expected_result - @pytest.mark.asyncio - async def test_send_actual_notification_new_no_permissions( + def test_send_actual_notification_new_no_permissions( self, dbsession, mock_configuration, mock_repo_provider, sample_comparison ): notifier = CommentNotifier( @@ -2226,7 +2196,7 @@ async def test_send_actual_notification_new_no_permissions( mock_repo_provider.delete_comment.side_effect = TorngitClientError( "code", "response", "message" ) - result = await notifier.send_actual_notification(data) + result = notifier.send_actual_notification(data) assert result.notification_attempted assert not result.notification_successful assert result.explanation == "no_permissions" @@ -2236,8 +2206,7 @@ async def test_send_actual_notification_new_no_permissions( assert not mock_repo_provider.post_comment.called assert not mock_repo_provider.edit_comment.called - @pytest.mark.asyncio - async def test_send_actual_notification_new( + def test_send_actual_notification_new( self, dbsession, mock_configuration, mock_repo_provider, sample_comparison ): notifier = CommentNotifier( @@ -2253,7 +2222,7 @@ async def test_send_actual_notification_new( data = {"message": ["message"], "commentid": "12345", "pullid": 98} mock_repo_provider.post_comment.return_value = {"id": 9865} mock_repo_provider.delete_comment.return_value = True - result = await notifier.send_actual_notification(data) + result = notifier.send_actual_notification(data) assert result.notification_attempted assert result.notification_successful assert result.explanation is None @@ -2263,8 +2232,7 @@ async def test_send_actual_notification_new( assert not mock_repo_provider.edit_comment.called mock_repo_provider.delete_comment.assert_called_with(98, "12345") - @pytest.mark.asyncio - async def test_send_actual_notification_new_no_permissions_post( + def test_send_actual_notification_new_no_permissions_post( self, dbsession, mock_configuration, mock_repo_provider, sample_comparison ): notifier = CommentNotifier( @@ -2284,7 +2252,7 @@ async def test_send_actual_notification_new_no_permissions_post( mock_repo_provider.edit_comment.side_effect = TorngitClientError( "code", "response", "message" ) - result = await notifier.send_actual_notification(data) + result = notifier.send_actual_notification(data) assert result.notification_attempted assert not result.notification_successful assert result.explanation == "comment_posting_permissions" @@ -2294,8 +2262,7 @@ async def test_send_actual_notification_new_no_permissions_post( assert not mock_repo_provider.delete_comment.called assert not mock_repo_provider.edit_comment.called - @pytest.mark.asyncio - async def test_send_actual_notification_new_deleted_comment( + def test_send_actual_notification_new_deleted_comment( self, dbsession, mock_configuration, mock_repo_provider, sample_comparison ): notifier = CommentNotifier( @@ -2313,7 +2280,7 @@ async def test_send_actual_notification_new_deleted_comment( mock_repo_provider.delete_comment.side_effect = TorngitObjectNotFoundError( "response", "message" ) - result = await notifier.send_actual_notification(data) + result = notifier.send_actual_notification(data) assert result.notification_attempted assert result.notification_successful assert result.explanation is None @@ -2323,8 +2290,7 @@ async def test_send_actual_notification_new_deleted_comment( assert not mock_repo_provider.edit_comment.called mock_repo_provider.delete_comment.assert_called_with(98, "12345") - @pytest.mark.asyncio - async def test_send_actual_notification_once_deleted_comment( + def test_send_actual_notification_once_deleted_comment( self, dbsession, mock_configuration, mock_repo_provider, sample_comparison ): notifier = CommentNotifier( @@ -2342,7 +2308,7 @@ async def test_send_actual_notification_once_deleted_comment( mock_repo_provider.edit_comment.side_effect = TorngitObjectNotFoundError( "response", "message" ) - result = await notifier.send_actual_notification(data) + result = notifier.send_actual_notification(data) assert result.notification_attempted is False assert result.notification_successful is None assert result.explanation == "comment_deleted" @@ -2352,8 +2318,7 @@ async def test_send_actual_notification_once_deleted_comment( mock_repo_provider.edit_comment.assert_called_with(98, "12345", "message") assert not mock_repo_provider.delete_comment.called - @pytest.mark.asyncio - async def test_send_actual_notification_once_non_existing_comment( + def test_send_actual_notification_once_non_existing_comment( self, dbsession, mock_configuration, mock_repo_provider, sample_comparison ): notifier = CommentNotifier( @@ -2371,7 +2336,7 @@ async def test_send_actual_notification_once_non_existing_comment( mock_repo_provider.edit_comment.side_effect = TorngitObjectNotFoundError( "response", "message" ) - result = await notifier.send_actual_notification(data) + result = notifier.send_actual_notification(data) assert result.notification_attempted assert result.notification_successful assert result.explanation is None @@ -2381,8 +2346,7 @@ async def test_send_actual_notification_once_non_existing_comment( assert not mock_repo_provider.delete_comment.called assert not mock_repo_provider.edit_comment.called - @pytest.mark.asyncio - async def test_send_actual_notification_once( + def test_send_actual_notification_once( self, dbsession, mock_configuration, mock_repo_provider, sample_comparison ): notifier = CommentNotifier( @@ -2398,7 +2362,7 @@ async def test_send_actual_notification_once( data = {"message": ["message"], "commentid": "12345", "pullid": 98} mock_repo_provider.post_comment.return_value = {"id": 9865} mock_repo_provider.edit_comment.return_value = {"id": "49"} - result = await notifier.send_actual_notification(data) + result = notifier.send_actual_notification(data) assert result.notification_attempted assert result.notification_successful assert result.explanation is None @@ -2408,8 +2372,7 @@ async def test_send_actual_notification_once( mock_repo_provider.edit_comment.assert_called_with(98, "12345", "message") assert not mock_repo_provider.delete_comment.called - @pytest.mark.asyncio - async def test_send_actual_notification_once_no_permissions( + def test_send_actual_notification_once_no_permissions( self, dbsession, mock_configuration, mock_repo_provider, sample_comparison ): notifier = CommentNotifier( @@ -2427,7 +2390,7 @@ async def test_send_actual_notification_once_no_permissions( mock_repo_provider.edit_comment.side_effect = TorngitClientError( "code", "response", "message" ) - result = await notifier.send_actual_notification(data) + result = notifier.send_actual_notification(data) assert result.notification_attempted assert not result.notification_successful assert result.explanation == "no_permissions" @@ -2437,8 +2400,7 @@ async def test_send_actual_notification_once_no_permissions( mock_repo_provider.edit_comment.assert_called_with(98, "12345", "message") assert not mock_repo_provider.delete_comment.called - @pytest.mark.asyncio - async def test_send_actual_notification_default( + def test_send_actual_notification_default( self, dbsession, mock_configuration, mock_repo_provider, sample_comparison ): notifier = CommentNotifier( @@ -2454,7 +2416,7 @@ async def test_send_actual_notification_default( data = {"message": ["message"], "commentid": "12345", "pullid": 98} mock_repo_provider.post_comment.return_value = {"id": 9865} mock_repo_provider.edit_comment.return_value = {"id": "49"} - result = await notifier.send_actual_notification(data) + result = notifier.send_actual_notification(data) assert result.notification_attempted assert result.notification_successful assert result.explanation is None @@ -2464,8 +2426,7 @@ async def test_send_actual_notification_default( mock_repo_provider.edit_comment.assert_called_with(98, "12345", "message") assert not mock_repo_provider.delete_comment.called - @pytest.mark.asyncio - async def test_send_actual_notification_default_no_permissions_edit( + def test_send_actual_notification_default_no_permissions_edit( self, dbsession, mock_configuration, mock_repo_provider, sample_comparison ): notifier = CommentNotifier( @@ -2483,7 +2444,7 @@ async def test_send_actual_notification_default_no_permissions_edit( mock_repo_provider.edit_comment.side_effect = TorngitClientError( "code", "response", "message" ) - result = await notifier.send_actual_notification(data) + result = notifier.send_actual_notification(data) assert result.notification_attempted assert result.notification_successful assert result.explanation is None @@ -2493,8 +2454,7 @@ async def test_send_actual_notification_default_no_permissions_edit( mock_repo_provider.edit_comment.assert_called_with(98, "12345", "message") assert not mock_repo_provider.delete_comment.called - @pytest.mark.asyncio - async def test_send_actual_notification_default_no_permissions_twice( + def test_send_actual_notification_default_no_permissions_twice( self, dbsession, mock_configuration, mock_repo_provider, sample_comparison ): notifier = CommentNotifier( @@ -2514,7 +2474,7 @@ async def test_send_actual_notification_default_no_permissions_twice( mock_repo_provider.edit_comment.side_effect = TorngitClientError( "code", "response", "message" ) - result = await notifier.send_actual_notification(data) + result = notifier.send_actual_notification(data) assert result.notification_attempted assert not result.notification_successful assert result.explanation == "comment_posting_permissions" @@ -2524,8 +2484,7 @@ async def test_send_actual_notification_default_no_permissions_twice( mock_repo_provider.edit_comment.assert_called_with(98, "12345", "message") assert not mock_repo_provider.delete_comment.called - @pytest.mark.asyncio - async def test_send_actual_notification_default_comment_not_found( + def test_send_actual_notification_default_comment_not_found( self, dbsession, mock_configuration, mock_repo_provider, sample_comparison ): notifier = CommentNotifier( @@ -2543,7 +2502,7 @@ async def test_send_actual_notification_default_comment_not_found( mock_repo_provider.edit_comment.side_effect = TorngitObjectNotFoundError( "response", "message" ) - result = await notifier.send_actual_notification(data) + result = notifier.send_actual_notification(data) assert result.notification_attempted assert result.notification_successful assert result.explanation is None @@ -2553,10 +2512,7 @@ async def test_send_actual_notification_default_comment_not_found( mock_repo_provider.post_comment.assert_called_with(98, "message") assert not mock_repo_provider.delete_comment.called - @pytest.mark.asyncio - async def test_notify_no_pull_request( - self, dbsession, sample_comparison_without_pull - ): + def test_notify_no_pull_request(self, dbsession, sample_comparison_without_pull): notifier = CommentNotifier( repository=sample_comparison_without_pull.head.commit.repository, title="title", @@ -2567,15 +2523,14 @@ async def test_notify_no_pull_request( notifier_site_settings=True, current_yaml={}, ) - result = await notifier.notify(sample_comparison_without_pull) + result = notifier.notify(sample_comparison_without_pull) assert not result.notification_attempted assert result.notification_successful == False assert result.explanation == "no_pull_request" assert result.data_sent is None assert result.data_received is None - @pytest.mark.asyncio - async def test_notify_pull_head_doesnt_match(self, dbsession, sample_comparison): + def test_notify_pull_head_doesnt_match(self, dbsession, sample_comparison): sample_comparison.pull.head = "aaaaaaaaaa" dbsession.flush() notifier = CommentNotifier( @@ -2588,15 +2543,14 @@ async def test_notify_pull_head_doesnt_match(self, dbsession, sample_comparison) notifier_site_settings=True, current_yaml={}, ) - result = await notifier.notify(sample_comparison) + result = notifier.notify(sample_comparison) assert not result.notification_attempted assert result.notification_successful is False assert result.explanation == "pull_head_does_not_match" assert result.data_sent is None assert result.data_received is None - @pytest.mark.asyncio - async def test_notify_pull_request_not_in_provider( + def test_notify_pull_request_not_in_provider( self, dbsession, sample_comparison_database_pull_without_provider ): notifier = CommentNotifier( @@ -2609,17 +2563,14 @@ async def test_notify_pull_request_not_in_provider( notifier_site_settings=True, current_yaml={}, ) - result = await notifier.notify(sample_comparison_database_pull_without_provider) + result = notifier.notify(sample_comparison_database_pull_without_provider) assert not result.notification_attempted assert result.notification_successful is False assert result.explanation == "pull_request_not_in_provider" assert result.data_sent is None assert result.data_received is None - @pytest.mark.asyncio - async def test_notify_server_unreachable( - self, mocker, dbsession, sample_comparison - ): + def test_notify_server_unreachable(self, mocker, dbsession, sample_comparison): mocked_send_actual_notification = mocker.patch.object( CommentNotifier, "send_actual_notification", @@ -2638,7 +2589,7 @@ async def test_notify_server_unreachable( notifier_site_settings=True, current_yaml={}, ) - result = await notifier.notify(sample_comparison) + result = notifier.notify(sample_comparison) assert result.notification_attempted assert not result.notification_successful assert result.explanation == "provider_issue" @@ -2649,8 +2600,7 @@ async def test_notify_server_unreachable( } assert result.data_received is None - @pytest.mark.asyncio - async def test_store_results(self, dbsession, sample_comparison): + def test_store_results(self, dbsession, sample_comparison): notifier = CommentNotifier( repository=sample_comparison.head.commit.repository, title="title", @@ -2675,8 +2625,7 @@ async def test_store_results(self, dbsession, sample_comparison): dbsession.refresh(sample_comparison.pull) assert sample_comparison.pull.commentid == "578263422" - @pytest.mark.asyncio - async def test_store_results_deleted_comment(self, dbsession, sample_comparison): + def test_store_results_deleted_comment(self, dbsession, sample_comparison): sample_comparison.pull.commentid = 12 dbsession.flush() notifier = CommentNotifier( @@ -2703,10 +2652,7 @@ async def test_store_results_deleted_comment(self, dbsession, sample_comparison) dbsession.refresh(sample_comparison.pull) assert sample_comparison.pull.commentid is None - @pytest.mark.asyncio - async def test_store_results_no_succesfull_result( - self, dbsession, sample_comparison - ): + def test_store_results_no_succesfull_result(self, dbsession, sample_comparison): notifier = CommentNotifier( repository=sample_comparison.head.commit.repository, title="title", @@ -2731,10 +2677,7 @@ async def test_store_results_no_succesfull_result( dbsession.refresh(sample_comparison.pull) assert sample_comparison.pull.commentid is None - @pytest.mark.asyncio - async def test_notify_unable_to_fetch_info( - self, dbsession, mocker, sample_comparison - ): + def test_notify_unable_to_fetch_info(self, dbsession, mocker, sample_comparison): mocked_build_message = mocker.patch.object( CommentNotifier, "build_message", @@ -2750,15 +2693,14 @@ async def test_notify_unable_to_fetch_info( notifier_site_settings=True, current_yaml={}, ) - result = await notifier.notify(sample_comparison) + result = notifier.notify(sample_comparison) assert not result.notification_attempted assert result.notification_successful is False assert result.explanation == "unable_build_message" assert result.data_sent is None assert result.data_received is None - @pytest.mark.asyncio - async def test_notify_not_enough_builds(self, dbsession, sample_comparison): + def test_notify_not_enough_builds(self, dbsession, sample_comparison): notifier = CommentNotifier( repository=sample_comparison.head.commit.repository, title="title", @@ -2770,7 +2712,7 @@ async def test_notify_not_enough_builds(self, dbsession, sample_comparison): notifier_site_settings=True, current_yaml={}, ) - result = await notifier.notify(sample_comparison) + result = notifier.notify(sample_comparison) assert not result.notification_attempted assert result.notification_successful is False assert result.explanation == "not_enough_builds" @@ -2814,7 +2756,7 @@ async def test_notify_with_enough_builds( sample_comparison.pull.state = pull_state dbsession.flush() dbsession.refresh(sample_comparison.pull) - result = await notifier.notify(sample_comparison) + result = notifier.notify(sample_comparison) assert result.notification_attempted assert result.notification_successful assert result.explanation is None @@ -2825,8 +2767,7 @@ async def test_notify_with_enough_builds( } assert result.data_received is None - @pytest.mark.asyncio - async def test_notify_exact_same_report_diff_unrelated_report( + def test_notify_exact_same_report_diff_unrelated_report( self, sample_comparison_no_change, mock_repo_provider ): compare_result = { @@ -2879,15 +2820,14 @@ async def test_notify_exact_same_report_diff_unrelated_report( notifier_site_settings=True, current_yaml={}, ) - res = await notifier.notify(sample_comparison_no_change) + res = notifier.notify(sample_comparison_no_change) assert res.notification_attempted is False assert res.notification_successful is False assert res.explanation == "changes_required" assert res.data_sent is None assert res.data_received is None - @pytest.mark.asyncio - async def test_notify_exact_same_report_diff_unrelated_report_update_comment( + def test_notify_exact_same_report_diff_unrelated_report_update_comment( self, sample_comparison_no_change, mock_repo_provider ): compare_result = { @@ -2942,13 +2882,12 @@ async def test_notify_exact_same_report_diff_unrelated_report_update_comment( notifier_site_settings=True, current_yaml={}, ) - res = await notifier.notify(sample_comparison_no_change) + res = notifier.notify(sample_comparison_no_change) assert res.notification_attempted is True assert res.notification_successful is True mock_repo_provider.edit_comment.assert_called() - @pytest.mark.asyncio - async def test_message_hide_details_github( + def test_message_hide_details_github( self, dbsession, mock_configuration, mock_repo_provider, sample_comparison ): mock_configuration.params["setup"]["codecov_dashboard_url"] = "test.example.br" @@ -2963,7 +2902,7 @@ async def test_message_hide_details_github( current_yaml={}, ) repository = sample_comparison.head.commit.repository - result = await notifier.build_message(comparison) + result = notifier.build_message(comparison) expected_result = [ f"## [Codecov](test.example.br/gh/{repository.slug}/pull/{pull.pullid}?dropdown=coverage&src=pr&el=h1) Report", "Attention: Patch coverage is `66.66667%` with `1 line` in your changes missing coverage. Please review.", @@ -2981,8 +2920,7 @@ async def test_message_hide_details_github( assert exp == res assert result == expected_result - @pytest.mark.asyncio - async def test_message_announcements_only( + def test_message_announcements_only( self, dbsession, mock_configuration, mock_repo_provider, sample_comparison ): mock_configuration.params["setup"]["codecov_dashboard_url"] = "test.example.br" @@ -2997,7 +2935,7 @@ async def test_message_announcements_only( current_yaml={}, ) repository = sample_comparison.head.commit.repository - result = await notifier.build_message(comparison) + result = notifier.build_message(comparison) expected_result = [ f"## [Codecov](test.example.br/gh/{repository.slug}/pull/{pull.pullid}?dropdown=coverage&src=pr&el=h1) Report", "Attention: Patch coverage is `66.66667%` with `1 line` in your changes missing coverage. Please review.", @@ -3018,8 +2956,7 @@ async def test_message_announcements_only( assert exp == res assert result == expected_result - @pytest.mark.asyncio - async def test_message_hide_details_bitbucket( + def test_message_hide_details_bitbucket( self, dbsession, mock_configuration, mock_repo_provider, sample_comparison ): mock_configuration.params["setup"]["codecov_dashboard_url"] = "test.example.br" @@ -3034,7 +2971,7 @@ async def test_message_hide_details_bitbucket( current_yaml={}, ) repository = sample_comparison.head.commit.repository - result = await notifier.build_message(comparison) + result = notifier.build_message(comparison) expected_result = [ f"## [Codecov](test.example.br/gh/{repository.slug}/pull/{pull.pullid}?dropdown=coverage&src=pr&el=h1) Report", "Attention: Patch coverage is `66.66667%` with `1 line` in your changes missing coverage. Please review.", @@ -3051,8 +2988,7 @@ async def test_message_hide_details_bitbucket( class TestFileSectionWriter(object): - @pytest.mark.asyncio - async def test_filesection_no_extra_settings(self, sample_comparison, mocker): + def test_filesection_no_extra_settings(self, sample_comparison, mocker): section_writer = FileSectionWriter( sample_comparison.head.commit.repository, "layout", @@ -3102,7 +3038,7 @@ async def test_filesection_no_extra_settings(self, sample_comparison, mocker): Change(path="added.py", new=True, in_diff=None, old_path=None, totals=None), ] lines = list( - await section_writer.write_section( + section_writer.write_section( sample_comparison, { "files": { @@ -3160,8 +3096,7 @@ def test_get_tree_cell_with_critical(self): == f"| [path/to/test\\_file.go](pull.link?src=pr&el=tree&filepath=path%2Fto%2Ftest_file.go#diff-cGF0aC90by90ZXN0X2ZpbGUuZ28=) **Critical** {metrics}" ) - @pytest.mark.asyncio - async def test_file_with_critical(self, sample_comparison, mocker): + def test_file_with_critical(self, sample_comparison, mocker): critical_path_report = mocker.MagicMock( get_critical_files_filenames=mocker.MagicMock( return_value=["file_1.go", "added.py"] @@ -3220,7 +3155,7 @@ async def test_file_with_critical(self, sample_comparison, mocker): Change(path="added.py", new=True, in_diff=None, old_path=None, totals=None), ] lines = list( - await section_writer.write_section( + section_writer.write_section( sample_comparison, { "files": { @@ -3254,8 +3189,7 @@ async def test_file_with_critical(self, sample_comparison, mocker): "... and [3 files with indirect coverage changes](pull.link/indirect-changes?src=pr&el=tree-more)", ] - @pytest.mark.asyncio - async def test_filesection_hide_project_cov(self, sample_comparison, mocker): + def test_filesection_hide_project_cov(self, sample_comparison, mocker): section_writer = NewFilesSectionWriter( sample_comparison.head.commit.repository, "layout", @@ -3286,7 +3220,7 @@ async def test_filesection_hide_project_cov(self, sample_comparison, mocker): Change(path="added.py", new=True, in_diff=None, old_path=None, totals=None), ] lines = list( - await section_writer.write_section( + section_writer.write_section( sample_comparison, { "files": { @@ -3336,8 +3270,7 @@ async def test_filesection_hide_project_cov(self, sample_comparison, mocker): "| [file\\_1.go](pull.link?src=pr&el=tree&filepath=file_1.go#diff-ZmlsZV8xLmdv) | 66.66% | [1 Missing :warning: ](pull.link?src=pr&el=tree) |", ] - @pytest.mark.asyncio - async def test_filesection_hide_project_cov_with_changed_files_but_no_missing_lines( + def test_filesection_hide_project_cov_with_changed_files_but_no_missing_lines( self, sample_comparison, mocker ): section_writer = NewFilesSectionWriter( @@ -3370,7 +3303,7 @@ async def test_filesection_hide_project_cov_with_changed_files_but_no_missing_li Change(path="added.py", new=True, in_diff=None, old_path=None, totals=None), ] lines = list( - await section_writer.write_section( + section_writer.write_section( sample_comparison, { "files": { @@ -3415,8 +3348,7 @@ async def test_filesection_hide_project_cov_with_changed_files_but_no_missing_li ) assert lines == [] - @pytest.mark.asyncio - async def test_filesection_hide_project_cov_no_files_changed( + def test_filesection_hide_project_cov_no_files_changed( self, sample_comparison, mocker ): section_writer = NewFilesSectionWriter( @@ -3449,7 +3381,7 @@ async def test_filesection_hide_project_cov_no_files_changed( Change(path="added.py", new=True, in_diff=None, old_path=None, totals=None), ] lines = list( - await section_writer.write_section( + section_writer.write_section( sample_comparison, {"files": {}}, changes, @@ -3460,8 +3392,7 @@ async def test_filesection_hide_project_cov_no_files_changed( class TestImpactedEndpointWriter(object): - @pytest.mark.asyncio - async def test_impacted_endpoints_table( + def test_impacted_endpoints_table( self, sample_comparison, mocker, mock_repo_provider ): mock_repo_provider.get_compare.return_value = { @@ -3522,7 +3453,7 @@ async def test_impacted_endpoints_table( current_yaml={}, ) lines = list( - await section_writer.write_section( + section_writer.write_section( sample_comparison, { "files": { @@ -3550,8 +3481,7 @@ async def test_impacted_endpoints_table( ) assert lines == ["| Related Entrypoints |", "|---|", "|banana|", "|GET /apple|"] - @pytest.mark.asyncio - async def test_impacted_endpoints_table_empty_list_result( + def test_impacted_endpoints_table_empty_list_result( self, sample_comparison, mocker, mock_repo_provider ): mocker.patch.object( @@ -3570,7 +3500,7 @@ async def test_impacted_endpoints_table_empty_list_result( current_yaml={}, ) lines = list( - await section_writer.write_section( + section_writer.write_section( sample_comparison, { "files": { @@ -3600,8 +3530,7 @@ async def test_impacted_endpoints_table_empty_list_result( "This change has been scanned for critical changes. [Learn more](https://docs.codecov.com/docs/impact-analysis)" ] - @pytest.mark.asyncio - async def test_impacted_endpoints_table_none_result( + def test_impacted_endpoints_table_none_result( self, sample_comparison, mocker, mock_repo_provider ): mocker.patch.object( @@ -3620,7 +3549,7 @@ async def test_impacted_endpoints_table_none_result( current_yaml={}, ) lines = list( - await section_writer.write_section( + section_writer.write_section( sample_comparison, { "files": { @@ -3650,8 +3579,7 @@ async def test_impacted_endpoints_table_none_result( class TestNewHeaderSectionWriter(object): - @pytest.mark.asyncio - async def test_new_header_section_writer(self, mocker, sample_comparison): + def test_new_header_section_writer(self, mocker, sample_comparison): writer = HeaderSectionWriter( mocker.MagicMock(), mocker.MagicMock(), @@ -3664,7 +3592,7 @@ async def test_new_header_section_writer(self, mocker, sample_comparison): return_value=Decimal(0), ) res = list( - await writer.write_section( + writer.write_section( sample_comparison, None, None, @@ -3676,10 +3604,7 @@ async def test_new_header_section_writer(self, mocker, sample_comparison): f"> Project coverage is 0%. Comparing base [(`{sample_comparison.project_coverage_base.commit.commitid[:7]}`)](urlurl?dropdown=coverage&el=desc) to head [(`{sample_comparison.head.commit.commitid[:7]}`)](headurl?dropdown=coverage&el=desc).", ] - @pytest.mark.asyncio - async def test_new_header_section_writer_with_behind_by( - self, mocker, sample_comparison - ): + def test_new_header_section_writer_with_behind_by(self, mocker, sample_comparison): writer = HeaderSectionWriter( mocker.MagicMock(), mocker.MagicMock(), @@ -3692,7 +3617,7 @@ async def test_new_header_section_writer_with_behind_by( return_value=Decimal(0), ) res = list( - await writer.write_section( + writer.write_section( sample_comparison, None, None, @@ -3706,8 +3631,7 @@ async def test_new_header_section_writer_with_behind_by( "> Report is 3 commits behind head on master.", ] - @pytest.mark.asyncio - async def test_new_header_section_writer_test_results_setup( + def test_new_header_section_writer_test_results_setup( self, mocker, sample_comparison ): sample_comparison.context = ComparisonContext(all_tests_passed=True) @@ -3723,7 +3647,7 @@ async def test_new_header_section_writer_test_results_setup( return_value=Decimal(0), ) res = list( - await writer.write_section( + writer.write_section( sample_comparison, None, None, @@ -3737,8 +3661,7 @@ async def test_new_header_section_writer_test_results_setup( ":white_check_mark: All tests successful. No failed tests found.", ] - @pytest.mark.asyncio - async def test_new_header_section_writer_test_results_error( + def test_new_header_section_writer_test_results_error( self, mocker, sample_comparison ): sample_comparison.context = ComparisonContext( @@ -3757,7 +3680,7 @@ async def test_new_header_section_writer_test_results_error( return_value=Decimal(0), ) res = list( - await writer.write_section( + writer.write_section( sample_comparison, None, None, @@ -3771,8 +3694,7 @@ async def test_new_header_section_writer_test_results_error( ":x: We are unable to process any of the uploaded JUnit XML files. Please ensure your files are in the right format.", ] - @pytest.mark.asyncio - async def test_new_header_section_writer_no_project_coverage( + def test_new_header_section_writer_no_project_coverage( self, mocker, sample_comparison ): writer = HeaderSectionWriter( @@ -3787,7 +3709,7 @@ async def test_new_header_section_writer_no_project_coverage( return_value=Decimal(0), ) res = list( - await writer.write_section( + writer.write_section( sample_comparison, None, None, @@ -3798,8 +3720,7 @@ async def test_new_header_section_writer_no_project_coverage( "All modified and coverable lines are covered by tests :white_check_mark:", ] - @pytest.mark.asyncio - async def test_new_header_section_writer_no_project_coverage_test_results_setup( + def test_new_header_section_writer_no_project_coverage_test_results_setup( self, mocker, sample_comparison ): sample_comparison.context = ComparisonContext(all_tests_passed=True) @@ -3815,7 +3736,7 @@ async def test_new_header_section_writer_no_project_coverage_test_results_setup( return_value=Decimal(0), ) res = list( - await writer.write_section( + writer.write_section( sample_comparison, None, None, @@ -3828,8 +3749,7 @@ async def test_new_header_section_writer_no_project_coverage_test_results_setup( ":white_check_mark: All tests successful. No failed tests found.", ] - @pytest.mark.asyncio - async def test_new_header_section_writer_no_project_coverage_test_results_error( + def test_new_header_section_writer_no_project_coverage_test_results_error( self, mocker, sample_comparison ): sample_comparison.context = ComparisonContext( @@ -3848,7 +3768,7 @@ async def test_new_header_section_writer_no_project_coverage_test_results_error( return_value=Decimal(0), ) res = list( - await writer.write_section( + writer.write_section( sample_comparison, None, None, @@ -3863,8 +3783,7 @@ async def test_new_header_section_writer_no_project_coverage_test_results_error( class TestAnnouncementsSectionWriter(object): - @pytest.mark.asyncio - async def test_announcement_section_writer(self, mocker): + def test_announcement_section_writer(self, mocker): writer = AnnouncementSectionWriter( mocker.MagicMock(), mocker.MagicMock(), @@ -3872,17 +3791,14 @@ async def test_announcement_section_writer(self, mocker): mocker.MagicMock(), mocker.MagicMock(), ) - res = list(await writer.write_section(mocker.MagicMock())) + res = list(writer.write_section(mocker.MagicMock())) assert len(res) == 1 line = res[0] assert line.startswith(":mega: ") message = line[7:] assert message in AnnouncementSectionWriter.current_active_messages - @pytest.mark.asyncio - async def test_announcement_section_writer_ats( - self, mocker, create_sample_comparison - ): + def test_announcement_section_writer_ats(self, mocker, create_sample_comparison): comparison = create_sample_comparison() current_yaml = UserYaml({}) @@ -3896,7 +3812,7 @@ async def test_announcement_section_writer_ats( writer.repository.language = "python" comparison.head.report._chunks = ["xx"] * 80_000_000 - res = list(await writer.write_section(comparison)) + res = list(writer.write_section(comparison)) assert len(res) == 1 line = res[0] assert line.startswith(":mega: ") @@ -3904,8 +3820,7 @@ async def test_announcement_section_writer_ats( class TestNewFooterSectionWriter(object): - @pytest.mark.asyncio - async def test_footer_section_writer_in_github(self, mocker): + def test_footer_section_writer_in_github(self, mocker): writer = NewFooterSectionWriter( mocker.MagicMock(), mocker.MagicMock(), @@ -3916,9 +3831,7 @@ async def test_footer_section_writer_in_github(self, mocker): mock_comparison = mocker.MagicMock() mock_comparison.repository_service.service = "github" res = list( - await writer.write_section( - mock_comparison, {}, [], links={"pull": "pull.link"} - ) + writer.write_section(mock_comparison, {}, [], links={"pull": "pull.link"}) ) assert res == [ "", @@ -3926,8 +3839,7 @@ async def test_footer_section_writer_in_github(self, mocker): ":loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/).", ] - @pytest.mark.asyncio - async def test_footer_section_writer_in_gitlab(self, mocker): + def test_footer_section_writer_in_gitlab(self, mocker): writer = NewFooterSectionWriter( mocker.MagicMock(), mocker.MagicMock(), @@ -3938,9 +3850,7 @@ async def test_footer_section_writer_in_gitlab(self, mocker): mock_comparison = mocker.MagicMock() mock_comparison.repository_service.service = "gitlab" res = list( - await writer.write_section( - mock_comparison, {}, [], links={"pull": "pull.link"} - ) + writer.write_section(mock_comparison, {}, [], links={"pull": "pull.link"}) ) assert res == [ "", @@ -3948,8 +3858,7 @@ async def test_footer_section_writer_in_gitlab(self, mocker): ":loudspeaker: Have feedback on the report? [Share it here](https://gitlab.com/codecov-open-source/codecov-user-feedback/-/issues/4).", ] - @pytest.mark.asyncio - async def test_footer_section_writer_in_bitbucket(self, mocker): + def test_footer_section_writer_in_bitbucket(self, mocker): writer = NewFooterSectionWriter( mocker.MagicMock(), mocker.MagicMock(), @@ -3960,9 +3869,7 @@ async def test_footer_section_writer_in_bitbucket(self, mocker): mock_comparison = mocker.MagicMock() mock_comparison.repository_service.service = "bitbucket" res = list( - await writer.write_section( - mock_comparison, {}, [], links={"pull": "pull.link"} - ) + writer.write_section(mock_comparison, {}, [], links={"pull": "pull.link"}) ) assert res == [ "", @@ -3970,8 +3877,7 @@ async def test_footer_section_writer_in_bitbucket(self, mocker): ":loudspeaker: Have feedback on the report? [Share it here](https://gitlab.com/codecov-open-source/codecov-user-feedback/-/issues/4).", ] - @pytest.mark.asyncio - async def test_footer_section_writer_with_project_cov_hidden(self, mocker): + def test_footer_section_writer_with_project_cov_hidden(self, mocker): writer = NewFooterSectionWriter( mocker.MagicMock(), mocker.MagicMock(), @@ -3985,9 +3891,7 @@ async def test_footer_section_writer_with_project_cov_hidden(self, mocker): mock_comparison = mocker.MagicMock() mock_comparison.repository_service.service = "bitbucket" res = list( - await writer.write_section( - mock_comparison, {}, [], links={"pull": "pull.link"} - ) + writer.write_section(mock_comparison, {}, [], links={"pull": "pull.link"}) ) assert res == [ "", @@ -3997,8 +3901,7 @@ async def test_footer_section_writer_with_project_cov_hidden(self, mocker): @pytest.mark.usefixtures("is_not_first_pull") class TestCommentNotifierInNewLayout(object): - @pytest.mark.asyncio - async def test_create_message_files_section_with_critical_files_new_layout( + def test_create_message_files_section_with_critical_files_new_layout( self, dbsession, mock_configuration, @@ -4152,7 +4055,7 @@ async def test_create_message_files_section_with_critical_files_new_layout( f"| [file\\_2.py](https://app.codecov.io/gh/{repository.slug}/pull/{pull.pullid}?src=pr&el=tree&filepath=file_2.py#diff-ZmlsZV8yLnB5) **Critical** | `50.00% <ø> (ø)` | `0.00 <0.00> (ø)` | |", "", ] - res = await notifier.build_message(comparison) + res = notifier.build_message(comparison) assert expected_result == res mocked_search_files_for_critical_changes.assert_called_with( {"file_2.py", "file_1.go"} @@ -4161,8 +4064,7 @@ async def test_create_message_files_section_with_critical_files_new_layout( mocked_search_files_for_critical_changes.call_count == 3 ) # called 2 times by FilesSectionWriter and 1 time by NewFilesSectionWriter - @pytest.mark.asyncio - async def test_build_message_no_base_commit_new_layout( + def test_build_message_no_base_commit_new_layout( self, dbsession, mock_configuration, @@ -4183,7 +4085,7 @@ async def test_build_message_no_base_commit_new_layout( current_yaml={}, ) repository = comparison.head.commit.repository - result = await notifier.build_message(comparison) + result = notifier.build_message(comparison) expected_result = [ f"## [Codecov](test.example.br/gh/{repository.slug}/pull/{pull.pullid}?dropdown=coverage&src=pr&el=h1) Report", "Attention: Patch coverage is `66.66667%` with `1 line` in your changes missing coverage. Please review.", @@ -4230,8 +4132,7 @@ async def test_build_message_no_base_commit_new_layout( assert exp == res assert result == expected_result - @pytest.mark.asyncio - async def test_build_message_no_base_report_new_layout( + def test_build_message_no_base_report_new_layout( self, dbsession, mock_configuration, @@ -4253,7 +4154,7 @@ async def test_build_message_no_base_report_new_layout( current_yaml={}, ) repository = comparison.head.commit.repository - result = await notifier.build_message(comparison) + result = notifier.build_message(comparison) expected_result = [ f"## [Codecov](test.example.br/gh/{repository.slug}/pull/{pull.pullid}?dropdown=coverage&src=pr&el=h1) Report", "Attention: Patch coverage is `66.66667%` with `1 line` in your changes missing coverage. Please review.", @@ -4303,8 +4204,7 @@ async def test_build_message_no_base_report_new_layout( assert exp == res assert result == expected_result - @pytest.mark.asyncio - async def test_build_message_no_project_coverage( + def test_build_message_no_project_coverage( self, dbsession, mock_configuration, @@ -4326,7 +4226,7 @@ async def test_build_message_no_project_coverage( current_yaml={}, ) repository = comparison.head.commit.repository - result = await notifier.build_message(comparison) + result = notifier.build_message(comparison) pull_url = f"test.example.br/gh/{repository.slug}/pull/{pull.pullid}" expected_result = [ f"## [Codecov]({pull_url}?dropdown=coverage&src=pr&el=h1) Report", @@ -4344,8 +4244,7 @@ async def test_build_message_no_project_coverage( assert exp == res assert result == expected_result - @pytest.mark.asyncio - async def test_build_message_no_project_coverage_files( + def test_build_message_no_project_coverage_files( self, dbsession, mock_configuration, @@ -4367,7 +4266,7 @@ async def test_build_message_no_project_coverage_files( current_yaml={}, ) repository = comparison.head.commit.repository - result = await notifier.build_message(comparison) + result = notifier.build_message(comparison) pull_url = f"test.example.br/gh/{repository.slug}/pull/{pull.pullid}" expected_result = [ f"## [Codecov]({pull_url}?dropdown=coverage&src=pr&el=h1) Report", @@ -4391,8 +4290,7 @@ async def test_build_message_no_project_coverage_files( for exp, res in zip(expected_result, result): assert exp == res - @pytest.mark.asyncio - async def test_build_message_no_project_coverage_condensed_yaml_configs( + def test_build_message_no_project_coverage_condensed_yaml_configs( self, dbsession, mock_configuration, @@ -4414,7 +4312,7 @@ async def test_build_message_no_project_coverage_condensed_yaml_configs( current_yaml={}, ) repository = comparison.head.commit.repository - result = await notifier.build_message(comparison) + result = notifier.build_message(comparison) pull_url = f"test.example.br/gh/{repository.slug}/pull/{pull.pullid}" expected_result = [ f"## [Codecov]({pull_url}?dropdown=coverage&src=pr&el=h1) Report", @@ -4432,8 +4330,7 @@ async def test_build_message_no_project_coverage_condensed_yaml_configs( assert exp == res assert result == expected_result - @pytest.mark.asyncio - async def test_build_message_head_and_pull_head_differ_new_layout( + def test_build_message_head_and_pull_head_differ_new_layout( self, dbsession, mock_configuration, @@ -4455,7 +4352,7 @@ async def test_build_message_head_and_pull_head_differ_new_layout( current_yaml={}, ) repository = comparison.head.commit.repository - result = await notifier.build_message(comparison) + result = notifier.build_message(comparison) expected_result = [ f"## [Codecov](test.example.br/gh/{repository.slug}/pull/{pull.pullid}?dropdown=coverage&src=pr&el=h1) Report", "Attention: Patch coverage is `66.66667%` with `1 line` in your changes missing coverage. Please review.", @@ -4502,8 +4399,7 @@ async def test_build_message_head_and_pull_head_differ_new_layout( assert exp == res assert result == expected_result - @pytest.mark.asyncio - async def test_build_message_head_and_pull_head_differ_with_components( + def test_build_message_head_and_pull_head_differ_with_components( self, dbsession, mock_configuration, @@ -4532,7 +4428,7 @@ async def test_build_message_head_and_pull_head_differ_with_components( }, ) repository = comparison.head.commit.repository - result = await notifier.build_message(comparison) + result = notifier.build_message(comparison) expected_result = [ f"## [Codecov](test.example.br/gh/{repository.slug}/pull/{pull.pullid}?dropdown=coverage&src=pr&el=h1) Report", "Attention: Patch coverage is `66.66667%` with `1 line` in your changes missing coverage. Please review.", @@ -4584,8 +4480,7 @@ async def test_build_message_head_and_pull_head_differ_with_components( assert exp == res assert result == expected_result - @pytest.mark.asyncio - async def test_build_message_team_plan_customer_missing_lines( + def test_build_message_team_plan_customer_missing_lines( self, dbsession, mock_configuration, @@ -4619,7 +4514,7 @@ async def test_build_message_team_plan_customer_missing_lines( pull = comparison.pull repository = sample_comparison_head_and_pull_head_differ.head.commit.repository - result = await notifier.build_message(comparison) + result = notifier.build_message(comparison) expected_result = [ f"## [Codecov](test.example.br/gh/{repository.slug}/pull/{pull.pullid}?dropdown=coverage&src=pr&el=h1) Report", "Attention: Patch coverage is `66.66667%` with `1 line` in your changes missing coverage. Please review.", @@ -4631,8 +4526,7 @@ async def test_build_message_team_plan_customer_missing_lines( ] assert result == expected_result - @pytest.mark.asyncio - async def test_build_message_team_plan_customer_all_lines_covered( + def test_build_message_team_plan_customer_all_lines_covered( self, dbsession, mock_configuration, @@ -4660,7 +4554,7 @@ async def test_build_message_team_plan_customer_all_lines_covered( ) pull = comparison.pull repository = sample_comparison_coverage_carriedforward.head.commit.repository - result = await notifier.build_message(comparison) + result = notifier.build_message(comparison) expected_result = [ f"## [Codecov](test.example.br/gh/{repository.slug}/pull/{pull.pullid}?dropdown=coverage&src=pr&el=h1) Report", @@ -4672,8 +4566,7 @@ async def test_build_message_team_plan_customer_all_lines_covered( ] assert result == expected_result - @pytest.mark.asyncio - async def test_build_message_team_plan_customer_all_lines_covered_test_results_error( + def test_build_message_team_plan_customer_all_lines_covered_test_results_error( self, dbsession, mock_configuration, @@ -4702,7 +4595,7 @@ async def test_build_message_team_plan_customer_all_lines_covered_test_results_e ) pull = comparison.pull repository = sample_comparison_coverage_carriedforward.head.commit.repository - result = await notifier.build_message(comparison) + result = notifier.build_message(comparison) expected_result = [ f"## [Codecov](test.example.br/gh/{repository.slug}/pull/{pull.pullid}?dropdown=coverage&src=pr&el=h1) Report", @@ -4714,8 +4607,7 @@ async def test_build_message_team_plan_customer_all_lines_covered_test_results_e ] assert result == expected_result - @pytest.mark.asyncio - async def test_build_message_team_plan_customer_all_lines_covered_no_third_line( + def test_build_message_team_plan_customer_all_lines_covered_no_third_line( self, dbsession, mock_configuration, @@ -4744,7 +4636,7 @@ async def test_build_message_team_plan_customer_all_lines_covered_no_third_line( ) pull = comparison.pull repository = sample_comparison_coverage_carriedforward.head.commit.repository - result = await notifier.build_message(comparison) + result = notifier.build_message(comparison) expected_result = [ f"## [Codecov](test.example.br/gh/{repository.slug}/pull/{pull.pullid}?dropdown=coverage&src=pr&el=h1) Report", @@ -4754,8 +4646,7 @@ async def test_build_message_team_plan_customer_all_lines_covered_no_third_line( ] assert result == expected_result - @pytest.mark.asyncio - async def test_build_message_no_patch_or_proj_change( + def test_build_message_no_patch_or_proj_change( self, dbsession, mock_configuration, @@ -4775,7 +4666,7 @@ async def test_build_message_no_patch_or_proj_change( current_yaml={}, ) repository = comparison.head.commit.repository - result = await notifier.build_message(comparison) + result = notifier.build_message(comparison) expected_result = [ f"## [Codecov](test.example.br/gh/{repository.slug}/pull/{pull.pullid}?dropdown=coverage&src=pr&el=h1) Report", "All modified and coverable lines are covered by tests :white_check_mark:", @@ -4825,8 +4716,7 @@ async def test_build_message_no_patch_or_proj_change( class TestComponentWriterSection(object): - @pytest.mark.asyncio - async def test_write_message_component_section_empty( + def test_write_message_component_section_empty( self, dbsession, mock_configuration, @@ -4841,14 +4731,13 @@ async def test_write_message_component_section_empty( settings={}, current_yaml={}, ) - message = await section_writer.write_section( + message = section_writer.write_section( comparison=comparison, diff=None, changes=None, links={"pull": "urlurl"} ) expected = [] assert message == expected - @pytest.mark.asyncio - async def test_get_component_data_for_table( + def test_get_component_data_for_table( self, dbsession, mock_configuration, @@ -4872,7 +4761,7 @@ async def test_get_component_data_for_table( ) all_components = get_components_from_yaml(section_writer.current_yaml) comparison = sample_comparison - component_data = await section_writer._get_table_data_for_components( + component_data = section_writer._get_table_data_for_components( all_components, comparison ) expected_result = [ @@ -4975,8 +4864,7 @@ async def test_get_component_data_for_table( ] assert component_data == expected_result - @pytest.mark.asyncio - async def test_get_component_data_for_table_no_base( + def test_get_component_data_for_table_no_base( self, dbsession, mock_configuration, @@ -5002,7 +4890,7 @@ async def test_get_component_data_for_table_no_base( ) all_components = get_components_from_yaml(section_writer.current_yaml) comparison = sample_comparison - component_data = await section_writer._get_table_data_for_components( + component_data = section_writer._get_table_data_for_components( all_components, comparison ) expected_result = [ @@ -5049,8 +4937,7 @@ async def test_get_component_data_for_table_no_base( ] assert component_data == expected_result - @pytest.mark.asyncio - async def test_write_message_component_section( + def test_write_message_component_section( self, dbsession, mock_configuration, @@ -5072,7 +4959,7 @@ async def test_write_message_component_section( } }, ) - message = await section_writer.write_section( + message = section_writer.write_section( comparison=comparison, diff=None, changes=None, links={"pull": "urlurl"} ) expected = [ @@ -5083,8 +4970,7 @@ async def test_write_message_component_section( ] assert message == expected - @pytest.mark.asyncio - async def test_write_message_component_section_no_base( + def test_write_message_component_section_no_base( self, dbsession, mock_configuration, @@ -5108,7 +4994,7 @@ async def test_write_message_component_section_no_base( } }, ) - message = await section_writer.write_section( + message = section_writer.write_section( comparison=comparison, diff=None, changes=None, links={"pull": "urlurl"} ) expected = [ @@ -5124,8 +5010,7 @@ async def test_write_message_component_section_no_base( class TestCommentNotifierWelcome: - @pytest.mark.asyncio - async def test_build_message( + def test_build_message( self, dbsession, mock_configuration, mock_repo_provider, sample_comparison ): mock_configuration.params["setup"]["codecov_dashboard_url"] = "test.example.br" @@ -5137,7 +5022,7 @@ async def test_build_message( notifier_site_settings=True, current_yaml={}, ) - result = await notifier.build_message(sample_comparison) + result = notifier.build_message(sample_comparison) expected_result = [ "## Welcome to [Codecov](https://codecov.io) :tada:", "", @@ -5149,8 +5034,7 @@ async def test_build_message( assert exp == res assert result == expected_result - @pytest.mark.asyncio - async def test_build_message_with_preexisting_bundle_pulls( + def test_build_message_with_preexisting_bundle_pulls( self, dbsession, mock_configuration, mock_repo_provider ): mock_configuration.params["setup"]["codecov_dashboard_url"] = "test.example.br" @@ -5240,7 +5124,7 @@ async def test_build_message_with_preexisting_bundle_pulls( notifier_site_settings=True, current_yaml={}, ) - result = await notifier.build_message(comparison) + result = notifier.build_message(comparison) expected_result = [ "## Welcome to [Codecov](https://codecov.io) :tada:", @@ -5256,8 +5140,7 @@ async def test_build_message_with_preexisting_bundle_pulls( pulls_in_db = dbsession.query(Pull).all() assert len(pulls_in_db) == 3 - @pytest.mark.asyncio - async def test_should_see_project_coverage_cta_public_repo( + def test_should_see_project_coverage_cta_public_repo( self, dbsession, mock_configuration, mock_repo_provider, sample_comparison ): mock_configuration.params["setup"]["codecov_dashboard_url"] = "test.example.br" @@ -5286,7 +5169,7 @@ async def test_should_see_project_coverage_cta_public_repo( notifier_site_settings=True, current_yaml={}, ) - result = await notifier.build_message(sample_comparison) + result = notifier.build_message(sample_comparison) assert PROJECT_COVERAGE_CTA not in result after_introduction_date = datetime(2024, 6, 1, 0, 0, 0).replace( @@ -5306,11 +5189,10 @@ async def test_should_see_project_coverage_cta_public_repo( notifier_site_settings=True, current_yaml={}, ) - result = await notifier.build_message(sample_comparison) + result = notifier.build_message(sample_comparison) assert PROJECT_COVERAGE_CTA in result - @pytest.mark.asyncio - async def test_should_see_project_coverage_cta_introduction_date( + def test_should_see_project_coverage_cta_introduction_date( self, dbsession, mock_configuration, mock_repo_provider, sample_comparison ): mock_configuration.params["setup"]["codecov_dashboard_url"] = "test.example.br" @@ -5339,7 +5221,7 @@ async def test_should_see_project_coverage_cta_introduction_date( notifier_site_settings=True, current_yaml={}, ) - result = await notifier.build_message(sample_comparison) + result = notifier.build_message(sample_comparison) assert PROJECT_COVERAGE_CTA not in result after_introduction_date = datetime(2024, 6, 1, 0, 0, 0).replace( @@ -5359,11 +5241,10 @@ async def test_should_see_project_coverage_cta_introduction_date( notifier_site_settings=True, current_yaml={}, ) - result = await notifier.build_message(sample_comparison) + result = notifier.build_message(sample_comparison) assert PROJECT_COVERAGE_CTA in result - @pytest.mark.asyncio - async def test_should_see_project_coverage_cta_team_plan( + def test_should_see_project_coverage_cta_team_plan( self, dbsession, mock_configuration, mock_repo_provider, sample_comparison ): mock_configuration.params["setup"]["codecov_dashboard_url"] = "test.example.br" @@ -5396,7 +5277,7 @@ async def test_should_see_project_coverage_cta_team_plan( notifier_site_settings=True, current_yaml={}, ) - result = await notifier.build_message(sample_comparison) + result = notifier.build_message(sample_comparison) assert PROJECT_COVERAGE_CTA not in result sample_comparison.head.commit.repository.owner.plan = ( @@ -5413,7 +5294,7 @@ async def test_should_see_project_coverage_cta_team_plan( notifier_site_settings=True, current_yaml={}, ) - result = await notifier.build_message(sample_comparison) + result = notifier.build_message(sample_comparison) assert PROJECT_COVERAGE_CTA in result diff --git a/services/notification/notifiers/tests/unit/test_comment_conditions.py b/services/notification/notifiers/tests/unit/test_comment_conditions.py index 393f64650..9ad9e2c7b 100644 --- a/services/notification/notifiers/tests/unit/test_comment_conditions.py +++ b/services/notification/notifiers/tests/unit/test_comment_conditions.py @@ -57,7 +57,6 @@ def _get_mock_compare_result(file_affected: str, lines_affected: List[str]) -> D } -@pytest.mark.asyncio @pytest.mark.parametrize( "comparison_name, condition, expected", [ @@ -99,7 +98,7 @@ def _get_mock_compare_result(file_affected: str, lines_affected: List[str]) -> D ), ], ) -async def test_condition_different_comparisons_no_diff( +def test_condition_different_comparisons_no_diff( comparison_name, condition, expected, mock_repo_provider, request ): comparison = request.getfixturevalue(comparison_name) @@ -107,12 +106,9 @@ async def test_condition_different_comparisons_no_diff( # Any change then needs to be a coverage change mock_repo_provider.get_compare.return_value = {"diff": {"files": {}, "commits": []}} notifier = _get_notifier(comparison.head.commit.repository, condition) - assert ( - await HasEnoughRequiredChanges.check_condition(notifier, comparison) == expected - ) + assert HasEnoughRequiredChanges.check_condition(notifier, comparison) == expected -@pytest.mark.asyncio @pytest.mark.parametrize( "condition, expected", [ @@ -136,7 +132,7 @@ async def test_condition_different_comparisons_no_diff( ), ], ) -async def test_condition_exact_same_report_coverage_not_affected_by_diff( +def test_condition_exact_same_report_coverage_not_affected_by_diff( sample_comparison_no_change, mock_repo_provider, condition, expected ): mock_repo_provider.get_compare.return_value = _get_mock_compare_result( @@ -146,14 +142,11 @@ async def test_condition_exact_same_report_coverage_not_affected_by_diff( sample_comparison_no_change.head.commit.repository, condition ) assert ( - await HasEnoughRequiredChanges.check_condition( - notifier, sample_comparison_no_change - ) + HasEnoughRequiredChanges.check_condition(notifier, sample_comparison_no_change) == expected ) -@pytest.mark.asyncio @pytest.mark.parametrize( "condition, expected", [ @@ -177,7 +170,7 @@ async def test_condition_exact_same_report_coverage_not_affected_by_diff( ), ], ) -async def test_condition_exact_same_report_coverage_affected_by_diff( +def test_condition_exact_same_report_coverage_affected_by_diff( sample_comparison_no_change, mock_repo_provider, condition, expected ): mock_repo_provider.get_compare.return_value = _get_mock_compare_result( @@ -187,14 +180,11 @@ async def test_condition_exact_same_report_coverage_affected_by_diff( sample_comparison_no_change.head.commit.repository, condition ) assert ( - await HasEnoughRequiredChanges.check_condition( - notifier, sample_comparison_no_change - ) + HasEnoughRequiredChanges.check_condition(notifier, sample_comparison_no_change) == expected ) -@pytest.mark.asyncio @pytest.mark.parametrize( "affected_lines, expected", [ @@ -202,7 +192,7 @@ async def test_condition_exact_same_report_coverage_affected_by_diff( pytest.param(["1", "8", "1", "8"], True, id="patch_NOT_100%_covered"), ], ) -async def test_uncovered_patch( +def test_uncovered_patch( sample_comparison_no_change, mock_repo_provider, affected_lines, expected ): mock_repo_provider.get_compare.return_value = _get_mock_compare_result( @@ -213,14 +203,11 @@ async def test_uncovered_patch( [CoverageCommentRequiredChanges.uncovered_patch.value], ) assert ( - await HasEnoughRequiredChanges.check_condition( - notifier, sample_comparison_no_change - ) + HasEnoughRequiredChanges.check_condition(notifier, sample_comparison_no_change) == expected ) -@pytest.mark.asyncio @pytest.mark.parametrize( "comparison_name, yaml, expected", [ @@ -245,7 +232,7 @@ async def test_uncovered_patch( ), ], ) -async def test_coverage_drop_with_different_project_configs( +def test_coverage_drop_with_different_project_configs( comparison_name, yaml, expected, request ): comparison: ComparisonProxy = request.getfixturevalue(comparison_name) @@ -254,6 +241,4 @@ async def test_coverage_drop_with_different_project_configs( comparison.head.commit.repository, [CoverageCommentRequiredChanges.coverage_drop.value], ) - assert ( - await HasEnoughRequiredChanges.check_condition(notifier, comparison) == expected - ) + assert HasEnoughRequiredChanges.check_condition(notifier, comparison) == expected diff --git a/services/notification/notifiers/tests/unit/test_generics.py b/services/notification/notifiers/tests/unit/test_generics.py index fef16cd64..29ade1103 100644 --- a/services/notification/notifiers/tests/unit/test_generics.py +++ b/services/notification/notifiers/tests/unit/test_generics.py @@ -1,5 +1,4 @@ import httpx -import pytest from database.tests.factories import RepositoryFactory from services.notification.notifiers.generics import ( @@ -12,7 +11,7 @@ class SampleNotifierForTest(StandardNotifier): def build_payload(self, comparison): return {"commitid": comparison.head.commit.commitid} - async def send_actual_notification(self, data): + def send_actual_notification(self, data): return { "notification_attempted": True, "notification_successful": True, @@ -193,8 +192,7 @@ def test_should_notify_comparison_is_above_threshold_no_coverage( ) assert not notifier.should_notify_comparison(actual_comparison) - @pytest.mark.asyncio - async def test_notify(self, sample_comparison): + def test_notify(self, sample_comparison): notifier = SampleNotifierForTest( repository=sample_comparison.head.commit.repository, title="title", @@ -205,15 +203,14 @@ async def test_notify(self, sample_comparison): notifier_site_settings=True, current_yaml={}, ) - res = await notifier.notify(sample_comparison) + res = notifier.notify(sample_comparison) assert res.notification_attempted assert res.notification_successful assert res.explanation is None assert res.data_sent == {"commitid": sample_comparison.head.commit.commitid} assert res.data_received is None - @pytest.mark.asyncio - async def test_notify_should_not_notify(self, sample_comparison, mocker): + def test_notify_should_not_notify(self, sample_comparison, mocker): notifier = SampleNotifierForTest( repository=sample_comparison.head.commit.repository, title="title", @@ -227,7 +224,7 @@ async def test_notify_should_not_notify(self, sample_comparison, mocker): mocker.patch.object( SampleNotifierForTest, "should_notify_comparison", return_value=False ) - res = await notifier.notify(sample_comparison) + res = notifier.notify(sample_comparison) assert not res.notification_attempted assert res.notification_successful is None assert res.explanation == "Did not fit criteria" @@ -236,9 +233,8 @@ async def test_notify_should_not_notify(self, sample_comparison, mocker): class TestRequestsYamlBasedNotifier(object): - @pytest.mark.asyncio - async def test_send_notification_exception(self, mocker, sample_comparison): - mocked_post = mocker.patch.object(httpx.AsyncClient, "post") + def test_send_notification_exception(self, mocker, sample_comparison): + mocked_post = mocker.patch.object(httpx.Client, "post") mocked_post.side_effect = httpx.HTTPError("message") notifier = RequestsYamlBasedNotifier( repository=sample_comparison.head.commit.repository, @@ -251,7 +247,7 @@ async def test_send_notification_exception(self, mocker, sample_comparison): current_yaml={}, ) data = {} - res = await notifier.send_actual_notification(data) + res = notifier.send_actual_notification(data) assert res == { "notification_attempted": True, "notification_successful": False, diff --git a/services/notification/notifiers/tests/unit/test_status.py b/services/notification/notifiers/tests/unit/test_status.py index 484b6787c..833c125c7 100644 --- a/services/notification/notifiers/tests/unit/test_status.py +++ b/services/notification/notifiers/tests/unit/test_status.py @@ -1,7 +1,7 @@ -from typing import Any, Coroutine from unittest.mock import MagicMock import pytest +from mock import AsyncMock from shared.reports.readonly import ReadOnlyReport from shared.reports.resources import Report, ReportFile, ReportLine from shared.torngit.exceptions import ( @@ -364,8 +364,7 @@ def test_can_we_set_this_status_no_pull(self, sample_comparison_without_pull): ) assert not exclude_branch_notifier.can_we_set_this_status(comparison) - @pytest.mark.asyncio - async def test_notify_after_n_builds_flags(self, sample_comparison, mocker): + def test_notify_after_n_builds_flags(self, sample_comparison, mocker): comparison = sample_comparison no_settings_notifier = StatusNotifier( repository=comparison.head.commit.repository, @@ -391,15 +390,14 @@ async def test_notify_after_n_builds_flags(self, sample_comparison, mocker): ), ) mocker.patch.object(StatusNotifier, "can_we_set_this_status", return_value=True) - result = await no_settings_notifier.notify(comparison) + result = no_settings_notifier.notify(comparison) assert not result.notification_attempted assert result.notification_successful is None assert result.explanation == "need_more_builds" assert result.data_sent is None assert result.data_received is None - @pytest.mark.asyncio - async def test_notify_after_n_builds_flags2(self, sample_comparison, mocker): + def test_notify_after_n_builds_flags2(self, sample_comparison, mocker): comparison = sample_comparison no_settings_notifier = StatusNotifier( repository=comparison.head.commit.repository, @@ -424,15 +422,14 @@ async def test_notify_after_n_builds_flags2(self, sample_comparison, mocker): ), ) mocker.patch.object(StatusNotifier, "can_we_set_this_status", return_value=True) - result = await no_settings_notifier.notify(comparison) + result = no_settings_notifier.notify(comparison) assert not result.notification_attempted assert result.notification_successful is None assert result.explanation == "need_more_builds" assert result.data_sent is None assert result.data_received is None - @pytest.mark.asyncio - async def test_notify_cannot_set_status(self, sample_comparison, mocker): + def test_notify_cannot_set_status(self, sample_comparison, mocker): comparison = sample_comparison no_settings_notifier = StatusNotifier( repository=comparison.head.commit.repository, @@ -444,15 +441,14 @@ async def test_notify_cannot_set_status(self, sample_comparison, mocker): mocker.patch.object( StatusNotifier, "can_we_set_this_status", return_value=False ) - result = await no_settings_notifier.notify(comparison) + result = no_settings_notifier.notify(comparison) assert not result.notification_attempted assert result.notification_successful is None assert result.explanation == "not_fit_criteria" assert result.data_sent is None assert result.data_received is None - @pytest.mark.asyncio - async def test_notify_no_base( + def test_notify_no_base( self, sample_comparison_without_base_with_pull, mocker, mock_repo_provider ): comparison = sample_comparison_without_base_with_pull @@ -484,7 +480,7 @@ async def test_notify_no_base( }, data_received={"id": "some_id"}, ) - result = await no_settings_notifier.notify(comparison) + result = no_settings_notifier.notify(comparison) assert result.notification_attempted assert result.notification_successful assert result.explanation is None @@ -495,8 +491,7 @@ async def test_notify_no_base( } assert result.data_received == {"id": "some_id"} - @pytest.mark.asyncio - async def test_notify_uncached( + def test_notify_uncached( self, sample_comparison, mocker, @@ -509,7 +504,7 @@ async def test_notify_uncached( } class TestNotifier(StatusNotifier): - async def build_payload(self, comparison): + def build_payload(self, comparison): return payload notifier = TestNotifier( @@ -522,11 +517,10 @@ async def build_payload(self, comparison): notifier.context = "fake" send_notification = mocker.patch.object(TestNotifier, "send_notification") - await notifier.notify(comparison) + notifier.notify(comparison) send_notification.assert_called_once - @pytest.mark.asyncio - async def test_notify_multiple_shas( + def test_notify_multiple_shas( self, sample_comparison, mocker, @@ -539,19 +533,19 @@ async def test_notify_multiple_shas( "url": get_pull_url(comparison.pull), } - async def set_status_side_effect(commit, *args, **kwargs): + def set_status_side_effect(commit, *args, **kwargs): return {"id": f"{commit}-status-set"} class TestNotifier(StatusNotifier): - async def build_payload(self, comparison): + def build_payload(self, comparison): return payload def get_github_app_used(self) -> None: return None - async def status_already_exists( + def status_already_exists( self, comparison: ComparisonProxy, title, state, description - ) -> Coroutine[Any, Any, bool]: + ) -> bool: return False notifier = TestNotifier( @@ -565,12 +559,12 @@ async def status_already_exists( fake_repo_service = MagicMock( name="fake_repo_provider", - set_commit_status=MagicMock(side_effect=set_status_side_effect), + set_commit_status=AsyncMock(side_effect=set_status_side_effect), ) mocker.patch.object( TestNotifier, "repository_service", return_value=fake_repo_service ) - result = await notifier.notify(comparison) + result = notifier.notify(comparison) assert result == NotificationResult( notification_attempted=True, notification_successful=True, @@ -585,8 +579,7 @@ async def status_already_exists( ) assert fake_repo_service.set_commit_status.call_count == 2 - @pytest.mark.asyncio - async def test_notify_cached( + def test_notify_cached( self, sample_comparison, mocker, @@ -600,7 +593,7 @@ async def test_notify_cached( } class TestNotifier(StatusNotifier): - async def build_payload(self, comparison): + def build_payload(self, comparison): return payload notifier = TestNotifier( @@ -618,7 +611,7 @@ async def build_payload(self, comparison): ) send_notification = mocker.patch.object(TestNotifier, "send_notification") - result = await notifier.notify(comparison) + result = notifier.notify(comparison) assert result == NotificationResult( notification_attempted=False, notification_successful=None, @@ -629,10 +622,7 @@ async def build_payload(self, comparison): # payload was cached - we do not send the notification assert not send_notification.called - @pytest.mark.asyncio - async def test_send_notification( - self, sample_comparison, mocker, mock_repo_provider - ): + def test_send_notification(self, sample_comparison, mocker, mock_repo_provider): comparison = sample_comparison no_settings_notifier = StatusNotifier( repository=comparison.head.commit.repository, @@ -650,7 +640,7 @@ async def test_send_notification( 403, "response", "message" ) payload = {"message": "something to say", "state": "success", "url": "url"} - result = await no_settings_notifier.send_notification(comparison, payload) + result = no_settings_notifier.send_notification(comparison, payload) assert result.notification_attempted assert not result.notification_successful assert result.explanation == "no_write_permission" @@ -662,10 +652,7 @@ async def test_send_notification( assert result.data_sent == expected_data_sent assert result.data_received is None - @pytest.mark.asyncio - async def test_notify_analytics( - self, sample_comparison, mocker, mock_repo_provider - ): + def test_notify_analytics(self, sample_comparison, mocker, mock_repo_provider): mocker.patch("helpers.environment.is_enterprise", return_value=False) comparison = sample_comparison no_settings_notifier = StatusNotifier( @@ -684,10 +671,9 @@ async def test_notify_analytics( 403, "response", "message" ) payload = {"message": "something to say", "state": "success", "url": "url"} - await no_settings_notifier.send_notification(comparison, payload) + no_settings_notifier.send_notification(comparison, payload) - @pytest.mark.asyncio - async def test_notify_analytics_enterprise( + def test_notify_analytics_enterprise( self, sample_comparison, mocker, mock_repo_provider ): mocker.patch("helpers.environment.is_enterprise", return_value=True) @@ -708,7 +694,7 @@ async def test_notify_analytics_enterprise( 403, "response", "message" ) payload = {"message": "something to say", "state": "success", "url": "url"} - await no_settings_notifier.send_notification(comparison, payload) + no_settings_notifier.send_notification(comparison, payload) def test_determine_status_check_behavior_to_apply(self, sample_comparison): # uses component level setting if provided @@ -887,8 +873,7 @@ def test_get_github_app_used_no_repository_service( class TestProjectStatusNotifier(object): - @pytest.mark.asyncio - async def test_build_payload( + def test_build_payload( self, sample_comparison, mock_repo_provider, mock_configuration ): mock_configuration.params["setup"]["codecov_dashboard_url"] = "test.example.br" @@ -904,11 +889,10 @@ async def test_build_payload( "message": f"60.00% (+10.00%) compared to {base_commit.commitid[:7]}", "state": "success", } - result = await notifier.build_payload(sample_comparison) + result = notifier.build_payload(sample_comparison) assert expected_result == result - @pytest.mark.asyncio - async def test_build_payload_passing_empty_upload( + def test_build_payload_passing_empty_upload( self, sample_comparison, mock_repo_provider, mock_configuration ): mock_configuration.params["setup"]["codecov_url"] = "test.example.br" @@ -924,11 +908,10 @@ async def test_build_payload_passing_empty_upload( "state": "success", "message": "Non-testable files changed.", } - result = await notifier.build_payload(sample_comparison) + result = notifier.build_payload(sample_comparison) assert expected_result == result - @pytest.mark.asyncio - async def test_build_payload_failing_empty_upload( + def test_build_payload_failing_empty_upload( self, sample_comparison, mock_repo_provider, mock_configuration ): mock_configuration.params["setup"]["codecov_url"] = "test.example.br" @@ -944,11 +927,10 @@ async def test_build_payload_failing_empty_upload( "state": "failure", "message": "Testable files changed", } - result = await notifier.build_payload(sample_comparison) + result = notifier.build_payload(sample_comparison) assert expected_result == result - @pytest.mark.asyncio - async def test_build_upgrade_payload( + def test_build_upgrade_payload( self, sample_comparison, mock_repo_provider, mock_configuration ): mock_configuration.params["setup"]["codecov_dashboard_url"] = "test.example.br" @@ -965,11 +947,10 @@ async def test_build_upgrade_payload( "message": "Please activate this user to display a detailed status check", "state": "success", } - result = await notifier.build_payload(sample_comparison) + result = notifier.build_payload(sample_comparison) assert expected_result == result - @pytest.mark.asyncio - async def test_build_payload_not_auto( + def test_build_payload_not_auto( self, sample_comparison, mock_repo_provider, mock_configuration ): mock_configuration.params["setup"]["codecov_dashboard_url"] = "test.example.br" @@ -984,11 +965,10 @@ async def test_build_payload_not_auto( current_yaml=UserYaml({}), ) expected_result = {"message": "60.00% (target 57.00%)", "state": "success"} - result = await notifier.build_payload(sample_comparison) + result = notifier.build_payload(sample_comparison) assert expected_result == result - @pytest.mark.asyncio - async def test_build_payload_not_auto_not_string( + def test_build_payload_not_auto_not_string( self, sample_comparison, mock_repo_provider, mock_configuration ): mock_configuration.params["setup"]["codecov_dashboard_url"] = "test.example.br" @@ -1000,11 +980,10 @@ async def test_build_payload_not_auto_not_string( current_yaml=UserYaml({}), ) expected_result = {"message": "60.00% (target 57.00%)", "state": "success"} - result = await notifier.build_payload(sample_comparison) + result = notifier.build_payload(sample_comparison) assert expected_result == result - @pytest.mark.asyncio - async def test_build_payload_no_base_report( + def test_build_payload_no_base_report( self, sample_comparison_without_base_report, mock_repo_provider, @@ -1025,11 +1004,10 @@ async def test_build_payload_no_base_report( "message": "No report found to compare against", "state": "success", } - result = await notifier.build_payload(comparison) + result = notifier.build_payload(comparison) assert expected_result == result - @pytest.mark.asyncio - async def test_notify_status_doesnt_exist( + def test_notify_status_doesnt_exist( self, sample_comparison, mock_repo_provider, mock_configuration ): mock_repo_provider.get_commit_statuses.return_value = Status([]) @@ -1054,11 +1032,10 @@ async def test_notify_status_doesnt_exist( }, data_received={"id": "some_id"}, ) - result = await notifier.notify(sample_comparison) + result = notifier.notify(sample_comparison) assert expected_result == result - @pytest.mark.asyncio - async def test_notify_client_side_exception( + def test_notify_client_side_exception( self, sample_comparison, mocker, mock_configuration ): mocker.patch.object( @@ -1088,11 +1065,10 @@ async def test_notify_client_side_exception( }, data_received=None, ) - result = await notifier.notify(sample_comparison) + result = notifier.notify(sample_comparison) assert expected_result == result - @pytest.mark.asyncio - async def test_notify_server_side_exception( + def test_notify_server_side_exception( self, sample_comparison, mocker, mock_configuration ): mocker.patch.object( @@ -1122,12 +1098,11 @@ async def test_notify_server_side_exception( }, data_received=None, ) - result = await notifier.notify(sample_comparison) + result = notifier.notify(sample_comparison) assert expected_result.data_sent == result.data_sent assert expected_result == result - @pytest.mark.asyncio - async def test_notify_pass_behavior_when_coverage_not_uploaded( + def test_notify_pass_behavior_when_coverage_not_uploaded( self, sample_comparison_coverage_carriedforward, mock_repo_provider ): mock_repo_provider.get_commit_statuses.return_value = Status([]) @@ -1156,11 +1131,10 @@ async def test_notify_pass_behavior_when_coverage_not_uploaded( }, data_received={"id": "some_id"}, ) - result = await notifier.notify(sample_comparison_coverage_carriedforward) + result = notifier.notify(sample_comparison_coverage_carriedforward) assert expected_result == result - @pytest.mark.asyncio - async def test_notify_pass_behavior_when_coverage_uploaded( + def test_notify_pass_behavior_when_coverage_uploaded( self, sample_comparison_coverage_carriedforward, mock_repo_provider ): mock_repo_provider.get_commit_statuses.return_value = Status([]) @@ -1189,11 +1163,10 @@ async def test_notify_pass_behavior_when_coverage_uploaded( }, data_received={"id": "some_id"}, ) - result = await notifier.notify(sample_comparison_coverage_carriedforward) + result = notifier.notify(sample_comparison_coverage_carriedforward) assert expected_result == result - @pytest.mark.asyncio - async def test_notify_include_behavior_when_coverage_not_uploaded( + def test_notify_include_behavior_when_coverage_not_uploaded( self, sample_comparison_coverage_carriedforward, mock_repo_provider ): mock_repo_provider.get_commit_statuses.return_value = Status([]) @@ -1222,11 +1195,10 @@ async def test_notify_include_behavior_when_coverage_not_uploaded( }, data_received={"id": "some_id"}, ) - result = await notifier.notify(sample_comparison_coverage_carriedforward) + result = notifier.notify(sample_comparison_coverage_carriedforward) assert expected_result == result - @pytest.mark.asyncio - async def test_notify_exclude_behavior_when_coverage_not_uploaded( + def test_notify_exclude_behavior_when_coverage_not_uploaded( self, sample_comparison_coverage_carriedforward, mock_repo_provider ): mock_repo_provider.get_commit_statuses.return_value = Status([]) @@ -1247,11 +1219,10 @@ async def test_notify_exclude_behavior_when_coverage_not_uploaded( data_sent=None, data_received=None, ) - result = await notifier.notify(sample_comparison_coverage_carriedforward) + result = notifier.notify(sample_comparison_coverage_carriedforward) assert expected_result == result - @pytest.mark.asyncio - async def test_notify_exclude_behavior_when_coverage_uploaded( + def test_notify_exclude_behavior_when_coverage_uploaded( self, sample_comparison_coverage_carriedforward, mock_repo_provider ): mock_repo_provider.get_commit_statuses.return_value = Status([]) @@ -1280,11 +1251,10 @@ async def test_notify_exclude_behavior_when_coverage_uploaded( }, data_received={"id": "some_id"}, ) - result = await notifier.notify(sample_comparison_coverage_carriedforward) + result = notifier.notify(sample_comparison_coverage_carriedforward) assert expected_result == result - @pytest.mark.asyncio - async def test_notify_exclude_behavior_when_some_coverage_uploaded( + def test_notify_exclude_behavior_when_some_coverage_uploaded( self, sample_comparison_coverage_carriedforward, mock_repo_provider ): mock_repo_provider.get_commit_statuses.return_value = Status([]) @@ -1317,11 +1287,10 @@ async def test_notify_exclude_behavior_when_some_coverage_uploaded( }, data_received={"id": "some_id"}, ) - result = await notifier.notify(sample_comparison_coverage_carriedforward) + result = notifier.notify(sample_comparison_coverage_carriedforward) assert expected_result == result - @pytest.mark.asyncio - async def test_notify_exclude_behavior_no_flags( + def test_notify_exclude_behavior_no_flags( self, sample_comparison_coverage_carriedforward, mock_repo_provider ): mock_repo_provider.get_commit_statuses.return_value = Status([]) @@ -1351,11 +1320,10 @@ async def test_notify_exclude_behavior_no_flags( }, data_received={"id": "some_id"}, ) - result = await notifier.notify(sample_comparison_coverage_carriedforward) + result = notifier.notify(sample_comparison_coverage_carriedforward) assert expected_result == result - @pytest.mark.asyncio - async def test_notify_path_filter( + def test_notify_path_filter( self, sample_comparison, mock_repo_provider, mock_configuration, mocker ): mocked_send_notification = mocker.patch.object( @@ -1375,12 +1343,11 @@ async def test_notify_path_filter( "state": "success", "url": f"test.example.br/gh/{sample_comparison.head.commit.repository.slug}/pull/{sample_comparison.pull.pullid}", } - result = await notifier.notify(sample_comparison) + result = notifier.notify(sample_comparison) assert result == mocked_send_notification.return_value mocked_send_notification.assert_called_with(sample_comparison, expected_result) - @pytest.mark.asyncio - async def test_notify_path_and_flags_filter_nothing_on_base( + def test_notify_path_and_flags_filter_nothing_on_base( self, sample_comparison, mock_repo_provider, mock_configuration, mocker ): mocked_send_notification = mocker.patch.object( @@ -1401,12 +1368,11 @@ async def test_notify_path_and_flags_filter_nothing_on_base( "state": "success", "url": f"test.example.br/gh/{sample_comparison.head.commit.repository.slug}/pull/{sample_comparison.pull.pullid}", } - result = await notifier.notify(sample_comparison) + result = notifier.notify(sample_comparison) assert result == mocked_send_notification.return_value mocked_send_notification.assert_called_with(sample_comparison, expected_result) - @pytest.mark.asyncio - async def test_notify_path_and_flags_filter_something_on_base( + def test_notify_path_and_flags_filter_something_on_base( self, sample_comparison_matching_flags, mock_repo_provider, @@ -1431,14 +1397,13 @@ async def test_notify_path_and_flags_filter_something_on_base( "state": "success", "url": f"test.example.br/gh/{sample_comparison_matching_flags.head.commit.repository.slug}/pull/{sample_comparison_matching_flags.pull.pullid}", } - result = await notifier.notify(sample_comparison_matching_flags) + result = notifier.notify(sample_comparison_matching_flags) assert result == mocked_send_notification.return_value mocked_send_notification.assert_called_with( sample_comparison_matching_flags, expected_result ) - @pytest.mark.asyncio - async def test_notify_pass_via_removals_only_behavior( + def test_notify_pass_via_removals_only_behavior( self, mock_configuration, sample_comparison, mocker ): mock_get_impacted_files = mocker.patch.object( @@ -1479,12 +1444,11 @@ async def test_notify_pass_via_removals_only_behavior( "message": "60.00% (target 80.00%), passed because this change only removed code", "state": "success", } - result = await notifier.build_payload(sample_comparison) + result = notifier.build_payload(sample_comparison) assert result == expected_result mock_get_impacted_files.assert_called() - @pytest.mark.asyncio - async def test_notify_pass_adjust_base_behavior( + def test_notify_pass_adjust_base_behavior( slef, mock_configuration, sample_comparison_negative_change, mocker ): sample_comparison = sample_comparison_negative_change @@ -1523,12 +1487,11 @@ async def test_notify_pass_adjust_base_behavior( "message": f"50.00% (-10.00%) compared to {sample_comparison.project_coverage_base.commit.commitid[:7]}, passed because coverage increased by +0.00% when compared to adjusted base (50.00%)", "state": "success", } - result = await notifier.build_payload(sample_comparison) + result = notifier.build_payload(sample_comparison) assert result == expected_result mock_get_impacted_files.assert_called() - @pytest.mark.asyncio - async def test_notify_removed_code_behavior_fail( + def test_notify_removed_code_behavior_fail( self, mock_configuration, sample_comparison, mocker ): mock_get_impacted_files = mocker.patch.object( @@ -1569,12 +1532,11 @@ async def test_notify_removed_code_behavior_fail( "message": "60.00% (target 80.00%)", "state": "failure", } - result = await notifier.build_payload(sample_comparison) + result = notifier.build_payload(sample_comparison) assert result == expected_result mock_get_impacted_files.assert_called() - @pytest.mark.asyncio - async def test_notify_adjust_base_behavior_fail( + def test_notify_adjust_base_behavior_fail( slef, mock_configuration, sample_comparison_negative_change, mocker ): sample_comparison = sample_comparison_negative_change @@ -1613,12 +1575,11 @@ async def test_notify_adjust_base_behavior_fail( "message": f"50.00% (-10.00%) compared to {sample_comparison.project_coverage_base.commit.commitid[:7]}", "state": "failure", } - result = await notifier.build_payload(sample_comparison) + result = notifier.build_payload(sample_comparison) assert result == expected_result mock_get_impacted_files.assert_called() - @pytest.mark.asyncio - async def test_notify_adjust_base_behavior_skips_if_target_coverage_defined( + def test_notify_adjust_base_behavior_skips_if_target_coverage_defined( slef, mock_configuration, sample_comparison_negative_change, mocker ): sample_comparison = sample_comparison_negative_change @@ -1640,12 +1601,11 @@ async def test_notify_adjust_base_behavior_skips_if_target_coverage_defined( "message": "50.00% (target 80.00%)", "state": "failure", } - result = await notifier.build_payload(sample_comparison) + result = notifier.build_payload(sample_comparison) assert result == expected_result mock_get_impacted_files.assert_not_called() - @pytest.mark.asyncio - async def test_notify_removed_code_behavior_unknown( + def test_notify_removed_code_behavior_unknown( self, mock_configuration, sample_comparison ): mock_configuration.params["setup"]["codecov_dashboard_url"] = "test.example.br" @@ -1663,11 +1623,10 @@ async def test_notify_removed_code_behavior_unknown( "message": "60.00% (target 80.00%)", "state": "failure", } - result = await notifier.build_payload(sample_comparison) + result = notifier.build_payload(sample_comparison) assert result == expected_result - @pytest.mark.asyncio - async def test_notify_fully_covered_patch_behavior_fail( + def test_notify_fully_covered_patch_behavior_fail( self, comparison_with_multiple_changes, mock_repo_provider, @@ -1715,12 +1674,11 @@ async def test_notify_fully_covered_patch_behavior_fail( "message": "50.00% (target 70.00%)", "state": "failure", } - result = await notifier.build_payload(comparison_with_multiple_changes) + result = notifier.build_payload(comparison_with_multiple_changes) assert result == expected_result mock_get_impacted_files.assert_called() - @pytest.mark.asyncio - async def test_notify_fully_covered_patch_behavior_success( + def test_notify_fully_covered_patch_behavior_success( self, comparison_100_percent_patch, mock_repo_provider, @@ -1768,12 +1726,11 @@ async def test_notify_fully_covered_patch_behavior_success( "message": "28.57% (target 70.00%), passed because patch was fully covered by tests, and no indirect coverage changes", "state": "success", } - result = await notifier.build_payload(comparison_100_percent_patch) + result = notifier.build_payload(comparison_100_percent_patch) assert result == expected_result mock_get_impacted_files.assert_called() - @pytest.mark.asyncio - async def test_notify_fully_covered_patch_behavior_no_coverage_change( + def test_notify_fully_covered_patch_behavior_no_coverage_change( self, mock_configuration, sample_comparison, mocker ): mock_configuration.params["setup"]["codecov_dashboard_url"] = "test.example.br" @@ -1929,14 +1886,13 @@ async def test_notify_fully_covered_patch_behavior_no_coverage_change( "message": "60.00% (target 70.00%), passed because coverage was not affected by patch", "state": "success", } - result = await notifier.build_payload(sample_comparison) + result = notifier.build_payload(sample_comparison) assert result == expected_result mock_get_impacted_files.assert_called() class TestPatchStatusNotifier(object): - @pytest.mark.asyncio - async def test_build_payload( + def test_build_payload( self, sample_comparison, mock_repo_provider, mock_configuration ): mock_configuration.params["setup"]["codecov_dashboard_url"] = "test.example.br" @@ -1951,11 +1907,10 @@ async def test_build_payload( "message": "66.67% of diff hit (target 50.00%)", "state": "success", } - result = await notifier.build_payload(sample_comparison) + result = notifier.build_payload(sample_comparison) assert expected_result == result - @pytest.mark.asyncio - async def test_build_upgrade_payload( + def test_build_upgrade_payload( self, sample_comparison, mock_repo_provider, mock_configuration ): mock_configuration.params["setup"]["codecov_dashboard_url"] = "test.example.br" @@ -1971,11 +1926,10 @@ async def test_build_upgrade_payload( "message": "Please activate this user to display a detailed status check", "state": "success", } - result = await notifier.build_payload(sample_comparison) + result = notifier.build_payload(sample_comparison) assert expected_result == result - @pytest.mark.asyncio - async def test_build_payload_target_coverage_failure( + def test_build_payload_target_coverage_failure( self, sample_comparison, mock_repo_provider, mock_configuration ): mock_configuration.params["setup"]["codecov_dashboard_url"] = "test.example.br" @@ -1990,11 +1944,10 @@ async def test_build_payload_target_coverage_failure( "message": "66.67% of diff hit (target 70.00%)", "state": "failure", } - result = await notifier.build_payload(sample_comparison) + result = notifier.build_payload(sample_comparison) assert expected_result == result - @pytest.mark.asyncio - async def test_build_payload_not_auto_not_string( + def test_build_payload_not_auto_not_string( self, sample_comparison, mock_repo_provider, mock_configuration ): mock_configuration.params["setup"]["codecov_dashboard_url"] = "test.example.br" @@ -2009,11 +1962,10 @@ async def test_build_payload_not_auto_not_string( "message": "66.67% of diff hit (target 57.00%)", "state": "success", } - result = await notifier.build_payload(sample_comparison) + result = notifier.build_payload(sample_comparison) assert expected_result == result - @pytest.mark.asyncio - async def test_build_payload_target_coverage_failure_within_threshold( + def test_build_payload_target_coverage_failure_within_threshold( self, sample_comparison, mock_repo_provider, mock_configuration ): mock_configuration.params["setup"]["codecov_dashboard_url"] = "test.example.br" @@ -2034,11 +1986,10 @@ async def test_build_payload_target_coverage_failure_within_threshold( "message": "66.67% of diff hit (within 5.00% threshold of 70.00%)", "state": "success", } - result = await notifier.build_payload(sample_comparison) + result = notifier.build_payload(sample_comparison) assert expected_result == result - @pytest.mark.asyncio - async def test_get_patch_status_bad_threshold( + def test_get_patch_status_bad_threshold( self, sample_comparison, mock_repo_provider, mock_configuration ): mock_configuration.params["setup"]["codecov_dashboard_url"] = "test.example.br" @@ -2059,11 +2010,10 @@ async def test_get_patch_status_bad_threshold( "message": "66.67% of diff hit (target 70.00%)", "state": "failure", } - result = await notifier.build_payload(sample_comparison) + result = notifier.build_payload(sample_comparison) assert expected_result == result - @pytest.mark.asyncio - async def test_get_patch_status_bad_threshold_fixed( + def test_get_patch_status_bad_threshold_fixed( self, sample_comparison, mock_repo_provider, mock_configuration ): mock_configuration.params["setup"]["codecov_dashboard_url"] = "test.example.br" @@ -2086,11 +2036,10 @@ async def test_get_patch_status_bad_threshold_fixed( "message": "66.67% of diff hit (within 5.00% threshold of 70.00%)", "state": "success", } - result = await notifier.build_payload(sample_comparison) + result = notifier.build_payload(sample_comparison) assert expected_result == result - @pytest.mark.asyncio - async def test_build_payload_no_diff( + def test_build_payload_no_diff( self, sample_comparison, mock_repo_provider, mock_configuration ): mock_repo_provider.get_compare.return_value = { @@ -2136,11 +2085,10 @@ async def test_build_payload_no_diff( "message": f"Coverage not affected when comparing {base_commit.commitid[:7]}...{head_commit.commitid[:7]}", "state": "success", } - result = await notifier.build_payload(sample_comparison) + result = notifier.build_payload(sample_comparison) assert expected_result == result - @pytest.mark.asyncio - async def test_build_payload_no_diff_no_base_report( + def test_build_payload_no_diff_no_base_report( self, sample_comparison_without_base_with_pull, mock_repo_provider, @@ -2185,11 +2133,10 @@ async def test_build_payload_no_diff_no_base_report( current_yaml=UserYaml({}), ) expected_result = {"message": "Coverage not affected", "state": "success"} - result = await notifier.build_payload(comparison) + result = notifier.build_payload(comparison) assert expected_result == result - @pytest.mark.asyncio - async def test_build_payload_without_base_report( + def test_build_payload_without_base_report( self, sample_comparison_without_base_report, mock_repo_provider, @@ -2208,11 +2155,10 @@ async def test_build_payload_without_base_report( "message": "No report found to compare against", "state": "success", } - result = await notifier.build_payload(comparison) + result = notifier.build_payload(comparison) assert expected_result == result - @pytest.mark.asyncio - async def test_build_payload_with_multiple_changes( + def test_build_payload_with_multiple_changes( self, comparison_with_multiple_changes, mock_repo_provider, @@ -2234,13 +2180,12 @@ async def test_build_payload_with_multiple_changes( "message": "50.00% of diff hit (target 76.92%)", "state": "failure", } - result = await notifier.build_payload(comparison_with_multiple_changes) + result = notifier.build_payload(comparison_with_multiple_changes) assert expected_result == result class TestChangesStatusNotifier(object): - @pytest.mark.asyncio - async def test_build_payload( + def test_build_payload( self, sample_comparison, mock_repo_provider, mock_configuration ): mock_configuration.params["setup"]["codecov_dashboard_url"] = "test.example.br" @@ -2255,11 +2200,10 @@ async def test_build_payload( "message": "No indirect coverage changes found", "state": "success", } - result = await notifier.build_payload(sample_comparison) + result = notifier.build_payload(sample_comparison) assert expected_result == result - @pytest.mark.asyncio - async def test_build_upgrade_payload( + def test_build_upgrade_payload( self, sample_comparison, mock_repo_provider, mock_configuration ): mock_configuration.params["setup"]["codecov_dashboard_url"] = "test.example.br" @@ -2275,11 +2219,10 @@ async def test_build_upgrade_payload( "message": "Please activate this user to display a detailed status check", "state": "success", } - result = await notifier.build_payload(sample_comparison) + result = notifier.build_payload(sample_comparison) assert expected_result == result - @pytest.mark.asyncio - async def test_build_payload_with_multiple_changes( + def test_build_payload_with_multiple_changes( self, comparison_with_multiple_changes, mock_repo_provider, @@ -2301,11 +2244,10 @@ async def test_build_payload_with_multiple_changes( "message": "3 files have indirect coverage changes not visible in diff", "state": "failure", } - result = await notifier.build_payload(comparison_with_multiple_changes) + result = notifier.build_payload(comparison_with_multiple_changes) assert expected_result == result - @pytest.mark.asyncio - async def test_build_payload_without_base_report( + def test_build_payload_without_base_report( self, sample_comparison_without_base_report, mock_repo_provider, @@ -2324,11 +2266,10 @@ async def test_build_payload_without_base_report( "message": "Unable to determine changes, no report found at pull request base", "state": "success", } - result = await notifier.build_payload(comparison) + result = notifier.build_payload(comparison) assert expected_result == result - @pytest.mark.asyncio - async def test_notify_path_filter( + def test_notify_path_filter( self, sample_comparison, mock_repo_provider, mock_configuration, mocker ): mocked_send_notification = mocker.patch.object( @@ -2348,12 +2289,11 @@ async def test_notify_path_filter( "state": "success", "url": f"test.example.br/gh/{sample_comparison.head.commit.repository.slug}/pull/{sample_comparison.pull.pullid}", } - result = await notifier.notify(sample_comparison) + result = notifier.notify(sample_comparison) assert result == mocked_send_notification.return_value mocked_send_notification.assert_called_with(sample_comparison, expected_result) - @pytest.mark.asyncio - async def test_build_passing_empty_upload_payload( + def test_build_passing_empty_upload_payload( self, sample_comparison, mock_repo_provider, mock_configuration ): mock_configuration.params["setup"]["codecov_url"] = "test.example.br" @@ -2366,5 +2306,5 @@ async def test_build_passing_empty_upload_payload( decoration_type=Decoration.passing_empty_upload, ) expected_result = {"state": "success", "message": "Non-testable files changed."} - result = await notifier.build_payload(sample_comparison) + result = notifier.build_payload(sample_comparison) assert expected_result == result diff --git a/services/notification/tests/unit/test_comparison.py b/services/notification/tests/unit/test_comparison.py index 1c5c3b915..bec5650ec 100644 --- a/services/notification/tests/unit/test_comparison.py +++ b/services/notification/tests/unit/test_comparison.py @@ -1,12 +1,10 @@ -import pytest from shared.reports.types import Change from services.comparison import ComparisonProxy, FilteredComparison class TestFilteredComparison(object): - @pytest.mark.asyncio - async def test_get_existing_statuses(self, mocker): + def test_get_existing_statuses(self, mocker): mocked_get_existing_statuses = mocker.patch.object( ComparisonProxy, "get_existing_statuses" ) @@ -14,11 +12,10 @@ async def test_get_existing_statuses(self, mocker): comparison = ComparisonProxy(mocker.MagicMock()) filtered_comparison = comparison.get_filtered_comparison(flags, path_patterns) assert isinstance(filtered_comparison, FilteredComparison) - res = await filtered_comparison.get_existing_statuses() + res = filtered_comparison.get_existing_statuses() assert res == mocked_get_existing_statuses.return_value - @pytest.mark.asyncio - async def test_get_changes_rust_vs_python(self, mocker): + def test_get_changes_rust_vs_python(self, mocker): mocker.patch.object(ComparisonProxy, "get_diff") mocker.patch( "services.comparison.get_changes", @@ -29,6 +26,6 @@ async def test_get_changes_rust_vs_python(self, mocker): return_value=[Change(path="banana"), Change(path="pear")], ) comparison = ComparisonProxy(mocker.MagicMock()) - res = await comparison.get_changes() + res = comparison.get_changes() expected_result = [Change(path="apple"), Change(path="pear")] assert expected_result == res diff --git a/services/notification/tests/unit/test_notification_service.py b/services/notification/tests/unit/test_notification_service.py index c49dedcba..409f66608 100644 --- a/services/notification/tests/unit/test_notification_service.py +++ b/services/notification/tests/unit/test_notification_service.py @@ -382,8 +382,7 @@ def test_get_notifiers_instances_comment( assert len(instances) == 1 assert instances[0].gh_installation_name == gh_installation_name - @pytest.mark.asyncio - async def test_notify_general_exception(self, mocker, dbsession, sample_comparison): + def test_notify_general_exception(self, mocker, dbsession, sample_comparison): current_yaml = {} commit = sample_comparison.head.commit good_notifier = mocker.MagicMock( @@ -391,7 +390,7 @@ async def test_notify_general_exception(self, mocker, dbsession, sample_comparis title="good_notifier", notification_type=Notification.comment, decoration_type=Decoration.standard, - notify=mock.AsyncMock(), + notify=mock.Mock(), ) bad_notifier = mocker.MagicMock( is_enabled=mocker.MagicMock(return_value=True), @@ -404,7 +403,7 @@ async def test_notify_general_exception(self, mocker, dbsession, sample_comparis title="disabled_notifier", notification_type=Notification.status_patch, decoration_type=Decoration.standard, - notify=mock.AsyncMock(), + notify=mock.Mock(), ) good_notifier.notify.return_value = NotificationResult( notification_attempted=True, @@ -436,22 +435,21 @@ async def test_notify_general_exception(self, mocker, dbsession, sample_comparis ), }, ] - res = await notifications_service.notify(sample_comparison) + res = notifications_service.notify(sample_comparison) assert expected_result == res - @pytest.mark.asyncio - async def test_notify_individual_notifier_timeout(self, mocker, sample_comparison): + def test_notify_individual_notifier_timeout(self, mocker, sample_comparison): current_yaml = {} commit = sample_comparison.head.commit notifier = mocker.MagicMock( title="fake_notifier", - notify=mock.AsyncMock(), + notify=mock.Mock(), notification_type=Notification.comment, decoration_type=Decoration.standard, ) notifier.notify.side_effect = AsyncioTimeoutError notifications_service = NotificationService(commit.repository, current_yaml) - res = await notifications_service.notify_individual_notifier( + res = notifications_service.notify_individual_notifier( notifier, sample_comparison ) assert res == { @@ -460,8 +458,7 @@ async def test_notify_individual_notifier_timeout(self, mocker, sample_compariso "title": "fake_notifier", } - @pytest.mark.asyncio - async def test_notify_individual_checks_notifier( + def test_notify_individual_checks_notifier( self, mocker, sample_comparison, mock_repo_provider, mock_configuration ): mock_configuration._params["setup"] = {"codecov_dashboard_url": "test"} @@ -485,10 +482,11 @@ async def test_notify_individual_checks_notifier( current_yaml=UserYaml({}), ) notifications_service = NotificationService(commit.repository, current_yaml) - res = await notifications_service.notify_individual_notifier( + res = notifications_service.notify_individual_notifier( notifier, sample_comparison ) - expected_result = { + + assert res == { "notifier": "checks-project", "title": "title", "result": NotificationResult( @@ -506,25 +504,21 @@ async def test_notify_individual_checks_notifier( data_received=None, ), } - assert res["result"].data_sent == expected_result["result"].data_sent - assert res["result"] == expected_result["result"] - assert res == expected_result - @pytest.mark.asyncio - async def test_notify_individual_notifier_timeout_notification_created( + def test_notify_individual_notifier_timeout_notification_created( self, mocker, dbsession, sample_comparison ): current_yaml = {} commit = sample_comparison.head.commit notifier = mocker.MagicMock( title="fake_notifier", - notify=mock.AsyncMock(), + notify=mock.Mock(), notification_type=Notification.comment, decoration_type=Decoration.standard, ) notifier.notify.side_effect = AsyncioTimeoutError notifications_service = NotificationService(commit.repository, current_yaml) - res = await notifications_service.notify_individual_notifier( + res = notifications_service.notify_individual_notifier( notifier, sample_comparison ) assert res == { @@ -543,22 +537,21 @@ async def test_notify_individual_notifier_timeout_notification_created( assert pull_commit_notification.decoration_type == notifier.decoration_type assert pull_commit_notification.state == NotificationState.error - @pytest.mark.asyncio - async def test_notify_individual_notifier_notification_created_then_updated( + def test_notify_individual_notifier_notification_created_then_updated( self, mocker, dbsession, sample_comparison ): current_yaml = {} commit = sample_comparison.head.commit notifier = mocker.MagicMock( title="fake_notifier", - notify=mock.AsyncMock(), + notify=mock.Mock(), notification_type=Notification.comment, decoration_type=Decoration.standard, ) # first attempt not successful notifier.notify.side_effect = AsyncioTimeoutError notifications_service = NotificationService(commit.repository, current_yaml) - res = await notifications_service.notify_individual_notifier( + res = notifications_service.notify_individual_notifier( notifier, sample_comparison ) assert res == { @@ -585,52 +578,48 @@ async def test_notify_individual_notifier_notification_created_then_updated( data_sent={"some": "data"}, ) ] - res = await notifications_service.notify_individual_notifier( + res = notifications_service.notify_individual_notifier( notifier, sample_comparison ) dbsession.commit() assert pull_commit_notification.state == NotificationState.success - @pytest.mark.asyncio - async def test_notify_individual_notifier_cancellation( - self, mocker, sample_comparison - ): + def test_notify_individual_notifier_cancellation(self, mocker, sample_comparison): current_yaml = {} commit = sample_comparison.head.commit notifier = mocker.MagicMock( title="fake_notifier", - notify=mock.AsyncMock(), + notify=mock.Mock(), notification_type=Notification.comment, decoration_type=Decoration.standard, ) notifier.notify.side_effect = CancelledError() notifications_service = NotificationService(commit.repository, current_yaml) with pytest.raises(CancelledError): - await notifications_service.notify_individual_notifier( + notifications_service.notify_individual_notifier( notifier, sample_comparison ) - @pytest.mark.asyncio - async def test_notify_timeout_exception(self, mocker, dbsession, sample_comparison): + def test_notify_timeout_exception(self, mocker, dbsession, sample_comparison): current_yaml = {} commit = sample_comparison.head.commit good_notifier = mocker.MagicMock( is_enabled=mocker.MagicMock(return_value=True), - notify=mock.AsyncMock(), + notify=mock.Mock(), title="good_notifier", notification_type=Notification.comment, decoration_type=Decoration.standard, ) no_attempt_notifier = mocker.MagicMock( is_enabled=mocker.MagicMock(return_value=True), - notify=mock.AsyncMock(), + notify=mock.Mock(), title="no_attempt_notifier", notification_type=Notification.status_project, decoration_type=Decoration.standard, ) bad_notifier = mocker.MagicMock( is_enabled=mocker.MagicMock(return_value=True), - notify=mock.AsyncMock(), + notify=mock.Mock(), title="bad_notifier", notification_type=Notification.status_patch, decoration_type=Decoration.standard, @@ -669,11 +658,11 @@ async def test_notify_timeout_exception(self, mocker, dbsession, sample_comparis ) notifications_service = NotificationService(commit.repository, current_yaml) with pytest.raises(SoftTimeLimitExceeded): - await notifications_service.notify(sample_comparison) + notifications_service.notify(sample_comparison) dbsession.flush() pull_commit_notifications = sample_comparison.enriched_pull.database_pull.get_head_commit_notifications() - assert len(pull_commit_notifications) == 2 + assert len(pull_commit_notifications) == 1 for commit_notification in pull_commit_notifications: assert commit_notification.state in ( NotificationState.success, @@ -685,8 +674,7 @@ async def test_notify_timeout_exception(self, mocker, dbsession, sample_comparis Notification.status_patch, ) - @pytest.mark.asyncio - async def test_not_licensed_enterprise(self, mocker, dbsession, sample_comparison): + def test_not_licensed_enterprise(self, mocker, dbsession, sample_comparison): mocker.patch("services.notification.is_properly_licensed", return_value=False) mock_notify_individual_notifier = mocker.patch.object( NotificationService, "notify_individual_notifier" @@ -695,12 +683,11 @@ async def test_not_licensed_enterprise(self, mocker, dbsession, sample_compariso commit = sample_comparison.head.commit notifications_service = NotificationService(commit.repository, current_yaml) expected_result = [] - res = await notifications_service.notify(sample_comparison) + res = notifications_service.notify(sample_comparison) assert expected_result == res assert not mock_notify_individual_notifier.called - @pytest.mark.asyncio - async def test_get_statuses(self, mocker, dbsession, sample_comparison): + def test_get_statuses(self, mocker, dbsession, sample_comparison): current_yaml = { "coverage": {"status": {"project": True, "patch": True, "changes": True}}, "flags": {"banana": {"carryforward": False}}, @@ -732,8 +719,7 @@ async def test_get_statuses(self, mocker, dbsession, sample_comparison): res = list(notifications_service.get_statuses(["unit", "banana", "strawberry"])) assert expected_result == res - @pytest.mark.asyncio - async def test_get_component_statuses(self, mocker, dbsession, sample_comparison): + def test_get_component_statuses(self, mocker, dbsession, sample_comparison): current_yaml = { "component_management": { "default_rules": { diff --git a/services/repository.py b/services/repository.py index 9556a988f..551677d23 100644 --- a/services/repository.py +++ b/services/repository.py @@ -7,9 +7,7 @@ import sentry_sdk import shared.torngit as torngit from asgiref.sync import async_to_sync -from shared.bots import ( - get_adapter_auth_information, -) +from shared.bots import get_adapter_auth_information from shared.bots.github_apps import ( get_github_app_token, get_specific_github_app_details, @@ -22,11 +20,7 @@ TorngitError, TorngitObjectNotFoundError, ) -from shared.typings.torngit import ( - OwnerInfo, - RepoInfo, - TorngitInstanceData, -) +from shared.typings.torngit import OwnerInfo, RepoInfo, TorngitInstanceData from shared.validation.exceptions import InvalidYamlException from shared.yaml import UserYaml from shared.yaml.user_yaml import OwnerContext @@ -36,9 +30,7 @@ from database.enums import CommitErrorTypes from database.models import Commit, Owner, Pull, Repository -from database.models.core import ( - GITHUB_APP_INSTALLATION_DEFAULT_NAME, -) +from database.models.core import GITHUB_APP_INSTALLATION_DEFAULT_NAME from helpers.save_commit_error import save_commit_error from helpers.token_refresh import get_token_refresh_callback from services.github import get_github_app_for_commit @@ -468,7 +460,7 @@ class EnrichedPull(object): @sentry_sdk.trace async def fetch_and_update_pull_request_information_from_commit( - repository_service, commit, current_yaml + repository_service: TorngitBaseAdapter, commit, current_yaml ) -> Optional[EnrichedPull]: db_session = commit.get_db_session() pullid = commit.pullid diff --git a/services/test_results.py b/services/test_results.py index d64bcf2cd..6ea81a69f 100644 --- a/services/test_results.py +++ b/services/test_results.py @@ -292,10 +292,10 @@ def build_message(self) -> str: message.append(generate_view_test_analytics_line(self.commit)) return "\n".join(message) - async def error_comment(self): + def error_comment(self): self._repo_service = get_repo_provider_service(self.commit.repository) - pull = await self.get_pull() + pull = self.get_pull() if pull is None: log.info( "Not notifying since there is no pull request associated with this commit", @@ -307,17 +307,17 @@ async def error_comment(self): message = ":x: We are unable to process any of the uploaded JUnit XML files. Please ensure your files are in the right format." - sent_to_provider = await self.send_to_provider(pull, message) + sent_to_provider = self.send_to_provider(pull, message) if sent_to_provider == False: return (False, "torngit_error") return (True, "comment_posted") - async def upgrade_comment(self): + def upgrade_comment(self): if self._repo_service is None: self._repo_service = get_repo_provider_service(self.commit.repository) - pull = await self.get_pull() + pull = self.get_pull() if pull is None: log.info( "Not notifying since there is no pull request associated with this commit", @@ -355,7 +355,7 @@ async def upgrade_comment(self): ] ) - sent_to_provider = await self.send_to_provider(pull, message) + sent_to_provider = self.send_to_provider(pull, message) if sent_to_provider == False: return (False, "torngit_error") diff --git a/services/tests/test_test_results.py b/services/tests/test_test_results.py index d0a0ace53..6ae9be28a 100644 --- a/services/tests/test_test_results.py +++ b/services/tests/test_test_results.py @@ -1,5 +1,4 @@ import mock -import pytest from shared.torngit.exceptions import TorngitClientError from database.tests.factories import CommitFactory @@ -16,16 +15,23 @@ from services.urls import services_short_dict -@pytest.mark.asyncio -async def test_send_to_provider(): +def mock_repo_service(): + repo_service = mock.Mock( + post_comment=mock.AsyncMock(), + edit_comment=mock.AsyncMock(), + ) + return repo_service + + +def test_send_to_provider(): tn = TestResultsNotifier(CommitFactory(), None) - tn._pull = mock.AsyncMock() + tn._pull = mock.Mock() tn._pull.database_pull.commentid = None - tn._repo_service = mock.AsyncMock() + tn._repo_service = mock_repo_service() m = dict(id=1) tn._repo_service.post_comment.return_value = m - res = await tn.send_to_provider(tn._pull, "hello world") + res = tn.send_to_provider(tn._pull, "hello world") assert res == True @@ -35,16 +41,15 @@ async def test_send_to_provider(): assert tn._pull.database_pull.commentid == 1 -@pytest.mark.asyncio -async def test_send_to_provider_edit(): +def test_send_to_provider_edit(): tn = TestResultsNotifier(CommitFactory(), None) - tn._pull = mock.AsyncMock() + tn._pull = mock.Mock() tn._pull.database_pull.commentid = 1 - tn._repo_service = mock.AsyncMock() + tn._repo_service = mock_repo_service() m = dict(id=1) tn._repo_service.edit_comment.return_value = m - res = await tn.send_to_provider(tn._pull, "hello world") + res = tn.send_to_provider(tn._pull, "hello world") assert res == True tn._repo_service.edit_comment.assert_called_with( @@ -52,22 +57,19 @@ async def test_send_to_provider_edit(): ) -@pytest.mark.asyncio -async def test_send_to_provider_fail(): +def test_send_to_provider_fail(): tn = TestResultsNotifier(CommitFactory(), None) - tn._pull = mock.AsyncMock() + tn._pull = mock.Mock() tn._pull.database_pull.commentid = 1 - tn._repo_service = mock.AsyncMock() - m = dict(id=1) + tn._repo_service = mock_repo_service() tn._repo_service.edit_comment.side_effect = TorngitClientError - res = await tn.send_to_provider(tn._pull, "hello world") + res = tn.send_to_provider(tn._pull, "hello world") assert res == False def test_generate_failure_info(): - tn = TestResultsNotifier(CommitFactory(), None) flags_hash = generate_flags_hash([]) test_id = generate_test_id(1, "testsuite", "testname", flags_hash) fail = TestResultsNotificationFailure( @@ -191,60 +193,49 @@ def test_build_message_with_flake(): ) -@pytest.mark.asyncio -async def test_notify(mocker): - mocker.patch( - "helpers.notifier.get_repo_provider_service", return_value=mock.AsyncMock() - ) +def test_notify(mocker): + mocker.patch("helpers.notifier.get_repo_provider_service", return_value=mock.Mock()) mocker.patch( "helpers.notifier.fetch_and_update_pull_request_information_from_commit", - return_value=mock.AsyncMock(), + return_value=mock.Mock(), ) tn = TestResultsNotifier(CommitFactory(), None) - tn.build_message = mock.Mock() - tn.send_to_provider = mock.AsyncMock() + tn.send_to_provider = mock.Mock() - notification_result = await tn.notify() + notification_result = tn.notify() assert notification_result == NotifierResult.COMMENT_POSTED -@pytest.mark.asyncio -async def test_notify_fail_torngit_error( +def test_notify_fail_torngit_error( mocker, ): - mocker.patch( - "helpers.notifier.get_repo_provider_service", return_value=mock.AsyncMock() - ) + mocker.patch("helpers.notifier.get_repo_provider_service", return_value=mock.Mock()) mocker.patch( "helpers.notifier.fetch_and_update_pull_request_information_from_commit", - return_value=mock.AsyncMock(), + return_value=mock.Mock(), ) tn = TestResultsNotifier(CommitFactory(), None) tn.build_message = mock.Mock() - tn.send_to_provider = mock.AsyncMock(return_value=False) + tn.send_to_provider = mock.Mock(return_value=False) - notification_result = await tn.notify() + notification_result = tn.notify() assert notification_result == NotifierResult.TORNGIT_ERROR -@pytest.mark.asyncio -async def test_notify_fail_no_pull( +def test_notify_fail_no_pull( mocker, ): - mocker.patch( - "helpers.notifier.get_repo_provider_service", return_value=mock.AsyncMock() - ) + mocker.patch("helpers.notifier.get_repo_provider_service", return_value=mock.Mock()) mocker.patch( "helpers.notifier.fetch_and_update_pull_request_information_from_commit", return_value=None, ) tn = TestResultsNotifier(CommitFactory(), None) - tn.build_message = mock.Mock() - tn.send_to_provider = mock.AsyncMock(return_value=False) + tn.send_to_provider = mock.Mock(return_value=False) - notification_result = await tn.notify() + notification_result = tn.notify() assert notification_result == NotifierResult.NO_PULL diff --git a/tasks/compute_comparison.py b/tasks/compute_comparison.py index e8ae99f96..461e3642b 100644 --- a/tasks/compute_comparison.py +++ b/tasks/compute_comparison.py @@ -67,7 +67,7 @@ def run_impl( # At this point we can calculate the patch coverage # Because we have a HEAD report and a base commit to get the diff from if comparison.patch_totals is None: - patch_totals = async_to_sync(comparison_proxy.get_patch_totals)() + patch_totals = comparison_proxy.get_patch_totals() comparison.patch_totals = minimal_totals(patch_totals) if not comparison_proxy.has_project_coverage_base_report(): @@ -82,7 +82,7 @@ def run_impl( comparison.error = None try: - impacted_files = self.serialize_impacted_files(comparison_proxy) + impacted_files = comparison_proxy.get_impacted_files() except TorngitRateLimitError: log.warning( "Unable to compute comparison due to rate limit error", @@ -106,6 +106,7 @@ def run_impl( db_session.commit() self.compute_component_comparisons(db_session, comparison, comparison_proxy) db_session.commit() + return {"successful": True} def compute_flag_comparison(self, db_session, comparison, comparison_proxy): @@ -199,7 +200,7 @@ def get_flag_comparison_totals( totals = dict( head_totals=head_totals, base_totals=base_totals, patch_totals=None ) - diff = async_to_sync(comparison_proxy.get_diff)() + diff = comparison_proxy.get_diff() if diff: patch_totals = flag_head_report.apply_diff(diff) if patch_totals: @@ -277,7 +278,7 @@ def compute_component_comparison( filtered.project_coverage_base.report.totals.asdict() ) component_comparison.head_totals = filtered.head.report.totals.asdict() - diff = async_to_sync(comparison_proxy.get_diff)() + diff = comparison_proxy.get_diff() if diff: patch_totals = filtered.head.report.apply_diff(diff) if patch_totals: @@ -318,10 +319,6 @@ def get_comparison_proxy( ), ) - @sentry_sdk.trace - def serialize_impacted_files(self, comparison_proxy): - return async_to_sync(comparison_proxy.get_impacted_files)() - @sentry_sdk.trace def store_results(self, comparison, impacted_files): repository = comparison.compare_commit.repository diff --git a/tasks/notify.py b/tasks/notify.py index 097f0a746..8c9b64ca5 100644 --- a/tasks/notify.py +++ b/tasks/notify.py @@ -1,4 +1,3 @@ -import asyncio import logging from typing import Optional @@ -381,9 +380,9 @@ def run_impl_within_lock( } if commit.repository.service == "gitlab": - gitlab_extra_shas_to_notify = async_to_sync( - self.get_gitlab_extra_shas_to_notify - )(commit, repository_service) + gitlab_extra_shas_to_notify = self.get_gitlab_extra_shas_to_notify( + commit, repository_service + ) else: gitlab_extra_shas_to_notify = None @@ -542,7 +541,7 @@ def save_patch_totals(self, comparison: ComparisonProxy) -> None: if db_session is None: log.warning("Failed to save patch_totals. dbsession is None") return - patch_coverage = async_to_sync(comparison.get_patch_totals)() + patch_coverage = comparison.get_patch_totals() if ( comparison.project_coverage_base is not None and comparison.project_coverage_base.commit is not None @@ -583,7 +582,7 @@ def save_patch_totals(self, comparison: ComparisonProxy) -> None: compare_commit.patch_totals = minimal_totals(patch_coverage) @sentry_sdk.trace - async def get_gitlab_extra_shas_to_notify( + def get_gitlab_extra_shas_to_notify( self, commit: Commit, repository_service: TorngitBaseAdapter ) -> set[str]: """ " @@ -605,14 +604,11 @@ async def get_gitlab_extra_shas_to_notify( ) return set() project_id = commit.repository.service_id - job_ids = [ + job_ids = ( upload.job_code for upload in report.uploads if upload.job_code is not None - ] - tasks = [ - repository_service.get_pipeline_details(project_id, job_id) - for job_id in job_ids - ] - results = await asyncio.gather(*tasks) + ) + _get_pipeline_details = async_to_sync(repository_service.get_pipeline_details) + results = [_get_pipeline_details(project_id, job_id) for job_id in job_ids] return set( filter(lambda sha: sha is not None and sha != commit.commitid, results) ) @@ -683,7 +679,7 @@ def submit_third_party_notifications( decoration_type, gh_installation_name_to_use=installation_name_to_use, ) - return async_to_sync(notifications_service.notify)(comparison) + return notifications_service.notify(comparison) def send_notifications_if_commit_differs_from_pulls_head( self, commit, enriched_pull, current_yaml @@ -800,7 +796,7 @@ def schedule_new_user_activated_task(self, org_ownerid, user_ownerid): @sentry_sdk.trace def fetch_and_update_whether_ci_passed( - self, repository_service, commit, current_yaml + self, repository_service: TorngitBaseAdapter, commit, current_yaml ): all_statuses = async_to_sync(repository_service.get_commit_statuses)( commit.commitid diff --git a/tasks/notify_error.py b/tasks/notify_error.py index d235f1f5d..c66b01f9e 100644 --- a/tasks/notify_error.py +++ b/tasks/notify_error.py @@ -1,7 +1,6 @@ import logging from dataclasses import dataclass -from asgiref.sync import async_to_sync from sqlalchemy.orm import Session from celery_config import notify_error_task_name @@ -87,7 +86,7 @@ def is_failed(upload): failed_upload=num_failed_upload, total_upload=num_total_upload, ) - notification_result: NotifierResult = async_to_sync(error_notifier.notify)() + notification_result: NotifierResult = error_notifier.notify() match notification_result: case NotifierResult.COMMENT_POSTED: log.info( diff --git a/tasks/save_report_results.py b/tasks/save_report_results.py index 2e9dcced0..a4498e2f2 100644 --- a/tasks/save_report_results.py +++ b/tasks/save_report_results.py @@ -84,7 +84,7 @@ def run_impl( notifier_site_settings=True, current_yaml=current_yaml, ) - result = async_to_sync(notifier.build_payload)(comparison) + result = notifier.build_payload(comparison) report = self.fetch_report(commit, report_code) log.info( "Saving report results into the db", diff --git a/tasks/status_set_error.py b/tasks/status_set_error.py index ffc1bdeeb..ff3ef8749 100644 --- a/tasks/status_set_error.py +++ b/tasks/status_set_error.py @@ -59,18 +59,9 @@ def run_impl(self, db_session, repoid, commitid, *, message=None, **kwargs): message or "Coverage not measured fully because CI failed" ) if context in statuses: - # async_to_sync has its own "context" argument so we - # have to hide ours in a wrapper - async def wrapper(): - await repo_service.set_commit_status( - commit=commitid, - status=state, - context=context, - description=message, - url=url, - ) - - async_to_sync(wrapper)() + async_to_sync(repo_service.set_commit_status)( + commitid, state, context, message, url + ) status_set = True log.info( "Status set", diff --git a/tasks/status_set_pending.py b/tasks/status_set_pending.py index e52233cda..b90909fcd 100644 --- a/tasks/status_set_pending.py +++ b/tasks/status_set_pending.py @@ -82,18 +82,13 @@ def run_impl( ), "Pending status disabled in YAML" assert title not in statuses, "Pending status already set" - # async_to_sync has its own "context" argument so we - # have to hide ours in a wrapper - async def wrapper(): - await repo_service.set_commit_status( - commit=commitid, - status="pending", - context=title, - description="Collecting reports and waiting for CI to complete", - url=url, - ) - - async_to_sync(wrapper)() + async_to_sync(repo_service.set_commit_status)( + commitid, + "pending", + title, + "Collecting reports and waiting for CI to complete", + url, + ) status_set = True log.info( "Status set", extra=dict(context=title, state="pending") diff --git a/tasks/test_results_finisher.py b/tasks/test_results_finisher.py index c2f0bea9c..29705e4b7 100644 --- a/tasks/test_results_finisher.py +++ b/tasks/test_results_finisher.py @@ -182,7 +182,7 @@ def process_impl_within_lock( # make an attempt to make test results comment notifier = TestResultsNotifier(commit, commit_yaml, None) - success, reason = async_to_sync(notifier.error_comment)() + success, reason = notifier.error_comment() # also make attempt to make coverage comment queue_notify = True @@ -302,7 +302,7 @@ def process_impl_within_lock( if should_show_upgrade_message: notifier = TestResultsNotifier(commit, commit_yaml, pull) - success, reason = async_to_sync(notifier.upgrade_comment)() + success, reason = notifier.upgrade_comment() metrics.incr( f"test_results.finisher.test_result_notifier_upgrade_comment.{"success" if success else "failure"}.{reason}", @@ -344,7 +344,7 @@ def process_impl_within_lock( f"test_results_notif_latency.{"flaky" if should_read_flaky_detection(repoid, commit_yaml) else "non_flaky"}", begin_to_notify, ) - notifier_result: NotifierResult = async_to_sync(notifier.notify)() + notifier_result: NotifierResult = notifier.notify() match notifier_result: case NotifierResult.COMMENT_POSTED: diff --git a/tasks/tests/unit/test_compute_comparison.py b/tasks/tests/unit/test_compute_comparison.py index 7f8450026..721dafbc3 100644 --- a/tasks/tests/unit/test_compute_comparison.py +++ b/tasks/tests/unit/test_compute_comparison.py @@ -383,9 +383,8 @@ def test_run_task_ratelimit_error(self, dbsession, mocker, sample_report): "tasks.compute_comparison.ComparisonProxy.get_patch_totals", return_value=patch_totals, ) - mocker.patch.object( - ComputeComparisonTask, - "serialize_impacted_files", + mocker.patch( + "tasks.compute_comparison.ComparisonProxy.get_impacted_files", side_effect=TorngitRateLimitError("response_data", "message", "reset"), ) task = ComputeComparisonTask() diff --git a/tasks/tests/unit/test_notify_task.py b/tasks/tests/unit/test_notify_task.py index d302e191f..261c57771 100644 --- a/tasks/tests/unit/test_notify_task.py +++ b/tasks/tests/unit/test_notify_task.py @@ -352,8 +352,7 @@ def test_possibly_pin_commit_to_github_app_new_selection(self, mocker, dbsession mock_refresh_selection.assert_called_with(commit) mock_set_gh_app_for_commit.assert_called_with(12, commit) - @pytest.mark.asyncio - async def test_get_gitlab_extra_shas(self, dbsession): + def test_get_gitlab_extra_shas(self, dbsession): commit = CommitFactory( repository__owner__service="gitlab", repository__service_id=1000 ) @@ -394,9 +393,7 @@ async def test_get_gitlab_extra_shas(self, dbsession): ) repository_service = Gitlab(token={"key": "some_token"}) task = NotifyTask() - assert await task.get_gitlab_extra_shas_to_notify( - commit, repository_service - ) == { + assert task.get_gitlab_extra_shas_to_notify(commit, repository_service) == { "508c25daba5bbc77d8e7cf3c1917d5859153cfd3", } diff --git a/tasks/tests/unit/test_status_set_error.py b/tasks/tests/unit/test_status_set_error.py index 2fa2e7d45..f10e4c171 100644 --- a/tasks/tests/unit/test_status_set_error.py +++ b/tasks/tests/unit/test_status_set_error.py @@ -94,11 +94,11 @@ def test_set_error( StatusSetErrorTask().run_impl(dbsession, repoid, commitid) if cc_status_exists: repo.set_commit_status.assert_called_with( - commit=commitid, - status="error", - context="codecov/" + context, - description="Coverage not measured fully because CI failed", - url=f"https://codecov.io/gh/owner/repo/commit/{commitid}", + commitid, + "error", + "codecov/" + context, + "Coverage not measured fully because CI failed", + f"https://codecov.io/gh/owner/repo/commit/{commitid}", ) else: assert not repo.set_commit_status.called @@ -153,9 +153,9 @@ def test_set_error_custom_message(self, mocker, mock_configuration, dbsession): ) repo.set_commit_status.assert_called_with( - commit=commitid, - status="error", - context="codecov/" + context, - description=custom_message, - url=f"https://codecov.io/gh/owner/repo/commit/{commitid}", + commitid, + "error", + "codecov/" + context, + custom_message, + f"https://codecov.io/gh/owner/repo/commit/{commitid}", ) diff --git a/tasks/tests/unit/test_status_set_pending.py b/tasks/tests/unit/test_status_set_pending.py index ca4f65769..ffb2673fd 100644 --- a/tasks/tests/unit/test_status_set_pending.py +++ b/tasks/tests/unit/test_status_set_pending.py @@ -224,11 +224,11 @@ def test_set_pending( ) if branch == "master" and not cc_status_exists: repo.set_commit_status.assert_called_with( - commit=commitid, - status="pending", - context="codecov/" + context + "/custom", - description="Collecting reports and waiting for CI to complete", - url=f"https://codecov.io/gh/owner/repo/commit/{commitid}", + commitid, + "pending", + "codecov/" + context + "/custom", + "Collecting reports and waiting for CI to complete", + f"https://codecov.io/gh/owner/repo/commit/{commitid}", ) else: assert not repo.set_commit_status.called