Skip to content

Commit e5ac455

Browse files
authored
Merge pull request #57 from github/copilot/fix-c728582b-46a4-4f10-9c69-d5442b58f08d
feat: Add warning and graceful handling for missing users and contributors in org chart
2 parents 61c5e71 + 8b3d1ec commit e5ac455

File tree

2 files changed

+191
-1
lines changed

2 files changed

+191
-1
lines changed

measure_innersource.py

Lines changed: 23 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -168,10 +168,22 @@ def main(): # pragma: no cover
168168
logger.info("Analyzing first commit...")
169169
commits = repo_data.commits()
170170
# Paginate to the last page to get the oldest commit
171-
# commits is a GitHubIterator, so you can use .count to get total, then get the last one
171+
# commits is a GitHubIterator, so you can use .count to get total,
172+
# then get the last one
172173
commit_list = list(commits)
173174
first_commit = commit_list[-1] # The last in the list is the oldest
174175
original_commit_author = first_commit.author.login
176+
177+
# Check if original commit author exists in org chart
178+
if original_commit_author not in org_data:
179+
logger.warning(
180+
"Original commit author '%s' not found in org chart. "
181+
"Cannot determine team boundaries for InnerSource "
182+
"measurement.",
183+
original_commit_author,
184+
)
185+
return
186+
175187
original_commit_author_manager = org_data[original_commit_author]["manager"]
176188
logger.info(
177189
"Original commit author: %s, with manager: %s",
@@ -220,6 +232,16 @@ def main(): # pragma: no cover
220232
logger.info("Analyzing all contributors in the repository...")
221233
for contributor in repo_data.contributors():
222234
all_contributors.append(contributor.login)
235+
236+
# Check if contributor is not found in org chart
237+
if contributor.login not in org_data:
238+
logger.warning(
239+
"Contributor '%s' not found in org chart. "
240+
"Excluding from InnerSource analysis.",
241+
contributor.login,
242+
)
243+
continue
244+
223245
if (
224246
contributor.login not in team_members_that_own_the_repo
225247
and "[bot]" not in contributor.login

test_measure_innersource.py

Lines changed: 168 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,8 @@
11
"""Unit tests for measure_innersource.evaluate_markdown_file_size."""
22

3+
import json
4+
from unittest.mock import MagicMock, patch
5+
36
import measure_innersource as mi
47
import pytest
58

@@ -67,3 +70,168 @@ def test_evaluate_markdown_file_size_no_split(tmp_path, monkeypatch):
6770
# The file should remain unchanged
6871
assert file.exists()
6972
assert file.read_text(encoding="utf-8") == "small"
73+
74+
75+
def test_main_missing_user_in_org_chart(tmp_path, monkeypatch):
76+
"""Test main function logs warning and exits when original commit author
77+
is missing from org chart."""
78+
# Switch working directory to tmp_path
79+
monkeypatch.chdir(tmp_path)
80+
81+
# Create org-data.json with some users, but not the original commit author
82+
org_data = {
83+
"manager1": {"manager": "director1"},
84+
"user1": {"manager": "manager1"},
85+
"user2": {"manager": "manager1"},
86+
}
87+
88+
org_file = tmp_path / "org-data.json"
89+
org_file.write_text(json.dumps(org_data), encoding="utf-8")
90+
91+
# Mock GitHub repository and commit data
92+
mock_author = MagicMock()
93+
mock_author.login = "missing_user" # This user is not in org_data
94+
95+
mock_commit = MagicMock()
96+
mock_commit.author = mock_author
97+
98+
mock_repo = MagicMock()
99+
mock_repo.full_name = "test/repo"
100+
101+
# Mock commits to return our mock commit as the first/oldest commit
102+
mock_repo.commits.return_value = [mock_commit]
103+
104+
mock_github = MagicMock()
105+
mock_github.repository.return_value = mock_repo
106+
107+
# Mock environment variables
108+
mock_env_vars = MagicMock()
109+
mock_env_vars.github_token = "fake_token"
110+
mock_env_vars.github_enterprise_hostname = None
111+
mock_env_vars.github_org = "test"
112+
mock_env_vars.github_repo = "repo"
113+
mock_env_vars.gh_app_id = None
114+
mock_env_vars.gh_app_installation_id = None
115+
mock_env_vars.gh_app_private_key_bytes = None
116+
mock_env_vars.gh_app_enterprise_only = False
117+
mock_env_vars.report_title = "Test Report"
118+
mock_env_vars.output_file = "test_output.md"
119+
120+
# Apply mocks
121+
with patch("measure_innersource.get_env_vars", return_value=mock_env_vars), patch(
122+
"measure_innersource.auth_to_github", return_value=mock_github
123+
), patch("measure_innersource.setup_logging") as mock_setup_logging:
124+
125+
# Configure logging to capture our test
126+
mock_logger = MagicMock()
127+
mock_setup_logging.return_value = mock_logger
128+
129+
with patch("measure_innersource.get_logger", return_value=mock_logger):
130+
# Call main function
131+
mi.main()
132+
133+
# Verify that warning was logged about missing user
134+
mock_logger.warning.assert_called_with(
135+
"Original commit author '%s' not found in org chart. "
136+
"Cannot determine team boundaries for InnerSource "
137+
"measurement.",
138+
"missing_user",
139+
)
140+
141+
# Verify that the function returned early (didn't process further)
142+
# by checking that subsequent info logs were not called
143+
info_calls = [
144+
call[0][0] for call in mock_logger.info.call_args_list if call[0]
145+
]
146+
147+
# Should have logged about reading org data and analyzing first
148+
# commit, but should NOT have logged about original commit author
149+
# with manager
150+
assert "Reading in org data from org-data.json..." in info_calls
151+
assert "Analyzing first commit..." in info_calls
152+
153+
# Should NOT contain the log message about
154+
# "Original commit author: X, with manager: Y"
155+
assert not any(
156+
isinstance(msg, str)
157+
and "Original commit author:" in msg
158+
and "with manager:" in msg
159+
for msg in info_calls
160+
)
161+
162+
163+
def test_contributors_missing_from_org_chart_excluded(tmp_path, monkeypatch):
164+
"""Test that contributors missing from org chart are excluded from
165+
InnerSource analysis."""
166+
# Switch working directory to tmp_path
167+
monkeypatch.chdir(tmp_path)
168+
169+
# Create org-data.json with some users
170+
org_data = {
171+
"original_author": {"manager": "manager1"},
172+
"manager1": {"manager": "director1"},
173+
"user1": {"manager": "manager1"},
174+
}
175+
176+
org_file = tmp_path / "org-data.json"
177+
org_file.write_text(json.dumps(org_data), encoding="utf-8")
178+
179+
# Mock GitHub repository and commit data
180+
mock_original_author = MagicMock()
181+
mock_original_author.login = "original_author"
182+
183+
mock_commit = MagicMock()
184+
mock_commit.author = mock_original_author
185+
186+
# Mock contributors - include one that's not in org_data
187+
mock_contributor1 = MagicMock()
188+
mock_contributor1.login = "unknown_contributor" # Not in org_data
189+
190+
mock_repo = MagicMock()
191+
mock_repo.full_name = "test/repo"
192+
mock_repo.commits.return_value = [mock_commit]
193+
mock_repo.contributors.return_value = [mock_contributor1]
194+
# Mock empty pull requests and issues to avoid infinite loops
195+
mock_repo.pull_requests.return_value = iter([])
196+
mock_repo.issues.return_value = iter([])
197+
198+
mock_github = MagicMock()
199+
mock_github.repository.return_value = mock_repo
200+
201+
# Mock environment variables
202+
mock_env_vars = MagicMock()
203+
mock_env_vars.github_token = "fake_token"
204+
mock_env_vars.github_enterprise_hostname = None
205+
mock_env_vars.github_org = "test"
206+
mock_env_vars.github_repo = "repo"
207+
mock_env_vars.gh_app_id = None
208+
mock_env_vars.gh_app_installation_id = None
209+
mock_env_vars.gh_app_private_key_bytes = None
210+
mock_env_vars.gh_app_enterprise_only = False
211+
mock_env_vars.report_title = "Test Report"
212+
mock_env_vars.output_file = "test_output.md"
213+
mock_env_vars.chunk_size = 100
214+
215+
# Apply mocks
216+
with patch("measure_innersource.get_env_vars", return_value=mock_env_vars), patch(
217+
"measure_innersource.auth_to_github", return_value=mock_github
218+
), patch("measure_innersource.setup_logging") as mock_setup_logging, patch(
219+
"measure_innersource.write_to_markdown"
220+
), patch(
221+
"measure_innersource.evaluate_markdown_file_size"
222+
):
223+
224+
# Configure logging to capture our test
225+
mock_logger = MagicMock()
226+
mock_setup_logging.return_value = mock_logger
227+
228+
with patch("measure_innersource.get_logger", return_value=mock_logger):
229+
# Call main function
230+
mi.main()
231+
232+
# Verify that warning was logged about missing contributor
233+
mock_logger.warning.assert_any_call(
234+
"Contributor '%s' not found in org chart. "
235+
"Excluding from InnerSource analysis.",
236+
"unknown_contributor",
237+
)

0 commit comments

Comments
 (0)