test(cli): fast fixtures for scan-format argparse tests + fix doubled scripts path (refs #271) - #284
Merged
Merged
Conversation
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
… scripts path (refs #271) Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
4 tasks
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.



Fixes the test-design subset of #271 (Group C + the path-bug from Group D). These are test-only fixes — no product code changed.
Fixes
1 & 2.
TestArgparseFormatConflictRegression::test_scan_with_format_{long,short}_does_not_crashThe tests ran
codelens scan . --format json/-f jsonvia subprocess withtimeout=60..resolved to the realscripts/dir, so scanning the whole codebase exceeded 60s and the subprocess timed out. The test's only purpose is to verify argparse does not raise a--format/-fconflict — it does not need a real codebase.Fix: point the scan at a tiny throwaway
tempfile.mkdtemp()workspace containing one trivial.pyfile, cleaned up infinallywithshutil.rmtree(..., ignore_errors=True). Same argparse path exercised; assertions unchanged ("argparse.ArgumentError" not in stderr,"conflicting option string" not in stderr).3.
TestDeepSingleInvocation::test_deep_unsupported_command_sets_hintBuilt the subprocess path as the relative
"scripts/codelens.py"withPYTHONPATH="scripts". When pytest runs from thescripts/cwd this becamescripts/scripts/codelens.py→can't open file ... No such file or directory.Fix: use the absolute
os.path.join(SCRIPT_DIR, "codelens.py")andPYTHONPATH=SCRIPT_DIR, mirroring every other subprocess invocation in the file. Resolves correctly regardless of cwd.Before / after
Before (all 3 failing):
After (the 3 targeted tests):
Scope note
Closes only the test-design subset of #271. Two other failures remain in these files and are out of scope / pre-existing on main (untouched here):
test_cli.py::TestCheckCommandArgs::test_check_full_cli_invocation_with_positional— same doubledscripts/scriptsrelative-path pattern, but not part of the audit(ci): triage 13 test failures unmasked by segfault fix (#266) #271 Group C/D scope.test_hybrid_engine.py::TestLSPClient::test_uri_conversion— Windows drive-letter path assertion.🤖 Generated with Claude Code