From cf6125391026d6d189285dae5835a1c738ea55e6 Mon Sep 17 00:00:00 2001 From: Danack Date: Tue, 10 Jul 2018 15:21:59 +0100 Subject: [PATCH 1/5] Fixing method covers taking priority over class coversNothing. --- src/Framework/TestResult.php | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/Framework/TestResult.php b/src/Framework/TestResult.php index e8a84a1563f..544fa3bf6c3 100644 --- a/src/Framework/TestResult.php +++ b/src/Framework/TestResult.php @@ -568,8 +568,13 @@ public function run(Test $test): void $annotations = $test->getAnnotations(); - if (isset($annotations['class']['coversNothing']) || isset($annotations['method']['coversNothing'])) { + if (isset($annotations['method']['coversNothing'])) { $coversNothing = true; + } elseif (isset($annotations['class']['coversNothing'])) { + if (!isset($annotations['method']['covers'])) { + // method covers takes priority over class coversNothing + $coversNothing = true; + } } } From 12f28abbe8e96ab88c6a797f8baeb09d7a5ea2ae Mon Sep 17 00:00:00 2001 From: Danack Date: Sat, 21 Jul 2018 10:38:24 +0100 Subject: [PATCH 2/5] Extract function and add test for whether coverage is needed --- src/Framework/TestResult.php | 32 +++++++++++++++++---------- tests/Framework/TestResultTest.php | 35 ++++++++++++++++++++++++++++++ 2 files changed, 56 insertions(+), 11 deletions(-) create mode 100644 tests/Framework/TestResultTest.php diff --git a/src/Framework/TestResult.php b/src/Framework/TestResult.php index 544fa3bf6c3..c038a649fff 100644 --- a/src/Framework/TestResult.php +++ b/src/Framework/TestResult.php @@ -543,6 +543,25 @@ public function getCollectCodeCoverageInformation(): bool return $this->codeCoverage !== null; } + public static function isAnyCoverageRequired(TestCase $test) + { + $annotations = $test->getAnnotations(); + + // If any methods have covers, coverage must me generated + if (isset($annotations['method']['covers'])) { + return true; + } + + // If there are no explicit covers, and the test class is + // marked as covers nothing, all coverage can be skipped + if (isset($annotations['class']['coversNothing'])) { + return false; + } + + // Otherwise each test method can generate coverage + return true; + } + /** * Runs a TestCase. * @@ -566,16 +585,7 @@ public function run(Test $test): void $this->registerMockObjectsFromTestArgumentsRecursively ); - $annotations = $test->getAnnotations(); - - if (isset($annotations['method']['coversNothing'])) { - $coversNothing = true; - } elseif (isset($annotations['class']['coversNothing'])) { - if (!isset($annotations['method']['covers'])) { - // method covers takes priority over class coversNothing - $coversNothing = true; - } - } + $isAnyCoverageRequired = self::isAnyCoverageRequired($test); } $error = false; @@ -604,7 +614,7 @@ public function run(Test $test): void $collectCodeCoverage = $this->codeCoverage !== null && !$test instanceof WarningTestCase && - !$coversNothing; + $isAnyCoverageRequired; if ($collectCodeCoverage) { $this->codeCoverage->start($test); diff --git a/tests/Framework/TestResultTest.php b/tests/Framework/TestResultTest.php new file mode 100644 index 00000000000..bfe73a3b689 --- /dev/null +++ b/tests/Framework/TestResultTest.php @@ -0,0 +1,35 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ +namespace PHPUnit\Framework; + +class TestResultTest extends TestCase +{ + public function canSkipCoverageProvider() + { + return [ + ['CoverageClassTest', true], + ['CoverageNothingTest', true], + ['CoverageCoversOverridesCoversNothingTest', false], + ]; + } + + /** + * @group foo + * @dataProvider canSkipCoverageProvider + */ + public function testCanSkipCoverage($testCase, $expectedCanSkip) + { + require_once __DIR__ . "/../_files/" . $testCase . ".php"; + + $test = new $testCase(); + $canSkipCoverage = TestResult::isAnyCoverageRequired($test); + $this->assertEquals($expectedCanSkip, $canSkipCoverage); + } +} From 5982d5d111c4dd932cfdce65b096a1256cbba289 Mon Sep 17 00:00:00 2001 From: Danack Date: Sat, 21 Jul 2018 15:11:18 +0100 Subject: [PATCH 3/5] CS --- src/Framework/TestResult.php | 38 +++++++++++++++--------------- tests/Framework/TestResultTest.php | 4 ++-- 2 files changed, 21 insertions(+), 21 deletions(-) diff --git a/src/Framework/TestResult.php b/src/Framework/TestResult.php index c038a649fff..4c3db3d6e91 100644 --- a/src/Framework/TestResult.php +++ b/src/Framework/TestResult.php @@ -184,6 +184,25 @@ class TestResult implements Countable */ private $registerMockObjectsFromTestArgumentsRecursively = false; + public static function isAnyCoverageRequired(TestCase $test) + { + $annotations = $test->getAnnotations(); + + // If any methods have covers, coverage must me generated + if (isset($annotations['method']['covers'])) { + return true; + } + + // If there are no explicit covers, and the test class is + // marked as covers nothing, all coverage can be skipped + if (isset($annotations['class']['coversNothing'])) { + return false; + } + + // Otherwise each test method can generate coverage + return true; + } + /** * Registers a TestListener. */ @@ -543,25 +562,6 @@ public function getCollectCodeCoverageInformation(): bool return $this->codeCoverage !== null; } - public static function isAnyCoverageRequired(TestCase $test) - { - $annotations = $test->getAnnotations(); - - // If any methods have covers, coverage must me generated - if (isset($annotations['method']['covers'])) { - return true; - } - - // If there are no explicit covers, and the test class is - // marked as covers nothing, all coverage can be skipped - if (isset($annotations['class']['coversNothing'])) { - return false; - } - - // Otherwise each test method can generate coverage - return true; - } - /** * Runs a TestCase. * diff --git a/tests/Framework/TestResultTest.php b/tests/Framework/TestResultTest.php index bfe73a3b689..deee88cab9f 100644 --- a/tests/Framework/TestResultTest.php +++ b/tests/Framework/TestResultTest.php @@ -26,9 +26,9 @@ public function canSkipCoverageProvider() */ public function testCanSkipCoverage($testCase, $expectedCanSkip) { - require_once __DIR__ . "/../_files/" . $testCase . ".php"; + require_once __DIR__ . '/../_files/' . $testCase . '.php'; - $test = new $testCase(); + $test = new $testCase(); $canSkipCoverage = TestResult::isAnyCoverageRequired($test); $this->assertEquals($expectedCanSkip, $canSkipCoverage); } From 888f03d4170b751efa80a16e33158d4c03bf731e Mon Sep 17 00:00:00 2001 From: Danack Date: Sat, 21 Jul 2018 15:20:58 +0100 Subject: [PATCH 4/5] Remove 'foo'. --- tests/Framework/TestResultTest.php | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/Framework/TestResultTest.php b/tests/Framework/TestResultTest.php index deee88cab9f..06a97044131 100644 --- a/tests/Framework/TestResultTest.php +++ b/tests/Framework/TestResultTest.php @@ -21,7 +21,6 @@ public function canSkipCoverageProvider() } /** - * @group foo * @dataProvider canSkipCoverageProvider */ public function testCanSkipCoverage($testCase, $expectedCanSkip) From 49a4028fa26e3457637e4d2db9ad164f0f8d85ca Mon Sep 17 00:00:00 2001 From: Sebastian Bergmann Date: Sat, 21 Jul 2018 17:09:53 +0200 Subject: [PATCH 5/5] Update ChangeLog --- ChangeLog-7.2.md | 1 + 1 file changed, 1 insertion(+) diff --git a/ChangeLog-7.2.md b/ChangeLog-7.2.md index 5fba7547d7f..8af396b0233 100644 --- a/ChangeLog-7.2.md +++ b/ChangeLog-7.2.md @@ -7,6 +7,7 @@ All notable changes of the PHPUnit 7.2 release series are documented in this fil ### Fixed * Fixed [#3218](https://github.com/sebastianbergmann/phpunit/issues/3218): `prefix` attribute for `directory` node missing from `phpunit.xml` XSD +* Fixed [#3222](https://github.com/sebastianbergmann/phpunit/pull/3222): Priority of `@covers` and `@coversNothing` is wrong ## [7.2.7] - 2018-07-15