From ee29cf84f89a746c92e51b40d46ad15e3ae804a5 Mon Sep 17 00:00:00 2001 From: Glenn Jocher Date: Thu, 2 Jul 2026 11:37:46 +0200 Subject: [PATCH] Add coverage for Actions failure paths (#784) --- tests/test_dispatch_actions.py | 118 +++++++++++++++++++++++++++++++++ tests/test_github_report.py | 110 ++++++++++++++++++++++++++++++ 2 files changed, 228 insertions(+) diff --git a/tests/test_dispatch_actions.py b/tests/test_dispatch_actions.py index eef0333..a371f1f 100644 --- a/tests/test_dispatch_actions.py +++ b/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:test-token@github.com") + + 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() diff --git a/tests/test_github_report.py b/tests/test_github_report.py index d960ada..68bb22d 100644 --- a/tests/test_github_report.py +++ b/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."""