Join GitHub today
GitHub is home to over 50 million developers working together to host and review code, manage projects, and build software together.
Sign uptests: `fork()`: Automatically exit child process after closure returns #139
Conversation
Rather than panicking, just exit the process cleanly. The idea with this was that the closure is never supposed to return, since chaos would ensue otherwise, as the child process and the parent process would continue running the rest of the test suite. However, the existing `unreachable!()` panic that was supposed to enforce this, effectively just terminated the process anyway. The panic message wouldn't even show up unless using `--nocapture`. So we can just as well make this official, so callers are no longer required to exit the process explicitly. This actually makes use of `fork()` more ergonomic.
metajack
commented
Feb 8, 2017
|
The only case where this might be different is that panics may be trapped in the application and thus not cause an exit. Is that not a likely scenario? |
|
@metajack that's an interesting point, since the test runner does trap panics AIUI? I haven't seen this cause any trouble though when there is a panic in a sub-process... (Aside from the parent sometimes hanging waiting for an answer until the test timeout kicks in -- but that's OK I guess...) Maybe that's because a forked process only clones the thread that called At any rate, more explicitly terminating the process is actually more correct in this regard. |
metajack
commented
Feb 8, 2017
|
@bors-servo r+ |
|
|
tests: `fork()`: Automatically exit child process after closure returns Rather than panicking, just exit the process cleanly. The idea with this was that the closure is never supposed to return, since chaos would ensue otherwise, as the child process and the parent process would continue running the rest of the test suite. However, the existing `unreachable!()` panic that was supposed to enforce this, effectively just terminated the process anyway. The panic message wouldn't even show up unless using `--nocapture`. So we can just as well make this official, so callers are no longer required to exit the process explicitly. This actually makes use of `fork()` more ergonomic.
|
|
antrik commentedFeb 8, 2017
Rather than panicking, just exit the process cleanly.
The idea with this was that the closure is never supposed to return,
since chaos would ensue otherwise, as the child process and the parent
process would continue running the rest of the test suite.
However, the existing
unreachable!()panic that was supposed toenforce this, effectively just terminated the process anyway. The panic
message wouldn't even show up unless using
--nocapture. So we can justas well make this official, so callers are no longer required to exit
the process explicitly. This actually makes use of
fork()moreergonomic.