[Process] Make tests more deterministic #17070
Conversation
4bac9b9 to
0f982e6
Compare
| @@ -422,6 +429,8 @@ public function signal($signal) | |||
| public function disableOutput() | |||
| { | |||
| if ($this->isRunning()) { | |||
| $this->stop(0); | |||
There was a problem hiding this comment.
this drastically changes the bahavior of Symfony Process. I consider this as a BC break
There was a problem hiding this comment.
reverted, even if I think that stopping the process when an exception is triggered would make things more robust (less process left behind)
0f982e6 to
da0b78d
Compare
| if (!is_resource($this->process)) { | ||
| throw new RuntimeException('Unable to launch a new process.'); | ||
| } | ||
| $this->status = self::STATUS_STARTED; | ||
|
|
||
| if (isset($descriptors[3])) { | ||
| fclose($ptsWorkaround); |
There was a problem hiding this comment.
moving this line looks wrong to me: you are not closing it anymore in case launching the process fails.
There was a problem hiding this comment.
resources are automatically closed when they are not used anymore, isn't it?
There was a problem hiding this comment.
I don't think so. they are closed at the end of the process but maybe not on GC
There was a problem hiding this comment.
I don't think so. they are closed at the end of the process
I tried with this script:
for ($i = 100000; $i; --$i) { $h[] = fopen(__FILE__, 'r'); }
if fails as expected with failed to open stream: Too many open files
but if I replace $h[] by $h it doesn't fail, which for me means the
files are closed once resources are freed. Isn't it?
0f56981 to
86de2ed
Compare
|
Status: needs work |
86de2ed to
bf2b62a
Compare
bf2b62a to
0dc9389
Compare
This PR was merged into the 2.7 branch. Discussion ---------- [Process] Make tests more deterministic | Q | A | ------------- | --- | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | #16988 | License | MIT | Doc PR | - This makes running the Process tests faster and more deterministic. Commits ------- 0dc9389 [Process] Make tests more deterministic
This makes running the Process tests faster and more deterministic.