fix(tests,registry): outdated language test-list + cache overwrite tiebreak bug - #233
Merged
Merged
Conversation
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
…ebreak bug 1. tests/test_node_types.py: EXPECTED_LANGUAGES only listed the original 7 languages (python/rust/js/ts/tsx/css/html). Issue #198 added 6 more (go/java/php/ruby/c/cpp) to node_types.yaml, but this test wasn't updated — test_no_extra_languages_beyond_expected failed with an explicit 'Update EXPECTED_LANGUAGES in this test' message pointing at its own staleness. Updated the set to all 13 current languages. 2. scripts/persistent_registry.py: get_cached_result() ordered by 'timestamp DESC LIMIT 1' only. set_cached_result() does a plain INSERT (not an upsert), so two writes for the same (command, file_set_hash) key in quick succession can land on the same time.time() value (limited clock resolution) -- the ORDER BY tie then resolves arbitrarily, sometimes returning the stale row instead of the newest. Found via baseline comparison: test_overwrite_frontend/test_overwrite_backend in test_persistent_registry_extra.py failed intermittently on this exact pattern (store 'old', store 'new', load should return 'new'). Fix: add 'id DESC' (autoincrement, monotonic) as a tiebreaker. Both found via comparing today's full test-suite failures against a clean baseline (commit c722f4a, before this session's work) -- confirmed these are test-debt from other legitimate changes this session, not regressions introduced by unrelated code. Verified: tests/test_node_types.py 32 passed. tests/test_persistent_registry_extra.py 26 passed (was 24 passed, 2 failed).
|
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
1 participant
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.
Found via comparing full test-suite results against a clean baseline (commit c722f4a, before this session).
test_node_types.py—EXPECTED_LANGUAGESonly listed the original 7 languages; issue feat(languages): add Go, Java, PHP, Ruby, C/C++ tree-sitter support #198 added 6 more (go/java/php/ruby/c/cpp) without updating this test. The failure message literally said "Update EXPECTED_LANGUAGES in this test."persistent_registry.py—get_cached_result()ordered bytimestamp DESC LIMIT 1only. Sinceset_cached_result()does a plain INSERT (not upsert), two writes for the same cache key in quick succession can land on the sametime.time()value, making the ORDER BY tie resolve arbitrarily — sometimes returning the stale row. Fix: addid DESC(monotonic autoincrement) as tiebreaker.Verified:
test_node_types.py32 passed.test_persistent_registry_extra.py26 passed (was 24 passed, 2 failed).