fix: make stdin quit monitoring non-blocking#562
fix: make stdin quit monitoring non-blocking#562MilesCranmerBot wants to merge 8 commits intoastroautomata:masterfrom
Conversation
Benchmark Results (Julia v1)Time benchmarks
Memory benchmarks
|
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #562 +/- ##
==========================================
+ Coverage 95.02% 95.12% +0.09%
==========================================
Files 42 42
Lines 3882 3900 +18
==========================================
+ Hits 3689 3710 +21
+ Misses 193 190 -3 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
MilesCranmer
left a comment
There was a problem hiding this comment.
Looks actually pretty good.
However one thing I would like is for a test that does not rely on fake streams but on the actual REPL.
I think we can do this with Linux expect tool. Or are there other options?
@MilesCranmerBot please investigate and add such a test.
I recommend doing some mutation testing as well - try introducing silent failures - does the test catch it? Or does the test always pass?
|
@codex review |
|
Codex Review: Didn't find any major issues. Keep them coming! ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
Co-authored-by: Miles Cranmer <miles.cranmer@gmail.com>
Co-authored-by: Miles Cranmer <miles.cranmer@gmail.com>
There was a problem hiding this comment.
@MilesCranmerBot Can we do this with linux expect rather than hacking together a reimplementation
There was a problem hiding this comment.
Swapped the REPL stdin nonblocking check over to a plain Linux expect script (so we’re not reimplementing an expect loop in Python).
- Added: test/repl/test_repl_stdin_nonblocking.exp
- CI: installs expect (ubuntu-latest + evaluation only) and runs the script
- Removed: test/repl/test_repl_stdin_nonblocking.py
Pushed in f3cbb63.
Fixes intermittent hangs/freezes caused by stdin quit monitoring when stdin is a pipe/non-interactive (notably on Windows).
TDD: adds a fast unit reproducer using a FakeInputStream that simulates bytesavailable>0 with a blocking read, then fixes the monitor to use non-blocking reads + robust CRLF/partial-chunk parsing.
Related PySR issues: #832, #1101, #1092.