[Process] Remove enhanced sigchild compatibility #23380
Conversation
194c2ce to
00647fa
Compare
|
Could you help me to fix tests? |
|
I am afraid I will not be of much help here (I tried to do that before in #22836 but then excluded this part as I didn't manage to make the tests pass) |
|
Ok, I see your previous comment here #22836 (comment). I'll try more... |
| * @throws ProcessFailedException if the process didn't terminate successfully | ||
| * | ||
| * @final since version 3.3 | ||
| */ | ||
| public function mustRun(callable $callback = null, array $env = array()) | ||
| { | ||
| if (!$this->enhanceSigchildCompatibility && $this->isSigchildEnabled()) { | ||
| throw new RuntimeException('This PHP has been compiled with --enable-sigchild. You must use setEnhanceSigchildCompatibility() to use this method.'); | ||
| if ($this->isSigchildEnabled()) { |
There was a problem hiding this comment.
this if block should be removed (and the annotation above)
| */ | ||
| public function getExitCode() | ||
| { | ||
| if (!$this->enhanceSigchildCompatibility && $this->isSigchildEnabled()) { | ||
| throw new RuntimeException('This PHP has been compiled with --enable-sigchild. You must use setEnhanceSigchildCompatibility() to use this method.'); | ||
| if ($this->isSigchildEnabled()) { |
There was a problem hiding this comment.
to be remove, also the related line in the docblock
| @@ -723,7 +721,7 @@ public function hasBeenSignaled() | |||
| { | |||
| $this->requireProcessIsTerminated(__FUNCTION__); | |||
|
|
|||
| if (!$this->enhanceSigchildCompatibility && $this->isSigchildEnabled()) { | |||
| if ($this->isSigchildEnabled()) { | |||
| } | ||
|
|
||
| if ($expectException) { | ||
| if (method_exists($this, 'expectException')) { |
There was a problem hiding this comment.
this is wrong. All the logic setting an expected exception was triggered only inside a self::$notEnhancedSigchild. And now, sigchild compat is always enhanced. So this if must become a if (false), not a if (true)
There was a problem hiding this comment.
Ok but... I'm not sure I'll be able to fix this alone... Could you show me what I need to do more exactly please?
|
Tests are failing because half of the places removing the usage of the field in conditionals performed the wrong replacement, making the code equivalent to disabling enhanced compat all the time instead of enabling it all the time. |
00647fa to
7838da4
Compare
| @@ -420,7 +419,7 @@ public function testExitCodeCommandFailed() | |||
| if ('\\' === DIRECTORY_SEPARATOR) { | |||
| $this->markTestSkipped('Windows does not support POSIX exit code'); | |||
| } | |||
| $this->skipIfNotEnhancedSigchild(); | |||
| $this->skipIfSigchildEnabled(); | |||
There was a problem hiding this comment.
all these should be just removed
| @@ -1517,19 +1501,21 @@ private function getProcessForCode($code, $cwd = null, array $env = null, $input | |||
| return $this->getProcess(array(self::$phpBin, '-r', $code), $cwd, $env, $input, $timeout); | |||
| } | |||
|
|
|||
| private function skipIfNotEnhancedSigchild($expectException = true) | |||
| private function skipIfSigchildEnabled($expectException = true) | |||
There was a problem hiding this comment.
this method should be removed
| @@ -716,17 +703,12 @@ public function isSuccessful() | |||
| * | |||
| * @return bool | |||
| * | |||
| * @throws RuntimeException In case --enable-sigchild is activated | |||
| * @throws LogicException In case the process is not terminated | |||
There was a problem hiding this comment.
cs fix required, see fabbot
7838da4 to
65501b7
Compare
65501b7 to
1aa7d68
Compare
|
Green tests! |
|
Thank you @maidmaid. |
…maid) This PR was merged into the 4.0-dev branch. Discussion ---------- [Process] Remove enhanced sigchild compatibility | Q | A | ------------- | --- | Branch? | master | Bug fix? | no | New feature? | no | BC breaks? | yes | Deprecations? | no | Tests pass? | yes | Fixed tickets | #23376 (comment), #22836 (comment) | License | MIT | Doc PR | / In 4.0, enhanced sigchild compatibility is always enabled. Commits ------- 1aa7d68 Remove enhance sigchild compatibility
In 4.0, enhanced sigchild compatibility is always enabled.