Replace ChildProcess with Process.spawn #892
Conversation
|
Many high-fives for this amazing piece! |
This implements a new private ProcessRunner class that broadly follows the ChildProcess interface. With this implementation, specs pass but cucumber scenarios fail.
mvz
force-pushed
the
replace-childprocess-with-process-spawn
branch
from
0ba94f4
to
ff6c4cf
Compare
mvz
force-pushed
the
replace-childprocess-with-process-spawn
branch
2 times, most recently
from
aac3658
to
1c9d78b
Compare
|
@mvz This is quite exciting - what could be the signal that triggers the EINVAL? Would you care to pair while finding out? |
|
@mvz Around the signals: The Thin web server also skips handling HUP on Windows. |
|
I made it log the signal and it's the TERM signal which I've currently mapped to signal 3 for Windows (I'm going to undo that mapping since just sending the signal name should work just fine). However, now I found this explanation of how things work on Windows: enkessler/childprocess#114 (comment). So, I'm going to try that experiment with that now. I unfortunately have no Windows machine available so debugging is via GitHub Actions. I may have to skip the spec that tests HUP on Windows since the whole point is to send a custom signal, and Windows seems to only support KILL. |
mvz
force-pushed
the
replace-childprocess-with-process-spawn
branch
from
12d7af0
to
d282def
Compare
Process.spawn takes care of the escaping for us.
On Windows, the TERM signal is not supported. Just skip it for now.
mvz
force-pushed
the
replace-childprocess-with-process-spawn
branch
from
d282def
to
424c4f6
Compare
|
@mvz Great job! Also, interesting to learn from enkessler (well pevtsoff) here. |
|
Thanks @olleolleolle. pevtsoff does not seem very active in the Ruby world at the moment. I've asked the submitter of #884 to comment, though. |
Summary
This replaces ChildProcess with Process.spawn for the purpose of starting, stopping and controlling the processes under test.
Details
To make this change be the least invasive, I created a new class that emulates the interface provided by ChildProcess, with a few differences where this was more practical. For example, where ChildProcess provides an #io method that returns an object giving access to stdout etc., this new class simply privides #stdout and related methods directly.
This also removes the childprocess dependency.
Motivation and Context
This fixes #884.
How Has This Been Tested?
I ran the full test suite with MRI 3.2 and JRuby 9.4.
I also added specs for SpawnProcess so its tested more fully in the RSpec suite.
Types of changes
Checklist: