Fix: Always fail on risky whatever the error

.github/workflows/CI.yml

@@ -90,6 +90,7 @@ jobs:
         run: |
           composer require -W ${{ env.COMPOSER_OPTIONS }} ${{ matrix.composer-flag }} \
             phpunit/phpunit:^${{ matrix.phpunit-version }} \
+            phpunit/php-code-coverage:^${{ matrix.phpunit-version }} \
           && composer update ${{ matrix.composer-flag }} \
           && make build
 
@@ -155,7 +156,7 @@ jobs:
         run: make build
 
       - name: ComposerRequireChecker
-        uses: docker://webfactory/composer-require-checker:3.2.0
+        uses: docker://webfactory/composer-require-checker:4.5.0
 
       - name: Dependencies check
         if: ${{ github.event_name == 'pull_request' }}
@@ -232,8 +233,11 @@ jobs:
 
       - name: Build
         run: |
-          composer require -W ${{ env.COMPOSER_OPTIONS }} \
+          composer config minimum-stability dev \
+          && composer require -W ${{ env.COMPOSER_OPTIONS }} \
             phpunit/phpunit:^${{ matrix.phpunit-version }} \
+            phpunit/php-code-coverage:^${{ matrix.phpunit-version }} \
+            phpspec/prophecy-phpunit:">=2" \
           && composer update \
           && make build
 

composer.json

@@ -29,7 +29,7 @@
   "require": {
     "php": "^8.0",
     "phpunit/phpunit": "^9.0",
-    "phpunit/php-code-coverage": "^9.2.4"
+    "phpunit/php-code-coverage": "^9.0"
   },
   "require-dev": {
     "squizlabs/php_codesniffer": "^3.5",

features/bootstrap/FeatureContext.php

@@ -61,6 +61,11 @@ public function iRunPhpunitTestSuite()
         $this->process = new Process($argList, null, $env);
 
         $this->process->run();
+
+        //var_dump(['env' => $env]);
+        //var_dump(['argList' => $argList]);
+        //var_dump(['output' => $this->process->getOutput()]);
+        //var_dump(['error' => $this->process->getErrorOutput()]);
     }
 
     /**
@@ -70,7 +75,11 @@ public function iShouldHaveXFailures($expectedFailureCount)
     {
         $output = $this->process->getOutput();
 
-        $failureCount = preg_match_all(sprintf('#There was %s failure#', $expectedFailureCount), $output);
+        $matched = preg_match_all('#There (?:was|were) (\d+) failure#', $output, $matches);
+        if ($matched === false) {
+            throw new \Exception('Error when executing preg_match_all');
+        }
+        $failureCount = $matches[1][0];
         if ($failureCount != $expectedFailureCount) {
             throw new \Exception(sprintf('Found %d failure, but %d expected', $failureCount, $expectedFailureCount));
         }

features/demo_app/tests/RiskyOutputTest.php

@@ -1,5 +1,8 @@
 <?php
 
+/**
+ * @coversNothing
+ */
 class RiskyOutputTest extends \PHPUnit\Framework\TestCase
 {
     /**

src/Listener/DelegatingListener.php

@@ -74,10 +74,10 @@ public function addIncompleteTest(Test $test, \Throwable $e, float $time) : void
     /**
      * {@inheritdoc}
      */
-    public function addRiskyTest(Test $test, \Throwable $e, float $time) : void
+    public function addRiskyTest(Test $test, \Throwable $exception, float $time) : void
     {
         foreach ($this->listenerList as $listener) {
-            $listener->addRiskyTest($test, $e, $time);
+            $listener->addRiskyTest($test, $exception, $time);
         }
     }
 

src/Listener/RiskyToFailedListener.php

@@ -2,11 +2,15 @@
 namespace Yoanm\PhpUnitExtended\Listener;
 
 use PHPUnit\Framework\AssertionFailedError;
+use PHPUnit\Framework\CoveredCodeNotExecutedException;
+use PHPUnit\Framework\InvalidCoversTargetException;
+use PHPUnit\Framework\MissingCoversAnnotationException;
 use PHPUnit\Framework\OutputError;
 use PHPUnit\Framework\Test;
 use PHPUnit\Framework\TestCase;
 use PHPUnit\Framework\TestListener;
 use PHPUnit\Framework\TestListenerDefaultImplementation;
+use PHPUnit\Framework\TestResult;
 use PHPUnit\Framework\UnintentionallyCoveredCodeError;
 use PHPUnit\Framework\Warning;
 
@@ -28,72 +32,68 @@ public function addWarning(Test $test, Warning $e, float $time) : void
     /**
      * {@inheritdoc}
      */
-    public function addRiskyTest(Test $test, \Throwable $e, float $time) : void
+    public function addRiskyTest(Test $test, \Throwable $exception, float $time) : void
     {
-        $this->addErrorIfNeeded($test, $e, $time);
+        $this->addErrorIfNeeded($test, $exception, $time);
     }
 
-    /**
-     * @param Test $test
-     * @param \Throwable $e
-     * @param $time
-     */
-    protected function addErrorIfNeeded(Test $test, \Throwable $e, $time)
+    protected function addErrorIfNeeded(Test $test, \Throwable $exception, $time)
     {
         /* Must be TestCase instance to have access to "getTestResultObject" method */
-        if ($test instanceof TestCase && $test->getTestResultObject() !== null) {
-            $reason = $this->getErrorReason($e);
-            if (null !== $reason) {
-                $test->getTestResultObject()->addFailure(
-                    $test,
-                    new AssertionFailedError(
-                        sprintf(
-                            "Strict mode - %s :\n%s",
-                            $reason,
-                            $e->getMessage()
-                        )
-                    ),
-                    $time
-                );
+        if ($test instanceof TestCase) {
+            if (!$test->getTestResultObject()) {
+                $test->setTestResultObject(new TestResult());
             }
+            $test->getTestResultObject()->addFailure(
+                $test,
+                new AssertionFailedError(
+                    sprintf(
+                        "Strict mode - %s :\n%s",
+                        $this->getErrorReason($exception),
+                        $exception->getMessage()
+                    )
+                ),
+                $time
+            );
         }
     }
 
-    /**
-     * @param \Throwable $e
-     *
-     * @return null|string
-     */
-    protected function getErrorReason(\Throwable $e)
+    protected function getErrorReason(\Throwable $exception): string
     {
-        $reason = null;
-        switch (true) {
+        if ($exception instanceof OutputError) {
             /* beStrictAboutOutputDuringTests="true" */
-            case $e instanceof OutputError:
-                $reason = 'No output during test';
-                break;
+            return 'No output during test';
+        } elseif ($exception instanceof UnintentionallyCoveredCodeError
+            || $exception instanceof InvalidCoversTargetException
+        ) {
             /* checkForUnintentionallyCoveredCode="true" */
-            case $e instanceof UnintentionallyCoveredCodeError:
-                $reason = 'Executed code must be defined with @covers and @uses annotations';
-                break;
-            default:
-                if (preg_match('#\-\-\- Global variables before the test#', $e->getMessage())) {
-                    /* beStrictAboutChangesToGlobalState="true" (no specific exception) for globals */
-                    $reason = 'No global variable manipulation during test';
-                } elseif (preg_match('#\-\-\- Static attributes before the test#', $e->getMessage())) {
-                    /* beStrictAboutChangesToGlobalState="true" (no specific exception) for static var */
-                    /* Only when beStrictAboutChangesToGlobalState="true" */
-                    $reason = 'No static attribute manipulation during test';
-                } elseif (preg_match('#This test did not perform any assertions#', $e->getMessage())) {
-                    /* beStrictAboutTestsThatDoNotTestAnything="true" (no specific exception) */
-                    $reason = 'No test that do not test anything';
-                } elseif (preg_match('#"@covers [^"]+" is invalid#', $e->getMessage())) {
-                    /* forceCoversAnnotation="true" (no specific exception) */
-                    $reason = 'Only executed code must be defined with @covers and @uses annotations';
-                }
-                break;
+            return 'Executed code must be defined with @covers and @uses annotations';
+        } elseif (str_contains($exception->getMessage(), '--- Global variables before the test')) {
+            /* beStrictAboutChangesToGlobalState="true" (no specific exception) for globals */
+            return 'No global variable manipulation during test';
+        } elseif (str_contains($exception->getMessage(), '--- Static attributes before the test')) {
+            /* beStrictAboutChangesToGlobalState="true" (no specific exception) for static var */
+            /* Only when beStrictAboutChangesToGlobalState="true" */
+            return 'No static attribute manipulation during test';
+        } elseif (str_contains($exception->getMessage(), 'This test did not perform any assertions')) {
+            /* beStrictAboutTestsThatDoNotTestAnything="true" (no specific exception) */
+            return 'No test that do not test anything';
+        } elseif ($exception instanceof CoveredCodeNotExecutedException
+            || preg_match('#"@covers [^"]+" is invalid#', $exception->getMessage())
+        ) {
+            /* forceCoversAnnotation="true" (no specific exception) */
+            return 'Only executed code must be defined with @covers and @uses annotations';
+        } elseif ($exception instanceof MissingCoversAnnotationException
+            || str_contains(
+                $exception->getMessage(),
+                'This test does not have a @covers annotation but is expected to have one'
+            )
+        ) {
+            /* forceCoversAnnotation="true" (no specific exception) */
+            return 'Missing @covers or @coversNothing annotation';
         }
 
-        return $reason;
+        // Always return an error even if it's not a known/managed error
+        return $exception->getMessage();
     }
 }

src/Listener/StrictCoverageListener.php

@@ -17,19 +17,14 @@ class StrictCoverageListener implements TestListener
     /**
      * {@inheritdoc}
      */
-    public function addRiskyTest(Test $test, \Throwable $e, float $time) : void
+    public function addRiskyTest(Test $test, \Throwable $exception, float $time) : void
     {
-        if (/* Must be PHPUnit_Framework_TestCase instance to have access to "getTestResultObject" method */
-            $test instanceof TestCase
-            && $e instanceof OutputError
-        ) {
+        /* Must be PHPUnit\Framework\TestCase instance to have access to "getTestResultObject" method */
+        if ($test instanceof TestCase && $exception instanceof OutputError) {
             $this->removeCoverageFor($test);
         }
     }
 
-    /**
-     * @param TestCase $test
-     */
     protected function removeCoverageFor(TestCase $test)
     {
         $coverage = $test->getTestResultObject()->getCodeCoverage();

tests/Functional/Listener/RiskyToFailedListenerTest.php

@@ -1,6 +1,9 @@
 <?php
 namespace Tests\Functional\Listener;
 
+use PHPUnit\Framework\CoveredCodeNotExecutedException;
+use PHPUnit\Framework\InvalidCoversTargetException;
+use PHPUnit\Framework\MissingCoversAnnotationException;
 use PHPUnit\Framework\OutputError;
 use PHPUnit\Framework\RiskyTestError;
 use PHPUnit\Framework\TestCase;
@@ -50,32 +53,24 @@ public function testShouldHandleBadCoverageTagWarning()
      *
      * @param $exceptionClass
      * @param $expectedReason
-     * @param bool|true $called
      * @param null $exceptionMessage
      */
     public function testShouldHandleRiskyTestWith(
         $exceptionClass,
         $expectedReason,
-        $called,
         $exceptionMessage = 'my default exception message'
     ) {
         $time = 0.3;
         $test = new TestCaseMock();
         $exception = new $exceptionClass($exceptionMessage);
 
-        $test->setTestResultObject(new TestResult());
-
         $this->listener->addRiskyTest($test, $exception, $time);
 
         $failures = $test->getTestResultObject()->failures();
-        if ($called) {
-            $this->assertCount(1, $failures);
-            $failure = array_shift($failures);
-            $this->assertInstanceOf(TestFailure::class, $failure);
-            $this->assertStringContainsString($exceptionMessage, $failure->getExceptionAsString());
-        } else {
-            $this->assertCount(0, $failures);
-        }
+        $this->assertCount(1, $failures);
+        $failure = array_shift($failures);
+        $this->assertInstanceOf(TestFailure::class, $failure);
+        $this->assertStringContainsString($exceptionMessage, $failure->getExceptionAsString());
     }
 
     /**
@@ -87,35 +82,52 @@ public function getExceptionMessageProvider()
             'Output exception' => [
                 'exceptionClass' => OutputError::class,
                 'expectedMessage' => 'No output during test',
-                'called' => true,
             ],
-            'Coverage exception' => [
+            'Coverage overflow - UnintentionallyCoveredCodeError' => [
                 'exceptionClass' => UnintentionallyCoveredCodeError::class,
                 'expectedMessage' => 'Executed code must be defined with @covers and @uses annotations',
-                'called' => true,
+            ],
+            'Coverage overflow - InvalidCoversTargetException instance' => [
+                'exceptionClass' => InvalidCoversTargetException::class,
+                'expectedMessage' => 'Executed code must be defined with @covers and @uses annotations',
             ],
             'Globals manipulation - globals' => [
                 'exceptionClass' => RiskyTestError::class,
                 'expectedMessage' => 'No global variable manipulation during test',
-                'called' => true,
                 'exceptionMessage' => '--- Global variables before the test',
             ],
             'Globals manipulation - static' => [
                 'exceptionClass' => RiskyTestError::class,
                 'expectedMessage' => 'No static attribute manipulation during test',
-                'called' => true,
                 'exceptionMessage' => '--- Static attributes before the test',
             ],
             'Test nothing' => [
                 'exceptionClass' => RiskyTestError::class,
                 'expectedMessage' => 'No test that do not test anything',
-                'called' => true,
                 'exceptionMessage' => 'This test did not perform any assertions',
             ],
+            'Coverage exception - CoveredCodeNotExecutedException instance' => [
+                'exceptionClass' => CoveredCodeNotExecutedException::class,
+                'expectedMessage' => 'Only executed code must be defined with @covers and @uses annotations',
+            ],
+            'Coverage exception - by message' => [
+                'exceptionClass' => RiskyTestError::class,
+                'expectedMessage' => 'Only executed code must be defined with @covers and @uses annotations',
+                'exceptionMessage' => '"@covers A\Class" is invalid'
+            ],
+            'Missing @covers - MissingCoversAnnotationException instance' => [
+                'exceptionClass' => MissingCoversAnnotationException::class,
+                'expectedMessage' => 'Missing @covers or @coversNothing annotation',
+            ],
+            'Missing @covers - by message' => [
+                'exceptionClass' => RiskyTestError::class,
+                'expectedMessage' => 'Missing @covers or @coversNothing annotation',
+                'exceptionMessage' => '"@covers A\Class" is invalid'
+            ],
             'other exceptions' => [
                 'exceptionClass' => \Exception::class,
-                'expectedMessage' => 'Risky test',
-                'called' => false,
+                'expectedMessage' => 'Arghh !',
+                'exceptionMessage' => 'Arghh !',
             ],
         ];
     }