From 88dc330cd6d197102aa0a178d78458dc4eb4c787 Mon Sep 17 00:00:00 2001 From: Alex Moses Date: Fri, 31 Jul 2026 01:17:12 +0000 Subject: [PATCH 1/7] Bug-2038705: Migrate get_commit to PyGithub Migrate get_commit to use pygithub --- treeherder/utils/github.py | 21 ++++++++++++++++++++- 1 file changed, 20 insertions(+), 1 deletion(-) diff --git a/treeherder/utils/github.py b/treeherder/utils/github.py index 0256abf01d2..91734f3f813 100644 --- a/treeherder/utils/github.py +++ b/treeherder/utils/github.py @@ -89,7 +89,26 @@ def get_all_commits(owner, repo, params=None): def get_commit(owner, repo, sha, params=None): - return fetch_api(f"repos/{owner}/{repo}/commits/{sha}", params) + repo_object = pygithub_get_repo(owner, repo) + commit = repo_object.get_commit(sha) + # Create a commit dict to be returned + commit_dict = {} + + # Append file objects required by collector.py + commit_dict["files"] = [] + for file in commit.files: + f = {} + f["filename"] = file.filename + commit_dict["files"].append(f) + + # Append object required by ingest.py + ## Add committer date + commit_dict["commit"] = {"committer": {"date": commit.commit.committer.date}} + ## Add parent sha's + commit_dict["parents"] = [] + for parent in commit.parents: + commit_dict["parents"].append({"sha": parent.sha}) + return commit_dict def get_pull_request(owner, repo, pr_id): From 6ba5bdc6fccb22fa2309060f93171757894ff698 Mon Sep 17 00:00:00 2001 From: Alex Moses Date: Fri, 31 Jul 2026 01:54:56 +0000 Subject: [PATCH 2/7] Bug-2038705: Add docstring Add docstring to `get_commit` function --- treeherder/utils/github.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/treeherder/utils/github.py b/treeherder/utils/github.py index 91734f3f813..06d877a66d2 100644 --- a/treeherder/utils/github.py +++ b/treeherder/utils/github.py @@ -89,6 +89,10 @@ def get_all_commits(owner, repo, params=None): def get_commit(owner, repo, sha, params=None): + """ + Retrieve GitHub commit for a given sha. + Returns a standardized dictionary representing a commit. + """ repo_object = pygithub_get_repo(owner, repo) commit = repo_object.get_commit(sha) # Create a commit dict to be returned From 50fe34350466c709b9be7044a992e7dcc6c78fc7 Mon Sep 17 00:00:00 2001 From: Alex Moses Date: Fri, 31 Jul 2026 06:47:00 +0000 Subject: [PATCH 3/7] Bug-2038705: Add mocks for testing Added mocks for mocking PyGithub Commit objects --- tests/utils/test_github.py | 35 ++++++++++++++++++++++++++++++++++- 1 file changed, 34 insertions(+), 1 deletion(-) diff --git a/tests/utils/test_github.py b/tests/utils/test_github.py index aada7663a08..15a9942000c 100644 --- a/tests/utils/test_github.py +++ b/tests/utils/test_github.py @@ -5,6 +5,35 @@ from treeherder.utils.github import get_releases +# Mock GitCommit and it's related classes +class MockCommitParent: + def __init__(self, sha): + self.sha = sha + + +class MockCommitFile: + def __init__(self, filename): + self.filename = filename + + +class MockCommitter: + def __init__(self, date): + self.date = date + + +class MockInnerCommit: + def __init__(self, committer_date): + self.committer = MockCommitter(committer_date) + + +class MockCommit: + def __init__(self, sha, committer_date, parents=None, files=None): + self.sha = sha + self.commit = MockInnerCommit(committer_date) + self.parents = [MockCommitParent(parent_sha) for parent_sha in parents] if parents else [] + self.files = [MockCommitFile(filename) for filename in files] if files else [] + + # Helper for MockGitRelease class MockAuthor: def __init__(self, login): @@ -63,8 +92,9 @@ def __repr__(self): # Mock Repository class to simulate PyGithub's Repository objects class MockRepository: - def __init__(self, releases): + def __init__(self, releases=None, commits=None): self._releases = releases + self._commits = commits def get_releases(self): # PyGithub's get_releases returns an iterable (PaginatedList), @@ -72,6 +102,9 @@ def get_releases(self): # Returning a list directly simulates this behavior for the mock. return self._releases + def get_commits(self): + return self._commits + @patch("treeherder.utils.github.github") def test_get_releases_no_params(mock_github): From dc8cc25a04d5112e24a7ec1350fbf8029ea28233 Mon Sep 17 00:00:00 2001 From: Alex Moses Date: Fri, 31 Jul 2026 07:41:13 +0000 Subject: [PATCH 4/7] Bug-2038705: Add tests for testing get_commit Added tests for testing get_commit scenarios: - Standard commit - Initial commit without parents - Commit without files --- tests/utils/test_github.py | 117 +++++++++++++++++++++++++++++++++++-- 1 file changed, 111 insertions(+), 6 deletions(-) diff --git a/tests/utils/test_github.py b/tests/utils/test_github.py index 15a9942000c..44027cc83d8 100644 --- a/tests/utils/test_github.py +++ b/tests/utils/test_github.py @@ -1,6 +1,8 @@ from datetime import UTC, datetime from unittest.mock import patch +import pytest + # Import the function to be tested from treeherder.utils.github import get_releases @@ -30,8 +32,28 @@ class MockCommit: def __init__(self, sha, committer_date, parents=None, files=None): self.sha = sha self.commit = MockInnerCommit(committer_date) - self.parents = [MockCommitParent(parent_sha) for parent_sha in parents] if parents else [] - self.files = [MockCommitFile(filename) for filename in files] if files else [] + self.parents = [MockCommitParent(p_sha) for p_sha in parents] if parents else [] + self.files = [MockCommitFile(f_name) for f_name in files] if files else [] + + +@pytest.fixture +def github_commit_mock(): + """ + A factory fixture that patches the github object, sets up a MockRepository, + and returns a helper function to easily register commits. + """ + with patch("treeherder.utils.github.github") as mock_github: + mock_repo = MockRepository() + mock_github.get_repo.return_value = mock_repo + + def _register(sha, committer_date, parents=None, files=None): + commit_obj = MockCommit( + sha=sha, committer_date=committer_date, parents=parents, files=files + ) + mock_repo._commits[sha] = commit_obj + return mock_github, mock_repo, commit_obj + + yield _register # Helper for MockGitRelease @@ -93,8 +115,8 @@ def __repr__(self): # Mock Repository class to simulate PyGithub's Repository objects class MockRepository: def __init__(self, releases=None, commits=None): - self._releases = releases - self._commits = commits + self._releases = releases or [] + self._commits = commits or {} def get_releases(self): # PyGithub's get_releases returns an iterable (PaginatedList), @@ -102,8 +124,8 @@ def get_releases(self): # Returning a list directly simulates this behavior for the mock. return self._releases - def get_commits(self): - return self._commits + def get_commit(self, sha): + return self._commits[sha] @patch("treeherder.utils.github.github") @@ -326,3 +348,86 @@ def test_get_releases_with_number_and_since_params(mock_github): ] assert len(result_s3) == 3 assert result_s3 == expected_s3 + + +def test_get_commit_standard(github_commit_mock): + """ + Test get_commit returns a dictionary representing a standard commit with files, parents, and committer date. + """ + owner = "test-owner" + repo = "test-repo" + sha = "abc123commitsha" + date_str = "2023-01-01T12:00:00Z" + + mock_github, _, _ = github_commit_mock( + sha=sha, + committer_date=date_str, + parents=["parentsha1", "parentsha2"], + files=["file1.py", "file2.py"], + ) + + from treeherder.utils.github import get_commit + + result = get_commit(owner, repo, sha) + + # Assertions + mock_github.get_repo.assert_called_once_with(f"{owner}/{repo}") + assert result == { + "files": [{"filename": "file1.py"}, {"filename": "file2.py"}], + "commit": {"committer": {"date": date_str}}, + "parents": [{"sha": "parentsha1"}, {"sha": "parentsha2"}], + } + + +def test_get_commit_initial_commit(github_commit_mock): + """ + Test get_commit handles an initial/root commit with no parents. + """ + owner = "test-owner" + repo = "test-repo" + sha = "initialcommitsha" + date_str = "2023-01-01T00:00:00Z" + + github_commit_mock( + sha=sha, + committer_date=date_str, + parents=[], + files=["README.md"], + ) + + from treeherder.utils.github import get_commit + + result = get_commit(owner, repo, sha) + + assert result == { + "files": [{"filename": "README.md"}], + "commit": {"committer": {"date": date_str}}, + "parents": [], + } + + +def test_get_commit_no_files(github_commit_mock): + """ + Test get_commit handles a commit with no files changed. + """ + owner = "test-owner" + repo = "test-repo" + sha = "nofilescommitsha" + date_str = "2023-01-02T10:00:00Z" + + github_commit_mock( + sha=sha, + committer_date=date_str, + parents=["parentsha"], + files=[], + ) + + from treeherder.utils.github import get_commit + + result = get_commit(owner, repo, sha) + + assert result == { + "files": [], + "commit": {"committer": {"date": date_str}}, + "parents": [{"sha": "parentsha"}], + } From 85de84c22e0c9cc38a9f6ac6219216a96cf52246 Mon Sep 17 00:00:00 2001 From: Alex Moses Date: Fri, 31 Jul 2026 08:17:34 +0000 Subject: [PATCH 5/7] Bug-2038705: Update tests in changelog Update the tests in tests/changelog/ to reflect changes in github.py --- tests/changelog/test_collector.py | 14 ++++++++++++++ tests/changelog/test_tasks.py | 13 +++++++++++++ 2 files changed, 27 insertions(+) diff --git a/tests/changelog/test_collector.py b/tests/changelog/test_collector.py index c215a52d4c0..fea5d5f5b04 100644 --- a/tests/changelog/test_collector.py +++ b/tests/changelog/test_collector.py @@ -68,8 +68,22 @@ def test_collect(mock_pygithub_get_repo): mock_release.html_url = "mock_release_url" mock_release.author = mock_author + # Mock the file and commit object + mock_file1 = mock.Mock() + mock_file1.filename = "file1" + mock_file2 = mock.Mock() + mock_file2.filename = "file2" + + mock_commit = mock.Mock() + mock_commit.files = [mock_file1, mock_file2] + mock_commit.commit.comitter.date = now.isoformat() + mock_parent = mock.Mock() + mock_parent.sha = "mock_parent_sha" + mock_commit.parents = [mock_parent] + mock_repo = mock.Mock() mock_repo.get_releases.return_value = [mock_release] + mock_repo.get_commit.return_value = [mock_commit] mock_pygithub_get_repo.return_value = mock_repo prepare_responses() diff --git a/tests/changelog/test_tasks.py b/tests/changelog/test_tasks.py index 2ac82638847..f36d6193068 100644 --- a/tests/changelog/test_tasks.py +++ b/tests/changelog/test_tasks.py @@ -25,8 +25,21 @@ def test_update_changelog(mock_pygithub_get_repo): mock_release.html_url = "mock_release_url" mock_release.author = mock_author + mock_file1 = mock.Mock() + mock_file1.filename = "file1" + mock_file2 = mock.Mock() + mock_file2.filename = "file2" + + mock_commit = mock.Mock() + mock_commit.files = [mock_file1, mock_file2] + mock_commit.commit.comitter.date = now.isoformat() + mock_parent = mock.Mock() + mock_parent.sha = "mock_parent_sha" + mock_commit.parents = [mock_parent] + mock_repo = mock.Mock() mock_repo.get_releases.return_value = [mock_release] + mock_repo.get_commit.return_value = [mock_commit] mock_pygithub_get_repo.return_value = mock_repo prepare_responses() From ab84c6a5859eb09fa69d3bb13d2bb08f841c74b4 Mon Sep 17 00:00:00 2001 From: Alex Moses Date: Fri, 31 Jul 2026 08:25:50 +0000 Subject: [PATCH 6/7] Bug-2038705: Fixed issues with tests Fixed return value of get_commit mock in tests and typo in committer. --- tests/changelog/test_collector.py | 14 +++++++------- tests/changelog/test_tasks.py | 4 ++-- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/tests/changelog/test_collector.py b/tests/changelog/test_collector.py index fea5d5f5b04..94725df7db2 100644 --- a/tests/changelog/test_collector.py +++ b/tests/changelog/test_collector.py @@ -36,8 +36,8 @@ def _commit(): }, } - def commit(request): - return 200, {}, json.dumps(_commit()) + # def commit(request): + # return 200, {}, json.dumps(_commit()) def commits(request): return 200, {}, json.dumps([_commit()]) @@ -45,9 +45,9 @@ def commits(request): 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.add_callback( + # responses.GET, COMMIT_INFO, callback=commit, content_type="application/json" + # ) @responses.activate @@ -76,14 +76,14 @@ def test_collect(mock_pygithub_get_repo): mock_commit = mock.Mock() mock_commit.files = [mock_file1, mock_file2] - mock_commit.commit.comitter.date = now.isoformat() + mock_commit.commit.committer.date = now.isoformat() mock_parent = mock.Mock() mock_parent.sha = "mock_parent_sha" mock_commit.parents = [mock_parent] mock_repo = mock.Mock() mock_repo.get_releases.return_value = [mock_release] - mock_repo.get_commit.return_value = [mock_commit] + mock_repo.get_commit.return_value = mock_commit mock_pygithub_get_repo.return_value = mock_repo prepare_responses() diff --git a/tests/changelog/test_tasks.py b/tests/changelog/test_tasks.py index f36d6193068..c71eda7ad08 100644 --- a/tests/changelog/test_tasks.py +++ b/tests/changelog/test_tasks.py @@ -32,14 +32,14 @@ def test_update_changelog(mock_pygithub_get_repo): mock_commit = mock.Mock() mock_commit.files = [mock_file1, mock_file2] - mock_commit.commit.comitter.date = now.isoformat() + mock_commit.commit.committer.date = now.isoformat() mock_parent = mock.Mock() mock_parent.sha = "mock_parent_sha" mock_commit.parents = [mock_parent] mock_repo = mock.Mock() mock_repo.get_releases.return_value = [mock_release] - mock_repo.get_commit.return_value = [mock_commit] + mock_repo.get_commit.return_value = mock_commit mock_pygithub_get_repo.return_value = mock_repo prepare_responses() From 6fc943483138d9339807ead2972add70c09f45cf Mon Sep 17 00:00:00 2001 From: Alex Moses Date: Fri, 31 Jul 2026 08:27:45 +0000 Subject: [PATCH 7/7] Bug-2038705: Remove commit from prepare_responses prepare_responses no longer returns commit. It only patches get_commits --- tests/changelog/test_collector.py | 6 ------ 1 file changed, 6 deletions(-) diff --git a/tests/changelog/test_collector.py b/tests/changelog/test_collector.py index 94725df7db2..dc519f5f0c8 100644 --- a/tests/changelog/test_collector.py +++ b/tests/changelog/test_collector.py @@ -36,18 +36,12 @@ def _commit(): }, } - # def commit(request): - # return 200, {}, json.dumps(_commit()) - def commits(request): return 200, {}, json.dumps([_commit()]) 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