Skip to content

Fix shell_magic load failure on Windows by lazy-initializing shell process#364

Merged
blink1073 merged 2 commits intoCalysto:mainfrom
blink1073:fix-247
Mar 13, 2026
Merged

Fix shell_magic load failure on Windows by lazy-initializing shell process#364
blink1073 merged 2 commits intoCalysto:mainfrom
blink1073:fix-247

Conversation

@blink1073
Copy link
Copy Markdown
Contributor

@blink1073 blink1073 commented Mar 13, 2026

Closes #247

Summary

  • ShellMagic.__init__ was eagerly calling start_process(), which on Windows immediately tried to spawn a powershell process via pexpect
  • On some systems (Windows 7 per the report), the pexpect prompt detection timed out, raising a pexpect.TIMEOUT exception
  • This propagated through register_magics() to reload_magics(), which logged a spurious "Can't load shell_magic.py" error and left the magic unregistered
  • Fix: remove the start_process() call from __init__ and add lazy initialization in eval() — the process is now started on first use

Test plan

  • Existing tests/magics/test_shell_magic.py tests all pass (verified locally)
  • On Windows: confirm python -m octave_kernel.check (or any MetaKernel-based kernel check) no longer shows the "Can't load shell_magic.py" error
  • %shell / ! commands still work normally when invoked

…ndows

Previously, ShellMagic.__init__ eagerly called start_process(), which
launched powershell immediately on Windows. On some systems (e.g. Windows 7),
the pexpect prompt detection timed out, raising an exception that propagated
up through register_magics() and caused reload_magics() to log a spurious
"Can't load shell_magic.py" error — preventing the magic from loading at all.

The fix defers process startup to the first call to eval(), so shell_magic.py
loads cleanly and any startup errors only surface when %shell is actually used.

Closes Calysto#247
@blink1073 blink1073 added the bug label Mar 13, 2026
_make_kernel_with_parent created a ZMQ PUB socket but never closed it,
unlike get_kernel() which uses weakref.finalize. The open sockets were
GC'd during later tests, causing ResourceWarning → PytestUnraisableExceptionWarning
failures when the full test suite ran.
@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented Mar 13, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 90.95%. Comparing base (0084f02) to head (c6e495e).

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #364   +/-   ##
=======================================
  Coverage   90.95%   90.95%           
=======================================
  Files          51       51           
  Lines        2874     2875    +1     
  Branches      402      403    +1     
=======================================
+ Hits         2614     2615    +1     
  Misses        181      181           
  Partials       79       79           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@blink1073 blink1073 enabled auto-merge (squash) March 13, 2026 02:20
@blink1073 blink1073 merged commit cca4a2e into Calysto:main Mar 13, 2026
25 checks passed
@blink1073 blink1073 deleted the fix-247 branch March 13, 2026 02:22
@blink1073 blink1073 mentioned this pull request Mar 17, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Can't load shell_magic.py error: <pexpect.popen_spawn.PopenSpawn object

2 participants