Merge pull request #5 from codecov/joseph/diff-in-pr-comment

PR comment mention diff between PR base and head

Author: joseph-sentry

database/models/core.py

@@ -81,7 +81,6 @@ def __repr__(self):
 
 
 class Repository(CodecovBaseModel):
-
     __tablename__ = "repos"
 
     repoid = Column(types.Integer, primary_key=True)
@@ -127,7 +126,6 @@ def __repr__(self):
 
 
 class Commit(CodecovBaseModel):
-
     __tablename__ = "commits"
 
     id_ = Column("id", types.BigInteger, primary_key=True)
@@ -185,7 +183,6 @@ def report(self):
 
 
 class Branch(CodecovBaseModel):
-
     __tablename__ = "branches"
 
     repoid = Column(types.Integer, ForeignKey("repos.repoid"), primary_key=True)
@@ -204,7 +201,6 @@ def __repr__(self):
 
 
 class LoginSession(CodecovBaseModel):
-
     __tablename__ = "sessions"
 
     sessionid = Column(types.Integer, primary_key=True)
@@ -218,7 +214,6 @@ class LoginSession(CodecovBaseModel):
 
 
 class Pull(CodecovBaseModel):
-
     __tablename__ = "pulls"
 
     id_ = Column("id", types.BigInteger, primary_key=True)
@@ -238,6 +233,8 @@ class Pull(CodecovBaseModel):
     diff = Column(postgresql.JSON)
     flare = Column(postgresql.JSON)
     author_id = Column("author", types.Integer, ForeignKey("owners.ownerid"))
+    behind_by = Column(types.Integer)
+    behind_by_commit = Column(types.Text)
 
     author = relationship(Owner)
     repository = relationship(Repository, backref=backref("pulls", cascade="delete"))
@@ -276,7 +273,6 @@ def get_head_commit_notifications(self):
 
 
 class CommitNotification(CodecovBaseModel):
-
     __tablename__ = "commit_notifications"
 
     id_ = Column("id", types.BigInteger, primary_key=True)

services/comparison/__init__.py

@@ -42,9 +42,12 @@ def __init__(self, comparison: Comparison):
         self._diff = None
         self._changes = None
         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 = {}
 
@@ -144,6 +147,50 @@ async def get_changes(self) -> Optional[List[Change]]:
                         )
             return self._changes
 
+    async def get_behind_by(self):
+        async with self._behind_by_lock:
+            if self._behind_by is None:
+                if not getattr(self.comparison.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
+                if self._branch is None:
+                    branches = await self.repository_service.get_branches()
+                    branch = [
+                        b for b in branches if b[0] == provider_pull["base"]["branch"]
+                    ]
+
+                    if len(branch) == 0:
+                        log.warning(
+                            "Unable to find branch in list of branches on repo",
+                            extra=dict(
+                                branch=provider_pull["base"]["branch"],
+                            ),
+                        )
+                        return None
+
+                    self._branch = branch
+
+                distance = await self.repository_service.get_distance_in_commits(
+                    branch[0][1],
+                    self.comparison.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
+
     async def get_existing_statuses(self):
         async with self._existing_statuses_lock:
             if self._existing_statuses is None:

services/comparison/tests/unit/test_behind_by.py

@@ -0,0 +1,46 @@
+import pytest
+
+from services.comparison import ComparisonProxy
+
+
+class TestGetBehindBy(object):
+    @pytest.mark.asyncio
+    async 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")]
+        mock_repo_provider.get_distance_in_commits.return_value = {
+            "behind_by": 3,
+            "behind_by_commit": 123456,
+        }
+        mocker.patch(
+            "services.comparison.get_repo_provider_service",
+            return_value=mock_repo_provider,
+        )
+        res = await comparison.get_behind_by()
+        assert res == 3
+
+    @pytest.mark.asyncio
+    async def test_get_behind_by_no_base_commit(self, mocker):
+        comparison = ComparisonProxy(mocker.MagicMock())
+        del comparison.comparison.base.commit.commitid
+        res = await comparison.get_behind_by()
+        assert res is None
+
+    @pytest.mark.asyncio
+    async 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()
+        assert res is None
+
+    @pytest.mark.asyncio
+    async def test_get_behind_by_no_matching_branches(self, mocker, mock_repo_provider):
+        mock_repo_provider.get_branches.return_value = []
+        mocker.patch(
+            "services.comparison.get_repo_provider_service",
+            return_value=mock_repo_provider,
+        )
+        comparison = ComparisonProxy(mocker.MagicMock())
+        res = await comparison.get_behind_by()
+        assert res is None

services/notification/notifiers/mixins/message/__init__.py

@@ -39,6 +39,7 @@ async def create_message(
         """
         changes = await comparison.get_changes()
         diff = await comparison.get_diff()
+        behind_by = await comparison.get_behind_by()
         base_report = comparison.base.report
         head_report = comparison.head.report
         pull = comparison.pull
@@ -80,8 +81,9 @@ async def create_message(
             settings,
             current_yaml,
         )
+
         await self.write_section_to_msg(
-            comparison, changes, diff, links, write, section_writer
+            comparison, changes, diff, links, write, section_writer, behind_by
         )
 
         is_compact_message = should_message_be_compact(comparison, settings)
@@ -121,7 +123,12 @@ async def create_message(
                     )
 
                 await self.write_section_to_msg(
-                    comparison, changes, diff, links, write, section_writer
+                    comparison,
+                    changes,
+                    diff,
+                    links,
+                    write,
+                    section_writer,
                 )
 
             if is_compact_message:
@@ -141,7 +148,12 @@ async def create_message(
                         current_yaml,
                     )
                     await self.write_section_to_msg(
-                        comparison, changes, diff, links, write, section_writer
+                        comparison,
+                        changes,
+                        diff,
+                        links,
+                        write,
+                        section_writer,
                     )
 
         return [m for m in message if m is not None]
@@ -161,15 +173,16 @@ async def _possibly_write_gh_app_login_announcement(
             write("")
 
     async def write_section_to_msg(
-        self, comparison, changes, diff, links, write, section_writer
+        self, comparison, changes, diff, links, write, section_writer, behind_by=None
     ):
         with metrics.timer(
             f"worker.services.notifications.notifiers.comment.section.{section_writer.name}"
         ):
             for line in await section_writer.write_section(
-                comparison, diff, changes, links
+                comparison, diff, changes, links, behind_by=behind_by
             ):
                 write(line)
+
         write("")
 
     def get_middle_layout_section_names(self, settings):

services/notification/notifiers/mixins/message/sections.py

@@ -76,7 +76,7 @@ async def write_section(*args, **kwargs):
 
 
 class NewFooterSectionWriter(BaseSectionWriter):
-    async def do_write_section(self, comparison, diff, changes, links):
+    async def do_write_section(self, comparison, diff, changes, links, behind_by=None):
         repo_service = comparison.repository_service.service
         yield ("")
         yield (
@@ -94,7 +94,7 @@ async def do_write_section(self, comparison, diff, changes, links):
 
 
 class NewHeaderSectionWriter(BaseSectionWriter):
-    async def do_write_section(self, comparison, diff, changes, links):
+    async def do_write_section(self, comparison, diff, changes, links, behind_by=None):
         yaml = self.current_yaml
         base_report = comparison.base.report
         head_report = comparison.head.report
@@ -148,6 +148,7 @@ async def do_write_section(self, comparison, diff, changes, links):
                     head_cov=round_number(yaml, Decimal(head_report.totals.coverage)),
                 )
             )
+
         else:
             yield (
                 "> :exclamation: No coverage uploaded for {request_type} {what} (`{branch}@{commit}`). [Click here to learn what that means](https://docs.codecov.io/docs/error-reference#section-missing-{what}-commit).".format(
@@ -175,6 +176,11 @@ async def do_write_section(self, comparison, diff, changes, links):
             else:
                 yield "> Patch has no changes to coverable lines."
 
+        if behind_by:
+            yield (
+                f"> Report is {behind_by} commits behind head on {pull_dict['base']['branch']}."
+            )
+
         if (
             comparison.enriched_pull.provider_pull is not None
             and comparison.head.commit.commitid
@@ -226,7 +232,7 @@ async def do_write_section(self, comparison, diff, changes, links):
 
 
 class HeaderSectionWriter(BaseSectionWriter):
-    async def do_write_section(self, comparison, diff, changes, links):
+    async def do_write_section(self, comparison, diff, changes, links, behind_by=None):
         yaml = self.current_yaml
         base_report = comparison.base.report
         head_report = comparison.head.report
@@ -261,6 +267,7 @@ async def do_write_section(self, comparison, diff, changes, links):
                     links=links,
                 )
             )
+
         else:
             yield (
                 "> :exclamation: No coverage uploaded for pull request {what} (`{branch}@{commit}`). [Click here to learn what that means](https://docs.codecov.io/docs/error-reference#section-missing-{what}-commit).".format(
@@ -272,6 +279,11 @@ async def do_write_section(self, comparison, diff, changes, links):
                 )
             )
 
+        if isinstance(behind_by, int) and behind_by > 0:
+            yield (
+                f"> Report is {behind_by} commits behind head on {pull_dict['base']['branch']}."
+            )
+
         diff_totals = head_report.apply_diff(diff)
         if diff_totals and diff_totals.coverage is not None:
             yield (
@@ -347,7 +359,7 @@ async def do_write_section(*args, **kwargs):
 
 
 class ImpactedEntrypointsSectionWriter(BaseSectionWriter):
-    async def do_write_section(self, comparison, diff, changes, links):
+    async 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()
         if impacted_endpoints:
@@ -367,7 +379,7 @@ async def do_write_section(self, comparison, diff, changes, links):
 
 
 class FooterSectionWriter(BaseSectionWriter):
-    async def do_write_section(self, comparison, diff, changes, links):
+    async def do_write_section(self, comparison, diff, changes, links, behind_by=None):
         pull_dict = comparison.enriched_pull.provider_pull
         yield ("------")
         yield ("")
@@ -393,7 +405,7 @@ async def do_write_section(self, comparison, diff, changes, links):
 
 
 class ReachSectionWriter(BaseSectionWriter):
-    async def do_write_section(self, comparison, diff, changes, links):
+    async 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(
@@ -411,7 +423,7 @@ async def do_write_section(self, comparison, diff, changes, links):
 
 
 class DiffSectionWriter(BaseSectionWriter):
-    async def do_write_section(self, comparison, diff, changes, links):
+    async def do_write_section(self, comparison, diff, changes, links, behind_by=None):
         base_report = comparison.base.report
         head_report = comparison.head.report
         if base_report is None:
@@ -432,7 +444,7 @@ async def do_write_section(self, comparison, diff, changes, links):
 
 
 class FileSectionWriter(BaseSectionWriter):
-    async def do_write_section(self, comparison, diff, changes, links):
+    async def do_write_section(self, comparison, diff, changes, links, behind_by=None):
         # create list of files changed in diff
         base_report = comparison.base.report
         head_report = comparison.head.report
@@ -535,7 +547,7 @@ def tree_cell(typ, path, metrics, _=None):
 
 
 class FlagSectionWriter(BaseSectionWriter):
-    async def do_write_section(self, comparison, diff, changes, links):
+    async def do_write_section(self, comparison, diff, changes, links, behind_by=None):
         # flags
         base_report = comparison.base.report
         head_report = comparison.head.report
@@ -683,7 +695,9 @@ async def _get_table_data_for_components(
             )
         return component_data
 
-    async def do_write_section(self, comparison: ComparisonProxy, diff, changes, links):
+    async 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