Skip to content

Commit

Permalink
Core tests: safeguard against duplicate test markers
Browse files Browse the repository at this point in the history
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
  • Loading branch information
jrfnl committed Dec 22, 2024
1 parent b35dba4 commit 1bb7113
Show file tree
Hide file tree
Showing 6 changed files with 85 additions and 13 deletions.
51 changes: 51 additions & 0 deletions tests/Core/AbstractMethodUnitTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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($seenComments, array_unique($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.
*
Expand Down
2 changes: 1 addition & 1 deletion tests/Core/File/GetMethodParametersTest.inc
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
4 changes: 2 additions & 2 deletions tests/Core/File/GetMethodParametersTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
21 changes: 21 additions & 0 deletions tests/Core/Tokenizers/AbstractTokenizerTestCase.php
Original file line number Diff line number Diff line change
Expand Up @@ -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.
*
Expand Down
10 changes: 5 additions & 5 deletions tests/Core/Tokenizers/PHP/OtherContextSensitiveKeywordsTest.inc
Original file line number Diff line number Diff line change
Expand Up @@ -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 */
Expand Down
10 changes: 5 additions & 5 deletions tests/Core/Tokenizers/PHP/OtherContextSensitiveKeywordsTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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',
],

Expand Down

0 comments on commit 1bb7113

Please sign in to comment.