From 3c313e4322a376585264f5a03d7b6d540ecd941f Mon Sep 17 00:00:00 2001 From: Karim Geiger Date: Mon, 19 Mar 2018 15:00:23 +0100 Subject: [PATCH 1/3] #310 Implement new SqliteRunner which uses sqlite to dispatch tests. This commit implements a SqliteRunner which attempts to resolve the issues faced in #310 with the WrapperRunner. It's derived from the WrapperRunner and dispatches n workers upon start, which then independently fetch pending tests from a pre-populated sqlite database until all tests are completed. This commit also changes the way the runners are chosen during bootstrapping. It will now dynamically check for the class in the default namespace. In addition, this commit will remove the isRunning flag from the WrapperRunner because accessing the process information is more reliable. closes #310 --- README.md | 6 +- bin/phpunit-sqlite-wrapper | 36 ++++ src/Console/Testers/PHPUnit.php | 38 ++-- src/Runners/PHPUnit/BaseRunner.php | 15 +- src/Runners/PHPUnit/ExecutableTest.php | 2 +- src/Runners/PHPUnit/SqliteRunner.php | 152 ++++++++++++++++ .../{Worker.php => Worker/BaseWorker.php} | 170 ++++-------------- src/Runners/PHPUnit/Worker/SqliteWorker.php | 23 +++ src/Runners/PHPUnit/Worker/WrapperWorker.php | 110 ++++++++++++ src/Runners/PHPUnit/WrapperRunner.php | 14 +- test/functional/PHPUnitTest.php | 15 ++ .../ParaTest/Runners/PHPUnit/WorkerTest.php | 20 +-- test/functional/SqliteRunnerTest.php | 81 +++++++++ .../Console/Commands/ParaTestCommandTest.php | 2 +- test/unit/Console/Testers/PHPUnitTest.php | 2 +- 15 files changed, 509 insertions(+), 177 deletions(-) create mode 100755 bin/phpunit-sqlite-wrapper create mode 100644 src/Runners/PHPUnit/SqliteRunner.php rename src/Runners/PHPUnit/{Worker.php => Worker/BaseWorker.php} (50%) create mode 100644 src/Runners/PHPUnit/Worker/SqliteWorker.php create mode 100644 src/Runners/PHPUnit/Worker/WrapperWorker.php create mode 100644 test/functional/SqliteRunnerTest.php diff --git a/README.md b/README.md index 5d119710..ea3c21cc 100644 --- a/README.md +++ b/README.md @@ -50,7 +50,7 @@ Options: --max-batch-size (-m) Max batch size (only for functional mode). (default: 0) --filter Filter (only for functional mode). --phpunit The PHPUnit binary to execute. (default: vendor/bin/phpunit) - --runner Runner or WrapperRunner. (default: Runner) + --runner Runner, WrapperRunner or SqliteRunner. (default: Runner) --bootstrap The bootstrap file to be used by PHPUnit. --configuration (-c) The PHPUnit configuration file to use. --group (-g) Only runs tests from the specified group(s). @@ -75,9 +75,9 @@ To get the most out of paratest, you have to adjust the parameters carefully. Given you have few testcases (classes) with many long running methods, you should use the `-f` option to enable the `functional mode` and allow different methods of the same class to be executed in parallel. Keep in mind that the default is per-testcase-parallelization to address inter-testmethod dependencies. Note that in most projects, using `-f` is **slower** since each test **method** will need to be bootstrapped separately. - 3. **Use the WrapperRunner if possible** + 3. **Use the WrapperRunner or SqliteRunner if possible** - The default Runner for PHPUnit spawns a new process for each testcase (or method in functional mode). This provides the highest compatibility but comes with the cost of many spawned processes and a bootstrapping for each process. Especially when you have a slow bootstrapping in your tests (like a database setup) you should try the WrapperRunner with `--runner WrapperRunner`. It spawns one "worker"-process for each parallel process (`-p`), executes the bootstrapping once and reuses these processes for each test executed. That way the overhead of process spawning and bootstrapping is reduced to the minimum. + The default Runner for PHPUnit spawns a new process for each testcase (or method in functional mode). This provides the highest compatibility but comes with the cost of many spawned processes and a bootstrapping for each process. Especially when you have a slow bootstrapping in your tests (like a database setup) you should try the WrapperRunner with `--runner WrapperRunner` or the SqliteRunner with `--runner SqliteRunner`. It spawns one "worker"-process for each parallel process (`-p`), executes the bootstrapping once and reuses these processes for each test executed. That way the overhead of process spawning and bootstrapping is reduced to the minimum. 4. **Tune batch max size `--max-batch-size`** diff --git a/bin/phpunit-sqlite-wrapper b/bin/phpunit-sqlite-wrapper new file mode 100755 index 00000000..67c73136 --- /dev/null +++ b/bin/phpunit-sqlite-wrapper @@ -0,0 +1,36 @@ +query('SELECT id, command FROM tests WHERE reserved_by_process_id IS NULL ORDER BY file_name LIMIT 1')->fetch()) { + $db->prepare('UPDATE tests SET reserved_by_process_id = :procId WHERE id = :id') + ->execute([ + ':procId' => getmypid(), + ':id' => $test['id'], + ]); + + try { + if (!preg_match_all('/\'([^\']*)\'[ ]?/', $test['command'], $arguments)) { + throw new \Exception("Failed to parse arguments from command line: \"" . $test['command'] . "\""); + } + $_SERVER['argv'] = $arguments[1]; + + PHPUnit\TextUI\Command::main(false); + } finally { + $db->prepare('UPDATE tests SET completed = 1 WHERE id = :id') + ->execute([':id' => $test['id']]); + } +} diff --git a/src/Console/Testers/PHPUnit.php b/src/Console/Testers/PHPUnit.php index cd146a14..ae5400c6 100644 --- a/src/Console/Testers/PHPUnit.php +++ b/src/Console/Testers/PHPUnit.php @@ -4,9 +4,10 @@ namespace ParaTest\Console\Testers; +use InvalidArgumentException; +use ParaTest\Runners\PHPUnit\BaseRunner; use ParaTest\Runners\PHPUnit\Configuration; use ParaTest\Runners\PHPUnit\Runner; -use ParaTest\Runners\PHPUnit\WrapperRunner; use Symfony\Component\Console\Command\Command; use Symfony\Component\Console\Input\InputArgument; use Symfony\Component\Console\Input\InputInterface; @@ -35,7 +36,7 @@ public function configure(Command $command) { $command ->addOption('phpunit', null, InputOption::VALUE_REQUIRED, 'The PHPUnit binary to execute. (default: vendor/bin/phpunit)') - ->addOption('runner', null, InputOption::VALUE_REQUIRED, 'Runner or WrapperRunner. (default: Runner)') + ->addOption('runner', null, InputOption::VALUE_REQUIRED, 'Runner, WrapperRunner or SqliteRunner. (default: Runner)') ->addOption('bootstrap', null, InputOption::VALUE_REQUIRED, 'The bootstrap file to be used by PHPUnit.') ->addOption('configuration', 'c', InputOption::VALUE_REQUIRED, 'The PHPUnit configuration file to use.') ->addOption('group', 'g', InputOption::VALUE_REQUIRED, 'Only runs tests from the specified group(s).') @@ -64,22 +65,7 @@ public function execute(InputInterface $input, OutputInterface $output) $this->displayHelp($input, $output); } - if ($input->getOption('runner') === 'WrapperRunner') { - $runner = new WrapperRunner($this->getRunnerOptions($input)); - } else { - if ($input->getOption('runner') !== '') { - // because we want to have to bootstrap script inherited before check/initialization - $runnerOption = $this->getRunnerOptions($input); - $runnerClass = $input->getOption('runner'); - if (null !== $runnerClass && class_exists($runnerClass)) { - $runner = new $runnerClass($runnerOption); - } - } - } - - if (!isset($runner)) { - $runner = new Runner($this->getRunnerOptions($input)); - } + $runner = $this->initializeRunner($input); $runner->run(); @@ -236,4 +222,20 @@ protected function getBootstrapFile(InputInterface $input, array $options): stri return ($bootstrap) ? $config->getConfigDir() . $bootstrap : ''; } + + private function initializeRunner(InputInterface $input): BaseRunner + { + if ($input->getOption('runner')) { + $runnerClass = $input->getOption('runner') ?: ''; + $runnerClass = class_exists($runnerClass) ? $runnerClass : ('\\ParaTest\\Runners\\PHPUnit\\' . $runnerClass); + } else { + $runnerClass = Runner::class; + } + + if (!class_exists($runnerClass)) { + throw new InvalidArgumentException('Selected runner does not exist.'); + } + + return new $runnerClass($this->getRunnerOptions($input)); + } } diff --git a/src/Runners/PHPUnit/BaseRunner.php b/src/Runners/PHPUnit/BaseRunner.php index 3454afd9..aada9e07 100644 --- a/src/Runners/PHPUnit/BaseRunner.php +++ b/src/Runners/PHPUnit/BaseRunner.php @@ -31,7 +31,7 @@ abstract class BaseRunner * A collection of pending ExecutableTest objects that have * yet to run. * - * @var array + * @var ExecutableTest[] */ protected $pending = []; @@ -67,10 +67,7 @@ public function __construct(array $opts = []) public function run() { - $this->verifyConfiguration(); - $this->initCoverage(); - $this->load(new SuiteLoader($this->options)); - $this->printer->start($this->options); + $this->initialize(); } /** @@ -176,4 +173,12 @@ protected function getCoverage() { return $this->coverage; } + + protected function initialize(): void + { + $this->verifyConfiguration(); + $this->initCoverage(); + $this->load(new SuiteLoader($this->options)); + $this->printer->start($this->options); + } } diff --git a/src/Runners/PHPUnit/ExecutableTest.php b/src/Runners/PHPUnit/ExecutableTest.php index a110b33c..a67d4f69 100644 --- a/src/Runners/PHPUnit/ExecutableTest.php +++ b/src/Runners/PHPUnit/ExecutableTest.php @@ -51,7 +51,7 @@ abstract class ExecutableTest * * @var string */ - protected $lastCommand; + protected $lastCommand = ''; public function __construct(string $path, string $fullyQualifiedClassName = null) { diff --git a/src/Runners/PHPUnit/SqliteRunner.php b/src/Runners/PHPUnit/SqliteRunner.php new file mode 100644 index 00000000..2cb675bb --- /dev/null +++ b/src/Runners/PHPUnit/SqliteRunner.php @@ -0,0 +1,152 @@ +dbFileName = (string)($opts['database'] ?? tempnam(sys_get_temp_dir(), 'paratest_db_')); + $this->db = new PDO('sqlite:' . $this->dbFileName); + } + + public function run() + { + $this->initialize(); + + $this->createTable(); + $this->assignAllPendingTests(); + $this->startWorkers(); + $this->waitForAllToFinish(); + $this->complete(); + $this->checkIfWorkersCrashed(); + } + + /** + * Start all workers. + */ + protected function startWorkers(): void + { + $wrapper = realpath(__DIR__ . '/../../../bin/phpunit-sqlite-wrapper'); + + for ($i = 1; $i <= $this->options->processes; ++$i) { + $worker = new SqliteWorker($this->dbFileName); + if ($this->options->noTestTokens) { + $token = null; + $uniqueToken = null; + } else { + $token = $i; + $uniqueToken = uniqid(); + } + $worker->start($wrapper, $token, $uniqueToken); + $this->workers[] = $worker; + } + } + + /** + * Wait for all workers to complete their tests and print output. + */ + private function waitForAllToFinish(): void + { + do { + foreach ($this->workers as $key => $worker) { + if (!$worker->isRunning()) { + unset($this->workers[$key]); + } + } + usleep(10000); + $this->printOutput(); + } while (count($this->workers) > 0); + } + + /** + * Initialize test queue table. + * + * @throws Exception + */ + private function createTable(): void + { + $statement = 'CREATE TABLE tests ( + id INTEGER PRIMARY KEY, + command TEXT NOT NULL UNIQUE, + file_name TEXT NOT NULL, + reserved_by_process_id INTEGER, + completed INTEGER DEFAULT 0 + )'; + + if ($this->db->exec($statement) === false) { + throw new Exception('Error while creating sqlite database table: ' . $this->db->errorCode()); + } + } + + /** + * Push all tests onto test queue. + */ + private function assignAllPendingTests(): void + { + foreach ($this->pending as $fileName => $test) { + $this->db->prepare('INSERT INTO tests (command, file_name) VALUES (:command, :fileName)') + ->execute([ + ':command' => $test->command($this->options->phpunit, $this->options->filtered), + ':fileName' => $fileName + ]); + } + } + + /** + * Loop through all completed tests and print their output. + */ + private function printOutput(): void + { + foreach ($this->db->query('SELECT id, file_name FROM tests WHERE completed = 1')->fetchAll() as $test) { + $this->printer->printFeedback($this->pending[$test['file_name']]); + $this->db->prepare('DELETE FROM tests WHERE id = :id')->execute([ + 'id' => $test['id'] + ]); + } + } + + public function __destruct() + { + if ($this->db !== null) { + unset($this->db); + unlink($this->dbFileName); + } + } + + /** + * Make sure that all tests were executed successfully. + */ + private function checkIfWorkersCrashed(): void + { + if ($this->db->query('SELECT COUNT(id) FROM tests')->fetchColumn(0) === "0") { + return; + } + + throw new RuntimeException( + 'Some workers have crashed.' . PHP_EOL + . '----------------------' . PHP_EOL + . 'All workers have quit, but some tests are still to be executed.' . PHP_EOL + . 'This may be the case if some tests were killed forcefully (for example, using exit()).' . PHP_EOL + . '----------------------' . PHP_EOL + . 'Failed test command(s):' . PHP_EOL + . '----------------------' . PHP_EOL + . implode(PHP_EOL, $this->db->query('SELECT command FROM tests')->fetchAll(PDO::FETCH_COLUMN)) + ); + } +} \ No newline at end of file diff --git a/src/Runners/PHPUnit/Worker.php b/src/Runners/PHPUnit/Worker/BaseWorker.php similarity index 50% rename from src/Runners/PHPUnit/Worker.php rename to src/Runners/PHPUnit/Worker/BaseWorker.php index 78059cf8..af2146f6 100644 --- a/src/Runners/PHPUnit/Worker.php +++ b/src/Runners/PHPUnit/Worker/BaseWorker.php @@ -2,32 +2,25 @@ declare(strict_types=1); -namespace ParaTest\Runners\PHPUnit; +namespace ParaTest\Runners\PHPUnit\Worker; -use Exception; use Symfony\Component\Process\PhpExecutableFinder; -class Worker +abstract class BaseWorker { - private static $descriptorspec = [ - 0 => ['pipe', 'r'], - 1 => ['pipe', 'w'], - 2 => ['pipe', 'w'], + protected static $descriptorspec = [ + 0 => ['pipe', 'r'], + 1 => ['pipe', 'w'], + 2 => ['pipe', 'w'], ]; - private $proc; - private $pipes; - private $inExecution = 0; - private $isRunning = false; + protected $proc; + protected $pipes; + protected $inExecution = 0; private $exitCode = null; - private $commands = []; private $chunks = ''; private $alreadyReadOutput = ''; - /** - * @var ExecutableTest - */ - private $currentlyExecuting; - public function start(string $wrapperBinary, $token = 1, $uniqueToken = null) + public function start(string $wrapperBinary, $token = 1, $uniqueToken = null, array $parameters = []) { $bin = 'PARATEST=1 '; if (is_numeric($token)) { @@ -38,85 +31,12 @@ public function start(string $wrapperBinary, $token = 1, $uniqueToken = null) } $finder = new PhpExecutableFinder(); $bin .= $finder->find() . " \"$wrapperBinary\""; + if ($parameters) { + $bin .= ' ' . implode(' ', array_map('escapeshellarg', $parameters)); + } $pipes = []; $this->proc = proc_open($bin, self::$descriptorspec, $pipes); $this->pipes = $pipes; - $this->isRunning = true; - } - - public function stdout() - { - return $this->pipes[1]; - } - - public function execute(string $testCmd) - { - $this->checkStarted(); - $this->commands[] = $testCmd; - fwrite($this->pipes[0], $testCmd . "\n"); - ++$this->inExecution; - } - - public function assign(ExecutableTest $test, string $phpunit, array $phpunitOptions) - { - if ($this->currentlyExecuting !== null) { - throw new Exception('Worker already has a test assigned - did you forget to call reset()?'); - } - $this->currentlyExecuting = $test; - $this->execute($test->command($phpunit, $phpunitOptions)); - } - - public function printFeedback(ResultPrinter $printer) - { - if ($this->currentlyExecuting !== null) { - $printer->printFeedback($this->currentlyExecuting); - } - } - - public function reset() - { - $this->currentlyExecuting = null; - } - - public function isStarted(): bool - { - return $this->proc !== null && $this->pipes !== null; - } - - private function checkStarted() - { - if (!$this->isStarted()) { - throw new \RuntimeException('You have to start the Worker first!'); - } - } - - public function stop() - { - fwrite($this->pipes[0], "EXIT\n"); - fclose($this->pipes[0]); - } - - /** - * This is an utility function for tests. - * Refactor or write it only in the test case. - */ - public function waitForFinishedJob() - { - if ($this->inExecution === 0) { - return; - } - $tellsUsItHasFinished = false; - stream_set_blocking($this->pipes[1], true); - while ($line = fgets($this->pipes[1])) { - if (strstr($line, "FINISHED\n")) { - $tellsUsItHasFinished = true; - --$this->inExecution; - break; - } - } - if (!$tellsUsItHasFinished) { - throw new \RuntimeException('The Worker terminated without finishing the job.'); - } } public function isFree(): bool @@ -127,43 +47,20 @@ public function isFree(): bool return $this->inExecution === 0; } - /** - * @deprecated - * This function consumes a lot of CPU while waiting for - * the worker to finish. Use it only in testing paratest - * itself. - */ - public function waitForStop() + public function isRunning(): bool { - $status = proc_get_status($this->proc); - while ($status['running']) { - $status = proc_get_status($this->proc); - $this->setExitCode($status); + if ($this->proc === null) { + return false; } - } - public function getCoverageFileName() - { - if ($this->currentlyExecuting !== null) { - return $this->currentlyExecuting->getCoverageFileName(); - } - } + $status = proc_get_status($this->proc); - private function setExitCode(array $status) - { - if (!$status['running']) { - if ($this->exitCode === null) { - $this->exitCode = $status['exitcode']; - } - } + return $status['running']; } - public function isRunning(): bool + public function isStarted(): bool { - $this->checkNotCrashed(); - $this->updateStateFromAvailableOutput(); - - return $this->isRunning; + return $this->proc !== null && $this->pipes !== null; } public function isCrashed(): bool @@ -174,9 +71,6 @@ public function isCrashed(): bool $status = proc_get_status($this->proc); $this->updateStateFromAvailableOutput(); - if (!$this->isRunning) { - return false; - } $this->setExitCode($status); if ($this->exitCode === null) { @@ -186,11 +80,12 @@ public function isCrashed(): bool return $this->exitCode !== 0; } - private function checkNotCrashed() + public function checkNotCrashed() { if ($this->isCrashed()) { + $lastCommand = isset($this->commands) ? ' Last executed command: ' . end($this->commands) : ''; throw new \RuntimeException( - 'This worker has crashed. Last executed command: ' . end($this->commands) . PHP_EOL + 'This worker has crashed.' . $lastCommand . PHP_EOL . 'Output:' . PHP_EOL . '----------------------' . PHP_EOL . $this->alreadyReadOutput . PHP_EOL @@ -200,6 +95,20 @@ private function checkNotCrashed() } } + public function stop() + { + fclose($this->pipes[0]); + } + + protected function setExitCode(array $status) + { + if (!$status['running']) { + if ($this->exitCode === null) { + $this->exitCode = $status['exitcode']; + } + } + } + private function readAllStderr() { return stream_get_contents($this->pipes[2]); @@ -229,11 +138,8 @@ private function updateStateFromAvailableOutput() if (strstr($line, "FINISHED\n")) { --$this->inExecution; } - if (strstr($line, "EXITED\n")) { - $this->isRunning = false; - } } stream_set_blocking($this->pipes[1], true); } } -} +} \ No newline at end of file diff --git a/src/Runners/PHPUnit/Worker/SqliteWorker.php b/src/Runners/PHPUnit/Worker/SqliteWorker.php new file mode 100644 index 00000000..634a761a --- /dev/null +++ b/src/Runners/PHPUnit/Worker/SqliteWorker.php @@ -0,0 +1,23 @@ +dbFileName = $dbFileName; + } + + public function start(string $wrapperBinary, $token = 1, $uniqueToken = null, array $parameters = []) + { + $parameters[] = $this->dbFileName; + + parent::start($wrapperBinary, $token, $uniqueToken, $parameters); + } +} \ No newline at end of file diff --git a/src/Runners/PHPUnit/Worker/WrapperWorker.php b/src/Runners/PHPUnit/Worker/WrapperWorker.php new file mode 100644 index 00000000..bf983f6b --- /dev/null +++ b/src/Runners/PHPUnit/Worker/WrapperWorker.php @@ -0,0 +1,110 @@ +pipes[1]; + } + + public function execute(string $testCmd) + { + $this->checkStarted(); + $this->commands[] = $testCmd; + fwrite($this->pipes[0], $testCmd . "\n"); + ++$this->inExecution; + } + + public function assign(ExecutableTest $test, string $phpunit, array $phpunitOptions) + { + if ($this->currentlyExecuting !== null) { + throw new Exception('Worker already has a test assigned - did you forget to call reset()?'); + } + $this->currentlyExecuting = $test; + $this->execute($test->command($phpunit, $phpunitOptions)); + } + + public function printFeedback(ResultPrinter $printer) + { + if ($this->currentlyExecuting !== null) { + $printer->printFeedback($this->currentlyExecuting); + } + } + + public function reset() + { + $this->currentlyExecuting = null; + } + + protected function checkStarted() + { + if (!$this->isStarted()) { + throw new \RuntimeException('You have to start the Worker first!'); + } + } + + public function stop() + { + fwrite($this->pipes[0], "EXIT\n"); + parent::stop(); + } + + /** + * This is an utility function for tests. + * Refactor or write it only in the test case. + */ + public function waitForFinishedJob() + { + if ($this->inExecution === 0) { + return; + } + $tellsUsItHasFinished = false; + stream_set_blocking($this->pipes[1], true); + while ($line = fgets($this->pipes[1])) { + if (strstr($line, "FINISHED\n")) { + $tellsUsItHasFinished = true; + --$this->inExecution; + break; + } + } + if (!$tellsUsItHasFinished) { + throw new \RuntimeException('The Worker terminated without finishing the job.'); + } + } + + /** + * @deprecated + * This function consumes a lot of CPU while waiting for + * the worker to finish. Use it only in testing paratest + * itself. + */ + public function waitForStop() + { + $status = proc_get_status($this->proc); + while ($status['running']) { + $status = proc_get_status($this->proc); + $this->setExitCode($status); + } + } + + public function getCoverageFileName() + { + if ($this->currentlyExecuting !== null) { + return $this->currentlyExecuting->getCoverageFileName(); + } + } +} diff --git a/src/Runners/PHPUnit/WrapperRunner.php b/src/Runners/PHPUnit/WrapperRunner.php index a980ca2f..2ee06aa9 100644 --- a/src/Runners/PHPUnit/WrapperRunner.php +++ b/src/Runners/PHPUnit/WrapperRunner.php @@ -4,6 +4,8 @@ namespace ParaTest\Runners\PHPUnit; +use ParaTest\Runners\PHPUnit\Worker\WrapperWorker; + class WrapperRunner extends BaseRunner { const PHPUNIT_FAILURES = 1; @@ -15,7 +17,7 @@ class WrapperRunner extends BaseRunner protected $streams; /** - * @var Worker[] + * @var WrapperWorker[] */ protected $workers; @@ -43,11 +45,11 @@ protected function load(SuiteLoader $loader) parent::load($loader); } - private function startWorkers() + protected function startWorkers() { $wrapper = realpath(__DIR__ . '/../../../bin/phpunit-wrapper'); for ($i = 1; $i <= $this->options->processes; ++$i) { - $worker = new Worker(); + $worker = new WrapperWorker(); if ($this->options->noTestTokens) { $token = null; $uniqueToken = null; @@ -119,7 +121,7 @@ private function waitForStreamsToChange(array $modified) /** * put on WorkersPool. * - * @return Worker[] + * @return WrapperWorker[] */ private function progressedWorkers(): array { @@ -156,7 +158,7 @@ private function streamsOf(array $workers): array return $streams; } - private function complete() + protected function complete() { $this->setExitCode(); $this->printer->printResults(); @@ -180,7 +182,7 @@ private function setExitCode() } } - private function flushWorker(Worker $worker) + private function flushWorker(WrapperWorker $worker) { if ($this->hasCoverage()) { $this->getCoverage()->addCoverageFromFile($worker->getCoverageFileName()); diff --git a/test/functional/PHPUnitTest.php b/test/functional/PHPUnitTest.php index d35c2751..3d440662 100644 --- a/test/functional/PHPUnitTest.php +++ b/test/functional/PHPUnitTest.php @@ -41,6 +41,14 @@ public function testWithWrapperRunner() ])); } + public function testWithSqliteRunner() + { + $this->assertTestsPassed($this->invokeParatest('passing-tests', [ + 'configuration' => PHPUNIT_CONFIGURATION, + 'runner' => 'SqliteRunner', + ])); + } + public function testWithCustomRunner() { $cb = new ProcessCallback(); @@ -97,6 +105,13 @@ public function testParatestEnvironmentVariableWithWrapperRunnerandWithoutTestTo $this->assertRegexp('/Failures: 1/', $proc->getOutput()); } + public function testParatestEnvironmentVariableWithSqliteRunner() + { + $this->assertTestsPassed($this->invokeParatest('paratest-only-tests/EnvironmentTest.php', + ['bootstrap' => BOOTSTRAP, 'runner' => 'SqliteRunner'] + )); + } + public function testWithConfigurationInDirWithoutConfigFile() { chdir(dirname(FIXTURES)); diff --git a/test/functional/ParaTest/Runners/PHPUnit/WorkerTest.php b/test/functional/ParaTest/Runners/PHPUnit/WorkerTest.php index 2657223b..554f48ec 100644 --- a/test/functional/ParaTest/Runners/PHPUnit/WorkerTest.php +++ b/test/functional/ParaTest/Runners/PHPUnit/WorkerTest.php @@ -4,6 +4,7 @@ namespace ParaTest\Runners\PHPUnit; +use ParaTest\Runners\PHPUnit\Worker\WrapperWorker; use SimpleXMLElement; class WorkerTest extends \TestBase @@ -38,7 +39,7 @@ public function testReadsAPHPUnitCommandFromStdInAndExecutesItItsOwnProcess() { $testLog = '/tmp/test.xml'; $testCmd = $this->getCommand('passing-tests/TestOfUnits.php', $testLog); - $worker = new Worker(); + $worker = new WrapperWorker(); $worker->start($this->phpunitWrapper); $worker->execute($testCmd); @@ -52,7 +53,7 @@ public function testKnowsWhenAJobIsFinished() { $testLog = '/tmp/test.xml'; $testCmd = $this->getCommand('passing-tests/TestOfUnits.php', $testLog); - $worker = new Worker(); + $worker = new WrapperWorker(); $worker->start($this->phpunitWrapper); $worker->execute($testCmd); $worker->waitForFinishedJob(); @@ -64,7 +65,7 @@ public function testTellsWhenItsFree() { $testLog = '/tmp/test.xml'; $testCmd = $this->getCommand('passing-tests/TestOfUnits.php', $testLog); - $worker = new Worker(); + $worker = new WrapperWorker(); $worker->start($this->phpunitWrapper); $this->assertTrue($worker->isFree()); @@ -77,7 +78,7 @@ public function testTellsWhenItsFree() public function testTellsWhenItsStopped() { - $worker = new Worker(); + $worker = new WrapperWorker(); $this->assertFalse($worker->isRunning()); $worker->start($this->phpunitWrapper); @@ -88,14 +89,13 @@ public function testTellsWhenItsStopped() $this->assertFalse($worker->isRunning()); } - public function testProcessIsNotMarkedAsCrashedWhenItFinishesCorrectly() + public function testProcessIsMarkedAsCrashedWhenItFinishesWithNonZeroExitCode() { - // fake state: process has already exited (clean) but worker did not yet notice - $worker = new Worker(); - $this->setPerReflection($worker, 'isRunning', false); + // fake state: process has already exited (with non-zero exit code) but worker did not yet notice + $worker = new WrapperWorker(); $this->setPerReflection($worker, 'proc', $this->createSomeClosedProcess()); $this->setPerReflection($worker, 'pipes', [0 => true]); - $this->assertFalse($worker->isCrashed()); + $this->assertTrue($worker->isCrashed()); } private function createSomeClosedProcess() @@ -127,7 +127,7 @@ public function testCanExecuteMultiplePHPUnitCommands() { $bin = 'bin/phpunit-wrapper'; - $worker = new Worker(); + $worker = new WrapperWorker(); $worker->start($this->phpunitWrapper); $testLog = '/tmp/test.xml'; diff --git a/test/functional/SqliteRunnerTest.php b/test/functional/SqliteRunnerTest.php new file mode 100644 index 00000000..6e3ef30d --- /dev/null +++ b/test/functional/SqliteRunnerTest.php @@ -0,0 +1,81 @@ +generate(self::TEST_CLASSES, self::TEST_METHODS_PER_CLASS); + + $proc = $this->invokeParatest($generator->path, [ + 'runner' => 'SqliteRunner', + 'processes' => 3, + ]); + + $expected = self::TEST_CLASSES * self::TEST_METHODS_PER_CLASS; + $this->assertTestsPassed($proc, $expected, $expected); + } + + public function testMultiLineClassDeclarationWithFilenameDifferentThanClassnameIsSupported() + { + $this->assertTestsPassed($this->invokeParatest('special-classes', [ + 'runner' => 'SqliteRunner', + 'processes' => 3, + ])); + } + + public function testRunningFewerTestsThanTheWorkersIsPossible() + { + $generator = new TestGenerator(); + $generator->generate(1, 1); + + $proc = $this->invokeParatest($generator->path, [ + 'runner' => 'SqliteRunner', + 'processes' => 2, + ]); + + $this->assertTestsPassed($proc, 1, 1); + } + + public function testExitCodes() + { + $options = [ + 'runner' => 'SqliteRunner', + 'processes' => 1, + ]; + $proc = $this->invokeParatest('wrapper-runner-exit-code-tests/ErrorTest.php', $options); + $output = $proc->getOutput(); + + $this->assertContains('Tests: 1', $output); + $this->assertContains('Failures: 0', $output); + $this->assertContains('Errors: 1', $output); + $this->assertEquals(2, $proc->getExitCode()); + + $proc = $this->invokeParatest('wrapper-runner-exit-code-tests/FailureTest.php', $options); + $output = $proc->getOutput(); + + $this->assertContains('Tests: 1', $output); + $this->assertContains('Failures: 1', $output); + $this->assertContains('Errors: 0', $output); + $this->assertEquals(1, $proc->getExitCode()); + + $proc = $this->invokeParatest('wrapper-runner-exit-code-tests/SuccessTest.php', $options); + $output = $proc->getOutput(); + + $this->assertContains('OK (1 test, 1 assertion)', $output); + $this->assertEquals(0, $proc->getExitCode()); + + $options['processes'] = 3; + $proc = $this->invokeParatest('wrapper-runner-exit-code-tests', $options); + $output = $proc->getOutput(); + $this->assertContains('Tests: 3', $output); + $this->assertContains('Failures: 1', $output); + $this->assertContains('Errors: 1', $output); + $this->assertEquals(2, $proc->getExitCode()); // There is at least one error so the exit code must be 2 + } +} diff --git a/test/unit/Console/Commands/ParaTestCommandTest.php b/test/unit/Console/Commands/ParaTestCommandTest.php index ab8ab0c5..1fb4ec71 100644 --- a/test/unit/Console/Commands/ParaTestCommandTest.php +++ b/test/unit/Console/Commands/ParaTestCommandTest.php @@ -40,7 +40,7 @@ public function testConfiguredDefinitionWithPHPUnitTester() new InputOption('functional', 'f', InputOption::VALUE_NONE, 'Run methods instead of suites in separate processes.'), new InputOption('help', 'h', InputOption::VALUE_NONE, 'Display this help message.'), new InputOption('phpunit', null, InputOption::VALUE_REQUIRED, 'The PHPUnit binary to execute. (default: vendor/bin/phpunit)'), - new InputOption('runner', null, InputOption::VALUE_REQUIRED, 'Runner or WrapperRunner. (default: Runner)'), + new InputOption('runner', null, InputOption::VALUE_REQUIRED, 'Runner, WrapperRunner or SqliteRunner. (default: Runner)'), new InputOption('bootstrap', null, InputOption::VALUE_REQUIRED, 'The bootstrap file to be used by PHPUnit.'), new InputOption('configuration', 'c', InputOption::VALUE_REQUIRED, 'The PHPUnit configuration file to use.'), new InputOption('group', 'g', InputOption::VALUE_REQUIRED, 'Only runs tests from the specified group(s).'), diff --git a/test/unit/Console/Testers/PHPUnitTest.php b/test/unit/Console/Testers/PHPUnitTest.php index e681d5ca..e722e070 100644 --- a/test/unit/Console/Testers/PHPUnitTest.php +++ b/test/unit/Console/Testers/PHPUnitTest.php @@ -16,7 +16,7 @@ public function testConfigureAddsOptionsAndArgumentsToCommand() $testCommand = new TestCommand(); $definition = new InputDefinition([ new InputOption('phpunit', null, InputOption::VALUE_REQUIRED, 'The PHPUnit binary to execute. (default: vendor/bin/phpunit)'), - new InputOption('runner', null, InputOption::VALUE_REQUIRED, 'Runner or WrapperRunner. (default: Runner)'), + new InputOption('runner', null, InputOption::VALUE_REQUIRED, 'Runner, WrapperRunner or SqliteRunner. (default: Runner)'), new InputOption('bootstrap', null, InputOption::VALUE_REQUIRED, 'The bootstrap file to be used by PHPUnit.'), new InputOption('configuration', 'c', InputOption::VALUE_REQUIRED, 'The PHPUnit configuration file to use.'), new InputOption('group', 'g', InputOption::VALUE_REQUIRED, 'Only runs tests from the specified group(s).'), From 127cf5f61ede30938ff75b6cb318508da4c34d89 Mon Sep 17 00:00:00 2001 From: Karim Geiger Date: Tue, 20 Mar 2018 07:22:37 +0100 Subject: [PATCH 2/3] #310 Check update row count to avoid processing a test twice. --- bin/phpunit-sqlite-wrapper | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/bin/phpunit-sqlite-wrapper b/bin/phpunit-sqlite-wrapper index 67c73136..fc7f47f1 100755 --- a/bin/phpunit-sqlite-wrapper +++ b/bin/phpunit-sqlite-wrapper @@ -16,11 +16,16 @@ if (file_exists(__DIR__ . '/../../../autoload.php')) { } while ($test = $db->query('SELECT id, command FROM tests WHERE reserved_by_process_id IS NULL ORDER BY file_name LIMIT 1')->fetch()) { - $db->prepare('UPDATE tests SET reserved_by_process_id = :procId WHERE id = :id') - ->execute([ - ':procId' => getmypid(), - ':id' => $test['id'], - ]); + $statement = $db->prepare('UPDATE tests SET reserved_by_process_id = :procId WHERE id = :id AND reserved_by_process_id IS NULL'); + $statement->execute([ + ':procId' => getmypid(), + ':id' => $test['id'], + ]); + + if ($statement->rowCount() !== 1) { + // Seems like this test has already been reserved. Continue to the next one. + continue; + } try { if (!preg_match_all('/\'([^\']*)\'[ ]?/', $test['command'], $arguments)) { From c17e323bc3d641e3aa0602ab7e2fddda567a3840 Mon Sep 17 00:00:00 2001 From: Karim Geiger Date: Tue, 20 Mar 2018 08:09:15 +0100 Subject: [PATCH 3/3] #310 Set proc to null if proc_open fails. proc_open() may return false in case of error. Since we explicitly check for null down the road, it will only be set if a resource has been created. --- src/Runners/PHPUnit/Worker/BaseWorker.php | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/Runners/PHPUnit/Worker/BaseWorker.php b/src/Runners/PHPUnit/Worker/BaseWorker.php index af2146f6..abd8d40f 100644 --- a/src/Runners/PHPUnit/Worker/BaseWorker.php +++ b/src/Runners/PHPUnit/Worker/BaseWorker.php @@ -35,7 +35,8 @@ public function start(string $wrapperBinary, $token = 1, $uniqueToken = null, ar $bin .= ' ' . implode(' ', array_map('escapeshellarg', $parameters)); } $pipes = []; - $this->proc = proc_open($bin, self::$descriptorspec, $pipes); + $process = proc_open($bin, self::$descriptorspec, $pipes); + $this->proc = is_resource($process) ? $process : null; $this->pipes = $pipes; } @@ -55,7 +56,7 @@ public function isRunning(): bool $status = proc_get_status($this->proc); - return $status['running']; + return $status ? $status['running'] : false; } public function isStarted(): bool