Remove Framework\ErrorTestCase

sebastianbergmann · Aug 26, 2022 · 7ccf97c · 7ccf97c
1 parent d34f055
commit 7ccf97c
Show file tree

Hide file tree

Showing 22 changed files with 174 additions and 310 deletions.
diff --git a/.psalm/baseline.xml b/.psalm/baseline.xml
@@ -312,10 +312,6 @@
     <LessSpecificReturnStatement occurrences="1">
       <code>$test</code>
     </LessSpecificReturnStatement>
-    <MissingThrowsDocblock occurrences="2">
-      <code>Filter::getFilteredStacktrace($t)</code>
-      <code>Filter::getFilteredStacktrace($t)</code>
-    </MissingThrowsDocblock>
     <MoreSpecificReturnType occurrences="1">
       <code>Test</code>
     </MoreSpecificReturnType>
@@ -404,7 +400,11 @@
       <code>$this-&gt;name</code>
       <code>$this-&gt;name</code>
     </ArgumentTypeCoercion>
-    <MissingThrowsDocblock occurrences="1"/>
+    <MissingThrowsDocblock occurrences="4">
+      <code>Event\TestDataCollection::fromArray([])</code>
+      <code>Filter::getFilteredStacktrace($t)</code>
+      <code>Filter::getFilteredStacktrace($t)</code>
+    </MissingThrowsDocblock>
     <UnsafeInstantiation occurrences="2">
       <code>new static($class-&gt;getName())</code>
       <code>new static($name)</code>
@@ -467,11 +467,6 @@
       <code>className</code>
     </UndefinedMethod>
   </file>
-  <file src="src/Metadata/Api/DataProvider.php">
-    <MissingThrowsDocblock occurrences="1">
-      <code>dataProvidedByTestWithAnnotation</code>
-    </MissingThrowsDocblock>
-  </file>
   <file src="src/Metadata/Api/Dependencies.php">
     <RedundantCondition occurrences="2">
       <code>assert($metadata instanceof DependsOnClass)</code>

diff --git a/src/Event/Value/Test/TestMethod.php b/src/Event/Value/Test/TestMethod.php
@@ -14,7 +14,6 @@
 use function method_exists;
 use PHPUnit\Event\DataFromDataProvider;
 use PHPUnit\Event\TestDataCollection;
-use PHPUnit\Framework\ErrorTestCase;
 use PHPUnit\Framework\TestCase;
 use PHPUnit\Metadata\MetadataCollection;
 use PHPUnit\Metadata\Parser\Registry as MetadataRegistry;
@@ -39,24 +38,15 @@ final class TestMethod extends Test
 
     public static function fromTestCase(TestCase $testCase): self
     {
-        $className  = $testCase::class;
-        $methodName = $testCase->name();
-        $testData   = self::dataFor($testCase);
-
-        if ($testCase instanceof ErrorTestCase) {
-            $className  = $testCase->className();
-            $methodName = $testCase->methodName();
-        }
-
-        $location = self::sourceLocationFor($className, $methodName);
+        $location = self::sourceLocationFor($testCase::class, $testCase->name());
 
         return new self(
-            $className,
-            $methodName,
+            $testCase::class,
+            $testCase->name(),
             $location['file'],
             $location['line'],
-            self::metadataFor($className, $methodName),
-            $testData,
+            self::metadataFor($testCase::class, $testCase->name()),
+            self::dataFor($testCase),
         );
     }
 

diff --git a/src/Framework/ErrorTestCase.php b/src/Framework/ErrorTestCase.php
diff --git a/src/Framework/TestBuilder.php b/src/Framework/TestBuilder.php
@@ -10,48 +10,33 @@
 namespace PHPUnit\Framework;
 
 use function assert;
-use function sprintf;
-use function trim;
 use PHPUnit\Metadata\Api\DataProvider;
 use PHPUnit\Metadata\Api\Groups;
 use PHPUnit\Metadata\BackupGlobals;
 use PHPUnit\Metadata\BackupStaticProperties;
 use PHPUnit\Metadata\ExcludeGlobalVariableFromBackup;
 use PHPUnit\Metadata\ExcludeStaticPropertyFromBackup;
-use PHPUnit\Metadata\InvalidDataSetException;
 use PHPUnit\Metadata\Parser\Registry as MetadataRegistry;
 use PHPUnit\Metadata\PreserveGlobalState;
 use PHPUnit\TextUI\Configuration\Registry as ConfigurationRegistry;
-use PHPUnit\Util\Filter;
 use ReflectionClass;
-use Throwable;
 
 /**
  * @internal This class is not covered by the backward compatibility promise for PHPUnit
  */
 final class TestBuilder
 {
+    /**
+     * @throws InvalidDataProviderException
+     */
     public function build(ReflectionClass $theClass, string $methodName): Test
     {
         $className = $theClass->getName();
 
-        try {
-            $data = (new DataProvider)->providedData(
-                $className,
-                $methodName
-            );
-        } catch (Throwable $t) {
-            $data = new ErrorTestCase(
-                $className,
-                $methodName,
-                sprintf(
-                    "The data provider specified for %s::%s is invalid\n%s",
-                    $className,
-                    $methodName,
-                    $this->throwableToString($t)
-                )
-            );
-        }
+        $data = (new DataProvider)->providedData(
+            $className,
+            $methodName
+        );
 
         if ($data !== null) {
             $test = $this->buildDataProviderTestSuite(
@@ -84,34 +69,30 @@ public function build(ReflectionClass $theClass, string $methodName): Test
      * @psalm-param class-string $className
      * @psalm-param array{backupGlobals: ?bool, backupGlobalsExcludeList: list<string>, backupStaticProperties: ?bool, backupStaticPropertiesExcludeList: array<string,list<string>>} $backupSettings
      */
-    private function buildDataProviderTestSuite(string $methodName, string $className, array|ErrorTestCase $data, bool $runTestInSeparateProcess, ?bool $preserveGlobalState, bool $runClassInSeparateProcess, array $backupSettings): DataProviderTestSuite
+    private function buildDataProviderTestSuite(string $methodName, string $className, array $data, bool $runTestInSeparateProcess, ?bool $preserveGlobalState, bool $runClassInSeparateProcess, array $backupSettings): DataProviderTestSuite
     {
         $dataProviderTestSuite = DataProviderTestSuite::empty(
             $className . '::' . $methodName
         );
 
         $groups = (new Groups)->groups($className, $methodName);
 
-        if ($data instanceof ErrorTestCase) {
-            $dataProviderTestSuite->addTest($data, $groups);
-        } else {
-            foreach ($data as $_dataName => $_data) {
-                $_test = new $className($methodName);
+        foreach ($data as $_dataName => $_data) {
+            $_test = new $className($methodName);
 
-                assert($_test instanceof TestCase);
+            assert($_test instanceof TestCase);
 
-                $_test->setData($_dataName, $_data);
+            $_test->setData($_dataName, $_data);
 
-                $this->configureTestCase(
-                    $_test,
-                    $runTestInSeparateProcess,
-                    $preserveGlobalState,
-                    $runClassInSeparateProcess,
-                    $backupSettings
-                );
+            $this->configureTestCase(
+                $_test,
+                $runTestInSeparateProcess,
+                $preserveGlobalState,
+                $runClassInSeparateProcess,
+                $backupSettings
+            );
 
-                $dataProviderTestSuite->addTest($_test, $groups);
-            }
+            $dataProviderTestSuite->addTest($_test, $groups);
         }
 
         return $dataProviderTestSuite;
@@ -151,30 +132,6 @@ private function configureTestCase(TestCase $test, bool $runTestInSeparateProces
         $test->setBackupStaticPropertiesExcludeList($backupSettings['backupStaticPropertiesExcludeList']);
     }
 
-    private function throwableToString(Throwable $t): string
-    {
-        $message = $t->getMessage();
-
-        if (empty(trim($message))) {
-            $message = '<no message>';
-        }
-
-        if ($t instanceof InvalidDataSetException) {
-            return sprintf(
-                "%s\n%s",
-                $message,
-                Filter::getFilteredStacktrace($t)
-            );
-        }
-
-        return sprintf(
-            "%s: %s\n%s",
-            $t::class,
-            $message,
-            Filter::getFilteredStacktrace($t)
-        );
-    }
-
     /**
      * @psalm-param class-string $className
      *

diff --git a/src/Framework/TestCase.php b/src/Framework/TestCase.php
@@ -546,8 +546,7 @@ final public function status(): TestStatus
      */
     final public function run(): void
     {
-        if (!$this instanceof ErrorTestCase &&
-            !$this->handleDependencies()) {
+        if (!$this->handleDependencies()) {
             return;
         }
 

diff --git a/src/Framework/TestRunner.php b/src/Framework/TestRunner.php
@@ -77,7 +77,6 @@ public function run(TestCase $test): void
         ErrorHandler::instance()->enable();
 
         $collectCodeCoverage = CodeCoverage::isActive() &&
-                               !$test instanceof ErrorTestCase &&
                                $shouldCodeCoverageBeCollected;
 
         if ($collectCodeCoverage) {
@@ -388,10 +387,6 @@ private function canTimeLimitBeEnforced(): bool
 
     private function shouldTimeLimitBeEnforced(TestCase $test): bool
     {
-        if ($test instanceof ErrorTestCase) {
-            return false;
-        }
-
         if (!$this->configuration->enforceTimeLimit()) {
             return false;
         }

diff --git a/src/Framework/TestSuite.php b/src/Framework/TestSuite.php
@@ -25,15 +25,18 @@
 use Iterator;
 use IteratorAggregate;
 use PHPUnit\Event;
+use PHPUnit\Event\Code\TestMethod;
 use PHPUnit\Metadata\Api\Dependencies;
 use PHPUnit\Metadata\Api\Groups;
 use PHPUnit\Metadata\Api\HookMethods;
 use PHPUnit\Metadata\Api\Requirements;
+use PHPUnit\Metadata\MetadataCollection;
 use PHPUnit\Runner\Filter\Factory;
 use PHPUnit\Runner\PhptTestCase;
 use PHPUnit\Runner\TestSuiteLoader;
 use PHPUnit\TestRunner\TestResult\Facade;
 use PHPUnit\TextUI\Configuration\Registry;
+use PHPUnit\Util\Filter;
 use PHPUnit\Util\Reflection;
 use PHPUnit\Util\Test as TestUtil;
 use ReflectionClass;
@@ -545,9 +548,31 @@ public function sortId(): string
      */
     protected function addTestMethod(ReflectionClass $class, ReflectionMethod $method): void
     {
+        $className  = $class->getName();
         $methodName = $method->getName();
 
-        $test = (new TestBuilder)->build($class, $methodName);
+        try {
+            $test = (new TestBuilder)->build($class, $methodName);
+        } catch (InvalidDataProviderException $e) {
+            Event\Facade::emitter()->testTriggeredPhpunitError(
+                new TestMethod(
+                    $className,
+                    $methodName,
+                    $class->getFileName(),
+                    $method->getStartLine(),
+                    MetadataCollection::fromArray([]),
+                    Event\TestDataCollection::fromArray([])
+                ),
+                sprintf(
+                    "The data provider specified for %s::%s is invalid\n%s",
+                    $className,
+                    $methodName,
+                    $this->throwableToString($e)
+                )
+            );
+
+            return;
+        }
 
         if ($test instanceof TestCase || $test instanceof DataProviderTestSuite) {
             $test->setDependencies(
@@ -614,4 +639,28 @@ private function shouldStop(): bool
 
         return false;
     }
+
+    private function throwableToString(Throwable $t): string
+    {
+        $message = $t->getMessage();
+
+        if (empty(trim($message))) {
+            $message = '<no message>';
+        }
+
+        if ($t instanceof InvalidDataProviderException) {
+            return sprintf(
+                "%s\n%s",
+                $message,
+                Filter::getFilteredStacktrace($t)
+            );
+        }
+
+        return sprintf(
+            "%s: %s\n%s",
+            $t::class,
+            $message,
+            Filter::getFilteredStacktrace($t)
+        );
+    }
 }