From d1a178a43cb62137f2ebe4262ef1b740df2e140c Mon Sep 17 00:00:00 2001 From: Nicolas Grekas Date: Fri, 18 Dec 2015 21:16:23 +0100 Subject: [PATCH 1/7] [Process] More robustness and deterministic tests --- src/Symfony/Component/Process/PhpProcess.php | 6 + src/Symfony/Component/Process/Process.php | 89 ++++--- .../Process/Tests/NonStopableProcess.php | 8 +- .../Component/Process/Tests/ProcessTest.php | 217 ++++++++---------- 4 files changed, 147 insertions(+), 173 deletions(-) diff --git a/src/Symfony/Component/Process/PhpProcess.php b/src/Symfony/Component/Process/PhpProcess.php index 4a2a2625ff..42ecb66f2b 100644 --- a/src/Symfony/Component/Process/PhpProcess.php +++ b/src/Symfony/Component/Process/PhpProcess.php @@ -46,6 +46,12 @@ class PhpProcess extends Process $php .= ' '.ProcessUtils::escapeArgument($file); $script = null; } + if ('\\' !== DIRECTORY_SEPARATOR && null !== $php) { + // exec is mandatory to deal with sending a signal to the process + // see https://github.com/symfony/symfony/issues/5030 about prepending + // command with exec + $php = 'exec '.$php; + } parent::__construct($php, $cwd, $env, $script, $timeout, $options); } diff --git a/src/Symfony/Component/Process/Process.php b/src/Symfony/Component/Process/Process.php index d2af621d43..636ddb9513 100644 --- a/src/Symfony/Component/Process/Process.php +++ b/src/Symfony/Component/Process/Process.php @@ -66,12 +66,12 @@ class Process private static $sigchild; private static $posixSignals = array( - 1 => 1, // SIGHUP - 2 => 2, // SIGINT - 3 => 3, // SIGQUIT - 6 => 6, // SIGABRT - 14 => 14, // SIGALRM - 15 => 15, // SIGTERM + 1, // SIGHUP + 2, // SIGINT + 3, // SIGQUIT + 6, // SIGABRT + 14, // SIGALRM + 15, // SIGTERM ); /** @@ -242,11 +242,19 @@ class Process if (!isset($this->options['bypass_shell'])) { $this->options['bypass_shell'] = true; } - } + } elseif (!$this->useFileHandles && $this->enhanceSigchildCompatibility && $this->isSigchildEnabled()) { + // last exit code is output on the fourth pipe and caught to work around --enable-sigchild + $descriptors[3] = array('pipe', 'w'); - $ptsWorkaround = null; + $commandline = ''; + foreach (self::$posixSignals as $s) { + $commandline .= "trap 'echo s$s >&3' $s;"; + } + + // See https://unix.stackexchange.com/questions/71205/background-process-pipe-input + $commandline .= '{ ('.$this->commandline.') <&3 3<&- 3>/dev/null & } 3<&0;'; + $commandline .= 'pid=$!; echo $pid >&3; wait $pid; code=$?; echo x$code >&3; exit $code'; - if (!$this->useFileHandles && $this->enhanceSigchildCompatibility && $this->isSigchildEnabled()) { // Workaround for the bug, when PTS functionality is enabled. // @see : https://bugs.php.net/69442 $ptsWorkaround = fopen(__FILE__, 'r'); @@ -254,15 +262,14 @@ class Process $this->process = proc_open($commandline, $descriptors, $this->processPipes->pipes, $this->cwd, $this->env, $this->options); - if ($ptsWorkaround) { - fclose($ptsWorkaround); - } - if (!is_resource($this->process)) { throw new RuntimeException('Unable to launch a new process.'); } $this->status = self::STATUS_STARTED; + if (isset($descriptors[3])) { + $this->fallbackStatus['pid'] = (int) fgets($this->processPipes->pipes[3]); + } $this->processPipes->unblock(); if ($this->tty) { @@ -468,8 +475,6 @@ class Process public function getExitCode() { if (!$this->enhanceSigchildCompatibility && $this->isSigchildEnabled()) { - $this->stop(0); - throw new RuntimeException('This PHP has been compiled with --enable-sigchild. You must use setEnhanceSigchildCompatibility() to use this method.'); } @@ -523,8 +528,6 @@ class Process $this->requireProcessIsTerminated(__FUNCTION__); if (!$this->enhanceSigchildCompatibility && $this->isSigchildEnabled()) { - $this->stop(0); - throw new RuntimeException('This PHP has been compiled with --enable-sigchild. Term signal can not be retrieved.'); } @@ -546,8 +549,6 @@ class Process $this->requireProcessIsTerminated(__FUNCTION__); if ($this->isSigchildEnabled() && (!$this->enhanceSigchildCompatibility || -1 === $this->processInformation['termsig'])) { - $this->stop(0); - throw new RuntimeException('This PHP has been compiled with --enable-sigchild. Term signal can not be retrieved.'); } @@ -990,22 +991,8 @@ class Process private function getDescriptors() { $this->processPipes = new ProcessPipes($this->useFileHandles, $this->tty); - $descriptors = $this->processPipes->getDescriptors(); - if (!$this->useFileHandles && $this->enhanceSigchildCompatibility && $this->isSigchildEnabled()) { - // last exit code is output on the fourth pipe and caught to work around --enable-sigchild - $descriptors[3] = array('pipe', 'w'); - - $trap = ''; - foreach (self::$posixSignals as $s) { - $trap .= "trap 'echo s$s >&3' $s;"; - } - - $this->commandline = $trap.'{ ('.$this->commandline.') <&3 3<&- 3>/dev/null & } 3<&0;'; - $this->commandline .= 'pid=$!; echo p$pid >&3; wait $pid; code=$?; echo x$code >&3; exit $code'; - } - - return $descriptors; + return $this->processPipes->getDescriptors(); } /** @@ -1106,8 +1093,11 @@ class Process $this->fallbackStatus['signaled'] = true; $this->fallbackStatus['exitcode'] = -1; $this->fallbackStatus['termsig'] = (int) substr($data, 1); - } elseif ('x' === $data[0] && !isset($this->fallbackStatus['signaled'])) { - $this->fallbackStatus['exitcode'] = (int) substr($data, 1); + } elseif ('x' === $data[0]) { + $this->fallbackStatus['running'] = false; + if (!isset($this->fallbackStatus['signaled'])) { + $this->fallbackStatus['exitcode'] = (int) substr($data, 1); + } } } } else { @@ -1189,14 +1179,6 @@ class Process return false; } - if ($this->enhanceSigchildCompatibility && $this->isSigchildEnabled() && !isset(self::$posixSignals[$signal]) && !(function_exists('posix_kill') && @posix_kill($this->getPid(), $signal))) { - if ($throwException) { - throw new RuntimeException('This PHP has been compiled with --enable-sigchild and posix_kill() is not available.'); - } - - return false; - } - if ('\\' === DIRECTORY_SEPARATOR) { exec(sprintf('taskkill /F /T /PID %d 2>&1', $this->getPid()), $output, $exitCode); if ($exitCode && $this->isRunning()) { @@ -1206,14 +1188,21 @@ class Process return false; } - } - - if (true !== @proc_terminate($this->process, $signal) && '\\' !== DIRECTORY_SEPARATOR) { - if ($throwException) { - throw new RuntimeException(sprintf('Error while sending signal `%s`.', $signal)); + } else { + if (!$this->enhanceSigchildCompatibility || !$this->isSigchildEnabled()) { + $ok = @proc_terminate($this->process, $signal); + } elseif (function_exists('posix_kill')) { + $ok = @posix_kill($this->getPid(), $signal); + } elseif ($ok = proc_open(sprintf('kill -%d %d', $signal, $this->getPid()), array(2 => array('pipe', 'w')), $pipes)) { + $ok = false === fgets($pipes[2]); } + if (!$ok) { + if ($throwException) { + throw new RuntimeException(sprintf('Error while sending signal `%s`.', $signal)); + } - return false; + return false; + } } $this->latestSignal = (int) $signal; diff --git a/src/Symfony/Component/Process/Tests/NonStopableProcess.php b/src/Symfony/Component/Process/Tests/NonStopableProcess.php index 54510c16a3..5643259657 100644 --- a/src/Symfony/Component/Process/Tests/NonStopableProcess.php +++ b/src/Symfony/Component/Process/Tests/NonStopableProcess.php @@ -30,16 +30,18 @@ function handleSignal($signal) break; } - echo "received signal $name\n"; + echo "signal $name\n"; } -declare (ticks = 1); pcntl_signal(SIGTERM, 'handleSignal'); pcntl_signal(SIGINT, 'handleSignal'); +echo 'received '; + $duration = isset($argv[1]) ? (int) $argv[1] : 3; $start = microtime(true); while ($duration > (microtime(true) - $start)) { - usleep(1000); + usleep(10000); + pcntl_signal_dispatch(); } diff --git a/src/Symfony/Component/Process/Tests/ProcessTest.php b/src/Symfony/Component/Process/Tests/ProcessTest.php index a49ededd7d..4b1d4d640b 100644 --- a/src/Symfony/Component/Process/Tests/ProcessTest.php +++ b/src/Symfony/Component/Process/Tests/ProcessTest.php @@ -23,12 +23,23 @@ use Symfony\Component\Process\ProcessPipes; class ProcessTest extends \PHPUnit_Framework_TestCase { private static $phpBin; + private static $sigchild; private static $notEnhancedSigchild = false; public static function setUpBeforeClass() { $phpBin = new PhpExecutableFinder(); self::$phpBin = 'phpdbg' === PHP_SAPI ? 'php' : $phpBin->find(); + if ('\\' !== DIRECTORY_SEPARATOR) { + // exec is mandatory to deal with sending a signal to the process + // see https://github.com/symfony/symfony/issues/5030 about prepending + // command with exec + self::$phpBin = 'exec '.self::$phpBin; + } + + ob_start(); + phpinfo(INFO_GENERAL); + self::$sigchild = false !== strpos(ob_get_clean(), '--enable-sigchild'); } public function testThatProcessDoesNotThrowWarningDuringRun() @@ -77,19 +88,18 @@ class ProcessTest extends \PHPUnit_Framework_TestCase */ public function testStopWithTimeoutIsActuallyWorking() { - // exec is mandatory here since we send a signal to the process - // see https://github.com/symfony/symfony/issues/5030 about prepending - // command with exec - $p = $this->getProcess('exec '.self::$phpBin.' '.__DIR__.'/NonStopableProcess.php 3'); + $p = $this->getProcess(self::$phpBin.' '.__DIR__.'/NonStopableProcess.php 30'); $p->start(); - usleep(100000); - $start = microtime(true); - $p->stop(1.1, SIGKILL); - while ($p->isRunning()) { + + while (false === strpos($p->getOutput(), 'received')) { usleep(1000); } + $start = microtime(true); + $p->stop(0.1); - $this->assertLessThan(4, microtime(true) - $start); + $p->wait(); + + $this->assertLessThan(15, microtime(true) - $start); } public function testAllOutputIsActuallyReadOnTermination() @@ -107,12 +117,16 @@ class ProcessTest extends \PHPUnit_Framework_TestCase $p = $this->getProcess(sprintf('%s -r %s', self::$phpBin, escapeshellarg($code))); $p->start(); - // Let's wait enough time for process to finish... - // Here we don't call Process::run or Process::wait to avoid any read of pipes - usleep(500000); - if ($p->isRunning()) { - $this->markTestSkipped('Process execution did not complete in the required time frame'); + // Don't call Process::run nor Process::wait to avoid any read of pipes + $h = new \ReflectionProperty($p, 'process'); + $h->setAccessible(true); + $h = $h->getValue($p); + $s = proc_get_status($h); + + while ($s['running']) { + usleep(1000); + $s = proc_get_status($h); } $o = $p->getOutput(); @@ -122,18 +136,14 @@ class ProcessTest extends \PHPUnit_Framework_TestCase public function testCallbacksAreExecutedWithStart() { - $data = ''; - - $process = $this->getProcess('echo foo && '.self::$phpBin.' -r "sleep(1);" && echo foo'); + $process = $this->getProcess('echo foo'); $process->start(function ($type, $buffer) use (&$data) { $data .= $buffer; }); - while ($process->isRunning()) { - usleep(10000); - } + $process->wait(); - $this->assertEquals(2, preg_match_all('/foo/', $data, $matches)); + $this->assertSame('foo'.PHP_EOL, $data); } /** @@ -173,7 +183,7 @@ class ProcessTest extends \PHPUnit_Framework_TestCase */ public function testSetStdinWhileRunningThrowsAnException() { - $process = $this->getProcess(self::$phpBin.' -r "usleep(500000);"'); + $process = $this->getProcess(self::$phpBin.' -r "sleep(30);"'); $process->start(); try { $process->setStdin('foobar'); @@ -193,7 +203,7 @@ class ProcessTest extends \PHPUnit_Framework_TestCase */ public function testInvalidStdin($value) { - $process = $this->getProcess(self::$phpBin.' -v'); + $process = $this->getProcess('foo'); $process->setStdin($value); } @@ -211,7 +221,7 @@ class ProcessTest extends \PHPUnit_Framework_TestCase */ public function testValidStdin($expected, $value) { - $process = $this->getProcess(self::$phpBin.' -v'); + $process = $this->getProcess('foo'); $process->setStdin($value); $this->assertSame($expected, $process->getStdin()); } @@ -413,7 +423,7 @@ class ProcessTest extends \PHPUnit_Framework_TestCase public function testTTYCommand() { if ('\\' === DIRECTORY_SEPARATOR) { - $this->markTestSkipped('Windows does have /dev/tty support'); + $this->markTestSkipped('Windows does not have /dev/tty support'); } $process = $this->getProcess('echo "foo" >> /dev/null && '.self::$phpBin.' -r "usleep(100000);"'); @@ -481,13 +491,13 @@ class ProcessTest extends \PHPUnit_Framework_TestCase $start = microtime(true); $process->start(); $end = microtime(true); - $this->assertLessThan(1, $end - $start); - $process->wait(); + $this->assertLessThan(0.4, $end - $start); + $process->stop(); } public function testUpdateStatus() { - $process = $this->getProcess(self::$phpBin.' -v'); + $process = $this->getProcess('echo foo'); $process->run(); $this->assertTrue(strlen($process->getOutput()) > 0); } @@ -496,7 +506,7 @@ class ProcessTest extends \PHPUnit_Framework_TestCase { $this->skipIfNotEnhancedSigchild(); - $process = $this->getProcess(self::$phpBin.' -r "usleep(200000);"'); + $process = $this->getProcess(self::$phpBin.' -r "usleep(100000);"'); $this->assertNull($process->getExitCode()); $process->start(); $this->assertNull($process->getExitCode()); @@ -508,7 +518,7 @@ class ProcessTest extends \PHPUnit_Framework_TestCase { $this->skipIfNotEnhancedSigchild(); - $process = $this->getProcess(self::$phpBin.' -r "usleep(200000);"'); + $process = $this->getProcess(self::$phpBin.' -r "usleep(100000);"'); $process->run(); $this->assertEquals(0, $process->getExitCode()); $process->start(); @@ -521,14 +531,14 @@ class ProcessTest extends \PHPUnit_Framework_TestCase { $this->skipIfNotEnhancedSigchild(); - $process = $this->getProcess(self::$phpBin.' -v'); + $process = $this->getProcess('echo foo'); $process->run(); $this->assertSame(0, $process->getExitCode()); } public function testStatus() { - $process = $this->getProcess(self::$phpBin.' -r "usleep(500000);"'); + $process = $this->getProcess(self::$phpBin.' -r "usleep(100000);"'); $this->assertFalse($process->isRunning()); $this->assertFalse($process->isStarted()); $this->assertFalse($process->isTerminated()); @@ -547,7 +557,7 @@ class ProcessTest extends \PHPUnit_Framework_TestCase public function testStop() { - $process = $this->getProcess(self::$phpBin.' -r "sleep(4);"'); + $process = $this->getProcess(self::$phpBin.' -r "sleep(31);"'); $process->start(); $this->assertTrue($process->isRunning()); $process->stop(); @@ -558,7 +568,7 @@ class ProcessTest extends \PHPUnit_Framework_TestCase { $this->skipIfNotEnhancedSigchild(); - $process = $this->getProcess(self::$phpBin.' -v'); + $process = $this->getProcess('echo foo'); $process->run(); $this->assertTrue($process->isSuccessful()); } @@ -567,14 +577,12 @@ class ProcessTest extends \PHPUnit_Framework_TestCase { $this->skipIfNotEnhancedSigchild(); - $process = $this->getProcess(self::$phpBin.' -r "sleep(1);"'); + $process = $this->getProcess(self::$phpBin.' -r "usleep(100000);"'); $process->start(); $this->assertFalse($process->isSuccessful()); - while ($process->isRunning()) { - usleep(300000); - } + $process->wait(); $this->assertTrue($process->isSuccessful()); } @@ -583,10 +591,8 @@ class ProcessTest extends \PHPUnit_Framework_TestCase { $this->skipIfNotEnhancedSigchild(); - $process = $this->getProcess(self::$phpBin.' -r "usleep(500000);throw new \Exception(\'BOUM\');"'); - $process->start(); - $this->assertTrue($process->isRunning()); - $process->wait(); + $process = $this->getProcess(self::$phpBin.' -r "throw new \Exception(\'BOUM\');"'); + $process->run(); $this->assertFalse($process->isSuccessful()); } @@ -597,19 +603,7 @@ class ProcessTest extends \PHPUnit_Framework_TestCase } $this->skipIfNotEnhancedSigchild(); - $process = $this->getProcess(self::$phpBin.' -v'); - $process->run(); - $this->assertFalse($process->hasBeenSignaled()); - } - - public function testProcessWithoutTermSignalIsNotSignaled() - { - if ('\\' === DIRECTORY_SEPARATOR) { - $this->markTestSkipped('Windows does not support POSIX signals'); - } - $this->skipIfNotEnhancedSigchild(); - - $process = $this->getProcess(self::$phpBin.' -v'); + $process = $this->getProcess('echo foo'); $process->run(); $this->assertFalse($process->hasBeenSignaled()); } @@ -621,7 +615,7 @@ class ProcessTest extends \PHPUnit_Framework_TestCase } $this->skipIfNotEnhancedSigchild(); - $process = $this->getProcess(self::$phpBin.' -v'); + $process = $this->getProcess('echo foo'); $process->run(); $this->assertEquals(0, $process->getTermSignal()); } @@ -633,23 +627,10 @@ class ProcessTest extends \PHPUnit_Framework_TestCase } $this->skipIfNotEnhancedSigchild(); - $process = $this->getProcess(self::$phpBin.' -r "sleep(4);"'); + $process = $this->getProcess(self::$phpBin.' -r "sleep(32);"'); $process->start(); $process->stop(); $this->assertTrue($process->hasBeenSignaled()); - } - - public function testProcessWithTermSignal() - { - if ('\\' === DIRECTORY_SEPARATOR) { - $this->markTestSkipped('Windows does not support POSIX signals'); - } - $this->skipIfNotEnhancedSigchild(); - - $process = $this->getProcess(self::$phpBin.' -r "sleep(4);"'); - $process->start(); - $process->stop(); - $this->assertEquals(15, $process->getTermSignal()); // SIGTERM } @@ -664,11 +645,9 @@ class ProcessTest extends \PHPUnit_Framework_TestCase } $this->skipIfNotEnhancedSigchild(false); - $termSignal = defined('SIGKILL') ? SIGKILL : 9; - - $process = $this->getProcess('exec '.self::$phpBin.' -r "while (true) {}"'); + $process = $this->getProcess(self::$phpBin.' -r "while (true) usleep(100);"'); $process->start(); - posix_kill($process->getPid(), $termSignal); + posix_kill($process->getPid(), 9); // SIGKILL $process->wait(); } @@ -697,8 +676,8 @@ class ProcessTest extends \PHPUnit_Framework_TestCase */ public function testRunProcessWithTimeout() { - $timeout = 0.5; - $process = $this->getProcess(self::$phpBin.' -r "usleep(600000);"'); + $timeout = 0.1; + $process = $this->getProcess(self::$phpBin.' -r "sleep(1);"'); $process->setTimeout($timeout); $start = microtime(true); try { @@ -719,15 +698,15 @@ class ProcessTest extends \PHPUnit_Framework_TestCase public function testCheckTimeoutOnNonStartedProcess() { - $process = $this->getProcess(self::$phpBin.' -r "sleep(3);"'); - $process->checkTimeout(); + $process = $this->getProcess('echo foo'); + $this->assertNull($process->checkTimeout()); } public function testCheckTimeoutOnTerminatedProcess() { - $process = $this->getProcess(self::$phpBin.' -v'); + $process = $this->getProcess('echo foo'); $process->run(); - $process->checkTimeout(); + $this->assertNull($process->checkTimeout()); } /** @@ -736,8 +715,8 @@ class ProcessTest extends \PHPUnit_Framework_TestCase */ public function testCheckTimeoutOnStartedProcess() { - $process = $this->getProcess(self::$phpBin.' -r "sleep(3);"'); - $process->setTimeout(0.5); + $process = $this->getProcess(self::$phpBin.' -r "sleep(33);"'); + $process->setTimeout(0.1); $process->start(); $start = microtime(true); @@ -763,37 +742,37 @@ class ProcessTest extends \PHPUnit_Framework_TestCase */ public function testStartAfterATimeout() { - $process = $this->getProcess(sprintf('%s -r %s', self::$phpBin, escapeshellarg('$n = 1000; while ($n--) {echo \'\'; usleep(1000); }'))); + $process = $this->getProcess(self::$phpBin.' -r "sleep(35);"'); $process->setTimeout(0.1); try { $process->run(); $this->fail('A RuntimeException should have been raised.'); } catch (RuntimeException $e) { } + $this->assertFalse($process->isRunning()); $process->start(); - usleep(10000); - $process->stop(); + $process->stop(0); throw $e; } public function testGetPid() { - $process = $this->getProcess(self::$phpBin.' -r "usleep(500000);"'); + $process = $this->getProcess(self::$phpBin.' -r "sleep(36);"'); $process->start(); $this->assertGreaterThan(0, $process->getPid()); - $process->wait(); + $process->stop(0); } public function testGetPidIsNullBeforeStart() { - $process = $this->getProcess(self::$phpBin.' -r "sleep(1);"'); + $process = $this->getProcess('foo'); $this->assertNull($process->getPid()); } public function testGetPidIsNullAfterRun() { - $process = $this->getProcess(self::$phpBin.' -v'); + $process = $this->getProcess('echo foo'); $process->run(); $this->assertNull($process->getPid()); } @@ -803,7 +782,7 @@ class ProcessTest extends \PHPUnit_Framework_TestCase */ public function testSignal() { - $process = $this->getProcess('exec '.self::$phpBin.' '.__DIR__.'/SignalListener.php'); + $process = $this->getProcess(self::$phpBin.' '.__DIR__.'/SignalListener.php'); $process->start(); while (false === strpos($process->getOutput(), 'Caught')) { @@ -842,7 +821,7 @@ class ProcessTest extends \PHPUnit_Framework_TestCase */ public function testSignalProcessNotRunning() { - $process = $this->getProcess(self::$phpBin.' -v'); + $process = $this->getProcess('foo'); $process->signal(1); // SIGHUP } @@ -851,7 +830,7 @@ class ProcessTest extends \PHPUnit_Framework_TestCase */ public function testMethodsThatNeedARunningProcess($method) { - $process = $this->getProcess(self::$phpBin.' -v'); + $process = $this->getProcess('foo'); $this->setExpectedException('Symfony\Component\Process\Exception\LogicException', sprintf('Process must be started before calling %s.', $method)); $process->{$method}(); } @@ -874,7 +853,7 @@ class ProcessTest extends \PHPUnit_Framework_TestCase */ public function testMethodsThatNeedATerminatedProcess($method) { - $process = $this->getProcess(self::$phpBin.' -r "sleep(1);"'); + $process = $this->getProcess(self::$phpBin.' -r "sleep(37);"'); $process->start(); try { $process->{$method}(); @@ -898,36 +877,38 @@ class ProcessTest extends \PHPUnit_Framework_TestCase } /** + * @dataProvider provideWrongSignal * @expectedException \Symfony\Component\Process\Exception\RuntimeException */ - public function testSignalWithWrongIntSignal() + public function testWrongSignal($signal) { if ('\\' === DIRECTORY_SEPARATOR) { $this->markTestSkipped('POSIX signals do not work on Windows'); } - $process = $this->getProcess(self::$phpBin.' -r "sleep(3);"'); + $process = $this->getProcess(self::$phpBin.' -r "sleep(38);"'); $process->start(); - $process->signal(-4); + try { + $process->signal($signal); + $this->fail('A RuntimeException must have been thrown'); + } catch (RuntimeException $e) { + $process->stop(0); + } + + throw $e; } - /** - * @expectedException \Symfony\Component\Process\Exception\RuntimeException - */ - public function testSignalWithWrongNonIntSignal() + public function provideWrongSignal() { - if ('\\' === DIRECTORY_SEPARATOR) { - $this->markTestSkipped('POSIX signals do not work on Windows'); - } - - $process = $this->getProcess(self::$phpBin.' -r "sleep(3);"'); - $process->start(); - $process->signal('Céphalopodes'); + return array( + array(-4), + array('Céphalopodes'), + ); } public function testStopTerminatesProcessCleanly() { - $process = $this->getProcess(self::$phpBin.' -r "echo \'foo\'; sleep(1); echo \'bar\';"'); + $process = $this->getProcess(self::$phpBin.' -r "echo 123; sleep(42);"'); $process->run(function () use ($process) { $process->stop(); }); @@ -936,22 +917,18 @@ class ProcessTest extends \PHPUnit_Framework_TestCase public function testKillSignalTerminatesProcessCleanly() { - $process = $this->getProcess(self::$phpBin.' -r "echo \'foo\'; sleep(1); echo \'bar\';"'); + $process = $this->getProcess(self::$phpBin.' -r "echo 123; sleep(43);"'); $process->run(function () use ($process) { - if ($process->isRunning()) { - $process->signal(9); // SIGKILL - } + $process->signal(9); // SIGKILL }); $this->assertTrue(true, 'A call to signal() is not expected to cause wait() to throw a RuntimeException'); } public function testTermSignalTerminatesProcessCleanly() { - $process = $this->getProcess(self::$phpBin.' -r "echo \'foo\'; sleep(1); echo \'bar\';"'); + $process = $this->getProcess(self::$phpBin.' -r "echo 123; sleep(44);"'); $process->run(function () use ($process) { - if ($process->isRunning()) { - $process->signal(15); // SIGTERM - } + $process->signal(15); // SIGTERM }); $this->assertTrue(true, 'A call to signal() is not expected to cause wait() to throw a RuntimeException'); } @@ -1041,11 +1018,11 @@ class ProcessTest extends \PHPUnit_Framework_TestCase private function skipIfNotEnhancedSigchild($expectException = true) { - if (self::$notEnhancedSigchild) { - if ($expectException) { + if (self::$sigchild) { + if (!$expectException) { + $this->markTestSkipped('PHP is compiled with --enable-sigchild.'); + } elseif (self::$notEnhancedSigchild) { $this->setExpectedException('Symfony\Component\Process\Exception\RuntimeException', 'This PHP has been compiled with --enable-sigchild.'); - } else { - $this->markTestSkipped('PHP is compiled with --enable-sigchild and enhanced mode is disabled.'); } } } From cb23212bd6da702b03a5e58934295226a48a8fab Mon Sep 17 00:00:00 2001 From: Nicolas Grekas Date: Tue, 22 Dec 2015 14:43:34 +0100 Subject: [PATCH 2/7] [Routing] Skip PhpGeneratorDumperTest::testDumpWithTooManyRoutes on HHVM --- .../Routing/Tests/Generator/Dumper/PhpGeneratorDumperTest.php | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/Symfony/Component/Routing/Tests/Generator/Dumper/PhpGeneratorDumperTest.php b/src/Symfony/Component/Routing/Tests/Generator/Dumper/PhpGeneratorDumperTest.php index 783d694d1c..8d4a70f844 100644 --- a/src/Symfony/Component/Routing/Tests/Generator/Dumper/PhpGeneratorDumperTest.php +++ b/src/Symfony/Component/Routing/Tests/Generator/Dumper/PhpGeneratorDumperTest.php @@ -85,6 +85,10 @@ class PhpGeneratorDumperTest extends \PHPUnit_Framework_TestCase public function testDumpWithTooManyRoutes() { + if (defined('HHVM_VERSION_ID')) { + $this->markTestSkipped('HHVM consumes too much memory on this test.'); + } + $this->routeCollection->add('Test', new Route('/testing/{foo}')); for ($i = 0; $i < 32769; ++$i) { $this->routeCollection->add('route_'.$i, new Route('/route_'.$i)); From 9859125130bc62be5898c8aa425ff375c7bd6fa9 Mon Sep 17 00:00:00 2001 From: Javier Eguiluz Date: Tue, 22 Dec 2015 14:54:17 +0100 Subject: [PATCH 3/7] Improved the design of the web debug toolbar --- .../views/Profiler/profiler.css.twig | 25 +++++++++++-------- .../Resources/views/Profiler/toolbar.css.twig | 22 +++++++++------- 2 files changed, 27 insertions(+), 20 deletions(-) diff --git a/src/Symfony/Bundle/WebProfilerBundle/Resources/views/Profiler/profiler.css.twig b/src/Symfony/Bundle/WebProfilerBundle/Resources/views/Profiler/profiler.css.twig index e24f3ba70d..b61f1c4204 100644 --- a/src/Symfony/Bundle/WebProfilerBundle/Resources/views/Profiler/profiler.css.twig +++ b/src/Symfony/Bundle/WebProfilerBundle/Resources/views/Profiler/profiler.css.twig @@ -7,6 +7,9 @@ 'subtle_border_and_shadow': 'background: #FFF; border: 1px solid #E0E0E0; box-shadow: 0px 0px 1px rgba(128, 128, 128, .2);' } %} +{# when updating any of these colors, do the same in toolbar.css.twig #} +{% set colors = { 'success': '#4F805D', 'warning': '#A46A1F', 'error': '#B0413E' } %} + {# Normalization (normalize.css v3.0.3 | MIT License | github.com/necolas/normalize.css) ========================================================================= #} @@ -232,9 +235,9 @@ table tbody ul { padding: 3px 7px; white-space: nowrap; } -.label.status-success { background: #5E976E; color: #FFF; } -.label.status-warning { background: #BC8034; color: #FFF; } -.label.status-error { background: #B0413E; color: #FFF; } +.label.status-success { background: {{ colors.success|raw }}; color: #FFF; } +.label.status-warning { background: {{ colors.warning|raw }}; color: #FFF; } +.label.status-error { background: {{ colors.error|raw }}; color: #FFF; } {# Metrics ------------------------------------------------------------------------- #} @@ -341,11 +344,11 @@ tr.status-warning td { border-top: 1px solid #FAFAFA; } -.status-warning .colored { - color: #BC8034; +.status-warning .colored { + color: {{ colors.warning|raw }}; } .status-error .colored { - color: #B0413E; + color: {{ colors.error|raw }}; } {# Syntax highlighting @@ -469,9 +472,9 @@ tr.status-warning td { text-decoration: underline; } -#summary .status-success { background: #5E976E; } -#summary .status-warning { background: #BC8034; } -#summary .status-error { background: #B0413E; } +#summary .status-success { background: {{ colors.success|raw }}; } +#summary .status-warning { background: {{ colors.warning|raw }}; } +#summary .status-error { background: {{ colors.error|raw }}; } #summary .status-success h2, #summary .status-success h2 a, @@ -670,10 +673,10 @@ tr.status-warning td { } #menu-profiler .label-status-warning .count { - background: #BC8034; + background: {{ colors.warning|raw }}; } #menu-profiler .label-status-error .count { - background: #B0413E; + background: {{ colors.error|raw }}; } {# Timeline panel diff --git a/src/Symfony/Bundle/WebProfilerBundle/Resources/views/Profiler/toolbar.css.twig b/src/Symfony/Bundle/WebProfilerBundle/Resources/views/Profiler/toolbar.css.twig index c0456f6131..4ae44c0326 100644 --- a/src/Symfony/Bundle/WebProfilerBundle/Resources/views/Profiler/toolbar.css.twig +++ b/src/Symfony/Bundle/WebProfilerBundle/Resources/views/Profiler/toolbar.css.twig @@ -1,3 +1,6 @@ +{# when updating any of these colors, do the same in profiler.css.twig #} +{% set colors = { 'success': '#4F805D', 'warning': '#A46A1F', 'error': '#B0413E' } %} + .sf-minitoolbar { background-color: #222; border-top-left-radius: 4px; @@ -46,8 +49,8 @@ } .sf-toolbarreset svg, .sf-toolbarreset img { - max-height: 24px; - max-width: 24px; + max-height: 20px; + max-width: 20px; } .sf-toolbarreset .hide-button { @@ -180,30 +183,31 @@ padding: 3px 6px; margin-bottom: 2px; vertical-align: middle; - min-width: 6px; + min-width: 15px; min-height: 13px; + text-align: center; } .sf-toolbar-block .sf-toolbar-status-green { - background-color: rgba(117, 158, 43, 0.8); + background-color: {{ colors.success|raw }}; } .sf-toolbar-block .sf-toolbar-status-red { - background-color: rgba(200, 43, 43, 0.8); + background-color: {{ colors.error|raw }}; } .sf-toolbar-block .sf-toolbar-status-yellow { - background-color: rgb(189, 132, 0); + background-color: {{ colors.warning|raw }}; } .sf-toolbar-block.sf-toolbar-status-green { - background-color: rgba(117, 158, 43, 0.8); + background-color: {{ colors.success|raw }}; color: #FFF; } .sf-toolbar-block.sf-toolbar-status-red { - background-color: rgba(200, 43, 43, 0.8); + background-color: {{ colors.error|raw }}; color: #FFF; } .sf-toolbar-block.sf-toolbar-status-yellow { - background-color: rgb(189, 132, 0); + background-color: {{ colors.warning|raw }}; color: #FFF; } From 99115243617612fc3985687f893bfd3c0bb134af Mon Sep 17 00:00:00 2001 From: Ener-Getick Date: Mon, 21 Dec 2015 13:34:11 +0100 Subject: [PATCH 4/7] [Validator] fixed wrong php docs --- src/Symfony/Component/Validator/Validation.php | 2 ++ src/Symfony/Component/Validator/ValidatorBuilderInterface.php | 1 + 2 files changed, 3 insertions(+) diff --git a/src/Symfony/Component/Validator/Validation.php b/src/Symfony/Component/Validator/Validation.php index de77e838fb..92ee6cf05b 100644 --- a/src/Symfony/Component/Validator/Validation.php +++ b/src/Symfony/Component/Validator/Validation.php @@ -11,6 +11,8 @@ namespace Symfony\Component\Validator; +use Symfony\Component\Validator\Validator\ValidatorInterface; + /** * Entry point for the Validator component. * diff --git a/src/Symfony/Component/Validator/ValidatorBuilderInterface.php b/src/Symfony/Component/Validator/ValidatorBuilderInterface.php index 918c8be392..aeab356908 100644 --- a/src/Symfony/Component/Validator/ValidatorBuilderInterface.php +++ b/src/Symfony/Component/Validator/ValidatorBuilderInterface.php @@ -15,6 +15,7 @@ use Doctrine\Common\Annotations\Reader; use Symfony\Component\Translation\TranslatorInterface; use Symfony\Component\Validator\Mapping\Cache\CacheInterface; use Symfony\Component\Validator\Mapping\Factory\MetadataFactoryInterface; +use Symfony\Component\Validator\Validator\ValidatorInterface; /** * A configurable builder for ValidatorInterface objects. From d0c0294a7b4dc9ab8c7225048a2ce42742de625d Mon Sep 17 00:00:00 2001 From: Alessandro Chitolina Date: Tue, 22 Dec 2015 18:06:45 +0100 Subject: [PATCH 5/7] [PropertyAccess] Reorder elements array after PropertyPathBuilder::replace | Q | A | ------------- | --- | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | #17102 | License | MIT | Doc PR | --- .../Component/PropertyAccess/PropertyPathBuilder.php | 1 + .../PropertyAccess/Tests/PropertyPathBuilderTest.php | 11 +++++++++++ 2 files changed, 12 insertions(+) diff --git a/src/Symfony/Component/PropertyAccess/PropertyPathBuilder.php b/src/Symfony/Component/PropertyAccess/PropertyPathBuilder.php index 5d7fdac6fd..39bf2da2e4 100644 --- a/src/Symfony/Component/PropertyAccess/PropertyPathBuilder.php +++ b/src/Symfony/Component/PropertyAccess/PropertyPathBuilder.php @@ -142,6 +142,7 @@ class PropertyPathBuilder $this->elements[$offset + $i] = $path->getElement($pathOffset + $i); $this->isIndex[$offset + $i] = $path->isIndex($pathOffset + $i); } + ksort($this->elements); } /** diff --git a/src/Symfony/Component/PropertyAccess/Tests/PropertyPathBuilderTest.php b/src/Symfony/Component/PropertyAccess/Tests/PropertyPathBuilderTest.php index 3767e08c82..6b4fdd8db9 100644 --- a/src/Symfony/Component/PropertyAccess/Tests/PropertyPathBuilderTest.php +++ b/src/Symfony/Component/PropertyAccess/Tests/PropertyPathBuilderTest.php @@ -252,6 +252,17 @@ class PropertyPathBuilderTest extends \PHPUnit_Framework_TestCase $this->assertEquals($path, $builder->getPropertyPath()); } + public function testReplaceWithLongerPathKeepsOrder() + { + $path = new PropertyPath('new1.new2.new3'); + $expected = new PropertyPath('new1.new2.new3.old2'); + + $builder = new PropertyPathBuilder(new PropertyPath('old1.old2')); + $builder->replace(0, 1, $path); + + $this->assertEquals($expected, $builder->getPropertyPath()); + } + public function testRemove() { $this->builder->remove(3); From 0dc9389982ae39fd161c98c7993c98412d2996c3 Mon Sep 17 00:00:00 2001 From: Nicolas Grekas Date: Fri, 18 Dec 2015 21:16:23 +0100 Subject: [PATCH 6/7] [Process] Make tests more deterministic --- src/Symfony/Component/Process/Process.php | 9 +-- .../Component/Process/Tests/ProcessTest.php | 68 ++++++++++--------- 2 files changed, 38 insertions(+), 39 deletions(-) diff --git a/src/Symfony/Component/Process/Process.php b/src/Symfony/Component/Process/Process.php index 92131ae6f2..4a9f3fbb26 100644 --- a/src/Symfony/Component/Process/Process.php +++ b/src/Symfony/Component/Process/Process.php @@ -1225,14 +1225,7 @@ class Process return $result = false; } - $proc = @proc_open('echo 1', array(array('pty'), array('pty'), array('pty')), $pipes); - if (is_resource($proc)) { - proc_close($proc); - - return $result = true; - } - - return $result = false; + return $result = (bool) @proc_open('echo 1', array(array('pty'), array('pty'), array('pty')), $pipes); } /** diff --git a/src/Symfony/Component/Process/Tests/ProcessTest.php b/src/Symfony/Component/Process/Tests/ProcessTest.php index e91c59e091..6f36accb28 100644 --- a/src/Symfony/Component/Process/Tests/ProcessTest.php +++ b/src/Symfony/Component/Process/Tests/ProcessTest.php @@ -828,30 +828,29 @@ class ProcessTest extends \PHPUnit_Framework_TestCase $process->checkTimeout(); usleep(100000); } - $this->fail('A RuntimeException should have been raised'); - } catch (RuntimeException $e) { + $this->fail('A ProcessTimedOutException should have been raised'); + } catch (ProcessTimedOutException $e) { } - $duration = microtime(true) - $start; - $this->assertLessThan(3, $duration); + $this->assertLessThan(15, microtime(true) - $start); throw $e; } public function testIdleTimeout() { - $process = $this->getProcess(self::$phpBin.' -r "sleep(3);"'); - $process->setTimeout(10); - $process->setIdleTimeout(0.5); + $process = $this->getProcess(self::$phpBin.' -r "sleep(34);"'); + $process->setTimeout(60); + $process->setIdleTimeout(0.1); try { $process->run(); $this->fail('A timeout exception was expected.'); - } catch (ProcessTimedOutException $ex) { - $this->assertTrue($ex->isIdleTimeout()); - $this->assertFalse($ex->isGeneralTimeout()); - $this->assertEquals(0.5, $ex->getExceededTimeout()); + } catch (ProcessTimedOutException $e) { + $this->assertTrue($e->isIdleTimeout()); + $this->assertFalse($e->isGeneralTimeout()); + $this->assertEquals(0.1, $e->getExceededTimeout()); } } @@ -860,17 +859,23 @@ class ProcessTest extends \PHPUnit_Framework_TestCase if ('\\' === DIRECTORY_SEPARATOR) { $this->markTestIncomplete('This test fails with a timeout on Windows, can someone investigate please?'); } - $process = $this->getProcess(sprintf('%s -r %s', self::$phpBin, escapeshellarg('$n = 30; while ($n--) {echo "foo\n"; usleep(100000); }'))); - $process->setTimeout(2); - $process->setIdleTimeout(1); + $process = $this->getProcess(sprintf('%s -r %s', self::$phpBin, escapeshellarg('while (true) {echo "foo\n"; usleep(10000);}'))); + $process->setTimeout(1); + $process->start(); + + while (false === strpos($process->getOutput(), 'foo')) { + usleep(1000); + } + + $process->setIdleTimeout(0.1); try { - $process->run(); + $process->wait(); $this->fail('A timeout exception was expected.'); } catch (ProcessTimedOutException $ex) { $this->assertTrue($ex->isGeneralTimeout(), 'A general timeout is expected.'); $this->assertFalse($ex->isIdleTimeout(), 'No idle timeout is expected.'); - $this->assertEquals(2, $ex->getExceededTimeout()); + $this->assertEquals(1, $ex->getExceededTimeout()); } } @@ -885,11 +890,12 @@ class ProcessTest extends \PHPUnit_Framework_TestCase try { $process->run(); - $this->fail('A RuntimeException should have been raised.'); - } catch (RuntimeException $e) { + $this->fail('A ProcessTimedOutException should have been raised.'); + } catch (ProcessTimedOutException $e) { } $this->assertFalse($process->isRunning()); $process->start(); + $this->assertTrue($process->isRunning()); $process->stop(0); throw $e; @@ -1047,7 +1053,7 @@ class ProcessTest extends \PHPUnit_Framework_TestCase public function testDisableOutputDisablesTheOutput() { - $p = $this->getProcess(self::$phpBin.' -r "usleep(500000);"'); + $p = $this->getProcess('foo'); $this->assertFalse($p->isOutputDisabled()); $p->disableOutput(); $this->assertTrue($p->isOutputDisabled()); @@ -1061,7 +1067,7 @@ class ProcessTest extends \PHPUnit_Framework_TestCase */ public function testDisableOutputWhileRunningThrowsException() { - $p = $this->getProcess(self::$phpBin.' -r "usleep(500000);"'); + $p = $this->getProcess(self::$phpBin.' -r "sleep(39);"'); $p->start(); $p->disableOutput(); } @@ -1072,7 +1078,7 @@ class ProcessTest extends \PHPUnit_Framework_TestCase */ public function testEnableOutputWhileRunningThrowsException() { - $p = $this->getProcess(self::$phpBin.' -r "usleep(500000);"'); + $p = $this->getProcess(self::$phpBin.' -r "sleep(40);"'); $p->disableOutput(); $p->start(); $p->enableOutput(); @@ -1080,12 +1086,12 @@ class ProcessTest extends \PHPUnit_Framework_TestCase public function testEnableOrDisableOutputAfterRunDoesNotThrowException() { - $p = $this->getProcess(self::$phpBin.' -r "usleep(500000);"'); + $p = $this->getProcess('echo foo'); $p->disableOutput(); - $p->start(); - $p->wait(); + $p->run(); $p->enableOutput(); $p->disableOutput(); + $this->assertTrue($p->isOutputDisabled()); } /** @@ -1094,7 +1100,7 @@ class ProcessTest extends \PHPUnit_Framework_TestCase */ public function testDisableOutputWhileIdleTimeoutIsSet() { - $process = $this->getProcess('sleep 3'); + $process = $this->getProcess('foo'); $process->setIdleTimeout(1); $process->disableOutput(); } @@ -1105,16 +1111,16 @@ class ProcessTest extends \PHPUnit_Framework_TestCase */ public function testSetIdleTimeoutWhileOutputIsDisabled() { - $process = $this->getProcess('sleep 3'); + $process = $this->getProcess('foo'); $process->disableOutput(); $process->setIdleTimeout(1); } public function testSetNullIdleTimeoutWhileOutputIsDisabled() { - $process = $this->getProcess('sleep 3'); + $process = $this->getProcess('foo'); $process->disableOutput(); - $process->setIdleTimeout(null); + $this->assertSame($process, $process->setIdleTimeout(null)); } /** @@ -1122,7 +1128,7 @@ class ProcessTest extends \PHPUnit_Framework_TestCase */ public function testStartWithACallbackAndDisabledOutput($startMethod, $exception, $exceptionMessage) { - $p = $this->getProcess(self::$phpBin.' -r "usleep(500000);"'); + $p = $this->getProcess('foo'); $p->disableOutput(); $this->setExpectedException($exception, $exceptionMessage); if ('mustRun' === $startMethod) { @@ -1147,7 +1153,7 @@ class ProcessTest extends \PHPUnit_Framework_TestCase */ public function testGetOutputWhileDisabled($fetchMethod) { - $p = $this->getProcess(self::$phpBin.' -r "usleep(500000);"'); + $p = $this->getProcess(self::$phpBin.' -r "sleep(41);"'); $p->disableOutput(); $p->start(); $p->{$fetchMethod}(); @@ -1162,7 +1168,7 @@ class ProcessTest extends \PHPUnit_Framework_TestCase array('getIncrementalErrorOutput'), ); } - + public function testStopTerminatesProcessCleanly() { $process = $this->getProcess(self::$phpBin.' -r "echo 123; sleep(42);"'); From 5517368265d1465a3bcd452a8850ca355d72e517 Mon Sep 17 00:00:00 2001 From: Nicolas Grekas Date: Wed, 23 Dec 2015 08:35:50 +0100 Subject: [PATCH 7/7] [Process] Fix transient test on Windows --- src/Symfony/Component/Process/Tests/ProcessTest.php | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/Symfony/Component/Process/Tests/ProcessTest.php b/src/Symfony/Component/Process/Tests/ProcessTest.php index 4b1d4d640b..d64eb07d30 100644 --- a/src/Symfony/Component/Process/Tests/ProcessTest.php +++ b/src/Symfony/Component/Process/Tests/ProcessTest.php @@ -645,7 +645,7 @@ class ProcessTest extends \PHPUnit_Framework_TestCase } $this->skipIfNotEnhancedSigchild(false); - $process = $this->getProcess(self::$phpBin.' -r "while (true) usleep(100);"'); + $process = $this->getProcess(self::$phpBin.' -r "sleep(32.1)"'); $process->start(); posix_kill($process->getPid(), 9); // SIGKILL @@ -729,9 +729,8 @@ class ProcessTest extends \PHPUnit_Framework_TestCase $this->fail('A RuntimeException should have been raised'); } catch (RuntimeException $e) { } - $duration = microtime(true) - $start; - $this->assertLessThan(3, $duration); + $this->assertLessThan(15, microtime(true) - $start); throw $e; }