Repository navigation
tests: add real coverage for _generate_installed_modules - #7525
hikmetba-bit wants to merge 1 commit into
Conversation
Fixes getsentry#4594. The previous test for this function (removed in getsentry#4593) just mirrored the implementation's own loop, so it never exercised any behavior independent of the code it was testing. This mocks importlib.metadata.distributions() with a small fixed set of fake distributions and asserts on the actual contract: names are normalized case-insensitively, entries with no Name are skipped, entries with no version are skipped, and a case-insensitive duplicate name is skipped in favor of the first occurrence. Verified the test actually catches a regression by temporarily breaking the dedup check locally and confirming the test fails, then reverting. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Want reviews to match your repository better? Bugbot Learning can learn team-specific rules from PR activity. A team admin can enable Learning in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 3e86efa. Configure here.
|
|
||
| with mock.patch( | ||
| "importlib.metadata.distributions", return_value=fake_distributions | ||
| ): |
There was a problem hiding this comment.
Test fails on older Python versions
Medium Severity
The new test patches importlib.metadata.distributions, which does not exist on Python 3.6 and 3.7. Common CI still runs tests/test_utils.py on those versions, so the test errors when applying the patch instead of covering _generate_installed_modules.
Reviewed by Cursor Bugbot for commit 3e86efa. Configure here.
|
We don't need tests for private helpers. I'll close the PR and the issue. |
Summary
Fixes #4594.
The previous test for
_generate_installed_moduleswas removed in #4593 because it was pointless — it just re-implemented the function's own loop and compared the two, so it never exercised any behavior independent of the implementation.Change
Added
test_generate_installed_modulestotests/test_utils.py, which mocksimportlib.metadata.distributions()with a small fixed set of fake distributions and asserts on the actual contract:Namein its metadata is skippedThis is independent of whatever packages happen to be installed in the test environment, so it doesn't need the
tox-pinned-environment approach suggested in the issue.Test plan
python -m pytest tests/test_utils.py -k installed_modules -v— bothtest_installed_modules_caching(existing) and the newtest_generate_installed_modulespass.normalized_name not in yieldeddedup check from_generate_installed_modules, confirmed the test fails with a clear diff, then reverted.🤖 Generated with Claude Code