Made changes to fix the head sha commit in diff comment

diff_poetry_lock/run_poetry.py

@@ -83,7 +83,7 @@ def post_comment(api: GithubApi, comment: str | None) -> None:
 def format_comment(
     packages: list[PackageSummary],
     base_commit_hash: str | None = None,
-    target_commit_hash: str | None = None,
+    head_commit_hash: str | None = None,
 ) -> str | None:
     added = sorted([p for p in packages if p.added()], key=attrgetter("name"))
     removed = sorted([p for p in packages if p.removed()], key=attrgetter("name"))
@@ -95,8 +95,8 @@ def format_comment(
 
     change_count = len(added + removed + updated)
     comment = f"### Detected {change_count} changes to dependencies in Poetry lockfile\n\n"
-    if base_commit_hash and target_commit_hash:
-        comment += f"From base {base_commit_hash} to target {target_commit_hash}:\n\n"
+    if base_commit_hash and head_commit_hash:
+        comment += f"From base {base_commit_hash} to head {head_commit_hash}:\n\n"
     summary_lines = [p.summary_line() for p in added + removed + updated]
     comment += "\n".join(summary_lines)
     comment += (
@@ -143,11 +143,11 @@ def do_diff(settings: Settings) -> None:
     if not any(package.changed() for package in packages):
         summary = None
     else:
-        target_commit_hash, base_commit_hash = api.resolve_commit_hashes(settings.ref, settings.base_ref)
+        head_commit_hash, base_commit_hash = api.resolve_commit_hashes(settings.ref, settings.base_ref)
         summary = format_comment(
             packages,
             base_commit_hash=base_commit_hash,
-            target_commit_hash=target_commit_hash,
+            head_commit_hash=head_commit_hash,
         )
 
     if summary:

diff_poetry_lock/settings.py

@@ -22,7 +22,6 @@ class Settings(ABC):
     ref: str
     repository: str
     base_ref: str
-    pr_num: str | None
 
     # from step config including secrets
     token: str
@@ -32,6 +31,31 @@ class Settings(ABC):
     sigil_envvar: ClassVar[str]
     """The envvar in this will always be present when this settings is valid."""
 
+    _pr_lookup_service: PrLookupService | None = None
+
+    def set_pr_lookup_service(self, service: PrLookupService) -> None:
+        self._pr_lookup_service = service
+
+    _pr_num_cached: str = ""
+
+    @property
+    def pr_num(self) -> str | None:
+        if self._pr_num_cached:
+            return self._pr_num_cached
+
+        if self._pr_lookup_service is None:
+            logger.warning("PR lookup requested before service configured; returning None")
+            return None
+
+        logger.debug("Settings.pr_num looking up PR for branch {}", self.ref)
+        pr_num = self._pr_lookup_service.find_pr_for_branch(self.ref)
+        object.__setattr__(self, "_pr_num_cached", pr_num)
+        if pr_num:
+            logger.debug("Settings.pr_num found PR #{}", pr_num)
+        else:
+            logger.warning("Settings.pr_num found no open PR")
+        return pr_num
+
     @classmethod
     def matches_env(cls, env: dict[str, str]) -> bool:
         """Check whether this CI's identifying env var is present."""
@@ -65,34 +89,15 @@ def __init__(self, **values: Any) -> None:  # noqa: ANN401
         logger.debug("VelaSettings ref={}", self.ref)
         logger.debug("VelaSettings event_name={}", self.event_name)
 
-    def set_pr_lookup_service(self, service: PrLookupService) -> None:
-        self._pr_lookup_service = service
-
-    @property
-    def pr_num(self) -> str | None:  # type: ignore[override]
-        if self._pr_num_cached:
-            return self._pr_num_cached
-
-        if self._pr_lookup_service is None:
-            logger.warning("PR lookup requested before service configured; returning None")
-            return None
-
-        logger.debug("VelaSettings.pr_num looking up PR for branch {}", self.ref)
-        pr_num = self._pr_lookup_service.find_pr_for_branch(self.ref)
-        self._pr_num_cached = pr_num
-        if pr_num:
-            logger.debug("VelaSettings.pr_num found PR #{}", pr_num)
-        else:
-            logger.warning("VelaSettings.pr_num found no open PR")
-        return pr_num
-
 
 class GitHubActionsSettings(BaseSettings, Settings):
     sigil_envvar: ClassVar[str] = "github_repository"
+    _pr_lookup_service: PrLookupService | None = PrivateAttr(default=None)
+    _pr_num_cached: str = PrivateAttr(default="")
 
     # from CI
     event_name: str = Field(env="github_event_name")  # must be 'pull_request'
-    ref: str = Field(env="github_ref")
+    ref: str = Field(env="github_head_ref")
     repository: str = Field(env="github_repository")
     base_ref: str = Field(env="github_base_ref")
 
@@ -120,12 +125,6 @@ def event_must_be_pull_request(cls, v: str) -> str:
             raise ValueError(msg)
         return v
 
-    @property
-    # todo: Avoid this MyPy error by having Pydantic compute the field
-    def pr_num(self) -> str | None:  # type: ignore[override]
-        # TODO: Validate early
-        return self.ref.split("/")[2]
-
 
 _CI_SETTINGS_CANDIDATES: list[type[Settings]] = [GitHubActionsSettings, VelaSettings]
 

diff_poetry_lock/test/test_poetry_diff.py

@@ -12,7 +12,13 @@
 from diff_poetry_lock import __version__
 from diff_poetry_lock.github import MAGIC_COMMENT_IDENTIFIER, GithubApi
 from diff_poetry_lock.run_poetry import PackageSummary, diff, do_diff, format_comment, load_packages, main
-from diff_poetry_lock.settings import GitHubActionsSettings, Settings
+from diff_poetry_lock.settings import (
+    GitHubActionsSettings,
+    PrLookupConfigurable,
+    PrLookupService,
+    Settings,
+    VelaSettings,
+)
 
 TESTFILE_1 = Path("diff_poetry_lock/test/res/poetry1.lock")
 TESTFILE_2 = Path("diff_poetry_lock/test/res/poetry2.lock")
@@ -33,21 +39,71 @@ def data2() -> bytes:
     return load_file(TESTFILE_2)
 
 
+@pytest.fixture(autouse=True)
+def patch_pr_lookup_setter(monkeypatch: MonkeyPatch) -> None:
+    def _safe_setter(self: Settings, service: PrLookupService) -> None:
+        object.__setattr__(self, "_pr_lookup_service", service)
+
+    monkeypatch.setattr(Settings, "set_pr_lookup_service", _safe_setter)
+
+
+class _LookupService:
+    def __init__(self, settings: Settings) -> None:
+        self.s = settings
+        if isinstance(self.s, PrLookupConfigurable):
+            self.s.set_pr_lookup_service(self)
+
+    def find_pr_for_branch(self, branch_ref: str) -> str:
+        if branch_ref == "refs/heads/github-actions":
+            return "42"
+        if branch_ref == "refs/heads/vela":
+            return "1"
+        return ""
+
+
 def test_settings(monkeypatch: MonkeyPatch) -> None:
     monkeypatch.setenv("GITHUB_EVENT_NAME", "pull_request")
-    monkeypatch.setenv("GITHUB_REF", "refs/pull/1/merge")
+    monkeypatch.setenv("GITHUB_HEAD_REF", "refs/heads/github-actions")
     monkeypatch.setenv("GITHUB_REPOSITORY", "account/repo")
     monkeypatch.setenv("INPUT_GITHUB_TOKEN", "foobar")
     monkeypatch.setenv("GITHUB_BASE_REF", "main")
 
     s = GitHubActionsSettings()
+    assert s.event_name == "pull_request"
+    assert s.ref == "refs/heads/github-actions"
+    assert s.repository == "account/repo"
+    assert s.base_ref == "main"
+    assert s.pr_num is None
+
+    _LookupService(s)
+
+    assert s.pr_num == "42"
+
+
+def test_vela_settings(monkeypatch: MonkeyPatch) -> None:
+    monkeypatch.setenv("VELA_BUILD_EVENT", "push")
+    monkeypatch.setenv("VELA_BUILD_REF", "refs/heads/vela")
+    monkeypatch.setenv("VELA_REPO_FULL_NAME", "account/repo")
+    monkeypatch.setenv("VELA_REPO_BRANCH", "main")
+    monkeypatch.setenv("PARAMETER_GITHUB_TOKEN", "vela-token")
+    monkeypatch.setenv("PARAMETER_LOCKFILE_PATH", "poetry.lock")
+    monkeypatch.setenv("PARAMETER_GITHUB_API_URL", "https://api.github.com")
+
+    s = VelaSettings()
+    assert s.event_name == "push"
+    assert s.ref == "refs/heads/vela"
+    assert s.repository == "account/repo"
+    assert s.base_ref == "refs/heads/main"
+    assert s.pr_num is None
+
+    _LookupService(s)
 
     assert s.pr_num == "1"
 
 
 def test_settings_not_pr(monkeypatch: MonkeyPatch) -> None:
     monkeypatch.setenv("GITHUB_EVENT_NAME", "push")
-    monkeypatch.setenv("GITHUB_REF", "refs/pull/1/merge")
+    monkeypatch.setenv("GITHUB_HEAD_REF", "deps-update")
     monkeypatch.setenv("GITHUB_REPOSITORY", "account/repo")
     monkeypatch.setenv("INPUT_GITHUB_TOKEN", "foobar")
     monkeypatch.setenv("GITHUB_BASE_REF", "main")
@@ -79,7 +135,7 @@ def test_diff() -> None:
     expected_comment = f"""\
     ### Detected 3 changes to dependencies in Poetry lockfile
 
-    From base new-sha to target old-sha:
+    From base new-sha to head old-sha:
 
     Removed **pydantic** (1.10.6)
     Removed **typing-extensions** (4.5.0)
@@ -93,7 +149,7 @@ def test_diff() -> None:
         format_comment(
             summary,
             base_commit_hash="new-sha",
-            target_commit_hash="old-sha",
+            head_commit_hash="old-sha",
         )
         or ""
     ).strip() == dedent(expected_comment).strip()
@@ -145,7 +201,7 @@ def test_diff_2() -> None:
     expected_comment = f"""\
     ### Detected 3 changes to dependencies in Poetry lockfile
 
-    From base new-sha to target old-sha:
+    From base new-sha to head old-sha:
 
     Added **pydantic** (1.10.6)
     Added **typing-extensions** (4.5.0)
@@ -159,7 +215,7 @@ def test_diff_2() -> None:
         format_comment(
             summary,
             base_commit_hash="new-sha",
-            target_commit_hash="old-sha",
+            head_commit_hash="old-sha",
         )
         or ""
     ).strip() == dedent(expected_comment).strip()
@@ -238,16 +294,17 @@ def test_e2e_diff_inexisting_comment(cfg: Settings, data1: bytes, data2: bytes)
     summary = format_comment(
         diff(load_packages(TESTFILE_2), load_packages(TESTFILE_1)),
         base_commit_hash="new-sha",
-        target_commit_hash="old-sha",
+        head_commit_hash="old-sha",
     )
 
     with requests_mock.Mocker() as m:
         mock_get_file(m, cfg, data1, cfg.base_ref, resolved_hash="old-sha")
         mock_get_file(m, cfg, data2, cfg.ref, resolved_hash="new-sha")
         mock_resolve_commit_hashes(m, cfg, head_hash="new-sha", base_hash="old-sha")
-        mock_list_comments(m, cfg, [])
+        mock_find_pr_for_branch(m, cfg, pr_num="1")
+        mock_list_comments(m, cfg, [], pr_num="1")
         m.post(
-            f"{cfg.api_url}/repos/{cfg.repository}/issues/{cfg.pr_num}/comments",
+            f"{cfg.api_url}/repos/{cfg.repository}/issues/1/comments",
             headers=GithubApi.Headers.JSON.headers(cfg.token),
             json={"body": f"{MAGIC_COMMENT_IDENTIFIER}{summary}"},
         )
@@ -259,19 +316,20 @@ def test_e2e_diff_existing_comment_same_data(cfg: Settings, data1: bytes, data2:
     summary = format_comment(
         diff(load_packages(TESTFILE_1), load_packages(TESTFILE_2)),
         base_commit_hash="old-sha",
-        target_commit_hash="new-sha",
+        head_commit_hash="new-sha",
     )
 
     with requests_mock.Mocker() as m:
         mock_get_file(m, cfg, data1, cfg.base_ref, resolved_hash="old-sha")
         mock_get_file(m, cfg, data2, cfg.ref, resolved_hash="new-sha")
         mock_resolve_commit_hashes(m, cfg, head_hash="new-sha", base_hash="old-sha")
+        mock_find_pr_for_branch(m, cfg, pr_num="1")
         comments = [
             {"body": "foobar", "id": 1334, "user": {"id": 123}},
             {"body": "foobar", "id": 1335, "user": {"id": 41898282}},
             {"body": f"{MAGIC_COMMENT_IDENTIFIER}{summary}", "id": 1337, "user": {"id": 41898282}},
         ]
-        mock_list_comments(m, cfg, comments)
+        mock_list_comments(m, cfg, comments, pr_num="1")
 
         do_diff(cfg)
 
@@ -280,19 +338,20 @@ def test_e2e_diff_existing_comment_different_data(cfg: Settings, data1: bytes, d
     summary = format_comment(
         diff(load_packages(TESTFILE_1), []),
         base_commit_hash="new-sha",
-        target_commit_hash="old-sha",
+        head_commit_hash="old-sha",
     )
 
     with requests_mock.Mocker() as m:
         mock_get_file(m, cfg, data1, cfg.base_ref, resolved_hash="old-sha")
         mock_get_file(m, cfg, data2, cfg.ref, resolved_hash="new-sha")
         mock_resolve_commit_hashes(m, cfg, head_hash="new-sha", base_hash="old-sha")
+        mock_find_pr_for_branch(m, cfg, pr_num="1")
         comments = [
             {"body": "foobar", "id": 1334, "user": {"id": 123}},
             {"body": "foobar", "id": 1335, "user": {"id": 41898282}},
             {"body": f"{MAGIC_COMMENT_IDENTIFIER}{summary}", "id": 1337, "user": {"id": 41898282}},
         ]
-        mock_list_comments(m, cfg, comments)
+        mock_list_comments(m, cfg, comments, pr_num="1")
         m.patch(
             f"{cfg.api_url}/repos/{cfg.repository}/issues/comments/1337",
             headers=GithubApi.Headers.JSON.headers(cfg.token),
@@ -306,20 +365,21 @@ def test_e2e_diff_commit_lookup_http_failure_falls_back_to_ref(cfg: Settings, da
     summary = format_comment(
         diff(load_packages(TESTFILE_2), load_packages(TESTFILE_1)),
         base_commit_hash=cfg.ref,
-        target_commit_hash=cfg.base_ref,
+        head_commit_hash=cfg.base_ref,
     )
 
     with requests_mock.Mocker() as m:
         mock_get_file(m, cfg, data1, cfg.base_ref)
         mock_get_file(m, cfg, data2, cfg.ref)
+        mock_find_pr_for_branch(m, cfg, pr_num="1")
         m.post(
             f"{cfg.api_url}/graphql",
             request_headers=GithubApi.Headers.JSON.headers(cfg.token),
             status_code=500,
         )
-        mock_list_comments(m, cfg, [])
+        mock_list_comments(m, cfg, [], pr_num="1")
         m.post(
-            f"{cfg.api_url}/repos/{cfg.repository}/issues/{cfg.pr_num}/comments",
+            f"{cfg.api_url}/repos/{cfg.repository}/issues/1/comments",
             headers=GithubApi.Headers.JSON.headers(cfg.token),
             json={"body": f"{MAGIC_COMMENT_IDENTIFIER}{summary}"},
         )
@@ -332,9 +392,18 @@ def load_file(filename: Path) -> bytes:
         return f.read()
 
 
-def mock_list_comments(m: Mocker, s: Settings, response_json: list[dict[Any, Any]]) -> None:
+def mock_find_pr_for_branch(m: Mocker, s: Settings, pr_num: str = "1") -> None:
+    head = f"{s.repository.split('/')[0]}:{s.ref}"
+    m.get(
+        f"{s.api_url}/repos/{s.repository}/pulls?head={head}&state=open",
+        request_headers=GithubApi.Headers.JSON.headers(s.token),
+        json=[{"number": int(pr_num)}],
+    )
+
+
+def mock_list_comments(m: Mocker, s: Settings, response_json: list[dict[Any, Any]], pr_num: str = "1") -> None:
     m.get(
-        f"{s.api_url}/repos/{s.repository}/issues/{s.pr_num}/comments?per_page=100&page=1",
+        f"{s.api_url}/repos/{s.repository}/issues/{pr_num}/comments?per_page=100&page=1",
         request_headers=GithubApi.Headers.JSON.headers(s.token),
         json=response_json,
     )
@@ -377,7 +446,7 @@ def create_settings(
 ) -> Settings:
     return GitHubActionsSettings(
         event_name="pull_request",
-        ref="refs/pull/1/merge",
+        ref="deps-update",
         repository=repository,
         token=token,
         base_ref="main",