Fix has_manifest_files failing to match root-level manifest files#168
Fix has_manifest_files failing to match root-level manifest files#168
Conversation
PurePath.match("**/package.json") returns False for root-level files
in Python 3.12+ because ** requires at least one directory component.
The function was unconditionally prepending **/ to all patterns,
causing root-level manifests like package.json and package-lock.json
to never match. This forced every scan into full scan mode instead of
diff scan mode, which meant MR/PR comments were never posted.
Fix by trying the direct pattern match first, then falling back to
the **/ prefixed pattern for subdirectory matching.
Fixes Zendesk #2447
|
🚀 Preview package published! Install with: pip install --index-url https://test.pypi.org/simple/ --extra-index-url https://pypi.org/simple socketsecurity==2.2.77.dev3Docker image: |
@dc-larsen you can resolve this locally by running |
lelia
left a comment
There was a problem hiding this comment.
@dc-larsen I recently added a GH workflow that will automatically runs unit tests for the CLI when PRs are opened, but I realized it won't pick up anything under /tests/core/**/*.py. Could you please make the following updates:
- Update
.github/workflows/python-tests.ymlto trigger ontests/core/**/*.py(not justtests/unit/**/*.py) - Update the workflow test command (L50) to run both suites (or all tests), e.g.
uv run pytest -q tests/unit/ tests/core/oruv run pytest -q tests/
Thanks!
dc-larsen
left a comment
There was a problem hiding this comment.
Done — added tests/core/**/*.py to the trigger paths and updated the test command to run both suites.
Signed-off-by: lelia <lelia@socket.dev>
…test construction Signed-off-by: lelia <lelia@socket.dev>
Thanks! Looks like that has surfaced some legitimate test failures, but I don't believe they were introduced by your PR. Rather, there's been some substantial drift between several If you're curious, here's the details for the fixes I just pushed up to get tests passing:
Now that we have these tests running regularly, we should be able to prevent a lot more CLI <> SDK drift! 🎉 |
Summary
has_manifest_files()unconditionally prepends**/to patterns without/, then matches usingPurePath.match(). In Python 3.12+,PurePath("package.json").match("**/package.json")returnsFalsebecause**requires at least one directory component. Root-level manifest files (the common case) never match.This sets
has_supported_files=False, forcing every scan into full scan mode instead of diff scan mode. Full scans don't post MR/PR comments.Fix
Try the direct pattern match first (handles root-level files), then fall back to
**/prefixed pattern for subdirectory matching.Reproduction
package.jsonat the rootsocketcli --target-path . --enable-debug --enable-diffhas_supported_files=False, falls back to full scanhas_supported_files=True, proceeds with diff scanTest plan
socketdev/cli:latestDocker image Python versionFixes Zendesk #2447