Skip to content

Commit 80f6d33

Browse files
authored
Merge pull request #60 from github/copilot/fix-f59d06a4-9b51-43be-ab8b-a323456aec3f
2 parents bffa376 + 2ad7d98 commit 80f6d33

File tree

4 files changed

+224
-6
lines changed

4 files changed

+224
-6
lines changed

README.md

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,7 @@ interface OrgData {
6161

6262
**Field Descriptions:**
6363

64-
- `username` (string): The GitHub username of the employee (case-sensitive)
64+
- `username` (string): The GitHub username of the employee (case-insensitive matching)
6565
- `manager` (string): The GitHub username of the employee's direct manager
6666

6767
#### Comprehensive org-data.json Examples
@@ -190,7 +190,7 @@ interface OrgData {
190190

191191
1. **All Contributors Must Be Included**: Every GitHub username that appears in the repository's contributor list must have an entry in org-data.json
192192
2. **Manager Chain**: Managers should also be included in the org-data.json file with their own manager relationships
193-
3. **Case Sensitivity**: GitHub usernames are case-sensitive and must match exactly
193+
3. **Case Insensitivity**: GitHub usernames are matched case-insensitively, so "username", "UserName", and "USERNAME" are treated as the same
194194
4. **JSON Validity**: The file must be valid JSON format
195195
5. **UTF-8 Encoding**: The file should be saved with UTF-8 encoding
196196

@@ -253,7 +253,7 @@ If Alice created the repository:
253253

254254
**Common Issues:**
255255

256-
1. **Username Mismatch**: Ensure GitHub usernames match exactly (case-sensitive)
256+
1. **Username Mismatch**: Usernames are matched case-insensitively, but ensure usernames exist in org-data.json
257257
2. **Missing Contributors**: All repository contributors must be in org-data.json
258258
3. **Invalid JSON**: Validate JSON syntax using online validators
259259
4. **Manager Loops**: Avoid circular manager relationships
@@ -692,7 +692,7 @@ The tool determines team ownership by:
692692
}
693693
}
694694
```
695-
2. Verify GitHub usernames are spelled correctly (case-sensitive)
695+
2. Verify GitHub usernames are spelled correctly (matched case-insensitively)
696696
3. Include bot accounts if needed (they're auto-excluded if containing "[bot]")
697697

698698
#### Memory and Performance Issues
@@ -762,7 +762,7 @@ The tool automatically splits large files, but you can:
762762
- [ ] `org-data.json` exists in repository root
763763
- [ ] `org-data.json` is valid JSON
764764
- [ ] All repository contributors are included in org-data.json
765-
- [ ] GitHub usernames match exactly (case-sensitive)
765+
- [ ] GitHub usernames are included in org-data.json (case-insensitive matching)
766766
- [ ] Manager relationships are defined for all users
767767

768768
### Debugging Steps

measure_innersource.py

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
from logging_config import get_logger, setup_logging
1717
from markdown_helpers import markdown_too_large_for_issue_body, split_markdown_file
1818
from markdown_writer import write_to_markdown
19+
from requests.structures import CaseInsensitiveDict
1920

2021

2122
def evaluate_markdown_file_size(output_file: str) -> None:
@@ -144,7 +145,9 @@ def main(): # pragma: no cover
144145
if org_data_path.exists():
145146
logger.info("Reading in org data from org-data.json...")
146147
with open(org_data_path, "r", encoding="utf-8") as org_file:
147-
org_data = json.load(org_file)
148+
raw_org_data = json.load(org_file)
149+
# Wrap org_data in case-insensitive dictionary for username lookups
150+
org_data = CaseInsensitiveDict(raw_org_data)
148151
logger.info("Org data read successfully.")
149152
else:
150153
logger.warning(

requirements.txt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,2 +1,3 @@
11
github3.py==4.0.1
22
python-dotenv==1.1.1
3+
requests==2.31.0

test_case_insensitive_lookup.py

Lines changed: 214 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,214 @@
1+
"""Unit tests for case-insensitive username lookup in org-data.json."""
2+
3+
import json
4+
import os
5+
import tempfile
6+
import unittest
7+
from pathlib import Path
8+
from unittest.mock import MagicMock, patch
9+
10+
from measure_innersource import CaseInsensitiveDict, main
11+
12+
13+
class TestCaseInsensitiveDict(unittest.TestCase):
14+
"""Test suite for the CaseInsensitiveDict class."""
15+
16+
def setUp(self):
17+
"""Set up test data."""
18+
self.test_data = {
19+
"alice": {"manager": "bob"},
20+
"Bob": {"manager": "charlie"},
21+
"Charlie": {"manager": "david"},
22+
"DAVID": {"manager": "eve"},
23+
}
24+
self.case_insensitive_dict = CaseInsensitiveDict(self.test_data)
25+
26+
def test_getitem_lowercase_key(self):
27+
"""Test getting an item with a lowercase key."""
28+
result = self.case_insensitive_dict["alice"]
29+
self.assertEqual(result, {"manager": "bob"})
30+
31+
def test_getitem_uppercase_key(self):
32+
"""Test getting an item with an uppercase key."""
33+
result = self.case_insensitive_dict["ALICE"]
34+
self.assertEqual(result, {"manager": "bob"})
35+
36+
def test_getitem_mixedcase_key(self):
37+
"""Test getting an item with a mixed case key."""
38+
result = self.case_insensitive_dict["Alice"]
39+
self.assertEqual(result, {"manager": "bob"})
40+
41+
def test_getitem_original_case_preserved(self):
42+
"""Test that original case in values is preserved."""
43+
result = self.case_insensitive_dict["bob"]
44+
self.assertEqual(result, {"manager": "charlie"})
45+
result = self.case_insensitive_dict["BOB"]
46+
self.assertEqual(result, {"manager": "charlie"})
47+
48+
def test_getitem_key_not_found(self):
49+
"""Test that KeyError is raised for missing keys."""
50+
with self.assertRaises(KeyError):
51+
_ = self.case_insensitive_dict["nonexistent"]
52+
53+
def test_contains_lowercase(self):
54+
"""Test __contains__ with lowercase key."""
55+
self.assertTrue("alice" in self.case_insensitive_dict)
56+
57+
def test_contains_uppercase(self):
58+
"""Test __contains__ with uppercase key."""
59+
self.assertTrue("ALICE" in self.case_insensitive_dict)
60+
61+
def test_contains_mixedcase(self):
62+
"""Test __contains__ with mixed case key."""
63+
self.assertTrue("Alice" in self.case_insensitive_dict)
64+
65+
def test_contains_not_found(self):
66+
"""Test __contains__ returns False for missing keys."""
67+
self.assertFalse("nonexistent" in self.case_insensitive_dict)
68+
69+
def test_get_with_default(self):
70+
"""Test get method with default value."""
71+
result = self.case_insensitive_dict.get("nonexistent", "default")
72+
self.assertEqual(result, "default")
73+
74+
def test_get_without_default(self):
75+
"""Test get method returns None if key not found and no default."""
76+
result = self.case_insensitive_dict.get("nonexistent")
77+
self.assertIsNone(result)
78+
79+
def test_get_case_insensitive(self):
80+
"""Test get method with case-insensitive key."""
81+
result = self.case_insensitive_dict.get("ALICE")
82+
self.assertEqual(result, {"manager": "bob"})
83+
84+
def test_items(self):
85+
"""Test items method returns original data."""
86+
items = list(self.case_insensitive_dict.items())
87+
self.assertEqual(len(items), 4)
88+
# Check that original keys are preserved
89+
keys = [k for k, v in items]
90+
self.assertIn("alice", keys)
91+
self.assertIn("Bob", keys)
92+
self.assertIn("Charlie", keys)
93+
self.assertIn("DAVID", keys)
94+
95+
def test_keys(self):
96+
"""Test keys method returns original keys."""
97+
keys = list(self.case_insensitive_dict.keys())
98+
self.assertEqual(len(keys), 4)
99+
self.assertIn("alice", keys)
100+
self.assertIn("Bob", keys)
101+
self.assertIn("Charlie", keys)
102+
self.assertIn("DAVID", keys)
103+
104+
def test_values(self):
105+
"""Test values method returns all values."""
106+
values = list(self.case_insensitive_dict.values())
107+
self.assertEqual(len(values), 4)
108+
109+
def test_duplicate_case_insensitive_keys(self):
110+
"""Test behavior with duplicate case-insensitive keys."""
111+
duplicate_data = {
112+
"alice": {"manager": "bob"},
113+
"Alice": {"manager": "charlie"}, # Duplicate!
114+
}
115+
# requests.structures.CaseInsensitiveDict handles duplicates by keeping the last value
116+
ci_dict = CaseInsensitiveDict(duplicate_data)
117+
# Both lowercase and mixed case should return the same value (the last one)
118+
self.assertEqual(ci_dict["alice"], {"manager": "charlie"})
119+
self.assertEqual(ci_dict["Alice"], {"manager": "charlie"})
120+
121+
122+
class TestCaseInsensitiveLookupIntegration(unittest.TestCase):
123+
"""Integration tests for case-insensitive username lookup in measure_innersource."""
124+
125+
@patch("measure_innersource.evaluate_markdown_file_size")
126+
@patch("measure_innersource.auth_to_github")
127+
@patch("measure_innersource.get_env_vars")
128+
@patch("measure_innersource.write_to_markdown")
129+
def test_username_lookup_case_insensitive(
130+
self,
131+
mock_write,
132+
mock_get_env_vars,
133+
mock_auth,
134+
_mock_evaluate,
135+
):
136+
"""Test that username lookups in org-data.json are case-insensitive."""
137+
# Create a temporary org-data.json file with lowercase username
138+
org_data = {
139+
"jeffrey-luszcz": {"manager": "team-lead"},
140+
"team-lead": {"manager": "director"},
141+
}
142+
143+
# Mock environment variables
144+
mock_env = MagicMock()
145+
mock_env.gh_token = "test_token"
146+
mock_env.owner = "test_owner"
147+
mock_env.repo = "test_repo"
148+
mock_env.report_title = "Test Report"
149+
mock_env.output_file = "test_output.md"
150+
mock_env.ghe = ""
151+
mock_env.gh_app_id = None
152+
mock_env.gh_app_installation_id = None
153+
mock_env.gh_app_private_key_bytes = None
154+
mock_env.gh_app_enterprise_only = False
155+
mock_env.chunk_size = 100
156+
mock_env.owning_team = None # Force using original commit author path
157+
mock_get_env_vars.return_value = mock_env
158+
159+
# Mock GitHub connection and repository
160+
mock_github = MagicMock()
161+
mock_auth.return_value = mock_github
162+
163+
mock_repo = MagicMock()
164+
mock_github.repository.return_value = mock_repo
165+
mock_repo.full_name = "test_owner/test_repo"
166+
167+
# Mock first commit with different case username
168+
mock_commit = MagicMock()
169+
mock_commit.author.login = "Jeffrey-Luszcz" # Different case!
170+
mock_repo.commits.return_value = iter([mock_commit])
171+
172+
# Mock contributors with different case
173+
mock_contributor = MagicMock()
174+
mock_contributor.login = "Jeffrey-Luszcz"
175+
mock_repo.contributors.return_value = [mock_contributor]
176+
177+
# Mock PRs and issues (empty for simplicity)
178+
mock_repo.pull_requests.return_value = iter([])
179+
mock_repo.issues.return_value = iter([])
180+
181+
# Create temp directory for org-data.json
182+
with tempfile.TemporaryDirectory() as tmpdir:
183+
org_data_path = Path(tmpdir) / "org-data.json"
184+
with open(org_data_path, "w", encoding="utf-8") as f:
185+
json.dump(org_data, f)
186+
187+
# Change to temp directory and run test
188+
original_dir = os.getcwd()
189+
try:
190+
os.chdir(tmpdir)
191+
192+
# Run main
193+
main()
194+
195+
# Verify that write_to_markdown was called (meaning no KeyError occurred)
196+
mock_write.assert_called_once()
197+
198+
# Get the arguments passed to write_to_markdown
199+
call_args = mock_write.call_args
200+
kwargs = call_args.kwargs if call_args.kwargs else {}
201+
202+
# Verify the original commit author is correctly identified
203+
self.assertEqual(kwargs.get("original_commit_author"), "Jeffrey-Luszcz")
204+
# Verify the manager was looked up correctly (case-insensitive)
205+
self.assertEqual(
206+
kwargs.get("original_commit_author_manager"), "team-lead"
207+
)
208+
209+
finally:
210+
os.chdir(original_dir)
211+
212+
213+
if __name__ == "__main__":
214+
unittest.main()

0 commit comments

Comments
 (0)