diff --git a/contributors.py b/contributors.py index 2885f92..ddb0fdc 100644 --- a/contributors.py +++ b/contributors.py @@ -155,41 +155,53 @@ def get_contributors(repo: object, start_date: str, end_date: str, ghe: str): Returns: contributors (list): A list of ContributorStats objects """ - all_repo_contributors = repo.contributors() contributors = [] + endpoint = ghe if ghe else "https://github.com" try: - for user in all_repo_contributors: - # Ignore contributors with [bot] in their name - if "[bot]" in user.login: - continue - - # Check if user has commits in the date range - if start_date and end_date: - user_commits = repo.commits( - author=user.login, since=start_date, until=end_date + if start_date and end_date: + # Fetch commits in the date range and extract unique authors. + # This is much more efficient than iterating all-time contributors + # and checking each one for commits, which causes rate limiting + # on large repositories. + contributor_data = {} + for commit in repo.commits(since=start_date, until=end_date): + if commit.author is None: + continue + login = commit.author.login + if "[bot]" in login: + continue + if login not in contributor_data: + contributor_data[login] = { + "avatar_url": commit.author.avatar_url, + "contribution_count": 0, + } + contributor_data[login]["contribution_count"] += 1 + + for username, data in contributor_data.items(): + commit_url = f"{endpoint}/{repo.full_name}/commits?author={username}&since={start_date}&until={end_date}" + contributor = contributor_stats.ContributorStats( + username, + False, + data["avatar_url"], + data["contribution_count"], + commit_url, + "", ) - - # If the user has no commits in the date range, skip them - try: - next(user_commits) - except StopIteration: + contributors.append(contributor) + else: + for user in repo.contributors(): + if "[bot]" in user.login: continue - - # Store the contributor information in a ContributorStats object - endpoint = ghe if ghe else "https://github.com" - if start_date and end_date: - commit_url = f"{endpoint}/{repo.full_name}/commits?author={user.login}&since={start_date}&until={end_date}" - else: commit_url = f"{endpoint}/{repo.full_name}/commits?author={user.login}" - contributor = contributor_stats.ContributorStats( - user.login, - False, - user.avatar_url, - user.contributions_count, - commit_url, - "", - ) - contributors.append(contributor) + contributor = contributor_stats.ContributorStats( + user.login, + False, + user.avatar_url, + user.contributions_count, + commit_url, + "", + ) + contributors.append(contributor) except Exception as e: print(f"Error getting contributors for repository: {repo.full_name}") print(e) diff --git a/markdown.py b/markdown.py index 88db305..08b9250 100644 --- a/markdown.py +++ b/markdown.py @@ -230,7 +230,10 @@ def get_contributor_table( sponsor_info = _is_truthy(sponsor_info) show_avatar = _is_truthy(show_avatar) link_to_profile = _is_truthy(link_to_profile) - columns = ["Username", "All Time Contribution Count"] + if start_date and end_date: + columns = ["Username", "Contribution Count"] + else: + columns = ["Username", "All Time Contribution Count"] if show_avatar: columns.insert(0, "Avatar") if start_date and end_date: diff --git a/test_contributors.py b/test_contributors.py index 19ba046..e14bd9c 100644 --- a/test_contributors.py +++ b/test_contributors.py @@ -2,7 +2,7 @@ import runpy import unittest -from unittest.mock import MagicMock, call, patch +from unittest.mock import MagicMock, patch import contributors as contributors_module from contributor_stats import ContributorStats @@ -19,24 +19,27 @@ def test_get_contributors(self, mock_contributor_stats): Test the get_contributors function. """ mock_repo = MagicMock() - mock_user = MagicMock() - mock_user.login = "user" - mock_user.avatar_url = "https://avatars.githubusercontent.com/u/12345678?v=4" - mock_user.contributions_count = 100 - mock_repo.contributors.return_value = [mock_user] + mock_commit = MagicMock() + mock_commit.author.login = "user" + mock_commit.author.avatar_url = ( + "https://avatars.githubusercontent.com/u/12345678?v=4" + ) mock_repo.full_name = "owner/repo" - mock_repo.commits.return_value = iter([object()]) + mock_repo.commits.return_value = iter([mock_commit]) - contributors_module.get_contributors(mock_repo, "2022-01-01", "2022-12-31", "") + result = contributors_module.get_contributors( + mock_repo, "2022-01-01", "2022-12-31", "" + ) + self.assertEqual(len(result), 1) mock_repo.commits.assert_called_once_with( - author="user", since="2022-01-01", until="2022-12-31" + since="2022-01-01", until="2022-12-31" ) mock_contributor_stats.assert_called_once_with( "user", False, "https://avatars.githubusercontent.com/u/12345678?v=4", - 100, + 1, "https://github.com/owner/repo/commits?author=user&since=2022-01-01&until=2022-12-31", "", ) @@ -124,41 +127,34 @@ def test_get_all_contributors_with_repository(self, mock_get_contributors): ) @patch("contributors.contributor_stats.ContributorStats") - def test_get_contributors_skip_users_with_no_commits(self, mock_contributor_stats): + def test_get_contributors_with_single_commit(self, mock_contributor_stats): """ - Test the get_contributors function skips users with no commits in the date range. + Test get_contributors returns a single contributor for one commit in the date range. """ mock_repo = MagicMock() - mock_user = MagicMock() - mock_user.login = "user" - mock_user.avatar_url = "https://avatars.githubusercontent.com/u/12345678?v=4" - mock_user.contributions_count = 100 - mock_user2 = MagicMock() - mock_user2.login = "user2" - mock_user2.avatar_url = "https://avatars.githubusercontent.com/u/12345679?v=4" - mock_user2.contributions_count = 102 + mock_commit = MagicMock() + mock_commit.author.login = "user" + mock_commit.author.avatar_url = ( + "https://avatars.githubusercontent.com/u/12345678?v=4" + ) - mock_repo.contributors.return_value = [mock_user, mock_user2] mock_repo.full_name = "owner/repo" - mock_repo.commits.side_effect = [ - iter([object()]), # user has commits in range - iter([]), # user2 has no commits in range and should be skipped - ] + mock_repo.commits.return_value = iter([mock_commit]) ghe = "" - contributors_module.get_contributors(mock_repo, "2022-01-01", "2022-12-31", ghe) + result = contributors_module.get_contributors( + mock_repo, "2022-01-01", "2022-12-31", ghe + ) - mock_repo.commits.assert_has_calls( - [ - call(author="user", since="2022-01-01", until="2022-12-31"), - call(author="user2", since="2022-01-01", until="2022-12-31"), - ] + self.assertEqual(len(result), 1) + mock_repo.commits.assert_called_once_with( + since="2022-01-01", until="2022-12-31" ) mock_contributor_stats.assert_called_once_with( "user", False, "https://avatars.githubusercontent.com/u/12345678?v=4", - 100, + 1, "https://github.com/owner/repo/commits?author=user&since=2022-01-01&until=2022-12-31", "", ) @@ -169,19 +165,22 @@ def test_get_contributors_skip_bot(self, mock_contributor_stats): Test if the get_contributors function skips the bot user. """ mock_repo = MagicMock() - mock_user = MagicMock() - mock_user.login = "[bot]" - mock_user.avatar_url = "https://avatars.githubusercontent.com/u/12345678?v=4" - mock_user.contributions_count = 100 + mock_commit = MagicMock() + mock_commit.author.login = "[bot]" + mock_commit.author.avatar_url = ( + "https://avatars.githubusercontent.com/u/12345678?v=4" + ) - mock_repo.contributors.return_value = [mock_user] mock_repo.full_name = "owner/repo" + mock_repo.commits.return_value = iter([mock_commit]) ghe = "" - contributors_module.get_contributors(mock_repo, "2022-01-01", "2022-12-31", ghe) + result = contributors_module.get_contributors( + mock_repo, "2022-01-01", "2022-12-31", ghe + ) + self.assertEqual(result, []) # Ensure that the bot user is skipped and ContributorStats is never instantiated - mock_repo.commits.assert_not_called() mock_contributor_stats.assert_not_called() @patch("contributors.contributor_stats.ContributorStats") @@ -212,13 +211,8 @@ def test_get_contributors_no_commit_end_date(self, mock_contributor_stats): ) def test_get_contributors_skips_when_no_commits_in_range(self): - """Test get_contributors skips users with no commits in the date range.""" + """Test get_contributors returns empty list when no commits in the date range.""" mock_repo = MagicMock() - mock_user = MagicMock() - mock_user.login = "user" - mock_user.avatar_url = "https://avatars.githubusercontent.com/u/12345678?v=4" - mock_user.contributions_count = 100 - mock_repo.contributors.return_value = [mock_user] mock_repo.full_name = "owner/repo" mock_repo.commits.return_value = iter([]) @@ -228,6 +222,39 @@ def test_get_contributors_skips_when_no_commits_in_range(self): self.assertEqual(result, []) + def test_get_contributors_skips_none_author(self): + """Test get_contributors skips commits with no linked GitHub author.""" + mock_repo = MagicMock() + mock_repo.full_name = "owner/repo" + mock_commit = MagicMock() + mock_commit.author = None + mock_repo.commits.return_value = iter([mock_commit]) + + result = contributors_module.get_contributors( + mock_repo, "2022-01-01", "2022-12-31", "" + ) + + self.assertEqual(result, []) + + def test_get_contributors_aggregates_multiple_commits(self): + """Test get_contributors counts multiple commits per author correctly.""" + mock_repo = MagicMock() + mock_repo.full_name = "owner/repo" + mock_commit1 = MagicMock() + mock_commit1.author.login = "user" + mock_commit1.author.avatar_url = "https://avatars.githubusercontent.com/u/1" + mock_commit2 = MagicMock() + mock_commit2.author.login = "user" + mock_commit2.author.avatar_url = "https://avatars.githubusercontent.com/u/1" + mock_repo.commits.return_value = iter([mock_commit1, mock_commit2]) + + result = contributors_module.get_contributors( + mock_repo, "2022-01-01", "2022-12-31", "" + ) + + self.assertEqual(len(result), 1) + self.assertEqual(result[0].contribution_count, 2) + def test_get_contributors_handles_exception(self): """Test get_contributors returns None when an exception is raised.""" @@ -239,7 +266,7 @@ def __iter__(self): mock_repo = MagicMock() mock_repo.full_name = "owner/repo" - mock_repo.contributors.return_value = BoomIterable() + mock_repo.commits.return_value = BoomIterable() with patch("builtins.print") as mock_print: result = contributors_module.get_contributors( diff --git a/test_markdown.py b/test_markdown.py index 88f21b2..459ace0 100644 --- a/test_markdown.py +++ b/test_markdown.py @@ -67,7 +67,7 @@ def test_write_to_markdown( "| Total Contributors | Total Contributions | % New Contributors |\n" "| --- | --- | --- |\n" "| 2 | 300 | 50.0% |\n\n" - "| Username | All Time Contribution Count | New Contributor | " + "| Username | Contribution Count | New Contributor | " "Commits between 2023-01-01 and 2023-01-02 |\n" "| --- | --- | --- | --- |\n" "| @user1 | 100 | False | commit url |\n" @@ -133,7 +133,7 @@ def test_write_to_markdown_with_sponsors( "| Total Contributors | Total Contributions | % New Contributors |\n" "| --- | --- | --- |\n" "| 2 | 300 | 50.0% |\n\n" - "| Username | All Time Contribution Count | New Contributor | " + "| Username | Contribution Count | New Contributor | " "Sponsor URL | Commits between 2023-01-01 and 2023-01-02 |\n" "| --- | --- | --- | --- | --- |\n" "| @user1 | 100 | False | [Sponsor Link](sponsor_url_1) | commit url |\n" @@ -200,7 +200,7 @@ def test_write_to_markdown_with_avatars( "| Total Contributors | Total Contributions | % New Contributors |\n" "| --- | --- | --- |\n" "| 2 | 300 | 50.0% |\n\n" - "| Avatar | Username | All Time Contribution Count | New Contributor | " + "| Avatar | Username | Contribution Count | New Contributor | " "Commits between 2023-01-01 and 2023-01-02 |\n" "| --- | --- | --- | --- | --- |\n" '| | ' @@ -268,7 +268,7 @@ def test_write_to_markdown_without_link_to_profile( "| Total Contributors | Total Contributions | % New Contributors |\n" "| --- | --- | --- |\n" "| 2 | 300 | 50.0% |\n\n" - "| Username | All Time Contribution Count | New Contributor | " + "| Username | Contribution Count | New Contributor | " "Commits between 2023-01-01 and 2023-01-02 |\n" "| --- | --- | --- | --- |\n" "| user1 | 100 | False | commit url |\n" @@ -394,7 +394,7 @@ def test_write_to_markdown_with_organization( "| Total Contributors | Total Contributions | % New Contributors |\n" "| --- | --- | --- |\n" "| 2 | 300 | 50.0% |\n\n" - "| Username | All Time Contribution Count | New Contributor | " + "| Username | Contribution Count | New Contributor | " "Commits between 2023-01-01 and 2023-01-02 |\n" "| --- | --- | --- | --- |\n" "| @user1 | 100 | False | " @@ -442,7 +442,7 @@ def test_write_to_markdown_empty_collaborators( "| Total Contributors | Total Contributions | % New Contributors |\n" "| --- | --- | --- |\n" "| 0 | 0 | 0% |\n\n" - "| Username | All Time Contribution Count | New Contributor | " + "| Username | Contribution Count | New Contributor | " "Commits between 2023-01-01 and 2023-01-02 |\n" "| --- | --- | --- | --- |\n" "\n _this file was generated by the " @@ -555,7 +555,7 @@ def test_write_to_markdown_with_ghe( "| Total Contributors | Total Contributions | % New Contributors |\n" "| --- | --- | --- |\n" "| 1 | 100 | 0.0% |\n\n" - "| Username | All Time Contribution Count | New Contributor | " + "| Username | Contribution Count | New Contributor | " "Commits between 2023-01-01 and 2023-01-02 |\n" "| --- | --- | --- | --- |\n" "| @user1 | 100 | False | "