Log file is empty: always output process outputs (#646)

* Log file is empty: always output process outputs * Escape according to OS * CS Fix
paratestphp · Dec 2, 2021 · c32a5c4 · c32a5c4
1 parent 9bd410d
commit c32a5c4
Show file tree

Hide file tree

Showing 9 changed files with 111 additions and 32 deletions.
diff --git a/psalm-baseline.xml b/psalm-baseline.xml
@@ -1,5 +1,5 @@
 <?xml version="1.0" encoding="UTF-8"?>
-<files psalm-version="4.9.2@00c062267d6e3229d91a1939992987e2d46f2393">
+<files psalm-version="4.13.1@5cf660f63b548ccd4a56f62d916ee4d6028e01a3">
   <file src="src/Logging/JUnit/Reader.php">
     <MixedArrayAccess occurrences="9">
       <code>$node['assertions']</code>
@@ -52,9 +52,6 @@
     </MixedArgumentTypeCoercion>
   </file>
   <file src="src/Runners/PHPUnit/SuiteLoader.php">
-    <MixedArgumentTypeCoercion occurrences="1">
-      <code>$groups</code>
-    </MixedArgumentTypeCoercion>
     <MixedPropertyTypeCoercion occurrences="4">
       <code>$this-&gt;files</code>
       <code>array_unique($this-&gt;files)</code>
@@ -73,4 +70,9 @@
       <code>$parameters</code>
     </MixedArgument>
   </file>
+  <file src="src/Runners/PHPUnit/WorkerCrashedException.php">
+    <MixedAssignment occurrences="1">
+      <code>$value</code>
+    </MixedAssignment>
+  </file>
 </files>
diff --git a/src/Runners/PHPUnit/ResultPrinter.php b/src/Runners/PHPUnit/ResultPrinter.php
@@ -224,30 +224,21 @@ public function printFeedback(ExecutableTest $test): Reader
         try {
             $reader = new Reader($test->getTempFile());
         } catch (InvalidArgumentException $invalidArgumentException) {
-            throw new EmptyLogFileException(sprintf(
-                "%s\n" .
-                "The process: %s\n" .
-                "This means a PHPUnit process was unable to run \"%s\"\n",
+            throw new EmptyLogFileException(
                 $invalidArgumentException->getMessage(),
-                $test->getLastCommand(),
-                $test->getPath()
-            ), 0, $invalidArgumentException);
+                0,
+                $invalidArgumentException
+            );
         }
 
         if ($this->teamcityLogFileHandle !== null) {
-            $log_file = $test->getTeamcityTempFile();
-
-            if (filesize($log_file) === 0) {
-                throw new EmptyLogFileException(sprintf(
-                    "Teamcity format file is empty.\n" .
-                    "The process: %s\n" .
-                    "This means a PHPUnit process was unable to run \"%s\"\n",
-                    $test->getLastCommand(),
-                    $test->getPath()
-                ));
+            $teamcityLogFile = $test->getTeamcityTempFile();
+
+            if (filesize($teamcityLogFile) === 0) {
+                throw new EmptyLogFileException("Teamcity format file ${teamcityLogFile} is empty");
             }
 
-            $result = file_get_contents($log_file);
+            $result = file_get_contents($teamcityLogFile);
             assert($result !== false);
             fwrite($this->teamcityLogFileHandle, $result);
         }

diff --git a/src/Runners/PHPUnit/Worker/WrapperWorker.php b/src/Runners/PHPUnit/Worker/WrapperWorker.php
@@ -5,6 +5,7 @@
 namespace ParaTest\Runners\PHPUnit\Worker;
 
 use ParaTest\Logging\JUnit\Reader;
+use ParaTest\Runners\PHPUnit\EmptyLogFileException;
 use ParaTest\Runners\PHPUnit\ExecutableTest;
 use ParaTest\Runners\PHPUnit\Options;
 use ParaTest\Runners\PHPUnit\ResultPrinter;
@@ -13,6 +14,7 @@
 use Symfony\Component\Process\InputStream;
 use Symfony\Component\Process\PhpExecutableFinder;
 use Symfony\Component\Process\Process;
+use Throwable;
 
 use function array_map;
 use function array_merge;
@@ -116,12 +118,12 @@ public function start(): void
         $this->process->start();
     }
 
-    public function getWorkerCrashedException(): WorkerCrashedException
+    public function getWorkerCrashedException(?Throwable $previousException = null): WorkerCrashedException
     {
         $command = end($this->commands);
         assert($command !== false);
 
-        return WorkerCrashedException::fromProcess($this->process, $command);
+        return WorkerCrashedException::fromProcess($this->process, $command, $previousException);
     }
 
     /**
@@ -150,7 +152,13 @@ public function printFeedback(ResultPrinter $printer): ?Reader
             return null;
         }
 
-        return $printer->printFeedback($this->currentlyExecuting);
+        try {
+            $reader = $printer->printFeedback($this->currentlyExecuting);
+        } catch (EmptyLogFileException $emptyLogException) {
+            throw $this->getWorkerCrashedException($emptyLogException);
+        }
+
+        return $reader;
     }
 
     public function reset(): void

diff --git a/src/Runners/PHPUnit/WorkerCrashedException.php b/src/Runners/PHPUnit/WorkerCrashedException.php
@@ -8,6 +8,7 @@
 use Symfony\Component\Process\Process;
 use Throwable;
 
+use function escapeshellarg;
 use function sprintf;
 
 /**
@@ -17,8 +18,14 @@ final class WorkerCrashedException extends RuntimeException
 {
     public static function fromProcess(Process $process, string $command, ?Throwable $previousException = null): self
     {
+        $envs = '';
+        foreach ($process->getEnv() as $key => $value) {
+            $envs .= sprintf('%s=%s ', $key, escapeshellarg((string) $value));
+        }
+
         $error = sprintf(
-            'The command "%s" failed.' . "\n\nExit Code: %s(%s)\n\nWorking directory: %s",
+            'The command "%s%s" failed.' . "\n\nExit Code: %s(%s)\n\nWorking directory: %s",
+            $envs,
             $command,
             (string) $process->getExitCode(),
             (string) $process->getExitCodeText(),

diff --git a/test/Unit/Runners/PHPUnit/ResultPrinterTest.php b/test/Unit/Runners/PHPUnit/ResultPrinterTest.php
@@ -512,30 +512,27 @@ public function testParallelSuiteProgressOverhead(): void
         static::assertStringContainsString('FAILURES', $this->output->fetch());
     }
 
-    public function testEmptyLogFileRaiseExceptionWithLastCommand(): void
+    public function testEmptyLogFileRaiseException(): void
     {
         $test = new ExecutableTestChild(uniqid(), false, false, TMP_DIR);
-        $test->setLastCommand(uniqid());
 
         $this->expectException(RuntimeException::class);
-        $this->expectExceptionMessageMatches(sprintf('/%s/', $test->getLastCommand()));
 
         $this->printer->printFeedback($test);
     }
 
-    public function testTeamcityEmptyLogFileRaiseExceptionWithLastCommand(): void
+    public function testTeamcityEmptyLogFileRaiseException(): void
     {
         $teamcityLog = TMP_DIR . DS . 'teamcity.log';
 
         $this->options = $this->createOptionsFromArgv(['--log-teamcity' => $teamcityLog]);
         $this->printer = new ResultPrinter($this->interpreter, $this->output, $this->options);
 
         $test = $this->getSuiteWithResult('single-passing.xml', 1);
-        $test->setLastCommand(uniqid());
 
         file_put_contents($test->getTeamcityTempFile(), '');
         $this->expectException(RuntimeException::class);
-        $this->expectExceptionMessageMatches(sprintf('/Teamcity.+%s/s', $test->getLastCommand()));
+        $this->expectExceptionMessageMatches('/Teamcity/');
 
         $this->printer->printFeedback($test);
     }

diff --git a/test/Unit/Runners/PHPUnit/RunnerTestCase.php b/test/Unit/Runners/PHPUnit/RunnerTestCase.php
@@ -14,6 +14,7 @@
 use function array_merge;
 use function array_reverse;
 use function defined;
+use function escapeshellarg;
 use function file_get_contents;
 use function posix_mkfifo;
 use function preg_match;
@@ -130,6 +131,20 @@ final public function testParallelSuiteOption(): void
         $this->assertTestsPassed($this->runRunner());
     }
 
+    final public function testRaiseVerboseExceptionWhenATestCallsErrorsOnListenerWithLogging(): void
+    {
+        $this->bareOptions['--configuration'] = $this->fixture('phpunit-failing-listener.xml');
+        $this->bareOptions['--log-junit']     = TMP_DIR . DS . uniqid('result_');
+        $this->bareOptions['--processes']     = '1';
+
+        $this->expectException(WorkerCrashedException::class);
+        $this->expectExceptionMessageMatches(
+            sprintf('/TEST_TOKEN=%s.+TestWithFailingListenerTest.+lorem/s', preg_quote(escapeshellarg('1')))
+        );
+
+        $this->runRunner();
+    }
+
     final public function testRaiseExceptionWhenATestCallsExitSilentlyWithCoverage(): void
     {
         $this->bareOptions['--path']         = $this->fixture('exit_tests' . DS . 'UnitTestThatExitsSilentlyTest.php');

diff --git a/test/fixtures/failing_listener/MyListener.php b/test/fixtures/failing_listener/MyListener.php
@@ -0,0 +1,23 @@
+<?php
+
+declare(strict_types=1);
+
+namespace ParaTest\Tests\fixtures\failing_listener;
+
+use PHPUnit\Framework\Test;
+use PHPUnit\Framework\TestListener;
+use PHPUnit\Framework\TestListenerDefaultImplementation;
+use RuntimeException;
+
+/**
+ * @internal
+ */
+final class MyListener implements TestListener
+{
+    use TestListenerDefaultImplementation;
+
+    public function startTest(Test $test): void
+    {
+        throw new RuntimeException('lorem');
+    }
+}
diff --git a/test/fixtures/failing_listener/TestWithFailingListenerTest.php b/test/fixtures/failing_listener/TestWithFailingListenerTest.php
@@ -0,0 +1,18 @@
+<?php
+
+declare(strict_types=1);
+
+namespace ParaTest\Tests\fixtures\failing_listener;
+
+use PHPUnit\Framework\TestCase;
+
+/**
+ * @internal
+ */
+final class TestWithFailingListenerTest extends TestCase
+{
+    public function testMe(): void
+    {
+        $this->assertTrue(true);
+    }
+}
diff --git a/test/fixtures/phpunit-failing-listener.xml b/test/fixtures/phpunit-failing-listener.xml
@@ -0,0 +1,18 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<phpunit
+        xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+        xsi:noNamespaceSchemaLocation="https://schema.phpunit.de/9.4/phpunit.xsd"
+        colors="true"
+>
+    <testsuites>
+        <testsuite name="ParaTest Fixtures">
+            <file>./failing_listener/TestWithFailingListenerTest.php</file>
+        </testsuite>
+    </testsuites>
+    <logging>
+        <junit outputFile="../tmp/junit.xml"/>
+    </logging>
+  <listeners>
+    <listener class="ParaTest\Tests\fixtures\failing_listener\MyListener"/>
+  </listeners>
+</phpunit>