Robust summarize_release.py previous tag (#475)

Signed-off-by: Glenn Jocher <[email protected]>
Co-authored-by: UltralyticsAssistant <[email protected]>

actions/__init__.py

@@ -22,4 +22,4 @@
 #     ├── test_summarize_pr.py
 #     └── ...
 
-__version__ = "0.0.85"
+__version__ = "0.0.86"

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)

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")