Repository navigation
test: add a fake GitHub API so collector fixes are tested in CI #563
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from all commits
Commits
Show all changes
4 commits
Select commit
Hold shift + click to select a range
75fdfa2
test: add a fake GitHub API so collector fixes are tested in CI
EngCaioFonseca 5596716
test: address review feedback on the fake GitHub fixture
EngCaioFonseca 9218f18
test: document what each resilience test failure means
EngCaioFonseca 650e7c3
test: document the celery_app stub order assumption
EngCaioFonseca File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,55 @@ | ||
| # GitHub resilience tests | ||
|
|
||
| Tests for collectors running against unusual repositories (issues disabled, PRs | ||
| disabled, repo deleted) using a fake GitHub API. They need no token, network, | ||
| database or Redis, and run in CI with the rest of `pytest`. | ||
|
|
||
| ```bash | ||
| uv run pytest tests/test_github_resilience | ||
| ``` | ||
|
|
||
| ## Testing a fix for a "collector crashes on repo X" bug | ||
|
|
||
| 1. Pick a scenario in `SCENARIOS` (`conftest.py`), or add one. Copy the status | ||
| codes from the real GitHub API (`curl -i` the failing URL from the bug | ||
| report) and note the repository and date beside it. Do not guess them. | ||
| 2. Call the collector's helper function against the scenario and assert what it | ||
| returns. Celery tasks (`@celery.task`) need a database and raise if called; | ||
| test the plain function the task calls, such as `retrieve_all_pr_data`. | ||
| 3. Confirm the test fails without your fix and passes with it. | ||
| 4. Check the neighboring scenarios still pass. A repo with only issues | ||
| disabled still has pull request comments on `/issues/comments`, so a fix | ||
| for one scenario must not skip data in another. | ||
| 5. Give the test a one-line docstring saying what a failure means. | ||
|
|
||
| Example: `test_message_collection.py` has a test for a known crash, marked | ||
| `xfail`. The fix makes it pass, and the marker is then removed: | ||
|
|
||
| ```python | ||
| import pytest | ||
| from unittest.mock import Mock | ||
|
|
||
| from collectoss.tasks.github.messages import fast_retrieve_all_pr_and_issue_messages | ||
|
|
||
|
|
||
| @pytest.mark.unit | ||
| def test_no_messages_when_issues_and_prs_disabled(fake_github): | ||
| fake_github("issues_and_prs_disabled") | ||
|
|
||
| messages = fast_retrieve_all_pr_and_issue_messages( | ||
| "https://github.com/owner/repo", Mock(), None, "test", None) | ||
|
|
||
| assert messages == [] | ||
| ``` | ||
|
|
||
| ## What the fixture gives you | ||
|
|
||
| - `gh = fake_github("scenario")` starts the fake. | ||
| - `gh.respond(path_regex, status, body)` adds a one-off response that takes | ||
| priority over the scenario. | ||
| - `gh.requests` lists every `(method, path)` the code asked for. | ||
| - A test fails if the code requests a path with no rule, even when the code | ||
| catches the error. | ||
|
|
||
| Not modeled yet: response headers (pagination, rate limits, redirects) and | ||
| query strings. | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,157 @@ | ||
| # SPDX-License-Identifier: MIT | ||
| """ | ||
| Fake GitHub API for testing collectors against unusual repositories (issues or | ||
| PRs disabled, repo deleted) with no token, network, database or Redis. | ||
|
|
||
| def test_something(fake_github): | ||
| gh = fake_github("issues_and_prs_disabled") | ||
| ...call a collector function and assert on the result... | ||
|
|
||
| See README.md for how to add a scenario. | ||
| """ | ||
| import os | ||
| import re | ||
| import sys | ||
| import types | ||
|
|
||
| import httpx | ||
| import pytest | ||
| from tenacity import wait_none | ||
| from unittest.mock import Mock | ||
|
|
||
| # issues.py reads this flag from the database at import time unless it is set. | ||
|
MoralCode marked this conversation as resolved.
Resolved
|
||
| os.environ.setdefault("COLLECTOSS_DEV", "1") | ||
|
|
||
| from collectoss.tasks.github.util import github_data_access as gda_module | ||
|
|
||
|
|
||
| def _task(*args, **kwargs): | ||
| """Stand-in for @celery.task: tasks need a database, so calling one raises.""" | ||
|
MoralCode marked this conversation as resolved.
Resolved
|
||
| def decorate(func): | ||
| def needs_database(*a, **kw): | ||
| raise RuntimeError( | ||
| f"{func.__name__} is a Celery task and needs a database. " | ||
| "Test the helper function it calls instead." | ||
| ) | ||
| return needs_database | ||
|
|
||
| if len(args) == 1 and callable(args[0]) and not kwargs: # bare @celery.task | ||
| return decorate(args[0]) | ||
| return decorate | ||
|
|
||
|
|
||
| # The real celery_app module reads Redis settings from the database at import time. | ||
| _celery_stub = types.ModuleType("collectoss.tasks.init.celery_app") | ||
| _celery_stub.celery_app = Mock(task=_task) | ||
| for _name in ("CoreRepoCollectionTask", "SecondaryRepoCollectionTask", | ||
| "FacadeRepoCollectionTask", "MLRepoCollectionTask"): | ||
| setattr(_celery_stub, _name, type(_name, (), {})) | ||
| # setdefault keeps any module already imported under this name, so if another | ||
| # test imports the real celery_app first, the stub is ignored and these tests | ||
| # depend on test order. Assigning sys.modules[...] directly would avoid that. | ||
| sys.modules.setdefault("collectoss.tasks.init.celery_app", _celery_stub) | ||
|
EngCaioFonseca marked this conversation as resolved.
Resolved
|
||
|
|
||
|
|
||
| REPO = r"^/repos/[^/]+/[^/]+$" | ||
| ISSUES = r"/issues$" | ||
| ISSUE_COMMENTS = r"/issues/\d+/comments$" # one issue or PR | ||
| ALL_COMMENTS = r"/issues/comments$" # whole repo, issues and PRs | ||
| PULLS = r"/pulls$" | ||
|
|
||
| NOT_FOUND = {"message": "Not Found"} | ||
| ISSUES_GONE = {"message": "Issues are disabled for this repo"} | ||
| COMMENT = {"id": 1, "body": "hello"} | ||
| PULL = {"number": 1} | ||
|
|
||
| # scenario -> [(path regex, status, JSON body)]; the first matching rule wins. | ||
| # "Observed" = checked against the real GitHub API on that repository and date. | ||
| SCENARIOS = { | ||
| "normal": [ | ||
| (REPO, 200, {"has_issues": True, "has_pull_requests": True}), | ||
| (ISSUE_COMMENTS, 200, [COMMENT]), | ||
| (ALL_COMMENTS, 200, [COMMENT]), | ||
| (ISSUES, 200, [{"number": 2}]), | ||
| (PULLS, 200, [PULL]), | ||
| ], | ||
| # Observed 2026-11-09 on openshift/release. | ||
| "issues_disabled": [ | ||
| (REPO, 200, {"has_issues": False, "has_pull_requests": True}), | ||
| (ISSUE_COMMENTS, 410, ISSUES_GONE), | ||
| (ALL_COMMENTS, 200, [COMMENT]), | ||
| (ISSUES, 200, [{"number": 1, "pull_request": {}}]), | ||
| (PULLS, 200, [PULL]), | ||
| ], | ||
| # Observed 2026-11-09 on kubernetes/apiextensions-apiserver. | ||
| "issues_and_prs_disabled": [ | ||
| (REPO, 200, {"has_issues": False, "has_pull_requests": False}), | ||
| (ISSUE_COMMENTS, 410, ISSUES_GONE), | ||
| (ALL_COMMENTS, 404, NOT_FOUND), | ||
| (ISSUES, 200, []), | ||
| (PULLS, 404, NOT_FOUND), | ||
| ], | ||
| # Observed 2026-11-09 on github/roadmap. | ||
| "prs_disabled": [ | ||
| (REPO, 200, {"has_issues": True, "has_pull_requests": False}), | ||
| (ISSUE_COMMENTS, 200, [COMMENT]), | ||
| (ALL_COMMENTS, 200, [COMMENT]), | ||
| (ISSUES, 200, [{"number": 2}]), | ||
| (PULLS, 404, NOT_FOUND), | ||
| ], | ||
| # Observed 2026-11-09 on a repository name that does not exist. | ||
| "repo_not_found": [ | ||
| (r"^/repos/", 404, NOT_FOUND), | ||
| ], | ||
| } | ||
|
|
||
|
|
||
| class FakeGitHub: | ||
| """Answers GitHub API requests from a list of rules and records them.""" | ||
|
|
||
| def __init__(self, rules): | ||
| self.rules = list(rules) | ||
| self.requests = [] # (method, path) of every request | ||
| self.unmatched = [] # requests no rule could answer | ||
|
|
||
| def respond(self, path_regex, status, body): | ||
| """Add a rule that takes priority over the scenario's rules.""" | ||
| self.rules.insert(0, (path_regex, status, body)) | ||
|
|
||
| def handle(self, request: httpx.Request) -> httpx.Response: | ||
| """Return the first matching rule's response, or 501 if none match.""" | ||
| path = request.url.path | ||
| self.requests.append((request.method, path)) | ||
| for regex, status, body in self.rules: | ||
| if re.search(regex, path): | ||
| return httpx.Response(status, json=body) | ||
| self.unmatched.append(f"{request.method} {path}") | ||
| return httpx.Response(501, json={"message": "FakeGitHub has no rule for this path"}) | ||
|
|
||
|
|
||
| @pytest.fixture | ||
| def fake_github(monkeypatch): | ||
| """Route all GitHub HTTP calls to a FakeGitHub scenario.""" | ||
| real_client = httpx.Client | ||
| started = [] | ||
|
|
||
| def start(scenario): | ||
| gh = FakeGitHub(SCENARIOS[scenario]) | ||
| started.append(gh) | ||
| monkeypatch.setattr( | ||
| gda_module.httpx, "Client", | ||
| lambda *a, **kw: real_client(transport=httpx.MockTransport(gh.handle)), | ||
| ) | ||
| key_client = Mock(request=Mock(return_value="fake-token")) | ||
| monkeypatch.setattr(gda_module, "KeyClient", Mock(return_value=key_client)) | ||
| # Keep the real retry logic but skip the 5s waits. | ||
| retrying = gda_module.GithubDataAccess._GithubDataAccess__make_request_with_retries.retry | ||
| monkeypatch.setattr(retrying, "wait", wait_none()) | ||
| return gh | ||
|
|
||
| yield start | ||
|
|
||
| # Fail even if the code under test caught the error. | ||
| for gh in started: | ||
| assert not gh.unmatched, ( | ||
| f"No rule for: {sorted(set(gh.unmatched))}. " | ||
| "Add one to the scenario or use gh.respond()." | ||
| ) | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,66 @@ | ||
| # SPDX-License-Identifier: MIT | ||
| """How the GitHub REST client reports unusual responses to collectors.""" | ||
| import pytest | ||
| from unittest.mock import Mock | ||
|
|
||
| from collectoss.tasks.github.util.github_data_access import ( | ||
| GithubDataAccess, ResourceGoneException, UrlNotFoundException, | ||
| ) | ||
|
|
||
|
|
||
| def client(): | ||
| return GithubDataAccess(None, Mock()) | ||
|
|
||
|
|
||
| @pytest.mark.unit | ||
| def test_404_raises_url_not_found(fake_github): | ||
| """Failure means a 404 no longer raises UrlNotFoundException, which | ||
| collectors catch to skip missing data.""" | ||
| fake_github("repo_not_found") | ||
| with pytest.raises(UrlNotFoundException): | ||
| client().get_resource("https://api.github.com/repos/owner/repo") | ||
|
|
||
|
|
||
| @pytest.mark.unit | ||
| def test_410_raises_resource_gone_with_github_message(fake_github): | ||
| """Failure means a 410 no longer raises ResourceGoneException with | ||
| GitHub's reason.""" | ||
| fake_github("issues_disabled") | ||
| with pytest.raises(ResourceGoneException, match="Issues are disabled"): | ||
| client().get_resource("https://api.github.com/repos/owner/repo/issues/1/comments") | ||
|
|
||
|
|
||
| @pytest.mark.unit | ||
| @pytest.mark.parametrize("scenario, expected", [ | ||
| ("normal", True), | ||
| ("prs_disabled", False), | ||
| ("issues_and_prs_disabled", False), | ||
| # Current behavior, not a requirement. | ||
| ("repo_not_found", False), | ||
| ]) | ||
| def test_check_prs_enabled(fake_github, scenario, expected): | ||
| """Failure means disabled pull requests are detected wrongly, so PR | ||
| collection would crash or be skipped by mistake.""" | ||
| fake_github(scenario) | ||
| assert client().check_prs_enabled("owner", "repo") is expected | ||
|
|
||
|
|
||
| @pytest.mark.unit | ||
| def test_respond_overrides_the_scenario(fake_github): | ||
| """Failure means gh.respond() no longer overrides a scenario rule.""" | ||
| gh = fake_github("normal") | ||
| gh.respond(r"/pulls$", 404, {"message": "Not Found"}) | ||
| assert client().check_prs_enabled("owner", "repo") is False | ||
|
|
||
|
|
||
| @pytest.mark.unit | ||
| def test_unknown_path_fails_the_test_even_if_the_error_is_caught(fake_github): | ||
| """Failure means requests with no rule are no longer recorded, so a test | ||
| could pass although the fake never answered.""" | ||
| gh = fake_github("normal") | ||
| try: | ||
| client().get_resource("https://api.github.com/repos/owner/repo/releases") | ||
| except Exception: | ||
| pass # as a collector with a broad except would | ||
| assert set(gh.unmatched) == {"GET /repos/owner/repo/releases"} | ||
| gh.unmatched.clear() # asserted above; skip the teardown check |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,32 @@ | ||
| # SPDX-License-Identifier: MIT | ||
| """Issue collection on repos with issues disabled.""" | ||
| import pytest | ||
| from unittest.mock import Mock | ||
|
|
||
| from collectoss.tasks.github.issues import collect_issues, retrieve_all_issue_data | ||
|
|
||
|
|
||
| def collect(): | ||
| return list(retrieve_all_issue_data("https://github.com/owner/repo", Mock(), None)) | ||
|
|
||
|
|
||
| @pytest.mark.unit | ||
| @pytest.mark.parametrize("scenario, expected", [ | ||
| ("normal", [{"number": 2}]), | ||
| # /issues still lists pull requests. | ||
| ("issues_disabled", [{"number": 1, "pull_request": {}}]), | ||
| ("issues_and_prs_disabled", []), | ||
| ]) | ||
| def test_issue_collection(fake_github, scenario, expected): | ||
| """Failure means issue collection crashes or returns the wrong items on a | ||
| repo with issues disabled.""" | ||
| fake_github(scenario) | ||
| assert collect() == expected | ||
|
|
||
|
|
||
| @pytest.mark.unit | ||
| def test_celery_tasks_cannot_be_called_directly(): | ||
| """Failure means a Celery task ran without a database, so a test calling | ||
| one could pass without testing anything.""" | ||
| with pytest.raises(RuntimeError, match="Celery task"): | ||
| collect_issues("https://github.com/owner/repo", True) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,32 @@ | ||
| # SPDX-License-Identifier: MIT | ||
| """Message collection on repos with issues and/or pull requests disabled.""" | ||
| import pytest | ||
| from unittest.mock import Mock | ||
|
|
||
| from collectoss.tasks.github.messages import fast_retrieve_all_pr_and_issue_messages | ||
| from collectoss.tasks.github.util.github_data_access import UrlNotFoundException | ||
|
|
||
| COMMENT = {"id": 1, "body": "hello"} | ||
|
|
||
|
|
||
| def collect_messages(): | ||
| return fast_retrieve_all_pr_and_issue_messages( | ||
| "https://github.com/owner/repo", Mock(), None, "test", None) | ||
|
|
||
|
|
||
| @pytest.mark.unit | ||
| @pytest.mark.parametrize("scenario", ["normal", "issues_disabled"]) | ||
| def test_comments_are_collected(fake_github, scenario): | ||
| """Failure means comments are being dropped. With issues disabled, | ||
| /issues/comments still returns the pull request comments.""" | ||
| fake_github(scenario) | ||
| assert collect_messages() == [COMMENT] | ||
|
|
||
|
|
||
| @pytest.mark.unit | ||
| @pytest.mark.xfail(strict=True, raises=UrlNotFoundException, reason="#503") | ||
| def test_no_messages_when_issues_and_prs_disabled(fake_github): | ||
| """Known crash (#503): this should return no messages. If it is reported | ||
| as XPASS, the crash is fixed: remove the xfail marker.""" | ||
| fake_github("issues_and_prs_disabled") | ||
| assert collect_messages() == [] |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,26 @@ | ||
| # SPDX-License-Identifier: MIT | ||
| """PR collection must skip repos with PRs disabled or missing instead of crashing.""" | ||
| import pytest | ||
| from unittest.mock import Mock | ||
|
|
||
| from collectoss.tasks.github.pull_requests.tasks import retrieve_all_pr_data | ||
|
|
||
|
|
||
| @pytest.mark.unit | ||
| @pytest.mark.parametrize("scenario, expected_prs", [ | ||
| ("normal", [{"number": 1}]), | ||
| ("issues_disabled", [{"number": 1}]), | ||
| ("prs_disabled", []), | ||
| ("issues_and_prs_disabled", []), | ||
| ("repo_not_found", []), | ||
| ]) | ||
| def test_pr_collection(fake_github, scenario, expected_prs): | ||
| """Failure means PR collection crashes, returns the wrong PRs, or fetches | ||
| the PR list from a repo without pull requests.""" | ||
| gh = fake_github(scenario) | ||
| prs = list(retrieve_all_pr_data("https://github.com/owner/repo", Mock(), None, None)) | ||
| assert prs == expected_prs | ||
|
|
||
| fetched_list = ("GET", "/repos/owner/repo/pulls") in gh.requests | ||
| assert fetched_list == bool(expected_prs), ( | ||
| "the PR list should only be fetched when pull requests are enabled") |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.