Add coverage for Actions failure paths (#784)

tests/test_dispatch_actions.py

@@ -51,6 +51,47 @@ def test_get_pr_branch_fork():
     assert mock_run.call_count == 4  # clone, remote add, fetch, push
 
 
+def test_get_pr_branch_fork_requires_token():
+    """Fork PRs need a token to push the temporary branch."""
+    mock_event = MagicMock()
+    mock_event.event_data = {"issue": {"number": 456}}
+    mock_event.get_repo_data.return_value = {
+        "head": {"ref": "fork-branch", "repo": {"id": 2, "full_name": "fork/repo"}},
+        "base": {"repo": {"id": 1}},
+    }
+
+    try:
+        with patch("os.environ.get", return_value=None):
+            get_pr_branch(mock_event)
+    except ValueError as e:
+        assert str(e) == "GITHUB_TOKEN environment variable is not set"
+    else:
+        raise AssertionError("Expected missing token to raise")
+
+
+def test_get_pr_branch_fork_sanitizes_git_errors():
+    """Git errors should not leak the GitHub token."""
+    import subprocess
+
+    mock_event = MagicMock()
+    mock_event.event_data = {"issue": {"number": 456}}
+    mock_event.repository = "base/repo"
+    mock_event.get_repo_data.return_value = {
+        "head": {"ref": "fork-branch", "repo": {"id": 2, "full_name": "fork/repo"}},
+        "base": {"repo": {"id": 1}},
+    }
+    error = subprocess.CalledProcessError(128, ["git"], stderr=b"https://x-access-token:[email protected]")
+
+    try:
+        with patch("subprocess.run", side_effect=error), patch("os.environ.get", return_value="test-token"):
+            get_pr_branch(mock_event)
+    except RuntimeError as e:
+        assert "test-token" not in str(e)
+        assert "***TOKEN***" in str(e)
+    else:
+        raise AssertionError("Expected git failure to raise")
+
+
 def test_trigger_and_get_workflow_info():
     """Test triggering workflows and getting info."""
     mock_event = MagicMock()
@@ -90,6 +131,31 @@ def test_trigger_and_get_workflow_info():
     assert results[0]["run_number"] == 42
 
 
+def test_trigger_and_get_workflow_info_reports_dispatch_failures():
+    """Failed workflow dispatches are returned without polling for runs."""
+    mock_event = MagicMock()
+    mock_event.repository = "test/repo"
+    response = MagicMock()
+    response.status_code = 422
+    response.json.return_value = {"message": "No ref found for: deleted-branch"}
+    mock_event.post.return_value = response
+
+    with patch("time.sleep") as mock_sleep:
+        results = trigger_and_get_workflow_info(mock_event, "deleted-branch", ["ci.yml"])
+
+    assert results == [
+        {
+            "name": "Ci",
+            "file": "ci.yml",
+            "url": "https://github.com/test/repo/actions/workflows/ci.yml",
+            "run_number": None,
+            "error": "No ref found for: deleted-branch",
+        }
+    ]
+    mock_sleep.assert_not_called()
+    mock_event.get.assert_not_called()
+
+
 def test_trigger_and_get_workflow_info_with_temp_branch():
     """Test that temp branches are deleted after triggering workflows."""
     mock_event = MagicMock()
@@ -158,6 +224,41 @@ def test_update_comment_function():
     )
 
 
+def test_update_comment_skips_empty_actions():
+    """No comment update is needed if no workflow was triggered."""
+    mock_event = MagicMock()
+
+    update_comment(mock_event, f"Run tests {RUN_CI_KEYWORD}", RUN_CI_KEYWORD, [])
+
+    mock_event.patch.assert_not_called()
+
+
+def test_update_comment_reports_failed_action_guidance():
+    """Failed dispatch comments include deleted-branch guidance from the GitHub error."""
+    mock_event = MagicMock()
+    mock_event.repository = "test/repo"
+    mock_event.event_data = {"comment": {"id": 456}}
+
+    update_comment(
+        mock_event,
+        f"Run tests {RUN_CI_KEYWORD}",
+        RUN_CI_KEYWORD,
+        [
+            {
+                "name": "CI",
+                "file": "ci.yml",
+                "url": "https://github.com/test/repo/actions/workflows/ci.yml",
+                "run_number": None,
+                "error": "No ref found for: deleted-branch",
+            }
+        ],
+    )
+
+    body = mock_event.patch.call_args.kwargs["json"]["body"]
+    assert "No GitHub Actions workflows were started" in body
+    assert "restore it or open a new PR from an existing branch" in body
+
+
 def test_main_triggers_ci_only():
     """Test main function triggers only CI for the CI command."""
     with patch("actions.dispatch_actions.Action") as MockAction:
@@ -245,3 +346,20 @@ def test_main_skips_non_pr_comments():
 
         # Verify toggle_eyes_reaction was not called
         mock_event.toggle_eyes_reaction.assert_not_called()
+
+
+def test_main_skips_unauthorized_comments():
+    """Only org members can trigger workflows from PR comments."""
+    with patch("actions.dispatch_actions.Action") as MockAction:
+        mock_event = MockAction.return_value
+        mock_event.event_name = "issue_comment"
+        mock_event.event_data = {
+            "action": "created",
+            "issue": {"pull_request": {}},
+            "comment": {"body": f"Please run CI {RUN_CI_KEYWORD}", "user": {"login": "external"}},
+        }
+        mock_event.is_org_member.return_value = False
+
+        main()
+
+        mock_event.toggle_eyes_reaction.assert_not_called()

tests/test_github_report.py

@@ -1,5 +1,6 @@
 # Ultralytics 🚀 AGPL-3.0 License - https://ultralytics.com/license
 
+import urllib.error
 from datetime import datetime, timedelta, timezone
 
 from actions import failed_scheduled_actions, github_report
@@ -25,6 +26,115 @@ def test_paginate_only_skips_http_errors_when_allowed(monkeypatch):
         raise AssertionError("Expected org listing failure to raise")
 
 
+def test_failed_scheduled_actions_visibility_guards(capsys):
+    """Invalid and unsafe visibility inputs fall back to public repositories."""
+    assert failed_scheduled_actions.parse_visibility("public,invalid", "private") == ["public"]
+    assert failed_scheduled_actions.parse_visibility("invalid", "private") == ["public"]
+    assert failed_scheduled_actions.parse_visibility("private", "public") == ["public"]
+
+    output = capsys.readouterr().out
+    assert "Invalid visibility values: invalid" in output
+    assert "No valid visibility values" in output
+    assert "Restricting to public only" in output
+
+
+def test_github_get_requires_token(monkeypatch):
+    """GitHub API calls require an auth token."""
+    monkeypatch.delenv("GH_TOKEN", raising=False)
+    monkeypatch.delenv("GITHUB_TOKEN", raising=False)
+
+    try:
+        failed_scheduled_actions.github_get("/user")
+    except RuntimeError as e:
+        assert str(e) == "GH_TOKEN or GITHUB_TOKEN is required"
+    else:
+        raise AssertionError("Expected missing token to raise")
+
+
+def test_github_get_fetches_json(monkeypatch):
+    """GitHub API responses are decoded from the requested URL."""
+    requests = []
+
+    class Response:
+        def __enter__(self):
+            return self
+
+        def __exit__(self, exc_type, exc, traceback):
+            return False
+
+        def read(self):
+            return b'{"ok": true}'
+
+    def fake_urlopen(request, timeout):
+        requests.append((request.full_url, timeout, request.headers["Authorization"]))
+        return Response()
+
+    monkeypatch.setenv("GH_TOKEN", "token")
+    monkeypatch.setattr(failed_scheduled_actions.urllib.request, "urlopen", fake_urlopen)
+
+    assert failed_scheduled_actions.github_get("/repos/ultralytics/actions", {"page": 1}) == {"ok": True}
+    assert requests == [("https://api.github.com/repos/ultralytics/actions?page=1", 60, "Bearer token")]
+
+
+def test_github_get_skips_allowed_repo_errors(monkeypatch, capsys):
+    """Allowed per-repo 403/404 API misses are skipped unless they are rate limits."""
+
+    class Body:
+        def __init__(self, text):
+            self.text = text
+
+        def read(self):
+            return self.text
+
+        def close(self):
+            pass
+
+    monkeypatch.setenv("GH_TOKEN", "token")
+    monkeypatch.setattr(
+        failed_scheduled_actions.urllib.request,
+        "urlopen",
+        lambda *args, **kwargs: (_ for _ in ()).throw(
+            urllib.error.HTTPError("url", 404, "Not Found", {"X-RateLimit-Remaining": "1"}, Body(b"missing"))
+        ),
+    )
+
+    assert failed_scheduled_actions.github_get("/repos/missing/actions/runs", allow_skip=True) is None
+    assert "Skipping /repos/missing/actions/runs: 404" in capsys.readouterr().out
+
+    monkeypatch.setattr(
+        failed_scheduled_actions.urllib.request,
+        "urlopen",
+        lambda *args, **kwargs: (_ for _ in ()).throw(
+            urllib.error.HTTPError("url", 403, "Forbidden", {"X-RateLimit-Remaining": "0"}, Body(b"rate limit"))
+        ),
+    )
+    try:
+        failed_scheduled_actions.github_get("/orgs/ultralytics/repos", allow_skip=True)
+    except urllib.error.HTTPError as e:
+        assert e.code == 403
+    else:
+        raise AssertionError("Expected rate limit to raise")
+
+
+def test_paginate_collects_until_short_page(monkeypatch):
+    """Pagination stops after the first short page and sleeps between full pages."""
+    pages = [
+        [{"id": i} for i in range(100)],
+        [{"id": 100}],
+    ]
+    sleeps = []
+
+    def fake_get(path, params=None, token=None, allow_skip=False):
+        assert path == "/items"
+        return pages[params["page"] - 1]
+
+    monkeypatch.setattr(failed_scheduled_actions, "github_get", fake_get)
+    monkeypatch.setattr(failed_scheduled_actions.time, "sleep", lambda seconds: sleeps.append(seconds))
+
+    assert len(failed_scheduled_actions.paginate("/items")) == 101
+    assert sleeps == [0.2]
+
+
 def test_collect_failed_actions_latest_run_per_workflow(monkeypatch):
     """Only the latest default-branch run for each workflow should determine whether it is reported."""