Skip to content

Commit

Permalink
Extract RunnerWorker from ExecutableTest (#501)
Browse files Browse the repository at this point in the history 
* Fix type hints dirtied by tests

* Extract RunnerWorker from ExecutableTest
  • Loading branch information
@Slamdunk
Slamdunk committed Aug 5, 2020
1 parent 6538c89 commit 43ac7a6
Show file tree
Hide file tree
Showing 15 changed files with 278 additions and 302 deletions.
4 changes: 2 additions & 2 deletions phpstan.neon.dist
View file
Original file line number Diff line number Diff line change
Expand Up @@ -88,9 +88,9 @@ parameters:
count: 1
path: src/Logging/JUnit/Writer.php
-
message: "#^Property ParaTest\\\\Runners\\\\PHPUnit\\\\ExecutableTest\\:\\:\\$process type has no value type specified in iterable type Symfony\\\\Component\\\\Process\\\\Process\\.$#"
message: "#^Property ParaTest\\\\Runners\\\\PHPUnit\\\\Worker\\\\RunnerWorker\\:\\:\\$process type has no value type specified in iterable type Symfony\\\\Component\\\\Process\\\\Process\\.$#"
count: 1
path: src/Runners/PHPUnit/ExecutableTest.php
path: src/Runners/PHPUnit/Worker/RunnerWorker.php
-
message: "#^Call to method PHPUnit\\\\Framework\\\\Assert\\:\\:assertTrue\\(\\) with false will always evaluate to false\\.$#"
count: 1
Expand Down
11 changes: 1 addition & 10 deletions src/Runners/PHPUnit/BaseRunner.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@
use ParaTest\Coverage\CoverageMerger;
use ParaTest\Logging\JUnit\Writer;
use ParaTest\Logging\LogInterpreter;
use ParaTest\Parser\ParsedFunction;
use Symfony\Component\Console\Output\OutputInterface;

use function array_merge;
Expand All @@ -27,18 +26,10 @@ abstract class BaseRunner implements RunnerInterface
* A collection of pending ExecutableTest objects that have
* yet to run.
*
* @var array<int|string, ExecutableTest|TestMethod|ParsedFunction>
* @var array<int|string, ExecutableTest>
*/
protected $pending = [];

/**
* A collection of ExecutableTest objects that have processes
* currently running.
*
* @var array|ExecutableTest[]
*/
protected $running = [];

/**
* A tallied exit code that returns the highest exit
* code returned out of the entire collection of tests.
Expand Down
166 changes: 0 additions & 166 deletions src/Runners/PHPUnit/ExecutableTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,19 +5,14 @@
namespace ParaTest\Runners\PHPUnit;

use PHPUnit\TextUI\Configuration\Configuration;
use RuntimeException;
use Symfony\Component\Process\PhpExecutableFinder;
use Symfony\Component\Process\Process;

use function array_map;
use function array_merge;
use function strlen;
use function sys_get_temp_dir;
use function tempnam;
use function unlink;

use const DIRECTORY_SEPARATOR;

abstract class ExecutableTest
{
/**
Expand All @@ -42,17 +37,6 @@ abstract class ExecutableTest
*/
protected $coverageFileName;

/** @var Process */
protected $process;

/**
* A unique token value for a given
* process.
*
* @var int
*/
protected $token;

/**
* Last executed process command.
*
Expand Down Expand Up @@ -92,23 +76,6 @@ final public function getTempFile(): string
return $this->temp;
}

/**
* Return the test process' stderr contents.
*/
final public function getStderr(): string
{
return $this->process->getErrorOutput();
}

/**
* Stop the process and return it's
* exit code.
*/
final public function stop(): ?int
{
return $this->process->stop();
}

/**
* Removes the test file.
*/
Expand All @@ -118,22 +85,6 @@ final public function deleteFile(): void
unlink($outputFile);
}

/**
* Check if the process has terminated.
*/
final public function isDoneRunning(): bool
{
return $this->process->isTerminated();
}

/**
* Return the exit code of the process.
*/
final public function getExitCode(): ?int
{
return $this->process->getExitCode();
}

/**
* Return the last process command.
*/
Expand All @@ -150,71 +101,6 @@ final public function setLastCommand(string $command): void
$this->lastCommand = $command;
}

/**
* Executes the test by creating a separate process.
*
* @param array<string, (string|bool|int|Configuration|string[]|null)> $options
* @param array<string, string|int> $environmentVariables
* @param string[]|null $passthru
* @param string[]|null $passthruPhp
*
* @return $this
*/
final public function run(
string $binary,
array $options = [],
array $environmentVariables = [],
?array $passthru = null,
?array $passthruPhp = null
) {
$environmentVariables['PARATEST'] = 1;
$this->handleEnvironmentVariables($environmentVariables);

$command = $this->getFullCommandlineString($binary, $options, $passthru, $passthruPhp);

$this->assertValidCommandLineLength($command);
$this->setLastCommand($command);

$this->process = Process::fromShellCommandline($command, null, $environmentVariables);
$this->process->start();

return $this;
}

/**
* Build the full executable as we would do on the command line, e.g.
* php -d zend_extension=xdebug.so vendor/bin/phpunit --teststuite suite1 --prepend xdebug-filter.php.
*
* @param array<string, (string|bool|int|Configuration|string[]|null)> $options
* @param string[]|null $passthru
* @param string[]|null $passthruPhp
*/
final protected function getFullCommandlineString(
string $binary,
array $options,
?array $passthru = null,
?array $passthruPhp = null
): string {
$finder = new PhpExecutableFinder();

$args = [$finder->find()];
if ($passthruPhp !== null) {
$args = array_merge($args, $passthruPhp);
}

$args = array_merge($args, $this->commandArguments($binary, $options, $passthru));

return (new Process($args))->getCommandLine();
}

/**
* Returns the unique token for this test process.
*/
final public function getToken(): int
{
return $this->token;
}

/**
* Generate command line arguments with passed options suitable to handle through paratest.
*
Expand Down Expand Up @@ -279,14 +165,6 @@ final public function getCoverageFileName(): string
return $this->coverageFileName;
}

/**
* Get process stdout content.
*/
final public function getStdout(): string
{
return $this->process->getOutput();
}

/**
* Set process temporary filename.
*/
Expand All @@ -295,35 +173,6 @@ final public function setTempFile(string $temp): void
$this->temp = $temp;
}

/**
* Assert that command line length is valid.
*
* In some situations process command line can became too long when combining different test
* cases in single --filter arguments so it's better to show error regarding that to user
* and propose him to decrease max batch size.
*
* @param string $cmd Command line
*
* @throws RuntimeException on too long command line.
*/
private function assertValidCommandLineLength(string $cmd): void
{
if (DIRECTORY_SEPARATOR === '\\') { // windows
// symfony's process wrapper
$cmd = 'cmd /V:ON /E:ON /C "(' . $cmd . ')';
if (strlen($cmd) > 32767) {
throw new RuntimeException('Command line is too long, try to decrease max batch size');
}
}

/*
* @todo Implement command line length validation for linux/osx/freebsd.
* Please note that on unix environment variables also became part of command line:
* - linux: echo | xargs --show-limits
* - osx/linux: getconf ARG_MAX
*/
}

/**
* A template method that can be overridden to add necessary options for a test.
*
Expand All @@ -333,21 +182,6 @@ private function assertValidCommandLineLength(string $cmd): void
*/
abstract protected function prepareOptions(array $options): array;

/**
* Checks environment variables for the presence of a TEST_TOKEN
* variable and sets $this->token based on its value.
*
* @param array<string, string|int> $environmentVariables
*/
private function handleEnvironmentVariables(array $environmentVariables): void
{
if (! isset($environmentVariables['TEST_TOKEN'])) {
return;
}

$this->token = $environmentVariables['TEST_TOKEN'];
}

/**
* Checks if the coverage-php option is set and redirects it to a unique temp file.
* This will ensure, that multiple tests write to separate coverage-files.
Expand Down
2 changes: 0 additions & 2 deletions src/Runners/PHPUnit/ResultPrinter.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -109,8 +109,6 @@ public function __construct(LogInterpreter $results, OutputInterface $output)

/**
* Adds an ExecutableTest to the tracked results.
*
* @return $this
*/
public function addTest(ExecutableTest $suite): self
{
Expand Down
53 changes: 36 additions & 17 deletions src/Runners/PHPUnit/Runner.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@

use Exception;
use Habitat\Habitat;
use ParaTest\Runners\PHPUnit\Worker\RunnerWorker;
use RuntimeException;
use Symfony\Component\Console\Output\OutputInterface;
use Throwable;
Expand All @@ -22,6 +23,14 @@ final class Runner extends BaseRunner
{
private const PHPUNIT_FATAL_ERROR = 255;

/**
* A collection of ExecutableTest objects that have processes
* currently running.
*
* @var RunnerWorker[]
*/
private $running = [];

/**
* A collection of available tokens based on the number
* of processes specified in $options.
Expand Down Expand Up @@ -53,7 +62,7 @@ public function run(): void
} catch (Throwable $e) {
if ($this->options->verbose > 0) {
$this->output->writeln("An error for $key: {$e->getMessage()}");
$this->output->writeln("Command: {$test->getLastCommand()}");
$this->output->writeln("Command: {$test->getExecutableTest()->getLastCommand()}");
$this->output->writeln('StdErr: ' . $test->getStderr());
$this->output->writeln('StdOut: ' . $test->getStdout());
}
Expand Down Expand Up @@ -102,18 +111,27 @@ private function fillRunQueue(): void
}

$this->acquireToken($tokenData['token']);
$env = [
$env = [
'TEST_TOKEN' => $tokenData['token'],
'UNIQUE_TEST_TOKEN' => $tokenData['unique'],
] + Habitat::getAll();
$this->running[$tokenData['token']] = array_shift($this->pending)
->run($opts->phpunit, $opts->filtered, $env, $opts->passthru, $opts->passthruPhp);

$executebleTest = array_shift($this->pending);
$this->running[$tokenData['token']] = new RunnerWorker($executebleTest);
$this->running[$tokenData['token']]->run(
$opts->phpunit,
$opts->filtered,
$env,
$opts->passthru,
$opts->passthruPhp
);

if ($opts->verbose === 0) {
continue;
}

$cmd = $this->running[$tokenData['token']];
$this->output->write("\nExecuting test via: {$cmd->getLastCommand()}\n");
$this->output->write("\nExecuting test via: {$cmd->getExecutableTest()->getLastCommand()}\n");
}
}

Expand All @@ -125,30 +143,31 @@ private function fillRunQueue(): void
*
* @throws Exception
*/
private function testIsStillRunning(ExecutableTest $test): bool
private function testIsStillRunning(RunnerWorker $worker): bool
{
if (! $test->isDoneRunning()) {
if (! $worker->isDoneRunning()) {
return true;
}

$this->setExitCode($test);
$test->stop();
if ($this->options->stopOnFailure && $test->getExitCode() > 0) {
$this->setExitCode($worker);
$worker->stop();
if ($this->options->stopOnFailure && $worker->getExitCode() > 0) {
$this->pending = [];
}

if ($test->getExitCode() === self::PHPUNIT_FATAL_ERROR) {
$errorOutput = $test->getStderr();
$executableTest = $worker->getExecutableTest();
if ($worker->getExitCode() === self::PHPUNIT_FATAL_ERROR) {
$errorOutput = $worker->getStderr();
if ($errorOutput === '') {
$errorOutput = $test->getStdout();
$errorOutput = $worker->getStdout();
}

throw new RuntimeException(sprintf("Fatal error in %s:\n%s", $test->getPath(), $errorOutput));
throw new RuntimeException(sprintf("Fatal error in %s:\n%s", $executableTest->getPath(), $errorOutput));
}

$this->printer->printFeedback($test);
$this->printer->printFeedback($executableTest);
if ($this->hasCoverage()) {
$this->addCoverage($test);
$this->addCoverage($executableTest);
}

return false;
Expand All @@ -159,7 +178,7 @@ private function testIsStillRunning(ExecutableTest $test): bool
* higher than the currently set exit code, that exit
* code will be set as the overall exit code.
*/
private function setExitCode(ExecutableTest $test): void
private function setExitCode(RunnerWorker $test): void
{
$exit = $test->getExitCode();
if ($exit <= $this->exitcode) {
Expand Down
Loading

0 comments on commit 43ac7a6

Please sign in to comment.