From 76f67ce3124c144fae7f066015d0615edf467529 Mon Sep 17 00:00:00 2001 From: jrfnl Date: Sun, 22 Dec 2024 14:28:34 +0100 Subject: [PATCH] Core tests: safeguard against duplicate test markers This commit adds a new test to both the `AbstractMethodUnitTest` and the `AbstractTokenizerTestCase` classes to automatically verify that the case file in use by the child test class only contains unique test markers. The actual logic for the test is in a custom, `static`, assertion `assertTestMarkersAreUnique()` to allow for calling the assertion directly if an additional test case file is tokenized for the test; and to prevent duplicating the logic in both test case classes. Includes fixing a few test markers which this new test identified as duplicates straight off. Fixes 773 --- tests/Core/AbstractMethodUnitTest.php | 51 +++++++++++++++++++ tests/Core/File/GetMethodParametersTest.inc | 2 +- tests/Core/File/GetMethodParametersTest.php | 4 +- .../Tokenizers/AbstractTokenizerTestCase.php | 21 ++++++++ .../PHP/OtherContextSensitiveKeywordsTest.inc | 10 ++-- .../PHP/OtherContextSensitiveKeywordsTest.php | 10 ++-- 6 files changed, 85 insertions(+), 13 deletions(-) diff --git a/tests/Core/AbstractMethodUnitTest.php b/tests/Core/AbstractMethodUnitTest.php index 3784fb072d..dbb7ec63d9 100644 --- a/tests/Core/AbstractMethodUnitTest.php +++ b/tests/Core/AbstractMethodUnitTest.php @@ -107,6 +107,57 @@ public static function reset() }//end reset() + /** + * 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. + * + * @coversNothing + * + * @return void + */ + public function testTestMarkersAreUnique() + { + $this->assertTestMarkersAreUnique(self::$phpcsFile); + + }//end testTestMarkersAreUnique() + + + /** + * Assertion to verify that a test case file does not contain any duplicate test markers. + * + * @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[$i] = $tokens[$i]['content']; + } + + self::assertSame(array_unique($seenComments), $seenComments, 'Duplicate test markers found.'); + + }//end assertTestMarkersAreUnique() + + /** * Get the token pointer for a target token based on a specific comment found on the line before. * diff --git a/tests/Core/File/GetMethodParametersTest.inc b/tests/Core/File/GetMethodParametersTest.inc index 1f72ccfaae..4ccabb33c7 100644 --- a/tests/Core/File/GetMethodParametersTest.inc +++ b/tests/Core/File/GetMethodParametersTest.inc @@ -118,7 +118,7 @@ function messyDeclaration( ?\MyNS /* comment */ \ SubCat // phpcs:ignore Standard.Cat.Sniff -- for reasons. \ MyClass $a, - $b /* test */ = /* test */ 'default' /* test*/, + $b /* comment */ = /* comment */ 'default' /* comment*/, // phpcs:ignore Stnd.Cat.Sniff -- For reasons. ? /*comment*/ bool // phpcs:disable Stnd.Cat.Sniff -- For reasons. diff --git a/tests/Core/File/GetMethodParametersTest.php b/tests/Core/File/GetMethodParametersTest.php index f8e7b22e27..db9e37bc6e 100644 --- a/tests/Core/File/GetMethodParametersTest.php +++ b/tests/Core/File/GetMethodParametersTest.php @@ -1268,8 +1268,8 @@ public function testMessyDeclaration() $expected[1] = [ 'token' => 29, 'name' => '$b', - 'content' => "\$b /* test */ = /* test */ 'default' /* test*/", - 'default' => "'default' /* test*/", + 'content' => "\$b /* comment */ = /* comment */ 'default' /* comment*/", + 'default' => "'default' /* comment*/", 'default_token' => 37, 'default_equal_token' => 33, 'has_attributes' => false, diff --git a/tests/Core/Tokenizers/AbstractTokenizerTestCase.php b/tests/Core/Tokenizers/AbstractTokenizerTestCase.php index a7c1a6b091..aa67aaf679 100644 --- a/tests/Core/Tokenizers/AbstractTokenizerTestCase.php +++ b/tests/Core/Tokenizers/AbstractTokenizerTestCase.php @@ -86,6 +86,27 @@ protected function initializeFile() }//end initializeFile() + /** + * 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. + * + * @coversNothing + * + * @return void + */ + public function testTestMarkersAreUnique() + { + AbstractMethodUnitTest::assertTestMarkersAreUnique($this->phpcsFile); + + }//end testTestMarkersAreUnique() + + /** * Get the token pointer for a target token based on a specific comment found on the line before. * diff --git a/tests/Core/Tokenizers/PHP/OtherContextSensitiveKeywordsTest.inc b/tests/Core/Tokenizers/PHP/OtherContextSensitiveKeywordsTest.inc index ef89caaeaa..028592f76b 100644 --- a/tests/Core/Tokenizers/PHP/OtherContextSensitiveKeywordsTest.inc +++ b/tests/Core/Tokenizers/PHP/OtherContextSensitiveKeywordsTest.inc @@ -212,11 +212,11 @@ class DNFTypes extends Something { const /* testSelfIsKeywordAsConstDNFType */ (self&B)|int NAME_SELF = SOME_CONST; const bool|(A&/* testParentIsKeywordAsConstDNFType */ parent) NAME_PARENT = SOME_CONST; - readonly public (A&B)/* testFalseIsKeywordAsConstDNFType */ |false $false; - protected /* testTrueIsKeywordAsConstDNFType */ true|(A&B) $true = SOME_CONST; - static private (A&B)|/* testNullIsKeywordAsConstDNFType */ null|(C&D) $null = SOME_CONST; - var string|/* testSelfIsKeywordAsConstDNFType */ (self&Stringable) $self = SOME_CONST; - protected (A/* testParentIsKeywordAsConstDNFType */ &parent)|float $parent = SOME_CONST; + readonly public (A&B)/* testFalseIsKeywordAsPropertyDNFType */ |false $false; + protected /* testTrueIsKeywordAsPropertyDNFType */ true|(A&B) $true = SOME_CONST; + static private (A&B)|/* testNullIsKeywordAsPropertyDNFType */ null|(C&D) $null = SOME_CONST; + var string|/* testSelfIsKeywordAsPropertyDNFType */ (self&Stringable) $self = SOME_CONST; + protected (A/* testParentIsKeywordAsPropertyDNFType */ &parent)|float $parent = SOME_CONST; public function DNFWithFalse( /* testFalseIsKeywordAsParamDNFType */ diff --git a/tests/Core/Tokenizers/PHP/OtherContextSensitiveKeywordsTest.php b/tests/Core/Tokenizers/PHP/OtherContextSensitiveKeywordsTest.php index 872fab0d78..b1a15bcb50 100644 --- a/tests/Core/Tokenizers/PHP/OtherContextSensitiveKeywordsTest.php +++ b/tests/Core/Tokenizers/PHP/OtherContextSensitiveKeywordsTest.php @@ -652,23 +652,23 @@ public static function dataKeywords() ], 'false: DNF type in property declaration' => [ - 'testMarker' => '/* testFalseIsKeywordAsConstDNFType */', + 'testMarker' => '/* testFalseIsKeywordAsPropertyDNFType */', 'expectedTokenType' => 'T_FALSE', ], 'true: DNF type in property declaration' => [ - 'testMarker' => '/* testTrueIsKeywordAsConstDNFType */', + 'testMarker' => '/* testTrueIsKeywordAsPropertyDNFType */', 'expectedTokenType' => 'T_TRUE', ], 'null: DNF type in property declaration' => [ - 'testMarker' => '/* testNullIsKeywordAsConstDNFType */', + 'testMarker' => '/* testNullIsKeywordAsPropertyDNFType */', 'expectedTokenType' => 'T_NULL', ], 'self: DNF type in property declaration' => [ - 'testMarker' => '/* testSelfIsKeywordAsConstDNFType */', + 'testMarker' => '/* testSelfIsKeywordAsPropertyDNFType */', 'expectedTokenType' => 'T_SELF', ], 'parent: DNF type in property declaration' => [ - 'testMarker' => '/* testParentIsKeywordAsConstDNFType */', + 'testMarker' => '/* testParentIsKeywordAsPropertyDNFType */', 'expectedTokenType' => 'T_PARENT', ],