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

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');
 

tests/end-to-end/logging/_files/TypeErrorTest.php

@@ -0,0 +1,29 @@
+<?php declare(strict_types=1);
+/*
+ * This file is part of PHPUnit.
+ *
+ * (c) Sebastian Bergmann <[email protected]>
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+namespace PHPUnit\TestFixture;
+
+use DateTime;
+use DateTimeImmutable;
+use PHPUnit\Framework\TestCase;
+
+final class TypeErrorTest extends TestCase
+{
+    private DateTimeImmutable $dateTime;
+
+    protected function setUp(): void
+    {
+        $this->dateTime = new DateTime;
+    }
+
+    public function testMe(): void
+    {
+        $this->assertTrue(true);
+    }
+}

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

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

tests/end-to-end/logging/log-junit-with-progress-with-errors.phpt

@@ -0,0 +1,53 @@
+--TEST--
+phpunit --log-junit php://stdout _files/TypeErrorTest.php
+--SKIPIF--
+<?php declare(strict_types=1);
+if (DIRECTORY_SEPARATOR === '\\') {
+    print "skip: this test does not work on Windows / GitHub Actions\n";
+}
+--FILE--
+<?php declare(strict_types=1);
+$logfile = tempnam(sys_get_temp_dir(), __FILE__);
+
+$_SERVER['argv'][] = '--do-not-cache-result';
+$_SERVER['argv'][] = '--no-configuration';
+$_SERVER['argv'][] = '--log-junit';
+$_SERVER['argv'][] = $logfile;
+$_SERVER['argv'][] = __DIR__ . '/_files/TypeErrorTest.php';
+
+require_once __DIR__ . '/../../bootstrap.php';
+
+(new PHPUnit\TextUI\Application)->run($_SERVER['argv']);
+
+print file_get_contents($logfile);
+
+unlink($logfile);
+--EXPECTF--
+PHPUnit %s by Sebastian Bergmann and contributors.
+
+Runtime: %s
+
+E                                                                   1 / 1 (100%)
+
+Time: %s, Memory: %s
+
+There was 1 error:
+
+1) PHPUnit\TestFixture\TypeErrorTest::testMe
+TypeError: Cannot assign DateTime to property PHPUnit\TestFixture\TypeErrorTest::$dateTime of type DateTimeImmutable
+
+%sTypeErrorTest.php:%d
+
+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" 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
+
+%sTypeErrorTest.php:%s</error>
+    </testcase>
+  </testsuite>
+</testsuites>