Use an argument list in test_typing_extensions_compiles_with_opt - #786
Open
rhettcarillo wants to merge 1 commit into
Open
Use an argument list in test_typing_extensions_compiles_with_opt#786rhettcarillo wants to merge 1 commit into
rhettcarillo wants to merge 1 commit into
Conversation
The test built a shell command string from sys.executable and typing_extensions.__file__ without quoting either. When the checkout or the interpreter lives under a path containing a space, the shell splits the path and the subprocess fails for that reason alone -- and because the except clause maps any CalledProcessError to "Module does not compile with optimize=2", the failure is indistinguishable from a real -OO compilation error. An argument list needs no quoting and no shell, so the test measures what it means to measure on every path. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
JelleZijlstra
approved these changes
Aug 13, 2026
Codecov Report✅ All modified and coverable lines are covered by tests. @@ Coverage Diff @@
## main #786 +/- ##
=======================================
Coverage 97.26% 97.26%
=======================================
Files 3 3
Lines 7859 7859
=======================================
Hits 7644 7644
Misses 215 215
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
Author
|
Closing and reopening to re-trigger the CLA check — the agreement has now been signed. Apologies for the notification noise. |
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.
test_typing_extensions_compiles_with_optbuilds a shell command string fromsys.executableandtyping_extensions.__file__without quoting either:When the checkout (or the interpreter) is under a path containing a space, the shell
splits the path and the subprocess fails for that reason alone. Because the
exceptclause maps any
CalledProcessErrorto'Module does not compile with optimize=2 (-OO flag).', the failure is indistinguishable from a real-OOcompilation error —which is the part that cost me some time before I looked at the command being run.
Replacing the string with an argument list needs no quoting and no shell.
Reproducing
On any checkout whose path contains a space (mine is on Windows, but the same applies
to
/home/user/my projects/...):typing_extensionscompiles fine under-OO; only the test is broken.With this change, from the same directory:
Full suite on the same checkout:
Ran 576 tests ... OK (skipped=24).Notes
typing_extensionsitself, so I have notadded a CHANGELOG entry — happy to add one if you would prefer.
commit carries a
Co-Authored-Bytrailer. The reproduction, the fix and the test runsabove are real and were executed on my machine; I am happy to adjust or drop the
trailer if it conflicts with your policy.