From f57d151131a5f710d559182a0c38f7392359d84d Mon Sep 17 00:00:00 2001 From: Glenn Jocher Date: Tue, 3 Jun 2025 17:49:30 -0700 Subject: [PATCH] Robust summarize_release.py previous tag (#475) Signed-off-by: Glenn Jocher Co-authored-by: UltralyticsAssistant --- actions/__init__.py | 2 +- actions/summarize_release.py | 23 ++++++++-- tests/test_summarize_release.py | 74 ++++++++++++++++++--------------- 3 files changed, 60 insertions(+), 39 deletions(-) diff --git a/actions/__init__.py b/actions/__init__.py index adbaf5c..d2d04bc 100644 --- a/actions/__init__.py +++ b/actions/__init__.py @@ -22,4 +22,4 @@ # ├── test_summarize_pr.py # └── ... -__version__ = "0.0.85" +__version__ = "0.0.86" diff --git a/actions/summarize_release.py b/actions/summarize_release.py index 6fb609b..d6522e6 100644 --- a/actions/summarize_release.py +++ b/actions/summarize_release.py @@ -25,7 +25,15 @@ def get_prs_between_tags(event, previous_tag: str, latest_tag: str) -> list: url = f"{GITHUB_API_URL}/repos/{event.repository}/compare/{previous_tag}...{latest_tag}" r = event.get(url) + if r.status_code != 200: + print(f"Failed to get comparison between {previous_tag} and {latest_tag}, status: {r.status_code}") + return [] + data = r.json() + if "commits" not in data: + print(f"No commits found in comparison between {previous_tag} and {latest_tag}") + return [] + pr_numbers = set() for commit in data["commits"]: pr_matches = re.findall(r"#(\d+)", commit["commit"]["message"]) @@ -139,9 +147,9 @@ def create_github_release(event, tag_name: str, name: str, body: str): event.post(url, json=data) -def get_previous_tag() -> str: - """Retrieves the previous Git tag, excluding the current tag, using the git describe command.""" - cmd = ["git", "describe", "--tags", "--abbrev=0", "--exclude", CURRENT_TAG] +def get_actual_previous_tag(current_tag: str) -> str: + """Gets the actual previous tag using git, excluding the current tag.""" + cmd = ["git", "describe", "--tags", "--abbrev=0", "--exclude", current_tag] try: return subprocess.run(cmd, check=True, text=True, capture_output=True).stdout.strip() except subprocess.CalledProcessError: @@ -156,7 +164,14 @@ def main(*args, **kwargs): if not all([event.token, CURRENT_TAG]): raise ValueError("One or more required environment variables are missing.") - previous_tag = PREVIOUS_TAG or get_previous_tag() + # Try PREVIOUS_TAG first, fall back to actual previous tag if comparison fails + previous_tag = PREVIOUS_TAG or get_actual_previous_tag(CURRENT_TAG) + + # Test if the previous tag works for comparison + test_url = f"{GITHUB_API_URL}/repos/{event.repository}/compare/{previous_tag}...{CURRENT_TAG}" + if event.get(test_url).status_code != 200: + previous_tag = get_actual_previous_tag(CURRENT_TAG) + print(f"Using actual previous tag: {previous_tag}") # Get the diff between the tags diff = get_release_diff(event, previous_tag, CURRENT_TAG) diff --git a/tests/test_summarize_release.py b/tests/test_summarize_release.py index 273aa86..e174dac 100644 --- a/tests/test_summarize_release.py +++ b/tests/test_summarize_release.py @@ -35,45 +35,51 @@ def test_get_prs_between_tags(): mock_event = MagicMock() mock_event.repository = "test/repo" - # Mock compare response - mock_compare_response = MagicMock() - mock_compare_response.json.return_value = { - "commits": [{"commit": {"message": "Fix bug #123"}}, {"commit": {"message": "Add feature #456"}}] - } + mock_event.get.side_effect = [ + MagicMock(status_code=200, json=lambda: {"commits": [{"commit": {"message": "Fix bug #123"}}]}), + MagicMock( + status_code=200, + json=lambda: { + "number": 123, + "title": "Fix bug", + "body": "Fix", + "user": {"login": "user1"}, + "html_url": "url", + "merged_at": "2023-01-01T12:00:00Z", + }, + ), + ] - # Mock PR responses - mock_pr1_response = MagicMock() - mock_pr1_response.status_code = 200 - mock_pr1_response.json.return_value = { - "number": 123, - "title": "Fix bug", - "body": "Fixes a bug", - "user": {"login": "user1"}, - "html_url": "https://github.com/test/repo/pull/123", - "merged_at": "2023-01-01T12:00:00Z", - } - - mock_pr2_response = MagicMock() - mock_pr2_response.status_code = 200 - mock_pr2_response.json.return_value = { - "number": 456, - "title": "Add feature", - "body": "Adds a new feature", - "user": {"login": "user2"}, - "html_url": "https://github.com/test/repo/pull/456", - "merged_at": "2023-01-02T12:00:00Z", - } - - # Set up get method to return different responses - mock_event.get.side_effect = [mock_compare_response, mock_pr1_response, mock_pr2_response] - - # Use patch to skip the sleep with patch("time.sleep"): prs = get_prs_between_tags(mock_event, "v1.0.0", "v1.1.0") - assert len(prs) == 2 + assert len(prs) == 1 assert prs[0]["number"] == 123 - assert prs[1]["number"] == 456 + + +def test_get_prs_between_tags_api_failure(): + """Test API failure returns empty list.""" + mock_event = MagicMock() + mock_event.get.return_value = MagicMock(status_code=404) + + assert get_prs_between_tags(mock_event, "v1.0.0", "v1.1.0") == [] + + +def test_get_prs_between_tags_missing_commits(): + """Test missing commits data returns empty list.""" + mock_event = MagicMock() + mock_event.get.return_value = MagicMock(status_code=200, json=lambda: {"message": "No commits"}) + + assert get_prs_between_tags(mock_event, "v1.0.0", "v1.1.0") == [] + + +def test_get_prs_between_tags_empty_commits(): + """Test empty commits list returns empty list.""" + mock_event = MagicMock() + mock_event.get.return_value = MagicMock(status_code=200, json=lambda: {"commits": []}) + + with patch("time.sleep"): + assert get_prs_between_tags(mock_event, "v1.0.0", "v1.1.0") == [] @patch("actions.summarize_release.get_completion")