Bug-2038705: Replace requests-based GitHub API calls with PyGithub across ETL modules

.vscode/settings.json

@@ -14,5 +14,7 @@
   "eslint.validate": ["javascript", "javascriptreact", "typescript", "typescriptreact"],
   "eslint.options": {
     "overrideConfigFile": "eslint.config.mjs"
-  }
+  },
+  "python.testing.unittestEnabled": false,
+  "python.testing.pytestEnabled": true
 }

tests/changelog/test_collector.py

@@ -1,10 +1,7 @@
 import binascii
-import json
 import os
-import re
 from datetime import datetime, timedelta
-
-import responses
+from unittest.mock import MagicMock, patch
 
 from treeherder.changelog.collector import collect
 
@@ -13,67 +10,47 @@ def random_id():
     return binascii.hexlify(os.urandom(16)).decode("utf8")
 
 
-RELEASES = re.compile(r"https://api.github.com/repos/.*/.*/releases.*")
-COMMITS = re.compile(r"https://api.github.com/repos/.*/.*/commits\?.*")
-COMMIT_INFO = re.compile(r"https://api.github.com/repos/.*/.*/commits/.*")
-
-
 def prepare_responses():
-    now = datetime.now().strftime("%Y-%m-%dT%H:%M:%S")
+    now = datetime.now()
 
-    def releases(request):
-        data = [
-            {
-                "name": "ok",
-                "published_at": now,
-                "id": random_id(),
-                "html_url": "url",
-                "tag_name": "some tag",
-                "author": {"login": "tarek"},
-            }
-        ]
-        return 200, {}, json.dumps(data)
+    mock_release = MagicMock()
+    mock_release.published_at = now
+    mock_release.author.login = "tarek"
+    mock_release.name = "ok"
+    mock_release.tag_name = "some tag"
+    mock_release.id = random_id()
+    mock_release.html_url = "url"
 
-    responses.add_callback(
-        responses.GET, RELEASES, callback=releases, content_type="application/json"
-    )
+    mock_commit = MagicMock()
+    mock_commit.sha = random_id()
+    mock_commit.html_url = "url"
+    mock_commit.commit.message = "yeah"
+    mock_commit.commit.author.name = "tarek"
+    mock_commit.commit.author.date = now
 
-    def _commit():
-        files = [{"filename": "file1"}, {"filename": "file2"}]
-        return {
-            "files": files,
-            "name": "ok",
-            "sha": random_id(),
-            "html_url": "url",
-            "tag_name": "some tag",
-            "commit": {
-                "message": "yeah",
-                "author": {"name": "tarek", "date": now},
-                "files": files,
-            },
-        }
+    mock_detailed_commit = MagicMock()
+    mock_file = MagicMock()
+    mock_file.filename = "file1"
+    mock_detailed_commit.files = [mock_file]
 
-    def commit(request):
-        return 200, {}, json.dumps(_commit())
+    mock_repo = MagicMock()
+    mock_repo.get_releases.return_value = [mock_release]
+    mock_repo.get_commits.return_value = [mock_commit]
+    mock_repo.get_commit.return_value = mock_detailed_commit
 
-    def commits(request):
-        return 200, {}, json.dumps([_commit()])
+    return mock_repo
 
-    responses.add_callback(
-        responses.GET, COMMITS, callback=commits, content_type="application/json"
-    )
-    responses.add_callback(
-        responses.GET, COMMIT_INFO, callback=commit, content_type="application/json"
-    )
 
-
-@responses.activate
 def test_collect():
     yesterday = datetime.now() - timedelta(days=1)
-    yesterday = yesterday.strftime("%Y-%m-%dT%H:%M:%S")
-    prepare_responses()
-    res = list(collect(yesterday))
+    mock_repo = prepare_responses()
+
+    with patch("treeherder.utils.github.github_client") as mock_gh:
+        mock_gh.get_repo.return_value = mock_repo
+        res = list(collect(yesterday))
 
-    # we're not looking into much details here, we can do this
-    # once we start to tweak the filters
-    assert len(res) > 0
+        assert len(res) > 0
+        # Verify it includes the release and the commit
+        types = [r["type"] for r in res]
+        assert "release" in types
+        assert "commit" in types

tests/changelog/test_tasks.py

@@ -1,18 +1,20 @@
+from unittest.mock import patch
+
 import pytest
-import responses
 
 from tests.changelog.test_collector import prepare_responses
 from treeherder.changelog.models import Changelog
 from treeherder.changelog.tasks import update_changelog
 
 
 @pytest.mark.django_db()
-@responses.activate
 def test_update_changelog():
-    prepare_responses()
+    mock_repo = prepare_responses()
     num_entries = Changelog.objects.count()
 
-    update_changelog()
+    with patch("treeherder.utils.github.github_client") as mock_gh:
+        mock_gh.get_repo.return_value = mock_repo
+        update_changelog()
 
     # we're not looking into much details here, we can do this
     # once we start to tweak the filters

tests/etl/test_ingest_command.py

@@ -1,3 +1,5 @@
+from unittest.mock import MagicMock
+
 from treeherder.etl.management.commands import ingest
 
 REPO_META = {
@@ -10,64 +12,59 @@
 
 
 def test_query_data_consumes_compare_dict(monkeypatch):
-    """query_data must read the GitHub compare REST response as a dict.
+    """query_data must use PyGithub objects correctly.
 
-    Regression guard for Bug 2009865, which switched ``compare_shas`` to return
-    a list of PyGithub commit objects (for the Pulse push loader) but left
-    query_data doing dict access (``.get("merge_base_commit")`` / ``["commits"]``),
-    breaking the ``ingest push`` command for GitHub repos.
+    Regression guard for Bug 2038705 refactor.
     """
-    compare_by_range = {
-        # base branch vs head: the head isn't on the base branch, so the API
-        # reports a merge base whose parent is the real fork point.
-        "main...HEAD": {
-            "merge_base_commit": {
-                "sha": "BASE",
-                "commit": {"committer": {"date": "2026-01-01T00:00:00Z"}},
-                "parents": [
-                    {
-                        "sha": "PARENT",
-                        "url": "https://api.github.com/repos/o/r/commits/PARENT",
-                    }
-                ],
-            },
-            "commits": [],
-        },
-        # re-compare with the corrected base yields the push's commits
-        "PARENT...HEAD": {
-            "merge_base_commit": {"sha": "PARENT", "parents": []},
-            "commits": [
-                {
-                    "sha": "C1",
-                    "commit": {
-                        "message": "Fix the thing",
-                        "author": {"name": "Dev", "email": "dev@example.com"},
-                        "committer": {"date": "2026-02-02T00:00:00Z"},
-                    },
-                }
-            ],
-        },
-    }
 
-    def fake_fetch_api(path, params=None):
-        return compare_by_range[path.split("/compare/")[1]]
+    mock_repo = MagicMock()
+
+    def create_mock_commit(sha, date, parents=None, message="Fix the issue"):
+        mock_commit = MagicMock()
+        mock_commit.sha = sha
+        mock_commit.commit.committer.name = "Dev"
+        mock_commit.commit.committer.email = "dev@example.com"
+        mock_commit.commit.committer.date = date
+        mock_commit.commit.author.name = "Dev"
+        mock_commit.commit.author.email = "dev@example.com"
+        mock_commit.commit.author.date = date
+        mock_commit.commit.message = message
+        mock_commit.parents = parents or []
+        return mock_commit
+
+    date1 = "2026-01-01T00:00:00Z"
+    date2 = "2026-02-02T00:00:00Z"
+
+    parent_commit = create_mock_commit("PARENT", date2)
+    base_commit = create_mock_commit("BASE", date1, parents=[parent_commit])
+    c1 = create_mock_commit("C1", date2)
+
+    mock_comparison1 = MagicMock()
+    mock_comparison1.merge_base_commit = base_commit
+    mock_comparison1.commits = []
+
+    mock_comparison2 = MagicMock()
+    mock_comparison2.merge_base_commit = parent_commit
+    mock_comparison2.commits = [c1]
 
-    def fake_fetch_api_full_url(url, params=None):
-        # The merge-base parent, with a committer date different from the merge
-        # base so query_data takes the simple (non-recursive) branch.
-        return {"sha": "PARENT", "commit": {"committer": {"date": "2026-02-02T00:00:00Z"}}}
+    def fake_compare(base, head):
+        if base == "main":
+            return mock_comparison1
+        if base == "PARENT":
+            return mock_comparison2
+        return MagicMock()
 
-    monkeypatch.setattr(ingest, "fetch_api", fake_fetch_api)
-    monkeypatch.setattr(ingest, "fetch_api_full_url", fake_fetch_api_full_url)
+    mock_repo.compare.side_effect = fake_compare
+    monkeypatch.setattr(ingest.github, "get_repository", lambda owner, repo_name: mock_repo)
 
     event_base_sha, commits = ingest.query_data(REPO_META, "HEAD")
 
     assert event_base_sha == "PARENT"
     assert commits == [
         {
-            "message": "Fix the thing",
-            "author": {"name": "Dev", "email": "dev@example.com"},
-            "committer": {"date": "2026-02-02T00:00:00Z"},
+            "message": "Fix the issue",
+            "author": {"name": "Dev", "email": "dev@example.com", "date": date2},
+            "committer": {"name": "Dev", "email": "dev@example.com", "date": date2},
             "id": "C1",
         }
     ]

tests/utils/test_github.py

@@ -0,0 +1,137 @@
+import unittest
+from unittest.mock import Mock, patch
+
+from treeherder.utils import github
+
+
+class TestGithubUtils(unittest.TestCase):
+    @patch("treeherder.utils.github.github_client")
+    def test_get_repository(self, mock_github_instance):
+        mock_repo = Mock()
+        mock_github_instance.get_repo.return_value = mock_repo
+        owner = "test_owner"
+        repo_name = "test_repo"
+        result = github.get_repository(owner, repo_name)
+        mock_github_instance.get_repo.assert_called_once_with(
+            full_name_or_id=f"{owner}/{repo_name}"
+        )
+        self.assertEqual(result, mock_repo)
+
+    @patch("treeherder.utils.github.get_repository")
+    def test_compare_shas(self, mock_get_repository):
+        mock_commit1 = Mock()
+        mock_commit2 = Mock()
+        mock_comparison = Mock()
+        mock_comparison.commits = [mock_commit1, mock_commit2]
+        mock_repo = Mock()
+        mock_repo.compare.return_value = mock_comparison
+        mock_get_repository.return_value = mock_repo
+
+        owner = "test_owner"
+        repo_name = "test_repo"
+        base = "base_sha"
+        head = "head_sha"
+        result = github.compare_shas(owner, repo_name, base, head)
+
+        mock_get_repository.assert_called_once_with(owner=owner, repo_name=repo_name)
+        mock_repo.compare.assert_called_once_with(base=base, head=head)
+        self.assertEqual(result, [mock_commit1, mock_commit2])
+
+    @patch("treeherder.utils.github.get_repository")
+    def test_get_comparison(self, mock_get_repository):
+        mock_comparison = Mock()
+        mock_repo = Mock()
+        mock_repo.compare.return_value = mock_comparison
+        mock_get_repository.return_value = mock_repo
+
+        owner = "test_owner"
+        repo_name = "test_repo"
+        base = "base_sha"
+        head = "head_sha"
+        result = github.get_comparison(owner, repo_name, base, head)
+
+        mock_get_repository.assert_called_once_with(owner=owner, repo_name=repo_name)
+        mock_repo.compare.assert_called_once_with(base=base, head=head)
+        self.assertEqual(result, mock_comparison)
+
+    @patch("treeherder.utils.github.get_repository")
+    def test_get_all_commits(self, mock_get_repository):
+        mock_commit1 = Mock()
+        mock_commit2 = Mock()
+        mock_repo = Mock()
+        mock_repo.get_commits.return_value = [mock_commit1, mock_commit2]
+        mock_get_repository.return_value = mock_repo
+
+        owner = "test_owner"
+        repo_name = "test_repo"
+        params = {"sha": "some_sha", "author": "test_author"}
+        result = github.get_all_commits(owner, repo_name, params)
+
+        mock_get_repository.assert_called_once_with(owner=owner, repo_name=repo_name)
+        mock_repo.get_commits.assert_called_once_with(**params)
+        self.assertEqual(result, [mock_commit1, mock_commit2])
+
+    @patch("treeherder.utils.github.get_repository")
+    def test_get_commit(self, mock_get_repository):
+        mock_commit = Mock()
+        mock_repo = Mock()
+        mock_repo.get_commit.return_value = mock_commit
+        mock_get_repository.return_value = mock_repo
+
+        owner = "test_owner"
+        repo_name = "test_repo"
+        sha = "test_sha"
+        result = github.get_commit(owner, repo_name, sha)
+
+        mock_get_repository.assert_called_once_with(owner=owner, repo_name=repo_name)
+        mock_repo.get_commit.assert_called_once_with(sha=sha)
+        self.assertEqual(result, mock_commit)
+
+    @patch("treeherder.utils.github.get_repository")
+    def test_get_pull_request(self, mock_get_repository):
+        mock_pr = Mock()
+        mock_repo = Mock()
+        mock_repo.get_pull.return_value = mock_pr
+        mock_get_repository.return_value = mock_repo
+
+        owner = "test_owner"
+        repo_name = "test_repo"
+        pr_number = 123
+        result = github.get_pull_request(owner, repo_name, pr_number)
+
+        mock_get_repository.assert_called_once_with(owner, repo_name)
+        mock_repo.get_pull.assert_called_once_with(pr_number)
+        self.assertEqual(result, mock_pr)
+
+    @patch("treeherder.utils.github.get_pull_request")
+    def test_get_pull_request_commits(self, mock_get_pull_request):
+        mock_commit1 = Mock()
+        mock_commit2 = Mock()
+        mock_pr = Mock()
+        mock_pr.get_commits.return_value = [mock_commit1, mock_commit2]
+        mock_get_pull_request.return_value = mock_pr
+
+        owner = "test_owner"
+        repo_name = "test_repo"
+        pr_number = 123
+        result = github.get_pull_request_commits(owner, repo_name, pr_number)
+
+        mock_get_pull_request.assert_called_once_with(owner, repo_name, pr_number)
+        mock_pr.get_commits.assert_called_once_with()
+        self.assertEqual(result, [mock_commit1, mock_commit2])
+
+    @patch("treeherder.utils.github.get_repository")
+    def test_get_releases(self, mock_get_repository):
+        mock_release1 = Mock()
+        mock_release2 = Mock()
+        mock_repo = Mock()
+        mock_repo.get_releases.return_value = [mock_release1, mock_release2]
+        mock_get_repository.return_value = mock_repo
+
+        owner = "test_owner"
+        repo_name = "test_repo"
+        result = github.get_releases(owner, repo_name)
+
+        mock_get_repository.assert_called_once_with(owner=owner, repo_name=repo_name)
+        mock_repo.get_releases.assert_called_once_with()
+        self.assertEqual(result, [mock_release1, mock_release2])