diff --git a/tests/changelog/test_collector.py b/tests/changelog/test_collector.py index c215a52d4c0..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 @@ -68,8 +62,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.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_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..c71eda7ad08 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.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_pygithub_get_repo.return_value = mock_repo prepare_responses() diff --git a/tests/utils/test_github.py b/tests/utils/test_github.py index aada7663a08..44027cc83d8 100644 --- a/tests/utils/test_github.py +++ b/tests/utils/test_github.py @@ -1,10 +1,61 @@ 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 +# 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(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 class MockAuthor: def __init__(self, login): @@ -63,8 +114,9 @@ def __repr__(self): # Mock Repository class to simulate PyGithub's Repository objects class MockRepository: - def __init__(self, releases): - self._releases = releases + def __init__(self, releases=None, commits=None): + self._releases = releases or [] + self._commits = commits or {} def get_releases(self): # PyGithub's get_releases returns an iterable (PaginatedList), @@ -72,6 +124,9 @@ def get_releases(self): # Returning a list directly simulates this behavior for the mock. return self._releases + def get_commit(self, sha): + return self._commits[sha] + @patch("treeherder.utils.github.github") def test_get_releases_no_params(mock_github): @@ -293,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"}], + } diff --git a/treeherder/utils/github.py b/treeherder/utils/github.py index 0256abf01d2..06d877a66d2 100644 --- a/treeherder/utils/github.py +++ b/treeherder/utils/github.py @@ -89,7 +89,30 @@ 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) + """ + 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 + 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):