Conversation
…d explicit file_id Add two new test classes to cover untested branches identified in #807: - TestPerFileSummaryCacheLRU: exercises lru_insert with Path keys and _CachedFileSummary values, verifying insert below capacity, eviction when full, update moves to MRU position, and update does not grow cache. - TestGetCachedVscodeRequestsExplicitFileId: tests the explicit file_id parameter path in _get_cached_vscode_requests, verifying that file_id=None populates cache with None, None→real id is a cache miss, and passing a real file_id avoids redundant safe_file_identity calls. Closes #807 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Adds targeted unit tests to improve coverage of VS Code log parser caching behavior in copilot_usage.vscode_parser, specifically around LRU eviction semantics and the explicit file_id fast-path for request caching.
Changes:
- Add unit-level LRU tests exercising
lru_insert()with the same key/value types used by the per-file summary cache. - Add dedicated tests for
_get_cached_vscode_requests(..., file_id=...), includingNoneidentity handling and cache-hit behavior.
- Use stat_spy binding in first patch block (assert call_count == 1) to avoid Ruff F841 and document post-parse stat behavior - Tighten weak assertion from 'cached.file_id == real_id or cached.file_id is not None' to 'cached.file_id == real_id' 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 with good coverage. Adds 7 meaningful unit tests across two new test classes (TestPerFileSummaryCacheLRU and TestGetCachedVscodeRequestsExplicitFileId) exercising previously untested LRU eviction semantics and explicit file_id cache paths. No production code changes. All CI checks pass. Auto-approving for merge.
microsasa
deleted the
add-bounded-file-summary-cache-tests-762c6d9977749213
branch
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 #807
Summary
Adds dedicated unit tests for two previously untested code paths in
vscode_parser.py:TestPerFileSummaryCacheLRUExercises
lru_insert()with the exact types used by_PER_FILE_SUMMARY_CACHE(Pathkeys,_CachedFileSummaryvalues):test_insert_below_capacity— inserting fewer entries thanmax_sizestores all of themtest_eviction_when_full— insertingmax_size + 1entries evicts the oldest (LRU) entrytest_update_existing_key_moves_to_mru— updating an existing key repositions it to the MRU endtest_update_existing_key_does_not_grow_cache— replacing an existing key keepslen(cache)unchangedTestGetCachedVscodeRequestsExplicitFileIdTests the
file_idparameter path in_get_cached_vscode_requests:test_none_file_id_populates_cache_with_none— passingfile_id=NonestoresNoneas cached identitytest_none_to_real_file_id_invalidates_cache— aNone→ real(mtime_ns, size)transition is a cache miss and triggers re-parsetest_real_file_id_avoids_extra_stat_call— passing a pre-computedfile_idbypassessafe_file_identityon cache hitVerification