Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
45 changes: 42 additions & 3 deletions src/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -176,6 +176,40 @@ def _env_required(name: str) -> str:
return val


def _pr_number_from_event() -> int | None:
"""Read the PR number from the event payload GitHub writes for the run.

Every pull_request payload carries `.pull_request.number`, for every
activity type, which GITHUB_REF does not.

Returns None rather than raising for anything unexpected — no payload path,
an unreadable or malformed file, a payload for some other event, a number
that is not one. The caller still has GITHUB_REF to fall back on, and a
raise here would turn a recoverable case into a failed run.

Returns:
The PR number, or None if the payload does not yield one.
"""
path = os.environ.get("GITHUB_EVENT_PATH")
if not path:
return None
try:
with open(path, encoding="utf-8") as handle:
payload = json.load(handle)
except (OSError, ValueError):
return None
if not isinstance(payload, dict):
return None
pull_request = payload.get("pull_request")
if not isinstance(pull_request, dict):
return None
number = pull_request.get("number")
# bool is an int subclass, and `true` is valid JSON in that position.
if isinstance(number, bool) or not isinstance(number, int):
return None
return number


@dataclass(frozen=True)
class Config:
"""Parsed and validated configuration from GitHub Actions inputs.
Expand Down Expand Up @@ -305,10 +339,15 @@ def parse_config() -> Config:
repo = _env_required("GITHUB_REPOSITORY")
event_name = os.environ.get("GITHUB_EVENT_NAME", "unknown")

# Extract PR number from GITHUB_REF (refs/pull/<number>/merge)
pr_number = None
# The event payload first, GITHUB_REF only as a fallback. Every
# pull_request payload carries .pull_request.number, whereas GITHUB_REF is
# refs/pull/<number>/merge only while GitHub has a merge ref to point the
# run at. When it points elsewhere the run used to abort with "Could not
# determine PR number" having measured nothing, which reads to the author
# as a coverage verdict on code the action never looked at.
pr_number = _pr_number_from_event()
github_ref = os.environ.get("GITHUB_REF", "")
match = re.search(r"refs/pull/(\d+)/", github_ref)
match = None if pr_number is not None else re.search(r"refs/pull/(\d+)/", github_ref)
if match:
pr_number = int(match.group(1))

Expand Down
6 changes: 5 additions & 1 deletion src/main.py
Original file line number Diff line number Diff line change
Expand Up @@ -197,7 +197,11 @@ def main() -> None:
return

if config.pr_number is None:
print("::error::Could not determine PR number from GITHUB_REF.")
print(
"::error::Could not determine the PR number from either the event payload "
"at GITHUB_EVENT_PATH or GITHUB_REF. Nothing was measured, so this is not a "
"test-adequacy verdict."
)
sys.exit(1)

try:
Expand Down
75 changes: 75 additions & 0 deletions tests/test_config.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
# pyright: reportUnknownParameterType=false, reportMissingParameterType=false, reportUnknownMemberType=false
"""Tests for configuration parsing."""

import json

import pytest

from src.config import parse_config
Expand Down Expand Up @@ -217,9 +219,82 @@ def test_pr_number_from_env(self, monkeypatch):
monkeypatch.setenv("GITHUB_REPOSITORY", "owner/repo")
monkeypatch.setenv("GITHUB_EVENT_NAME", "pull_request")
monkeypatch.setenv("GITHUB_REF", "refs/pull/42/merge")
# This suite runs inside Actions too, where GITHUB_EVENT_PATH points at a
# real payload naming a real PR. Without this the assertion below would
# measure that PR instead of the ref under test.
monkeypatch.delenv("GITHUB_EVENT_PATH", raising=False)
cfg = parse_config()
assert cfg.pr_number == 42

def test_pr_number_from_event_payload_when_the_ref_has_none(self, monkeypatch, tmp_path):
# The case the action used to abort on: a pull_request event whose ref is
# not refs/pull/<n>/merge. Observed on the `edited` activity type, where
# the run failed with "Could not determine PR number" without having
# measured anything.
payload = tmp_path / "event.json"
payload.write_text(json.dumps({"pull_request": {"number": 180}}), encoding="utf-8")
monkeypatch.setenv("GITHUB_TOKEN", "ghp_fake")
monkeypatch.setenv("GITHUB_REPOSITORY", "owner/repo")
monkeypatch.setenv("GITHUB_EVENT_NAME", "pull_request")
monkeypatch.setenv("GITHUB_REF", "refs/heads/main")
monkeypatch.setenv("GITHUB_EVENT_PATH", str(payload))
assert parse_config().pr_number == 180

def test_the_event_payload_wins_over_the_ref(self, monkeypatch, tmp_path):
# The payload describes the event being processed; the ref is a pointer
# that may lag it. When they disagree, the payload is the one to trust.
payload = tmp_path / "event.json"
payload.write_text(json.dumps({"pull_request": {"number": 180}}), encoding="utf-8")
monkeypatch.setenv("GITHUB_TOKEN", "ghp_fake")
monkeypatch.setenv("GITHUB_REPOSITORY", "owner/repo")
monkeypatch.setenv("GITHUB_EVENT_NAME", "pull_request")
monkeypatch.setenv("GITHUB_REF", "refs/pull/42/merge")
monkeypatch.setenv("GITHUB_EVENT_PATH", str(payload))
assert parse_config().pr_number == 180

@pytest.mark.parametrize(
"payload_text",
[
pytest.param('{"pull_request": {"number": ', id="truncated json"),
pytest.param("not json at all", id="not json"),
pytest.param('{"push": {"ref": "refs/heads/main"}}', id="another event"),
pytest.param('{"pull_request": null}', id="null pull_request"),
pytest.param('{"pull_request": {"number": "180"}}', id="number as a string"),
pytest.param('{"pull_request": {"number": true}}', id="number as a boolean"),
pytest.param('{"pull_request": {}}', id="no number"),
pytest.param("[]", id="payload is a list"),
],
)
def test_an_unusable_event_payload_falls_back_to_the_ref(
self, monkeypatch, tmp_path, payload_text
):
# None of these may raise. Each still has a usable ref, and a raise here
# would turn a recoverable run into a failed one.
payload = tmp_path / "event.json"
payload.write_text(payload_text, encoding="utf-8")
monkeypatch.setenv("GITHUB_TOKEN", "ghp_fake")
monkeypatch.setenv("GITHUB_REPOSITORY", "owner/repo")
monkeypatch.setenv("GITHUB_EVENT_NAME", "pull_request")
monkeypatch.setenv("GITHUB_REF", "refs/pull/42/merge")
monkeypatch.setenv("GITHUB_EVENT_PATH", str(payload))
assert parse_config().pr_number == 42

def test_a_missing_event_file_falls_back_to_the_ref(self, monkeypatch, tmp_path):
monkeypatch.setenv("GITHUB_TOKEN", "ghp_fake")
monkeypatch.setenv("GITHUB_REPOSITORY", "owner/repo")
monkeypatch.setenv("GITHUB_EVENT_NAME", "pull_request")
monkeypatch.setenv("GITHUB_REF", "refs/pull/42/merge")
monkeypatch.setenv("GITHUB_EVENT_PATH", str(tmp_path / "absent.json"))
assert parse_config().pr_number == 42

def test_no_pr_number_when_neither_source_has_one(self, monkeypatch):
monkeypatch.setenv("GITHUB_TOKEN", "ghp_fake")
monkeypatch.setenv("GITHUB_REPOSITORY", "owner/repo")
monkeypatch.setenv("GITHUB_EVENT_NAME", "push")
monkeypatch.setenv("GITHUB_REF", "refs/heads/main")
monkeypatch.delenv("GITHUB_EVENT_PATH", raising=False)
assert parse_config().pr_number is None

def test_coverage_threshold_range_validation(self, monkeypatch):
monkeypatch.setenv("GITHUB_TOKEN", "ghp_fake")
monkeypatch.setenv("GITHUB_REPOSITORY", "owner/repo")
Expand Down
Loading