test: cover remaining contributor paths

[vchrombie]

how much coverage is too much coverage

Pull Request

Proposed Changes

Readiness Checklist

Author/Contributor

• If documentation is needed for this change, has that been included in this pull request
• run make lint and fix any issues that you have introduced
• run make test and ensure you have test coverage for the lines you are introducing
• If publishing new data to the public (scorecards, security scan results, code quality results, live dashboards, etc.), please request review from @jeffrey-luszcz

Reviewer

• Label as either bug, documentation, enhancement, infrastructure, maintenance or breaking

[Copilot]

Pull request overview

This PR expands unit test coverage for remaining contributor-related execution paths and a couple of supporting utilities/docs.

Changes:

• Add tests for get_contributors() edge cases (no commits in range, exception handling) and main() execution paths.
• Add tests for env.get_int_env_var() invalid parsing and contributor_stats (__repr__, sponsor info error path).
• Adjust README badges formatting to be on separate lines.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.

File	Description
test_env.py	Adds coverage for invalid integer env var parsing.
test_contributors.py	Adds coverage for additional contributor filtering/error paths and script/main execution.
test_contributor_stats.py	Adds coverage for __repr__ and sponsor info error handling.
README.md	Splits badges onto separate lines for readability.

[Copilot]

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.

[Copilot]

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated 4 comments.

[Copilot]

env.get_env_vars() returns sponsor_info as a boolean (see env.get_bool_env_var / get_env_vars), but this test uses the string "true". This makes the test diverge from real behavior and can mask the fact that contributors.main() currently compares sponsor_info == "true" instead of treating it as a boolean.

Suggested change

	"true",
	True,

[Copilot]

In this test call, repository_list is passed as an empty string, but get_all_contributors() is typed/used as List[str]. Passing [] here would better match the real call sites and avoid accidentally exercising the for repo in repository_list branch if organization becomes falsy in the future.

Suggested change

	"org", "", "2022-01-01", "2022-12-31", mock_github_connection, ghe
	"org", [], "2022-01-01", "2022-12-31", mock_github_connection, ghe

[Copilot]

get_contributors() checks commits via repo.commits(...) and next(user_commits), but this test configures mock_repo.get_commits, which is never used. As written, it won’t reliably exercise the “skip users with no commits” path—configure mock_repo.commits to return an empty iterator for the user you want skipped (and consider including mock_user2 in mock_repo.contributors.return_value to validate the filtering).

[Copilot]

This test doesn’t assert anything meaningful: mock_contributor_stats.isEmpty() is just another mock call and won’t fail if ContributorStats was instantiated. Prefer asserting the constructor was not called (e.g., assert_not_called) and remove the unused mock_repo.get_commits setup (the implementation calls repo.commits).

Suggested change

	# Note that only user is returned and user2 is not returned here because there were no commits in the date range
	mock_contributor_stats.isEmpty()
	# Ensure that the bot user is skipped and ContributorStats is never instantiated
	mock_contributor_stats.assert_not_called()