Conversation
…ation, and partial OSError Add three tests addressing untested scenarios in vscode_parser.py: 1. _scan_child_ids symlink exclusion: Verify that symlinked directories are excluded from child_ids when follow_symlinks=False is used, documenting the intentional behaviour. 2. _cached_discover_vscode_logs child-dir cache miss: Verify that adding a new child directory under a watched root causes a cache miss (root mtime changes) and triggers a re-glob. 3. get_vscode_summary partial OSError: Verify that an OSError on one log file does not suppress data from other successfully parsed files, using real file discovery with monkeypatched read failure. Closes #876 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Adds regression tests to improve confidence in src/copilot_usage/vscode_parser.py’s discovery/caching behavior and its ability to return partial results when one file read fails.
Changes:
- Add a test documenting that
_scan_child_ids()excludes symlinked directories (withfollow_symlinks=False). - Add a test asserting
_cached_discover_vscode_logs()re-globs when a new child session directory is added (via cache invalidation). - Add a test asserting
get_vscode_summary()continues and returns data from readable files when one file raisesOSError.
- Wrap symlink_to in try/except with pytest.skip for environments where symlinks are not supported - Force deterministic mtime change via os.utime before cache-miss assertion to avoid flakiness on coarse-timestamp filesystems Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
|
Commit pushed:
|
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Low-impact test-only addition covering three untested edge cases in vscode_parser.py (symlink exclusion, cache invalidation on new child dirs, partial OSError resilience). Tests are well-structured, follow existing patterns, and CI is fully green. Auto-approving for merge.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #876
Adds three tests addressing untested scenarios in
vscode_parser.py:Gap 1 —
_scan_child_idssymlink exclusiontest_symlinked_directory_excludedverifies that symlinked directories are excluded fromchild_idswhenfollow_symlinks=Falseis used, documenting the current intentional behaviour.Gap 2 —
_cached_discover_vscode_logschild-dir cache invalidationtest_cache_misses_when_child_dir_addedverifies that adding a new child directory under a watched root causes a cache miss (rootst_mtime_nschanges) and triggers a re-glob that picks up log files in the new child. Uses a glob-counting spy to assert exactly two glob calls across two discovery invocations.Gap 3 —
get_vscode_summarypartial OSErrortest_partial_oserror_returns_data_from_surviving_filesverifies that anOSErroron one log file does not suppress data from other successfully parsed files. Uses real file discovery (no mock of_cached_discover_vscode_logs) with a monkeypatched_get_cached_vscode_requeststo simulate a read failure on one specific path.Verification
make fix— no formatting changes neededmake check— all checks pass (lint, typecheck, security, tests at 99% coverage)