codecov · giovanni-guidini · Sep 5, 2024 · Sep 3, 2024 · Sep 3, 2024 · Sep 5, 2024
diff --git a/helpers/comparison.py b/helpers/comparison.py
@@ -0,0 +1,21 @@
+from shared.reports.types import ReportTotals
+
+
+def minimal_totals(totals: ReportTotals | None) -> dict:
+    if totals is None:
+        return {
+            "hits": 0,
+            "misses": 0,
+            "partials": 0,
+            "coverage": None,
+        }
+    return {
+        "hits": totals.hits,
+        "misses": totals.misses,
+        "partials": totals.partials,
+        # ReportTotals has coverage as a string, we want float in the DB
+        # Also the coverage from ReportTotals is 0-100, while in the DB it's 0-1
+        "coverage": (
+            (float(totals.coverage) / 100) if totals.coverage is not None else None
+        ),
+    }
diff --git a/services/comparison/__init__.py b/services/comparison/__init__.py
@@ -59,6 +59,7 @@
         self._repository_service = None
         self._adjusted_base_diff = None
         self._original_base_diff = None
+        self._patch_totals = None
         self._changes = None
         self._existing_statuses = None
         self._behind_by = None
@@ -209,9 +210,11 @@
 
         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)
-        totals = self.head.report.apply_diff(diff)
-        return totals
+        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:
@@ -371,6 +374,7 @@
         self.flags = flags
         self.path_patterns = path_patterns
         self.real_comparison = real_comparison
+        self._patch_totals = None
         self._changes = None
         self.project_coverage_base = FullCommit(
             commit=real_comparison.project_coverage_base.commit,
@@ -394,6 +398,18 @@
     async def get_diff(self, use_original_base=False):
         return await self.real_comparison.get_diff(use_original_base=use_original_base)
 
+    @sentry_sdk.trace
+    async 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)
+        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()
 

diff --git a/services/notification/notifiers/mixins/status.py b/services/notification/notifiers/mixins/status.py
@@ -9,7 +9,9 @@
 
 
 class StatusPatchMixin(object):
-    async def get_patch_status(self, comparison) -> Tuple[str, str]:
+    async def get_patch_status(
+        self, comparison: ComparisonProxy | FilteredComparison
+    ) -> Tuple[str, str]:
         threshold = self.notifier_yaml_settings.get("threshold", "0.0")
 
         # check if user has erroneously added a % to this input and fix
@@ -21,8 +23,7 @@ async def get_patch_status(self, comparison) -> Tuple[str, str]:
         except (InvalidOperation, TypeError):
             threshold = Decimal("0.0")
 
-        diff = await comparison.get_diff(use_original_base=True)
-        totals = comparison.head.report.apply_diff(diff)
+        totals = await 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("%", "")

diff --git a/services/repository.py b/services/repository.py
@@ -471,6 +471,7 @@ class EnrichedPull(object):
     provider_pull: Optional[Mapping[str, Any]]
 
 
+@sentry_sdk.trace
 async def fetch_and_update_pull_request_information_from_commit(
     repository_service, commit, current_yaml
 ) -> Optional[EnrichedPull]:
@@ -540,6 +541,7 @@ async def _pick_best_base_comparedto_pair(
     return (candidates_to_base[0], None)
 
 
+@sentry_sdk.trace
 async def fetch_and_update_pull_request_information(
     repository_service, db_session, repoid, pullid, current_yaml
 ) -> EnrichedPull:

diff --git a/tasks/compute_comparison.py b/tasks/compute_comparison.py
@@ -7,14 +7,14 @@
 from shared.components import Component
 from shared.helpers.flag import Flag
 from shared.reports.readonly import ReadOnlyReport
-from shared.reports.types import ReportTotals
 from shared.torngit.exceptions import TorngitRateLimitError
 from shared.yaml import UserYaml
 
 from app import celery_app
 from database.enums import CompareCommitError, CompareCommitState
 from database.models import CompareCommit, CompareComponent, CompareFlag
 from database.models.reports import ReportLevelTotals, RepositoryFlag
+from helpers.comparison import minimal_totals
 from helpers.github_installation import get_installation_name_for_owner_for_task
 from services.archive import ArchiveService
 from services.comparison import ComparisonContext, ComparisonProxy, FilteredComparison
@@ -66,8 +66,9 @@ 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
-        patch_totals = async_to_sync(comparison_proxy.get_patch_totals)()
-        comparison.patch_totals = self.minimal_totals(patch_totals)
+        if comparison.patch_totals is None:
+            patch_totals = async_to_sync(comparison_proxy.get_patch_totals)()
+            comparison.patch_totals = minimal_totals(patch_totals)
 
         if not comparison_proxy.has_project_coverage_base_report():
             comparison.error = CompareCommitError.missing_base_report.value
@@ -96,18 +97,6 @@ def run_impl(
         path = self.store_results(comparison, impacted_files)
 
         comparison.report_storage_path = path
-        calculated_patch_totals = impacted_files.get("changes_summary").get(
-            "patch_totals"
-        )
-        if calculated_patch_totals != comparison.patch_totals:
-            log.warning(
-                "Calculated patch totals differ!",
-                extra={
-                    **log_extra,
-                    "calculated_patch_totals": calculated_patch_totals,
-                    "patch_totals": comparison.patch_totals,
-                },
-            )
 
         comparison.state = CompareCommitState.processed.value
         log.info("Computing comparison successful", extra=log_extra)
@@ -119,25 +108,6 @@ def run_impl(
         db_session.commit()
         return {"successful": True}
 
-    def minimal_totals(self, totals: ReportTotals | None) -> dict:
-        if totals is None:
-            return {
-                "hits": 0,
-                "misses": 0,
-                "partials": 0,
-                "coverage": None,
-            }
-        return {
-            "hits": totals.hits,
-            "misses": totals.misses,
-            "partials": totals.partials,
-            # ReportTotals has coverage as a string, we want float in the DB
-            # Also the coverage from ReportTotals is 0-100, while in the DB it's 0-1
-            "coverage": (
-                (float(totals.coverage) / 100) if totals.coverage is not None else None
-            ),
-        }
-
     def compute_flag_comparison(self, db_session, comparison, comparison_proxy):
         log_extra = dict(comparison_id=comparison.id)
         log.info("Computing flag comparisons", extra=log_extra)

diff --git a/tasks/notify.py b/tasks/notify.py
@@ -1,6 +1,7 @@
 import logging
 from typing import Optional
 
+import sentry_sdk
 from asgiref.sync import async_to_sync
 from celery.exceptions import MaxRetriesExceededError, SoftTimeLimitExceeded
 from shared.celery_config import (
@@ -14,21 +15,27 @@
 from shared.torngit.base import TokenType, TorngitBaseAdapter
 from shared.torngit.exceptions import TorngitClientError, TorngitServerFailureError
 from shared.yaml import UserYaml
+from sqlalchemy import and_
 from sqlalchemy.orm.session import Session
 
 from app import celery_app
 from database.enums import CommitErrorTypes, Decoration, NotificationState, ReportType
 from database.models import Commit, Pull
-from database.models.core import GITHUB_APP_INSTALLATION_DEFAULT_NAME
+from database.models.core import GITHUB_APP_INSTALLATION_DEFAULT_NAME, CompareCommit
 from helpers.checkpoint_logger import from_kwargs as checkpoints_from_kwargs
 from helpers.checkpoint_logger.flows import UploadFlow
 from helpers.clock import get_seconds_to_next_hour
+from helpers.comparison import minimal_totals
 from helpers.exceptions import NoConfiguredAppsAvailable, RepositoryWithoutValidBotError
 from helpers.github_installation import get_installation_name_for_owner_for_task
 from helpers.save_commit_error import save_commit_error
 from services.activation import activate_user
 from services.commit_status import RepositoryCIFilter
-from services.comparison import ComparisonContext, ComparisonProxy
+from services.comparison import (
+    ComparisonContext,
+    ComparisonProxy,
+    get_or_create_comparison,
+)
 from services.comparison.types import Comparison, FullCommit
 from services.decoration import determine_decoration_details
 from services.github import get_github_app_for_commit, set_github_app_for_commit
@@ -511,6 +518,52 @@
             return True
         return False
 
+    @sentry_sdk.trace
+    def save_patch_totals(self, comparison: ComparisonProxy) -> None:
+        """Saves patch coverage to the CompareCommit, if it exists.
+        This is done to make sure the patch coverage reported by notifications and UI is the same
+        (because they come from the same source)
+        """
+        if comparison.comparison.patch_coverage_base_commitid is None:
+            # Can't get diff to calculate patch totals
+            return
+        head_commit = comparison.head.commit
+        db_session = head_commit.get_db_session()
+        patch_coverage = async_to_sync(comparison.get_patch_totals)()
+        if comparison.project_coverage_base is not None:
+            # Update existing Comparison
+            statement = (
+                CompareCommit.__table__.update()
+                .where(
+                    and_(
+                        CompareCommit.compare_commit_id == head_commit.id,
+                        CompareCommit.base_commit_id
+                        == comparison.project_coverage_base.commit.id,
+                    )
+                )
+                .values(patch_totals=minimal_totals(patch_coverage))
+            )
+            db_session.execute(statement)
+        else:
+            # We calculated patch coverage, but there's no project base
+            # So we will create a comparison to save the patch_totals, to make sure
+            # the UI and the PR have the same information
+            base_commit = (
+                db_session.query(Commit)
+                .filter(
+                    Commit.commitid
+                    == comparison.comparison.patch_coverage_base_commitid,
+                    Commit.repository == head_commit.repository,
+                )
+                .first()
+            )
+            if base_commit:
+                compare_commit = get_or_create_comparison(
+                    db_session, base_commit, head_commit
+                )
+                compare_commit.patch_totals = minimal_totals(patch_coverage)
+
+    @sentry_sdk.trace
     def submit_third_party_notifications(
         self,
         current_yaml: UserYaml,
@@ -562,6 +615,8 @@
             ),
         )
 
+        self.save_patch_totals(comparison)
+
         decoration_type = self.determine_decoration_type_from_pull(
             enriched_pull, empty_upload
         )
@@ -687,6 +742,7 @@
             ),
         )
 
+    @sentry_sdk.trace
     def fetch_and_update_whether_ci_passed(
         self, repository_service, commit, current_yaml
     ):