Skip to content

Commit b40d8e5

Browse files
Copilotzkoppert
andcommitted
Address code review feedback: add duplicate key detection and move imports
Co-authored-by: zkoppert <6935431+zkoppert@users.noreply.github.com>
1 parent b103e8b commit b40d8e5

File tree

2 files changed

+34
-6
lines changed

2 files changed

+34
-6
lines changed

measure_innersource.py

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,17 +24,32 @@ class CaseInsensitiveDict:
2424
This class wraps a dictionary and provides case-insensitive access to its values
2525
while preserving the original data structure. It's used to handle GitHub usernames
2626
which should be matched case-insensitively.
27+
28+
Note: If the input dictionary contains keys that differ only in case (e.g., 'User'
29+
and 'user'), a ValueError will be raised during initialization to prevent ambiguity
30+
and data loss.
2731
"""
2832

2933
def __init__(self, data):
3034
"""Initialize the case-insensitive dictionary.
3135
3236
Args:
3337
data: A dictionary to wrap with case-insensitive key access
38+
39+
Raises:
40+
ValueError: If the dictionary contains duplicate keys that differ only in case
3441
"""
3542
self._data = data
3643
# Create a lowercase key mapping to original keys
37-
self._key_map = {key.lower(): key for key in data.keys()}
44+
self._key_map = {}
45+
for key in data.keys():
46+
normalized_key = key.lower()
47+
if normalized_key in self._key_map:
48+
raise ValueError(
49+
f"Duplicate case-insensitive keys found: '{self._key_map[normalized_key]}' "
50+
f"and '{key}' both normalize to '{normalized_key}'"
51+
)
52+
self._key_map[normalized_key] = key
3853

3954
def __getitem__(self, key):
4055
"""Get an item using case-insensitive key lookup.

test_case_insensitive_lookup.py

Lines changed: 18 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
"""Unit tests for case-insensitive username lookup in org-data.json."""
22

33
import json
4+
import os
5+
import tempfile
46
import unittest
57
from pathlib import Path
68
from unittest.mock import MagicMock, patch
@@ -104,6 +106,17 @@ def test_values(self):
104106
values = list(self.case_insensitive_dict.values())
105107
self.assertEqual(len(values), 4)
106108

109+
def test_duplicate_case_insensitive_keys(self):
110+
"""Test that ValueError is raised for duplicate case-insensitive keys."""
111+
duplicate_data = {
112+
"alice": {"manager": "bob"},
113+
"Alice": {"manager": "charlie"}, # Duplicate!
114+
}
115+
with self.assertRaises(ValueError) as context:
116+
CaseInsensitiveDict(duplicate_data)
117+
self.assertIn("Duplicate case-insensitive keys found", str(context.exception))
118+
self.assertIn("alice", str(context.exception).lower())
119+
107120

108121
class TestCaseInsensitiveLookupIntegration(unittest.TestCase):
109122
"""Integration tests for case-insensitive username lookup in measure_innersource."""
@@ -113,7 +126,11 @@ class TestCaseInsensitiveLookupIntegration(unittest.TestCase):
113126
@patch("measure_innersource.get_env_vars")
114127
@patch("measure_innersource.write_to_markdown")
115128
def test_username_lookup_case_insensitive(
116-
self, mock_write, mock_get_env_vars, mock_auth, mock_evaluate # pylint: disable=unused-argument
129+
self,
130+
mock_write,
131+
mock_get_env_vars,
132+
mock_auth,
133+
mock_evaluate, # pylint: disable=unused-argument
117134
):
118135
"""Test that username lookups in org-data.json are case-insensitive."""
119136
# Create a temporary org-data.json file with lowercase username
@@ -160,16 +177,12 @@ def test_username_lookup_case_insensitive(
160177
mock_repo.issues.return_value = iter([])
161178

162179
# Create temp directory for org-data.json
163-
import tempfile
164-
165180
with tempfile.TemporaryDirectory() as tmpdir:
166181
org_data_path = Path(tmpdir) / "org-data.json"
167182
with open(org_data_path, "w", encoding="utf-8") as f:
168183
json.dump(org_data, f)
169184

170185
# Change to temp directory and run test
171-
import os
172-
173186
original_dir = os.getcwd()
174187
try:
175188
os.chdir(tmpdir)

0 commit comments

Comments
 (0)