From 548954984be27f6c58e8627437e432d0f136c08c Mon Sep 17 00:00:00 2001 From: Remon van de Kamp Date: Wed, 9 May 2018 16:02:42 +0200 Subject: [PATCH] Use symfony/process to run isolated tests --- composer.json | 3 +- src/Util/PHP/AbstractPhpProcess.php | 42 +++--- src/Util/PHP/DefaultPhpProcess.php | 137 ++---------------- .../unit/Util/PHP/AbstractPhpProcessTest.php | 24 ++- 4 files changed, 59 insertions(+), 147 deletions(-) diff --git a/composer.json b/composer.json index c44cbc2f749..0349d61ec7e 100644 --- a/composer.json +++ b/composer.json @@ -44,7 +44,8 @@ "sebastian/global-state": "^3.0", "sebastian/object-enumerator": "^3.0.3", "sebastian/resource-operations": "^2.0", - "sebastian/version": "^2.0.1" + "sebastian/version": "^2.0.1", + "symfony/process": "~3.4|~4.0" }, "require-dev": { "ext-PDO": "*" diff --git a/src/Util/PHP/AbstractPhpProcess.php b/src/Util/PHP/AbstractPhpProcess.php index c88e362b773..0a82f868fd7 100644 --- a/src/Util/PHP/AbstractPhpProcess.php +++ b/src/Util/PHP/AbstractPhpProcess.php @@ -174,10 +174,15 @@ public function runTestJob(string $job, Test $test, TestResult $result): void /** * Returns the command based into the configurations. */ - public function getCommand(array $settings, string $file = null): string + public function getCommand(array $settings, string $file = null): array { - $command = $this->runtime->getBinary(); - $command .= $this->settingsToParameters($settings); + $command = $this->runtime->getBinary(); + $parameters = []; + + foreach ($settings as $i => $setting) { + $command .= ' -d ' . $this->buildEnvParameter('SETTING_' . $i); + $parameters['SETTING_' . $i] = $setting; + } if (\PHP_SAPI === 'phpdbg') { $command .= ' -qrr'; @@ -188,39 +193,42 @@ public function getCommand(array $settings, string $file = null): string } if ($file) { - $command .= ' ' . \escapeshellarg($file); + $command .= ' ' . $this->buildEnvParameter('FILE'); + $parameters['FILE'] = $file; } if ($this->args) { if (!$file) { $command .= ' --'; } - $command .= ' ' . $this->args; + $command .= ' ' . $this->buildEnvParameter('ARGS'); + $parameters['ARGS'] = $this->args; } if ($this->stderrRedirection) { $command .= ' 2>&1'; } - return $command; + return [ + 'command' => $command, + 'parameters' => $parameters, + ]; } - /** - * Runs a single job (PHP code) using a separate PHP process. - */ - abstract public function runJob(string $job, array $settings = []): array; - - protected function settingsToParameters(array $settings): string + public function buildEnvParameter(string $name): string { - $buffer = ''; - - foreach ($settings as $setting) { - $buffer .= ' -d ' . \escapeshellarg($setting); + if (\DIRECTORY_SEPARATOR === '\\') { + return '%' . $name . '%'; } - return $buffer; + return '$' . $name; } + /** + * Runs a single job (PHP code) using a separate PHP process. + */ + abstract public function runJob(string $job, array $settings = []): array; + /** * Processes the TestResult object from an isolated process. * diff --git a/src/Util/PHP/DefaultPhpProcess.php b/src/Util/PHP/DefaultPhpProcess.php index d49ef3b68b0..b0d67ea618b 100644 --- a/src/Util/PHP/DefaultPhpProcess.php +++ b/src/Util/PHP/DefaultPhpProcess.php @@ -10,6 +10,8 @@ namespace PHPUnit\Util\PHP; use PHPUnit\Framework\Exception; +use Symfony\Component\Process\Exception\RuntimeException; +use Symfony\Component\Process\Process; /** * @internal This class is not covered by the backward compatibility promise for PHPUnit @@ -42,14 +44,6 @@ public function runJob(string $job, array $settings = []): array return $this->runProcess($job, $settings); } - /** - * Returns an array of file handles to be used in place of pipes - */ - protected function getHandles(): array - { - return []; - } - /** * Handles creating the child process and returning the STDOUT and STDERR * @@ -57,8 +51,6 @@ protected function getHandles(): array */ protected function runProcess(string $job, array $settings): array { - $handles = $this->getHandles(); - $env = null; if ($this->env) { @@ -73,128 +65,29 @@ protected function runProcess(string $job, array $settings): array } } - $pipeSpec = [ - 0 => $handles[0] ?? ['pipe', 'r'], - 1 => $handles[1] ?? ['pipe', 'w'], - 2 => $handles[2] ?? ['pipe', 'w'], - ]; - - $process = \proc_open( - $this->getCommand($settings, $this->tempFile), - $pipeSpec, - $pipes, - null, - $env + ['command' => $command, 'parameters' => $parameters] = $this->getCommand($settings, $this->tempFile); + + $process = new Process( + $command, + \getcwd(), + $env, + $job, + $this->timeout === 0 ? null : $this->timeout ); - if (!\is_resource($process)) { + try { + $process->start(null, $parameters); + } catch (RuntimeException $e) { throw new Exception( 'Unable to spawn worker process' ); } - if ($job) { - $this->process($pipes[0], $job); - } - - \fclose($pipes[0]); - - $stderr = $stdout = ''; - - if ($this->timeout) { - unset($pipes[0]); - - while (true) { - $r = $pipes; - $w = null; - $e = null; - - $n = @\stream_select($r, $w, $e, $this->timeout); - - if ($n === false) { - break; - } - - if ($n === 0) { - \proc_terminate($process, 9); - - throw new Exception( - \sprintf( - 'Job execution aborted after %d seconds', - $this->timeout - ) - ); - } - - if ($n > 0) { - foreach ($r as $pipe) { - $pipeOffset = 0; - - foreach ($pipes as $i => $origPipe) { - if ($pipe === $origPipe) { - $pipeOffset = $i; - - break; - } - } - - if (!$pipeOffset) { - break; - } - - $line = \fread($pipe, 8192); - - if ($line === '') { - \fclose($pipes[$pipeOffset]); - - unset($pipes[$pipeOffset]); - } elseif ($pipeOffset === 1) { - $stdout .= $line; - } else { - $stderr .= $line; - } - } - - if (empty($pipes)) { - break; - } - } - } - } else { - if (isset($pipes[1])) { - $stdout = \stream_get_contents($pipes[1]); - - \fclose($pipes[1]); - } - - if (isset($pipes[2])) { - $stderr = \stream_get_contents($pipes[2]); - - \fclose($pipes[2]); - } - } - - if (isset($handles[1])) { - \rewind($handles[1]); - - $stdout = \stream_get_contents($handles[1]); - - \fclose($handles[1]); - } - - if (isset($handles[2])) { - \rewind($handles[2]); - - $stderr = \stream_get_contents($handles[2]); - - \fclose($handles[2]); - } - - \proc_close($process); + $process->wait(); $this->cleanup(); - return ['stdout' => $stdout, 'stderr' => $stderr]; + return ['stdout' => $process->getOutput(), 'stderr' => $process->getErrorOutput()]; } protected function process($pipe, string $job): void diff --git a/tests/unit/Util/PHP/AbstractPhpProcessTest.php b/tests/unit/Util/PHP/AbstractPhpProcessTest.php index ba2e33452af..03cdb0e2d61 100644 --- a/tests/unit/Util/PHP/AbstractPhpProcessTest.php +++ b/tests/unit/Util/PHP/AbstractPhpProcessTest.php @@ -55,10 +55,18 @@ public function testShouldUseGivenSettingsToCreateCommand(): void 'display_errors=1', ]; - $expectedCommandFormat = '%s -d %callow_url_fopen=1%c -d %cauto_append_file=%c -d %cdisplay_errors=1%c%S'; + $expectedCommandFormat = '%s -d %cSETTING_0%S -d %cSETTING_1%S -d %cSETTING_2%S'; $actualCommand = $this->phpProcess->getCommand($settings); - $this->assertStringMatchesFormat($expectedCommandFormat, $actualCommand); + $this->assertStringMatchesFormat($expectedCommandFormat, $actualCommand['command']); + $this->assertEquals( + [ + 'SETTING_0' => 'allow_url_fopen=1', + 'SETTING_1' => 'auto_append_file=', + 'SETTING_2' => 'display_errors=1', + ], + $actualCommand['parameters'] + ); } public function testShouldRedirectStderrToStdoutWhenDefined(): void @@ -68,25 +76,27 @@ public function testShouldRedirectStderrToStdoutWhenDefined(): void $expectedCommandFormat = '%s 2>&1'; $actualCommand = $this->phpProcess->getCommand([]); - $this->assertStringMatchesFormat($expectedCommandFormat, $actualCommand); + $this->assertStringMatchesFormat($expectedCommandFormat, $actualCommand['command']); } public function testShouldUseArgsToCreateCommand(): void { $this->phpProcess->setArgs('foo=bar'); - $expectedCommandFormat = '%s foo=bar'; + $expectedCommandFormat = '%s -- %cARGS%S'; $actualCommand = $this->phpProcess->getCommand([]); - $this->assertStringMatchesFormat($expectedCommandFormat, $actualCommand); + $this->assertStringMatchesFormat($expectedCommandFormat, $actualCommand['command']); + $this->assertEquals('foo=bar', $actualCommand['parameters']['ARGS']); } public function testShouldHaveFileToCreateCommand(): void { - $expectedCommandFormat = '%s %cfile.php%c'; + $expectedCommandFormat = '%s %cFILE%S'; $actualCommand = $this->phpProcess->getCommand([], 'file.php'); - $this->assertStringMatchesFormat($expectedCommandFormat, $actualCommand); + $this->assertStringMatchesFormat($expectedCommandFormat, $actualCommand['command']); + $this->assertEquals('file.php', $actualCommand['parameters']['FILE']); } public function testStdinGetterAndSetter(): void