Merge pull request #777 from PHPCSStandards/feature/773-safeguard-aga…

…inst-duplicate-test-markers Core tests: safeguard against duplicate test markers
PHPCSStandards · Jan 23, 2025 · 154bb91 · 154bb91
2 parents 296db10 + 0e9c8c5
commit 154bb91
Show file tree

Hide file tree

Showing 6 changed files with 85 additions and 13 deletions.
diff --git 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[] = $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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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',
             ],