Repository navigation
Use OS temp dir in runString - #330
StantonMatt wants to merge 1 commit into
Conversation
Sorry, something went wrong.
Signed-off-by: Matthew Stanton <stantonmatthewj@gmail.com>
|
I checked this locally on Windows. Validated:
The fix looks correct to me: tmpdir is imported as a function from os, so the old tmpdir + sep + ... path construction would stringify the function instead of using the OS temp directory. The new join(tmpdir(), ...) path and the focused test cover that behavior. I could not use the full npm test result as a signal in this local environment because the installed WindowsApps Python launcher fails to spawn here (spawn UNKNOWN), causing the Python-dependent baseline tests to fail independently of this PR. |
Sorry, something went wrong.
PNHD
left a comment
There was a problem hiding this comment.
Reviewed current head fc0422d.
This is the focused form of the runString() temp-path fix: it preserves the existing collision-resistant random suffix, changes only the broken tmpdir function reference to join(tmpdir(), ...), and the regression verifies the generated file location/content plus option forwarding without needing to spawn Python.
I prefer this scoped PR over the overlapping change in #322. LGTM.
Sorry, something went wrong.
Refreshes stale PR #320 with the same minimal fix and focused coverage.
PythonShell.runString() currently builds its temp file path with the imported tmpdir function reference instead of calling tmpdir(). That can produce an invalid path before PythonShell.run() is reached.
This change uses path.join(tmpdir(), ...) and adds a regression test that intercepts PythonShell.run() to verify:
Verification: