[#5188] Fix JunitXmlLogger handling of errors on unprepared tests

sebastianbergmann · Feb 23, 2023 · dc16f72 · dc16f72
1 parent e36c37c
commit dc16f72
Show file tree

Hide file tree

Showing 4 changed files with 62 additions and 38 deletions.
diff --git a/src/Logging/JUnit/JunitXmlLogger.php b/src/Logging/JUnit/JunitXmlLogger.php
@@ -20,11 +20,11 @@
 use DOMElement;
 use PHPUnit\Event\Code\Test;
 use PHPUnit\Event\Code\TestMethod;
-use PHPUnit\Event\Code\Throwable;
 use PHPUnit\Event\EventFacadeIsSealedException;
 use PHPUnit\Event\Facade;
 use PHPUnit\Event\InvalidArgumentException;
 use PHPUnit\Event\Telemetry\HRTime;
+use PHPUnit\Event\Telemetry\Info;
 use PHPUnit\Event\Test\Errored;
 use PHPUnit\Event\Test\Failed;
 use PHPUnit\Event\Test\Finished;
@@ -85,6 +85,7 @@ final class JunitXmlLogger
     private int $testSuiteLevel          = 0;
     private ?DOMElement $currentTestCase = null;
     private ?HRTime $time                = null;
+    private bool $prepared               = false;
 
     /**
      * @throws EventFacadeIsSealedException
@@ -186,39 +187,15 @@ public function testSuiteFinished(): void
     public function testPrepared(Prepared $event): void
     {
         $this->createTestCase($event);
+        $this->prepared = true;
     }
 
     /**
      * @throws InvalidArgumentException
      */
     public function testFinished(Finished $event): void
     {
-        assert($this->currentTestCase !== null);
-        assert($this->time !== null);
-
-        $time = $event->telemetryInfo()->time()->duration($this->time)->asFloat();
-
-        $this->testSuiteAssertions[$this->testSuiteLevel] += $event->numberOfAssertionsPerformed();
-
-        $this->currentTestCase->setAttribute(
-            'assertions',
-            (string) $event->numberOfAssertionsPerformed()
-        );
-
-        $this->currentTestCase->setAttribute(
-            'time',
-            sprintf('%F', $time)
-        );
-
-        $this->testSuites[$this->testSuiteLevel]->appendChild(
-            $this->currentTestCase
-        );
-
-        $this->testSuiteTests[$this->testSuiteLevel]++;
-        $this->testSuiteTimes[$this->testSuiteLevel] += $time;
-
-        $this->currentTestCase = null;
-        $this->time            = null;
+        $this->handleFinish($event->telemetryInfo(), $event->numberOfAssertionsPerformed());
     }
 
     /**
@@ -245,7 +222,7 @@ public function testSkipped(Skipped $event): void
      */
     public function testErrored(Errored $event): void
     {
-        $this->handleFault($event->test(), $event->throwable(), 'error');
+        $this->handleFault($event, 'error');
 
         $this->testSuiteErrors[$this->testSuiteLevel]++;
     }
@@ -256,11 +233,44 @@ public function testErrored(Errored $event): void
      */
     public function testFailed(Failed $event): void
     {
-        $this->handleFault($event->test(), $event->throwable(), 'failure');
+        $this->handleFault($event, 'failure');
 
         $this->testSuiteFailures[$this->testSuiteLevel]++;
     }
 
+    /**
+     * @throws InvalidArgumentException
+     */
+    private function handleFinish(Info $telemetryInfo, int $numberOfAssertionsPerformed): void
+    {
+        assert($this->currentTestCase !== null);
+        assert($this->time !== null);
+
+        $time = $telemetryInfo->time()->duration($this->time)->asFloat();
+
+        $this->testSuiteAssertions[$this->testSuiteLevel] += $numberOfAssertionsPerformed;
+
+        $this->currentTestCase->setAttribute(
+            'assertions',
+            (string) $numberOfAssertionsPerformed
+        );
+
+        $this->currentTestCase->setAttribute(
+            'time',
+            sprintf('%F', $time)
+        );
+
+        $this->testSuites[$this->testSuiteLevel]->appendChild(
+            $this->currentTestCase
+        );
+
+        $this->testSuiteTests[$this->testSuiteLevel]++;
+        $this->testSuiteTimes[$this->testSuiteLevel] += $time;
+
+        $this->currentTestCase = null;
+        $this->time            = null;
+    }
+
     /**
      * @throws EventFacadeIsSealedException
      * @throws UnknownSubscriberTypeException
@@ -293,12 +303,17 @@ private function createDocument(): void
      * @throws InvalidArgumentException
      * @throws NoDataSetFromDataProviderException
      */
-    private function handleFault(Test $test, Throwable $throwable, string $type): void
+    private function handleFault(Errored|Failed $event, string $type): void
     {
+        if (!$this->prepared) {
+            $this->createTestCase($event);
+        }
+
         assert($this->currentTestCase !== null);
 
-        $buffer = $this->testAsString($test);
+        $buffer = $this->testAsString($event->test());
 
+        $throwable = $event->throwable();
         $buffer .= trim(
             $throwable->description() . PHP_EOL .
             $throwable->stackTrace()
@@ -312,6 +327,10 @@ private function handleFault(Test $test, Throwable $throwable, string $type): vo
         $fault->setAttribute('type', $throwable->className());
 
         $this->currentTestCase->appendChild($fault);
+
+        if (!$this->prepared) {
+            $this->handleFinish($event->telemetryInfo(), 0);
+        }
     }
 
     /**
@@ -320,7 +339,7 @@ private function handleFault(Test $test, Throwable $throwable, string $type): vo
      */
     private function handleIncompleteOrSkipped(MarkedIncomplete|Skipped $event): void
     {
-        if ($this->currentTestCase === null) {
+        if (!$this->prepared) {
             $this->createTestCase($event);
         }
 
@@ -331,6 +350,10 @@ private function handleIncompleteOrSkipped(MarkedIncomplete|Skipped $event): voi
         $this->currentTestCase->appendChild($skipped);
 
         $this->testSuiteSkipped[$this->testSuiteLevel]++;
+
+        if (!$this->prepared) {
+            $this->handleFinish($event->telemetryInfo(), 0);
+        }
     }
 
     /**
@@ -389,8 +412,10 @@ private function name(Test $test): string
     /**
      * @throws InvalidArgumentException
      * @throws NoDataSetFromDataProviderException
+     *
+     * @psalm-assert !null $this->currentTestCase
      */
-    private function createTestCase(Prepared|MarkedIncomplete|Skipped $event): void
+    private function createTestCase(Prepared|MarkedIncomplete|Skipped|Errored|Failed $event): void
     {
         $testCase = $this->document->createElement('testcase');
 

diff --git a/tests/end-to-end/logging/log-junit-to-file.phpt b/tests/end-to-end/logging/log-junit-to-file.phpt
@@ -26,7 +26,7 @@ unlink($logfile);
 --EXPECTF--
 <?xml version="1.0" encoding="UTF-8"?>
 <testsuites>
-  <testsuite name="PHPUnit\SelfTest\Basic\StatusTest" file="%sStatusTest.php" tests="12" assertions="4" errors="2" failures="2" skipped="5" time="%f">
+  <testsuite name="PHPUnit\SelfTest\Basic\StatusTest" file="%sStatusTest.php" tests="12" assertions="4" errors="2" failures="2" skipped="4" time="%f">
     <testcase name="testSuccess" file="%sStatusTest.php" line="%d" class="PHPUnit\SelfTest\Basic\StatusTest" classname="PHPUnit.SelfTest.Basic.StatusTest" assertions="1" time="%f"/>
     <testcase name="testFailure" file="%sStatusTest.php" line="%d" class="PHPUnit\SelfTest\Basic\StatusTest" classname="PHPUnit.SelfTest.Basic.StatusTest" assertions="1" time="%f">
       <failure type="PHPUnit\Framework\ExpectationFailedException">PHPUnit\SelfTest\Basic\StatusTest::testFailure%A

diff --git a/tests/end-to-end/logging/log-junit-to-stdout.phpt b/tests/end-to-end/logging/log-junit-to-stdout.phpt
@@ -15,7 +15,7 @@ require_once __DIR__ . '/../../bootstrap.php';
 --EXPECTF--
 <?xml version="1.0" encoding="UTF-8"?>
 <testsuites>
-  <testsuite name="PHPUnit\SelfTest\Basic\StatusTest" file="%sStatusTest.php" tests="12" assertions="4" errors="2" failures="2" skipped="5" time="%f">
+  <testsuite name="PHPUnit\SelfTest\Basic\StatusTest" file="%sStatusTest.php" tests="12" assertions="4" errors="2" failures="2" skipped="4" time="%f">
     <testcase name="testSuccess" file="%sStatusTest.php" line="%d" class="PHPUnit\SelfTest\Basic\StatusTest" classname="PHPUnit.SelfTest.Basic.StatusTest" assertions="1" time="%f"/>
     <testcase name="testFailure" file="%sStatusTest.php" line="%d" class="PHPUnit\SelfTest\Basic\StatusTest" classname="PHPUnit.SelfTest.Basic.StatusTest" assertions="1" time="%f">
       <failure type="PHPUnit\Framework\ExpectationFailedException">PHPUnit\SelfTest\Basic\StatusTest::testFailure%A

diff --git a/tests/end-to-end/logging/log-junit-with-progress-with-errors.phpt b/tests/end-to-end/logging/log-junit-with-progress-with-errors.phpt
@@ -40,11 +40,10 @@ TypeError: Cannot assign DateTime to property PHPUnit\TestFixture\TypeErrorTest:
 
 ERRORS!
 Tests: 1, Assertions: 0, Errors: 1.
-
 <?xml version="1.0" encoding="UTF-8"?>
 <testsuites>
-  <testsuite name="PHPUnit\TestFixture\TypeErrorTest" file="%sTypeErrorTest.php" tests="1" assertions="0" errors="1" warnings="0" failures="0" skipped="0" time="%f">
-    <testcase name="testMe" class="PHPUnit\TestFixture\TypeErrorTest" classname="PHPUnit.TestFixture.TypeErrorTest" file="%sTypeErrorTest.php" line="%d" assertions="0" time="%f">
+  <testsuite name="PHPUnit\TestFixture\TypeErrorTest" file="%sTypeErrorTest.php" tests="1" assertions="0" errors="1" failures="0" skipped="0" time="%f">
+    <testcase name="testMe" file="%sTypeErrorTest.php" line="%d" class="PHPUnit\TestFixture\TypeErrorTest" classname="PHPUnit.TestFixture.TypeErrorTest" assertions="0" time="%f">
       <error type="TypeError">PHPUnit\TestFixture\TypeErrorTest::testMe
 TypeError: Cannot assign DateTime to property PHPUnit\TestFixture\TypeErrorTest::$dateTime of type DateTimeImmutable