diff --git a/PHPCSUtils/TestUtils/UtilityMethodTestCase.php b/PHPCSUtils/TestUtils/UtilityMethodTestCase.php index 5fa81154..4853fbf0 100644 --- a/PHPCSUtils/TestUtils/UtilityMethodTestCase.php +++ b/PHPCSUtils/TestUtils/UtilityMethodTestCase.php @@ -378,6 +378,57 @@ public static function usesPhp8NameTokens() return \version_compare(Helper::getVersion(), '3.99.99', '>='); } + /** + * Test QA: verify that a test case file does not contain any duplicate test markers. + * + * When a test case file contains a lot of test cases, it is easy to overlook that a test marker name + * is already in use. + * A test wouldn't necessarily fail on this, but would not be testing what is intended to be tested as + * it would be verifying token properties for the wrong token. + * + * This test safeguards against this. + * + * @since 1.1.0 + * + * @coversNothing + * + * @return void + */ + public function testTestMarkersAreUnique() + { + $this->assertTestMarkersAreUnique(self::$phpcsFile); + } + + /** + * Assertion to verify that a test case file does not contain any duplicate test markers. + * + * @since 1.1.0 + * + * @param \PHP_CodeSniffer\Files\File $phpcsFile The file to validate. + * + * @return void + */ + public static function assertTestMarkersAreUnique(File $phpcsFile) + { + $tokens = $phpcsFile->getTokens(); + + // Collect all marker comments in the file. + $seenComments = []; + for ($i = 0; $i < $phpcsFile->numTokens; $i++) { + if ($tokens[$i]['code'] !== \T_COMMENT) { + continue; + } + + if (\stripos($tokens[$i]['content'], '/* test') !== 0) { + continue; + } + + $seenComments[] = $tokens[$i]['content']; + } + + self::assertSame(\array_unique($seenComments), $seenComments, 'Duplicate test markers found.'); + } + /** * Get the token pointer for a target token based on a specific comment. * diff --git a/Tests/TestUtils/UtilityMethodTestCase/ExpectPhpcsExceptionTest.php b/Tests/TestUtils/UtilityMethodTestCase/ExpectPhpcsExceptionTest.php index 542911b2..ae1eb086 100644 --- a/Tests/TestUtils/UtilityMethodTestCase/ExpectPhpcsExceptionTest.php +++ b/Tests/TestUtils/UtilityMethodTestCase/ExpectPhpcsExceptionTest.php @@ -60,6 +60,19 @@ public static function resetTestFile() // Deliberately left empty. } + /** + * Overload the "normal" test marker QA check - this test class does not have a File object. + * + * @coversNothing + * @doesNotPerformAssertions + * + * @return void + */ + public function testTestMarkersAreUnique() + { + // Deliberately left empty. + } + /** * Test that the helper method to handle cross-version testing of exceptions in PHPUnit * works correctly. diff --git a/Tests/TestUtils/UtilityMethodTestCase/FailedToTokenizeTest.php b/Tests/TestUtils/UtilityMethodTestCase/FailedToTokenizeTest.php index 8ef79a21..afd34493 100644 --- a/Tests/TestUtils/UtilityMethodTestCase/FailedToTokenizeTest.php +++ b/Tests/TestUtils/UtilityMethodTestCase/FailedToTokenizeTest.php @@ -35,6 +35,19 @@ public static function setUpTestFile() // Deliberately left empty. } + /** + * Overload the "normal" test marker QA check - this test class does not have a valid File object. + * + * @coversNothing + * @doesNotPerformAssertions + * + * @return void + */ + public function testTestMarkersAreUnique() + { + // Deliberately left empty. + } + /** * Test that the setUpTestFile() fails a test when the tokenizer errored out. * diff --git a/Tests/TestUtils/UtilityMethodTestCase/GetTargetTokenFileNotFoundTest.php b/Tests/TestUtils/UtilityMethodTestCase/GetTargetTokenFileNotFoundTest.php index 01e112c7..e0d6b07d 100644 --- a/Tests/TestUtils/UtilityMethodTestCase/GetTargetTokenFileNotFoundTest.php +++ b/Tests/TestUtils/UtilityMethodTestCase/GetTargetTokenFileNotFoundTest.php @@ -34,6 +34,19 @@ public static function setUpTestFile() // Deliberately left empty. } + /** + * Overload the "normal" test marker QA check - this test class does not have a File object. + * + * @coversNothing + * @doesNotPerformAssertions + * + * @return void + */ + public function testTestMarkersAreUnique() + { + // Deliberately left empty. + } + /** * Test the behaviour of the getTargetToken() method when the test case file has not been tokenized. * diff --git a/Tests/TestUtils/UtilityMethodTestCase/MissingCaseFileTest.php b/Tests/TestUtils/UtilityMethodTestCase/MissingCaseFileTest.php index f849d830..660ecfee 100644 --- a/Tests/TestUtils/UtilityMethodTestCase/MissingCaseFileTest.php +++ b/Tests/TestUtils/UtilityMethodTestCase/MissingCaseFileTest.php @@ -35,6 +35,19 @@ public static function setUpTestFile() // Deliberately left empty. } + /** + * Overload the "normal" test marker QA check - this test class does not have a File object. + * + * @coversNothing + * @doesNotPerformAssertions + * + * @return void + */ + public function testTestMarkersAreUnique() + { + // Deliberately left empty. + } + /** * Test that the setUpTestFile() fails a test when the test case file is missing. * diff --git a/Tests/TestUtils/UtilityMethodTestCase/ResetTestFileTest.php b/Tests/TestUtils/UtilityMethodTestCase/ResetTestFileTest.php index a11ce667..6d4b1975 100644 --- a/Tests/TestUtils/UtilityMethodTestCase/ResetTestFileTest.php +++ b/Tests/TestUtils/UtilityMethodTestCase/ResetTestFileTest.php @@ -28,6 +28,10 @@ final class ResetTestFileTest extends PolyfilledTestCase /** * Overload the "normal" set up as it needs to be run from within the actual test(s) to ensure we have a valid test. * + * Note: We don't rely on this method being called "before class" as the tests in this class reset the statics on + * the UtilityMethodTestCase, so we need to be sure the static $caseFile property is set (again) before parsing + * a file and do that by calling this method directly from within each of the tests. + * * @beforeClass * * @return void @@ -38,6 +42,20 @@ public static function setUpTestFile() // Deliberately not running the actual setUpTestFile() method. } + /** + * Overload the "normal" test marker QA check - this test class resets the property containing the File object, + * so there will be no valid File + the case file is already validated in the `SetUpTestFileTest` class anyway. + * + * @coversNothing + * @doesNotPerformAssertions + * + * @return void + */ + public function testTestMarkersAreUnique() + { + // Deliberately left empty. + } + /** * Test that the static class properties in the class are correctly reset. * @@ -48,6 +66,7 @@ public function testTearDownCleansUpStaticTestCaseClassProperties() // Initialize a test, which should change the values of most static properties. self::$tabWidth = 2; self::$selectedSniff = ['Test.Test.Test']; + self::setUpTestFile(); parent::setUpTestFile(); // Verify that (most) properties no longer have their original value. diff --git a/Tests/TestUtils/UtilityMethodTestCase/SetUpTestFileTest.php b/Tests/TestUtils/UtilityMethodTestCase/SetUpTestFileTest.php index 47f656fa..faa92555 100644 --- a/Tests/TestUtils/UtilityMethodTestCase/SetUpTestFileTest.php +++ b/Tests/TestUtils/UtilityMethodTestCase/SetUpTestFileTest.php @@ -37,6 +37,22 @@ public static function setUpTestFile() // Deliberately not running the actual setUpTestFile() method. } + /** + * Overload the "normal" test marker QA check - this test class does not have a File object in the $phpcsFile property. + * + * @coversNothing + * + * @return void + */ + public function testTestMarkersAreUnique() + { + parent::setUpTestFile(); + + $this->assertTestMarkersAreUnique(self::$phpcsFile); + + parent::resetTestFile(); + } + /** * Test that the setUpTestFile() method works correctly. * diff --git a/Tests/TestUtils/UtilityMethodTestCase/TestMarkersAreUniqueFailsTest.inc b/Tests/TestUtils/UtilityMethodTestCase/TestMarkersAreUniqueFailsTest.inc new file mode 100644 index 00000000..6a9fa44a --- /dev/null +++ b/Tests/TestUtils/UtilityMethodTestCase/TestMarkersAreUniqueFailsTest.inc @@ -0,0 +1,13 @@ +expectException($exception); + $this->expectExceptionMessage($msg); + + parent::testTestMarkersAreUnique(); + } +} diff --git a/Tests/TestUtils/UtilityMethodTestCase/TestMarkersAreUniqueNoMarkersTest.inc b/Tests/TestUtils/UtilityMethodTestCase/TestMarkersAreUniqueNoMarkersTest.inc new file mode 100644 index 00000000..e5d2cb6a --- /dev/null +++ b/Tests/TestUtils/UtilityMethodTestCase/TestMarkersAreUniqueNoMarkersTest.inc @@ -0,0 +1,4 @@ +