From 333e4236d129a62e3e6858c7eb78b4adb01db707 Mon Sep 17 00:00:00 2001 From: jrfnl Date: Tue, 18 Apr 2023 18:11:28 +0200 Subject: [PATCH 1/9] GetMethodParametersTest: add some missing array indexes expectations This commit: 1. Fixes the order of a few array entries. 2. Adds some array entries which were supposed to be expected, but missing. The use of `assertArraySubset()` hid the fact that these entries were missing. As that assertion now needs to be replaced, these issues came to light. --- tests/Core/File/GetMethodParametersTest.php | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/tests/Core/File/GetMethodParametersTest.php b/tests/Core/File/GetMethodParametersTest.php index 6cd2d6e87b..f44f36c307 100644 --- a/tests/Core/File/GetMethodParametersTest.php +++ b/tests/Core/File/GetMethodParametersTest.php @@ -95,8 +95,8 @@ public function testSingleDefaultValue() $expected[0] = [ 'name' => '$var1', 'content' => '$var1=self::CONSTANT', - 'has_attributes' => false, 'default' => 'self::CONSTANT', + 'has_attributes' => false, 'pass_by_reference' => false, 'variable_length' => false, 'type_hint' => '', @@ -119,8 +119,8 @@ public function testDefaultValues() $expected[0] = [ 'name' => '$var1', 'content' => '$var1=1', - 'has_attributes' => false, 'default' => '1', + 'has_attributes' => false, 'pass_by_reference' => false, 'variable_length' => false, 'type_hint' => '', @@ -129,8 +129,8 @@ public function testDefaultValues() $expected[1] = [ 'name' => '$var2', 'content' => "\$var2='value'", - 'has_attributes' => false, 'default' => "'value'", + 'has_attributes' => false, 'pass_by_reference' => false, 'variable_length' => false, 'type_hint' => '', @@ -900,6 +900,7 @@ public function testPHP8ConstructorPropertyPromotionGlobalFunction() 'type_hint' => '', 'nullable_type' => false, 'property_visibility' => 'private', + 'property_readonly' => false, ]; $this->getMethodParametersTestHelper('/* '.__FUNCTION__.' */', $expected); @@ -924,6 +925,7 @@ public function testPHP8ConstructorPropertyPromotionAbstractMethod() 'type_hint' => 'callable', 'nullable_type' => false, 'property_visibility' => 'public', + 'property_readonly' => false, ]; $expected[1] = [ 'name' => '$x', @@ -934,6 +936,7 @@ public function testPHP8ConstructorPropertyPromotionAbstractMethod() 'type_hint' => '', 'nullable_type' => false, 'property_visibility' => 'private', + 'property_readonly' => false, ]; $this->getMethodParametersTestHelper('/* '.__FUNCTION__.' */', $expected); @@ -953,6 +956,7 @@ public function testCommentsInParameter() 'name' => '$param', 'content' => '// Leading comment. ?MyClass /*-*/ & /*-*/.../*-*/ $param /*-*/ = /*-*/ \'default value\' . /*-*/ \'second part\' // Trailing comment.', + 'default' => '\'default value\' . /*-*/ \'second part\' // Trailing comment.', 'has_attributes' => false, 'pass_by_reference' => true, 'variable_length' => true, @@ -982,6 +986,7 @@ public function testParameterAttributesInFunctionDeclaration() 'type_hint' => 'string', 'nullable_type' => false, 'property_visibility' => 'private', + 'property_readonly' => false, ]; $expected[1] = [ 'name' => '$typedParamSingleAttribute', From 69f66ccee32f0969c2c73501f7010baba6bb3764 Mon Sep 17 00:00:00 2001 From: jrfnl Date: Tue, 18 Apr 2023 18:11:28 +0200 Subject: [PATCH 2/9] GetMemberPropertiesTest: add some missing array indexes expectations This commit adds some array entries which were supposed to be expected, but missing. The use of `assertArraySubset()` hid the fact that these entries were missing. As that assertion now needs to be replaced, these issues came to light. --- tests/Core/File/GetMemberPropertiesTest.php | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/tests/Core/File/GetMemberPropertiesTest.php b/tests/Core/File/GetMemberPropertiesTest.php index a207e2c2ba..c9bd4c255f 100644 --- a/tests/Core/File/GetMemberPropertiesTest.php +++ b/tests/Core/File/GetMemberPropertiesTest.php @@ -764,6 +764,7 @@ public function dataGetMemberProperties() 'scope' => 'public', 'scope_specified' => true, 'is_static' => false, + 'is_readonly' => false, 'type' => 'Foo&Bar', 'nullable_type' => false, ], @@ -774,6 +775,7 @@ public function dataGetMemberProperties() 'scope' => 'public', 'scope_specified' => true, 'is_static' => false, + 'is_readonly' => false, 'type' => 'Foo&Bar&Baz', 'nullable_type' => false, ], @@ -784,6 +786,7 @@ public function dataGetMemberProperties() 'scope' => 'public', 'scope_specified' => true, 'is_static' => false, + 'is_readonly' => false, 'type' => 'int&string', 'nullable_type' => false, ], @@ -794,6 +797,7 @@ public function dataGetMemberProperties() 'scope' => 'public', 'scope_specified' => true, 'is_static' => false, + 'is_readonly' => false, 'type' => '?Foo&Bar', 'nullable_type' => true, ], From 2bdb734e380a28745f9c5cd8d494e5f044a95d5a Mon Sep 17 00:00:00 2001 From: jrfnl Date: Tue, 18 Apr 2023 18:19:58 +0200 Subject: [PATCH 3/9] File/Get*Tests: work round removal of `assertArraySubset()` The `assertArraySubset()` method was deprecated in PHPUnit 8.x and removed in PHPUnit 9.0.0 without replacement. The `assertArraySubset()` assertion was being used as not all token array indexes are being tested - to be specific: any index which is a token offset is not tested -. As the `assertArraySubset()` has been removed, I'm electing to unset the token offset array indexes and replacing the assertion with a strict type `assertSame()` comparison. --- tests/Core/File/GetMemberPropertiesTest.php | 5 ++++- tests/Core/File/GetMethodParametersTest.php | 18 +++++++++++++++++- tests/Core/File/GetMethodPropertiesTest.php | 5 ++++- 3 files changed, 25 insertions(+), 3 deletions(-) diff --git a/tests/Core/File/GetMemberPropertiesTest.php b/tests/Core/File/GetMemberPropertiesTest.php index c9bd4c255f..aaed35a29b 100644 --- a/tests/Core/File/GetMemberPropertiesTest.php +++ b/tests/Core/File/GetMemberPropertiesTest.php @@ -30,7 +30,10 @@ public function testGetMemberProperties($identifier, $expected) $variable = $this->getTargetToken($identifier, T_VARIABLE); $result = self::$phpcsFile->getMemberProperties($variable); - $this->assertArraySubset($expected, $result, true); + // Unset those indexes which are not being tested. + unset($result['type_token'], $result['type_end_token']); + + $this->assertSame($expected, $result); }//end testGetMemberProperties() diff --git a/tests/Core/File/GetMethodParametersTest.php b/tests/Core/File/GetMethodParametersTest.php index f44f36c307..fe52dbdaaa 100644 --- a/tests/Core/File/GetMethodParametersTest.php +++ b/tests/Core/File/GetMethodParametersTest.php @@ -1179,7 +1179,23 @@ private function getMethodParametersTestHelper($commentString, $expected) $function = $this->getTargetToken($commentString, [T_FUNCTION, T_CLOSURE, T_FN]); $found = self::$phpcsFile->getMethodParameters($function); - $this->assertArraySubset($expected, $found, true); + // Unset those indexes which are not being tested. + foreach ($found as $i => $param) { + unset( + $found[$i]['token'], + $found[$i]['reference_token'], + $found[$i]['variadic_token'], + $found[$i]['type_hint_token'], + $found[$i]['type_hint_end_token'], + $found[$i]['comma_token'], + $found[$i]['default_token'], + $found[$i]['default_equal_token'], + $found[$i]['visibility_token'], + $found[$i]['readonly_token'] + ); + } + + $this->assertSame($expected, $found); }//end getMethodParametersTestHelper() diff --git a/tests/Core/File/GetMethodPropertiesTest.php b/tests/Core/File/GetMethodPropertiesTest.php index 3a32aa5f61..ef881965e5 100644 --- a/tests/Core/File/GetMethodPropertiesTest.php +++ b/tests/Core/File/GetMethodPropertiesTest.php @@ -902,7 +902,10 @@ private function getMethodPropertiesTestHelper($commentString, $expected) $function = $this->getTargetToken($commentString, [T_FUNCTION, T_CLOSURE, T_FN]); $found = self::$phpcsFile->getMethodProperties($function); - $this->assertArraySubset($expected, $found, true); + // Unset those indexes which are not being tested. + unset($found['return_type_token'], $found['return_type_end_token']); + + $this->assertSame($expected, $found); }//end getMethodPropertiesTestHelper() From 7e14b7fc9016e3a7b89628f9ddfc409cca7590d4 Mon Sep 17 00:00:00 2001 From: jrfnl Date: Tue, 18 Apr 2023 18:22:16 +0200 Subject: [PATCH 4/9] File/Get*Tests: work round removal of exception related annotations Expecting exceptions via annotations was deprecated in PHPUnit 8.x and removed in PHPUnit 9.0.0 in favour of using the `expectException*()` methods. This does need a work around for PHPUnit 4.x in which the `expectException*()` methods didn't exist yet, but as this only applies to three tests, that's not a big deal. --- tests/Core/File/GetClassPropertiesTest.php | 15 ++++++++--- tests/Core/File/GetMemberPropertiesTest.php | 30 ++++++++++++++++----- 2 files changed, 36 insertions(+), 9 deletions(-) diff --git a/tests/Core/File/GetClassPropertiesTest.php b/tests/Core/File/GetClassPropertiesTest.php index 7eed507307..c5fe70b690 100644 --- a/tests/Core/File/GetClassPropertiesTest.php +++ b/tests/Core/File/GetClassPropertiesTest.php @@ -23,13 +23,22 @@ class GetClassPropertiesTest extends AbstractMethodUnitTest * * @dataProvider dataNotAClassException * - * @expectedException PHP_CodeSniffer\Exceptions\RuntimeException - * @expectedExceptionMessage $stackPtr must be of type T_CLASS - * * @return void */ public function testNotAClassException($testMarker, $tokenType) { + $msg = '$stackPtr must be of type T_CLASS'; + $exception = 'PHP_CodeSniffer\Exceptions\RuntimeException'; + + if (\method_exists($this, 'expectException') === true) { + // PHPUnit 5+. + $this->expectException($exception); + $this->expectExceptionMessage($msg); + } else { + // PHPUnit 4. + $this->setExpectedException($exception, $msg); + } + $target = $this->getTargetToken($testMarker, $tokenType); self::$phpcsFile->getClassProperties($target); diff --git a/tests/Core/File/GetMemberPropertiesTest.php b/tests/Core/File/GetMemberPropertiesTest.php index aaed35a29b..70d683ef2e 100644 --- a/tests/Core/File/GetMemberPropertiesTest.php +++ b/tests/Core/File/GetMemberPropertiesTest.php @@ -815,15 +815,24 @@ public function dataGetMemberProperties() * * @param string $identifier Comment which precedes the test case. * - * @expectedException PHP_CodeSniffer\Exceptions\RuntimeException - * @expectedExceptionMessage $stackPtr is not a class member var - * * @dataProvider dataNotClassProperty * * @return void */ public function testNotClassPropertyException($identifier) { + $msg = '$stackPtr is not a class member var'; + $exception = 'PHP_CodeSniffer\Exceptions\RuntimeException'; + + if (\method_exists($this, 'expectException') === true) { + // PHPUnit 5+. + $this->expectException($exception); + $this->expectExceptionMessage($msg); + } else { + // PHPUnit 4. + $this->setExpectedException($exception, $msg); + } + $variable = $this->getTargetToken($identifier, T_VARIABLE); $result = self::$phpcsFile->getMemberProperties($variable); @@ -855,13 +864,22 @@ public function dataNotClassProperty() /** * Test receiving an expected exception when a non variable is passed. * - * @expectedException PHP_CodeSniffer\Exceptions\RuntimeException - * @expectedExceptionMessage $stackPtr must be of type T_VARIABLE - * * @return void */ public function testNotAVariableException() { + $msg = '$stackPtr must be of type T_VARIABLE'; + $exception = 'PHP_CodeSniffer\Exceptions\RuntimeException'; + + if (\method_exists($this, 'expectException') === true) { + // PHPUnit 5+. + $this->expectException($exception); + $this->expectExceptionMessage($msg); + } else { + // PHPUnit 4. + $this->setExpectedException($exception, $msg); + } + $next = $this->getTargetToken('/* testNotAVariable */', T_RETURN); $result = self::$phpcsFile->getMemberProperties($next); From b82b84dfaf200d814553328df7f18206a5425839 Mon Sep 17 00:00:00 2001 From: jrfnl Date: Tue, 18 Apr 2023 18:09:40 +0200 Subject: [PATCH 5/9] GotoLabelTest: work round removal of `assertInternalType()` The `assertInternalType()` method was deprecated in PHPUnit 7.5.0 and removed in PHPUnit 9.0.0. PHPUnit 7.5.0 introduced dedicated `assertIs*()` (like `assertIsInt()`) methods as a replacement. As this is only a simple check in these two tests, a PHPUnit feature based toggle seems over the top, so I'm just replacing the assertion with an alternative which will work PHPUnit cross-version. --- tests/Core/Tokenizer/GotoLabelTest.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/Core/Tokenizer/GotoLabelTest.php b/tests/Core/Tokenizer/GotoLabelTest.php index f3799e755e..b4d1e8a4eb 100644 --- a/tests/Core/Tokenizer/GotoLabelTest.php +++ b/tests/Core/Tokenizer/GotoLabelTest.php @@ -32,7 +32,7 @@ public function testGotoStatement($testMarker, $testContent) $label = $this->getTargetToken($testMarker, T_STRING); - $this->assertInternalType('int', $label); + $this->assertTrue(is_int($label)); $this->assertSame($testContent, $tokens[$label]['content']); }//end testGotoStatement() @@ -78,7 +78,7 @@ public function testGotoDeclaration($testMarker, $testContent) $label = $this->getTargetToken($testMarker, T_GOTO_LABEL); - $this->assertInternalType('int', $label); + $this->assertTrue(is_int($label)); $this->assertSame($testContent, $tokens[$label]['content']); }//end testGotoDeclaration() From b4edac2f9e47d6fb1799ea85fef1da017eb80f23 Mon Sep 17 00:00:00 2001 From: jrfnl Date: Tue, 18 Apr 2023 18:43:13 +0200 Subject: [PATCH 6/9] RuleInclusion*Test: remove a few redundant assertions These assertions are checking whether explicitly declared properties exist, which is redundant. Removing the assertions does not diminish the value of the tests as there are follow-up assertions testing the value of the properties. Removing the assertions also gets rid of a warning thrown in PHPUnit 9.6.x about the `assertObjectHasAttribute()` assertion being removed in PHPUnit 10.0. Note: PHPUnit 10.1.0 adds these assertions back again, but under a different name `assertObjectHasProperty()`. --- tests/Core/Ruleset/RuleInclusionAbsoluteLinuxTest.php | 1 - tests/Core/Ruleset/RuleInclusionAbsoluteWindowsTest.php | 1 - tests/Core/Ruleset/RuleInclusionTest.php | 3 --- 3 files changed, 5 deletions(-) diff --git a/tests/Core/Ruleset/RuleInclusionAbsoluteLinuxTest.php b/tests/Core/Ruleset/RuleInclusionAbsoluteLinuxTest.php index a41c856882..1b4fcf13ec 100644 --- a/tests/Core/Ruleset/RuleInclusionAbsoluteLinuxTest.php +++ b/tests/Core/Ruleset/RuleInclusionAbsoluteLinuxTest.php @@ -91,7 +91,6 @@ public function tearDown() public function testLinuxStylePathRuleInclusion() { // Test that the sniff is correctly registered. - $this->assertObjectHasAttribute('sniffCodes', $this->ruleset); $this->assertCount(1, $this->ruleset->sniffCodes); $this->assertArrayHasKey('Generic.Formatting.SpaceAfterNot', $this->ruleset->sniffCodes); $this->assertSame( diff --git a/tests/Core/Ruleset/RuleInclusionAbsoluteWindowsTest.php b/tests/Core/Ruleset/RuleInclusionAbsoluteWindowsTest.php index b7c4410a06..3824a671a0 100644 --- a/tests/Core/Ruleset/RuleInclusionAbsoluteWindowsTest.php +++ b/tests/Core/Ruleset/RuleInclusionAbsoluteWindowsTest.php @@ -92,7 +92,6 @@ public function tearDown() public function testWindowsStylePathRuleInclusion() { // Test that the sniff is correctly registered. - $this->assertObjectHasAttribute('sniffCodes', $this->ruleset); $this->assertCount(1, $this->ruleset->sniffCodes); $this->assertArrayHasKey('Generic.Formatting.SpaceAfterCast', $this->ruleset->sniffCodes); $this->assertSame( diff --git a/tests/Core/Ruleset/RuleInclusionTest.php b/tests/Core/Ruleset/RuleInclusionTest.php index b74c54d394..3608f120d3 100644 --- a/tests/Core/Ruleset/RuleInclusionTest.php +++ b/tests/Core/Ruleset/RuleInclusionTest.php @@ -91,7 +91,6 @@ public function tearDown() */ public function testHasSniffCodes() { - $this->assertObjectHasAttribute('sniffCodes', self::$ruleset); $this->assertCount(48, self::$ruleset->sniffCodes); }//end testHasSniffCodes() @@ -336,7 +335,6 @@ public function dataRegisteredSniffCodes() */ public function testSettingProperties($sniffClass, $propertyName, $expectedValue) { - $this->assertObjectHasAttribute('sniffs', self::$ruleset); $this->assertArrayHasKey($sniffClass, self::$ruleset->sniffs); $this->assertObjectHasAttribute($propertyName, self::$ruleset->sniffs[$sniffClass]); @@ -426,7 +424,6 @@ public function dataSettingProperties() */ public function testSettingInvalidPropertiesOnStandardsAndCategoriesSilentlyFails($sniffClass, $propertyName) { - $this->assertObjectHasAttribute('sniffs', self::$ruleset, 'Ruleset does not have property sniffs'); $this->assertArrayHasKey($sniffClass, self::$ruleset->sniffs, 'Sniff class '.$sniffClass.' not listed in registered sniffs'); $sniffObject = self::$ruleset->sniffs[$sniffClass]; From 86e363c7158bcd5f94908b2345a5abbc857edaeb Mon Sep 17 00:00:00 2001 From: jrfnl Date: Tue, 18 Apr 2023 18:51:03 +0200 Subject: [PATCH 7/9] Ruleset Tests: work round removal of `assertObjectHasAttribute()` The `assertObjectHasAttribute()` method was deprecated in PHPUnit 9.6.x and removed in PHPUnit 10.0.0 without replacement. Note: PHPUnit 10.1.0 adds the assertion back again, but under a different name `assertObjectHasProperty()`. While only a deprecation warning is shown on PHPUnit 9.6.x and the tests will still pass, I'm electing to replace the assertion anyway with code which emulates what PHPUnit would assert. --- tests/Core/Ruleset/RuleInclusionTest.php | 11 +++++++++-- tests/Core/Ruleset/SetSniffPropertyTest.php | 7 +++++-- 2 files changed, 14 insertions(+), 4 deletions(-) diff --git a/tests/Core/Ruleset/RuleInclusionTest.php b/tests/Core/Ruleset/RuleInclusionTest.php index 3608f120d3..835d676298 100644 --- a/tests/Core/Ruleset/RuleInclusionTest.php +++ b/tests/Core/Ruleset/RuleInclusionTest.php @@ -12,6 +12,7 @@ use PHP_CodeSniffer\Config; use PHP_CodeSniffer\Ruleset; use PHPUnit\Framework\TestCase; +use ReflectionObject; class RuleInclusionTest extends TestCase { @@ -336,7 +337,10 @@ public function dataRegisteredSniffCodes() public function testSettingProperties($sniffClass, $propertyName, $expectedValue) { $this->assertArrayHasKey($sniffClass, self::$ruleset->sniffs); - $this->assertObjectHasAttribute($propertyName, self::$ruleset->sniffs[$sniffClass]); + + $hasProperty = (new ReflectionObject(self::$ruleset->sniffs[$sniffClass]))->hasProperty($propertyName); + $errorMsg = sprintf('Property %s does not exist on sniff class %s', $propertyName, $sniffClass); + $this->assertTrue($hasProperty, $errorMsg); $actualValue = self::$ruleset->sniffs[$sniffClass]->$propertyName; $this->assertSame($expectedValue, $actualValue); @@ -427,7 +431,10 @@ public function testSettingInvalidPropertiesOnStandardsAndCategoriesSilentlyFail $this->assertArrayHasKey($sniffClass, self::$ruleset->sniffs, 'Sniff class '.$sniffClass.' not listed in registered sniffs'); $sniffObject = self::$ruleset->sniffs[$sniffClass]; - $this->assertObjectNotHasAttribute($propertyName, $sniffObject, 'Property '.$propertyName.' registered for sniff '.$sniffClass.' which does not support it'); + + $hasProperty = (new ReflectionObject(self::$ruleset->sniffs[$sniffClass]))->hasProperty($propertyName); + $errorMsg = sprintf('Property %s registered for sniff %s which does not support it', $propertyName, $sniffClass); + $this->assertFalse($hasProperty, $errorMsg); }//end testSettingInvalidPropertiesOnStandardsAndCategoriesSilentlyFails() diff --git a/tests/Core/Ruleset/SetSniffPropertyTest.php b/tests/Core/Ruleset/SetSniffPropertyTest.php index 5f5b882103..1b3d955cc3 100644 --- a/tests/Core/Ruleset/SetSniffPropertyTest.php +++ b/tests/Core/Ruleset/SetSniffPropertyTest.php @@ -12,6 +12,7 @@ use PHP_CodeSniffer\Config; use PHP_CodeSniffer\Ruleset; use PHPUnit\Framework\TestCase; +use ReflectionObject; /** * These tests specifically focus on the changes made to work around the PHP 8.2 dynamic properties deprecation. @@ -118,8 +119,10 @@ public function testSetPropertyAppliesPropertyToMultipleSniffsInCategory() // Test that the property doesn't get set for the one sniff which doesn't support the property. $sniffClass = 'PHP_CodeSniffer\Standards\PEAR\Sniffs\Functions\ValidDefaultValueSniff'; $this->assertArrayHasKey($sniffClass, $ruleset->sniffs, 'Sniff class '.$sniffClass.' not listed in registered sniffs'); - $sniffObject = $ruleset->sniffs[$sniffClass]; - $this->assertObjectNotHasAttribute($propertyName, $sniffObject, 'Property registered for sniff '.$sniffClass.' which does not support it'); + + $hasProperty = (new ReflectionObject($ruleset->sniffs[$sniffClass]))->hasProperty($propertyName); + $errorMsg = sprintf('Property %s registered for sniff %s which does not support it', $propertyName, $sniffClass); + $this->assertFalse($hasProperty, $errorMsg); }//end testSetPropertyAppliesPropertyToMultipleSniffsInCategory() From bc302dd977877a22c5e60d42a2f6b7d9e9192dab Mon Sep 17 00:00:00 2001 From: jrfnl Date: Tue, 18 Apr 2023 17:07:28 +0200 Subject: [PATCH 8/9] Tests: rename fixture methods and use annotations As of PHPUnit 8.x, the method signature for the `setUpBeforeClass()`, `setUp()`, `tearDown()` and `tearDownAfterClass()` fixture methods has changed to require the `void` return type. As the `void` return type isn't available until PHP 7.1, this cannot be implemented. Annotations to the rescue. By renaming the `setUpBeforeClass()`, `setUp()`, `tearDown()` and `tearDownAfterClass()` methods to another, descriptive name and using the `@beforeClass`, `@before`, `@after` and `@afterClass` annotations, the tests can be made cross-version compatible up to PHPUnit 9.x. With this change, the unit tests can now be run on PHPUnit 4 - 9. This constitutes a minor BC-break for external standards which a) extend the PHPCS native testsuite and b) overload the `AbstractSniffUnitTest::setUp()` method. While quite a few external standards extends the PHPCS native testsuite, I very much doubt any of these overload the `setUp()` method, so IMO and taking into account that this is a test-only change, this is an acceptable change to include in the next PHPCS minor. Ref: https://docs.phpunit.de/en/7.5/annotations.html#before --- .../Generic/Tests/Debug/ESLintUnitTest.php | 16 +++++++++------- tests/Core/AbstractMethodUnitTest.php | 12 ++++++++---- .../Core/Autoloader/DetermineLoadedClassTest.php | 6 ++++-- tests/Core/Filters/Filter/AcceptTest.php | 6 ++++-- .../Ruleset/RuleInclusionAbsoluteLinuxTest.php | 12 ++++++++---- .../Ruleset/RuleInclusionAbsoluteWindowsTest.php | 12 ++++++++---- tests/Core/Ruleset/RuleInclusionTest.php | 12 ++++++++---- tests/Core/Sniffs/AbstractArraySniffTest.php | 8 +++++--- tests/Core/Tokenizer/HeredocNowdocCloserTest.php | 6 ++++-- tests/Standards/AbstractSniffUnitTest.php | 6 ++++-- 10 files changed, 62 insertions(+), 34 deletions(-) diff --git a/src/Standards/Generic/Tests/Debug/ESLintUnitTest.php b/src/Standards/Generic/Tests/Debug/ESLintUnitTest.php index aae2af2195..1aaec09d05 100644 --- a/src/Standards/Generic/Tests/Debug/ESLintUnitTest.php +++ b/src/Standards/Generic/Tests/Debug/ESLintUnitTest.php @@ -36,31 +36,33 @@ class ESLintUnitTest extends AbstractSniffUnitTest /** * Sets up this unit test. * + * @before + * * @return void */ - protected function setUp() + protected function setUpPrerequisites() { - parent::setUp(); + parent::setUpPrerequisites(); $cwd = getcwd(); file_put_contents($cwd.'/.eslintrc.json', self::ESLINT_CONFIG); - }//end setUp() + }//end setUpPrerequisites() /** * Remove artifact. * + * @after + * * @return void */ - protected function tearDown() + protected function resetProperties() { - parent::tearDown(); - $cwd = getcwd(); unlink($cwd.'/.eslintrc.json'); - }//end tearDown() + }//end resetProperties() /** diff --git a/tests/Core/AbstractMethodUnitTest.php b/tests/Core/AbstractMethodUnitTest.php index f9ecd81831..cd845d1309 100644 --- a/tests/Core/AbstractMethodUnitTest.php +++ b/tests/Core/AbstractMethodUnitTest.php @@ -41,9 +41,11 @@ abstract class AbstractMethodUnitTest extends TestCase * The test case file for a unit test class has to be in the same directory * directory and use the same file name as the test class, using the .inc extension. * + * @beforeClass + * * @return void */ - public static function setUpBeforeClass() + public static function initializeFile() { $config = new Config(); $config->standards = ['PSR1']; @@ -62,19 +64,21 @@ public static function setUpBeforeClass() self::$phpcsFile = new DummyFile($contents, $ruleset, $config); self::$phpcsFile->process(); - }//end setUpBeforeClass() + }//end initializeFile() /** * Clean up after finished test. * + * @afterClass + * * @return void */ - public static function tearDownAfterClass() + public static function resetFile() { self::$phpcsFile = null; - }//end tearDownAfterClass() + }//end resetFile() /** diff --git a/tests/Core/Autoloader/DetermineLoadedClassTest.php b/tests/Core/Autoloader/DetermineLoadedClassTest.php index af14d83266..e4a27258a5 100644 --- a/tests/Core/Autoloader/DetermineLoadedClassTest.php +++ b/tests/Core/Autoloader/DetermineLoadedClassTest.php @@ -19,13 +19,15 @@ class DetermineLoadedClassTest extends TestCase /** * Load the test files. * + * @beforeClass + * * @return void */ - public static function setUpBeforeClass() + public static function includeFixture() { include __DIR__.'/TestFiles/Sub/C.inc'; - }//end setUpBeforeClass() + }//end includeFixture() /** diff --git a/tests/Core/Filters/Filter/AcceptTest.php b/tests/Core/Filters/Filter/AcceptTest.php index 83affe9bc1..26e589e010 100644 --- a/tests/Core/Filters/Filter/AcceptTest.php +++ b/tests/Core/Filters/Filter/AcceptTest.php @@ -36,15 +36,17 @@ class AcceptTest extends TestCase /** * Initialize the config and ruleset objects based on the `AcceptTest.xml` ruleset file. * + * @beforeClass + * * @return void */ - public static function setUpBeforeClass() + public static function initializeConfigAndRuleset() { $standard = __DIR__.'/'.basename(__FILE__, '.php').'.xml'; self::$config = new Config(["--standard=$standard", "--ignore=*/somethingelse/*"]); self::$ruleset = new Ruleset(self::$config); - }//end setUpBeforeClass() + }//end initializeConfigAndRuleset() /** diff --git a/tests/Core/Ruleset/RuleInclusionAbsoluteLinuxTest.php b/tests/Core/Ruleset/RuleInclusionAbsoluteLinuxTest.php index 1b4fcf13ec..6e9e739a83 100644 --- a/tests/Core/Ruleset/RuleInclusionAbsoluteLinuxTest.php +++ b/tests/Core/Ruleset/RuleInclusionAbsoluteLinuxTest.php @@ -41,9 +41,11 @@ class RuleInclusionAbsoluteLinuxTest extends TestCase /** * Initialize the config and ruleset objects. * + * @before + * * @return void */ - public function setUp() + public function initializeConfigAndRuleset() { $this->standard = __DIR__.'/'.basename(__FILE__, '.php').'.xml'; $repoRootDir = dirname(dirname(dirname(__DIR__))); @@ -67,19 +69,21 @@ public function setUp() $config = new Config(["--standard={$this->standard}"]); $this->ruleset = new Ruleset($config); - }//end setUp() + }//end initializeConfigAndRuleset() /** * Reset ruleset file. * + * @after + * * @return void */ - public function tearDown() + public function resetRuleset() { file_put_contents($this->standard, $this->contents); - }//end tearDown() + }//end resetRuleset() /** diff --git a/tests/Core/Ruleset/RuleInclusionAbsoluteWindowsTest.php b/tests/Core/Ruleset/RuleInclusionAbsoluteWindowsTest.php index 3824a671a0..d838e01fbc 100644 --- a/tests/Core/Ruleset/RuleInclusionAbsoluteWindowsTest.php +++ b/tests/Core/Ruleset/RuleInclusionAbsoluteWindowsTest.php @@ -41,9 +41,11 @@ class RuleInclusionAbsoluteWindowsTest extends TestCase /** * Initialize the config and ruleset objects. * + * @before + * * @return void */ - public function setUp() + public function initializeConfigAndRuleset() { if (DIRECTORY_SEPARATOR === '/') { $this->markTestSkipped('Windows specific test'); @@ -66,21 +68,23 @@ public function setUp() $config = new Config(["--standard={$this->standard}"]); $this->ruleset = new Ruleset($config); - }//end setUp() + }//end initializeConfigAndRuleset() /** * Reset ruleset file. * + * @after + * * @return void */ - public function tearDown() + public function resetRuleset() { if (DIRECTORY_SEPARATOR !== '/') { file_put_contents($this->standard, $this->contents); } - }//end tearDown() + }//end resetRuleset() /** diff --git a/tests/Core/Ruleset/RuleInclusionTest.php b/tests/Core/Ruleset/RuleInclusionTest.php index 835d676298..cab58778e5 100644 --- a/tests/Core/Ruleset/RuleInclusionTest.php +++ b/tests/Core/Ruleset/RuleInclusionTest.php @@ -42,9 +42,11 @@ class RuleInclusionTest extends TestCase /** * Initialize the config and ruleset objects based on the `RuleInclusionTest.xml` ruleset file. * + * @beforeClass + * * @return void */ - public static function setUpBeforeClass() + public static function initializeConfigAndRuleset() { $standard = __DIR__.'/'.basename(__FILE__, '.php').'.xml'; self::$standard = $standard; @@ -70,19 +72,21 @@ public static function setUpBeforeClass() $config = new Config(["--standard=$standard"]); self::$ruleset = new Ruleset($config); - }//end setUpBeforeClass() + }//end initializeConfigAndRuleset() /** * Reset ruleset file. * + * @after + * * @return void */ - public function tearDown() + public function resetRuleset() { file_put_contents(self::$standard, self::$contents); - }//end tearDown() + }//end resetRuleset() /** diff --git a/tests/Core/Sniffs/AbstractArraySniffTest.php b/tests/Core/Sniffs/AbstractArraySniffTest.php index 3a9a6c4df2..c74513cac9 100644 --- a/tests/Core/Sniffs/AbstractArraySniffTest.php +++ b/tests/Core/Sniffs/AbstractArraySniffTest.php @@ -31,14 +31,16 @@ class AbstractArraySniffTest extends AbstractMethodUnitTest * The test case file for a unit test class has to be in the same directory * directory and use the same file name as the test class, using the .inc extension. * + * @beforeClass + * * @return void */ - public static function setUpBeforeClass() + public static function initializeFile() { self::$sniff = new AbstractArraySniffTestable(); - parent::setUpBeforeClass(); + parent::initializeFile(); - }//end setUpBeforeClass() + }//end initializeFile() /** diff --git a/tests/Core/Tokenizer/HeredocNowdocCloserTest.php b/tests/Core/Tokenizer/HeredocNowdocCloserTest.php index dbefd23f9d..9d34c5e1a4 100644 --- a/tests/Core/Tokenizer/HeredocNowdocCloserTest.php +++ b/tests/Core/Tokenizer/HeredocNowdocCloserTest.php @@ -29,9 +29,11 @@ class HeredocNowdocCloserTest extends AbstractMethodUnitTest * {@internal This is a near duplicate of the original method. Only difference is that * tab replacement is enabled for this test.} * + * @beforeClass + * * @return void */ - public static function setUpBeforeClass() + public static function initializeFile() { $config = new Config(); $config->standards = ['PSR1']; @@ -51,7 +53,7 @@ public static function setUpBeforeClass() self::$phpcsFile = new DummyFile($contents, $ruleset, $config); self::$phpcsFile->process(); - }//end setUpBeforeClass() + }//end initializeFile() /** diff --git a/tests/Standards/AbstractSniffUnitTest.php b/tests/Standards/AbstractSniffUnitTest.php index 78699a7639..d10689d29a 100644 --- a/tests/Standards/AbstractSniffUnitTest.php +++ b/tests/Standards/AbstractSniffUnitTest.php @@ -50,15 +50,17 @@ abstract class AbstractSniffUnitTest extends TestCase /** * Sets up this unit test. * + * @before + * * @return void */ - protected function setUp() + protected function setUpPrerequisites() { $class = get_class($this); $this->standardsDir = $GLOBALS['PHP_CODESNIFFER_STANDARD_DIRS'][$class]; $this->testsDir = $GLOBALS['PHP_CODESNIFFER_TEST_DIRS'][$class]; - }//end setUp() + }//end setUpPrerequisites() /** From 10b01fe41017b7b1f88d24b5537ea2cab9bdd1c6 Mon Sep 17 00:00:00 2001 From: jrfnl Date: Tue, 18 Apr 2023 15:37:18 +0200 Subject: [PATCH 9/9] Tests: allow the test suite to run on PHPUnit 8.x and 9.x Includes: * `composer.json`: widening the PHPUnit requirement to allow for PHPUnit 8.x and PHPUnit 9.x. Note: The recently released PHPUnit 10.x is not (yet) supported as it no longer supports the old-style test suite runner setup. It also would require for the abstract base test cases to be renamed as those classes are no longer allowed to end on `Test`. Refactoring the test suite to allow for PHPUnit 10.x is for a future PR. * `composer.json`: remove the script which was specific for PHP 8.1+ * Adjusting the PHPUnit configuration to ensure the tests are run in the same way and show all notices/warnings/deprecations on all PHPUnit versions. The default value for a number of configuration options has changed over time. This makes sure they are consistently set to values which are sensible for this codebase, independently of the PHPUnit version on which the tests are run. Includes adding a schema annotation (set to PHPUnit 9.2 as the schema has changed in PHPUnit 9.3, though that won't prevent the tests from running correctly). * GH Actions `test`/`quicktest` workflow: removing work-arounds which were in place related to running PHPUnit 7.x on PHP 8.x. * GH Actions `phpstan` workflow: let PHPStan run on the latest PHP version, now there's no need anymore to run it against a lower PHP version to prevent deprecation notices related to the use of an outdated PHPUnit version. * `AllTests`: Adjusting the condition which determines which `TestSuite` file to load to allow for PHPUnit 8.x and 9.x. * Adding the `.phpunit.result.cache` file to `.gitignore`. PHPUnit has a caching feature build in as of PHPUnit 8, so ignore the file that generates to prevent it from being committed. * CONTRIBUTING: remove references to the remove Composer script + instructions which have now become redundant. Related to 3395 --- .github/CONTRIBUTING.md | 7 ++----- .github/workflows/phpstan.yml | 2 +- .github/workflows/test.yml | 19 +------------------ .gitignore | 1 + CHANGELOG.md | 3 +++ composer.json | 7 +------ phpunit.xml.dist | 12 +++++++++++- tests/AllTests.php | 2 +- 8 files changed, 21 insertions(+), 32 deletions(-) diff --git a/.github/CONTRIBUTING.md b/.github/CONTRIBUTING.md index d73fd72ee3..deb27c2b5c 100644 --- a/.github/CONTRIBUTING.md +++ b/.github/CONTRIBUTING.md @@ -137,7 +137,6 @@ Note: There may be an issue or PR open already. If so, please join the discussio 1. Fork/clone the repository. 2. Run `composer install`. - When installing on PHP >= 8.0, use `composer install --ignore-platform-req=php+`. 3. Create a new branch off the `master` branch to hold your patch. If there is an open issue associated with your patch, including the issue number in the branch name is good practice. @@ -152,10 +151,8 @@ To help you with this, a number of convenience scripts are available: * `composer check-all` will run the `cs` + `test` checks in one go. * `composer cs` will check for code style violations. * `composer cbf` will run the autofixers for code style violations. -* `composer test` will run the unit tests (only works when on PHP < 8.1). -* `composer test-php8` will run the unit tests when you are working on PHP 8.1+. - Please note that using a `phpunit.xml` overload config file will not work with this script! -* `composer coverage` will run the unit tests with code coverage (only works when on PHP < 8.1). +* `composer test` will run the unit tests. +* `composer coverage` will run the unit tests with code coverage. Note: you may want to use a custom `phpunit.xml` overload config file to tell PHPUnit where to place an HTML report. Alternative run it like so: `composer coverage -- --coverage-html /path/to/report-dir/` to specify the location for the HTML report on the command line. * `composer build` will build the phpcs.phar and phpcbf.phar files. diff --git a/.github/workflows/phpstan.yml b/.github/workflows/phpstan.yml index 040afcb0a0..f4eb2c9677 100644 --- a/.github/workflows/phpstan.yml +++ b/.github/workflows/phpstan.yml @@ -30,7 +30,7 @@ jobs: - name: Install PHP uses: shivammathur/setup-php@v2 with: - php-version: '7.4' + php-version: 'latest' coverage: none tools: phpstan diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index a0ef078458..2af559af80 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -131,37 +131,20 @@ jobs: # Install dependencies and handle caching in one go. # @link https://github.com/marketplace/actions/install-php-dependencies-with-composer - - name: Install Composer dependencies - normal - if: ${{ matrix.php < '8.0' }} + - name: Install Composer dependencies uses: "ramsey/composer-install@v2" with: # Bust the cache at least once a month - output format: YYYY-MM. custom-cache-suffix: $(date -u "+%Y-%m") - # For PHP 8.0+, we need to install with ignore platform reqs as PHPUnit 7 is still used. - - name: Install Composer dependencies - with ignore platform - if: ${{ matrix.php >= '8.0' }} - uses: "ramsey/composer-install@v2" - with: - composer-options: --ignore-platform-req=php+ - custom-cache-suffix: $(date -u "+%Y-%m") - # Note: The code style check is run multiple times against every PHP version # as it also acts as an integration test. - name: 'PHPCS: set the path to PHP' run: php bin/phpcs --config-set php_path php - name: 'PHPUnit: run the tests' - if: ${{ matrix.php != '8.1' && matrix.php != '8.2' }} run: vendor/bin/phpunit tests/AllTests.php - # We need to ignore the config file so that PHPUnit doesn't try to read it. - # The config file causes an error on PHP 8.1+ with PHPunit 7, but it's not needed here anyway - # as we can pass all required settings in the phpunit command. - - name: 'PHPUnit: run the tests on PHP > 8.0' - if: ${{ matrix.php == '8.1' || matrix.php == '8.2' }} - run: vendor/bin/phpunit tests/AllTests.php --no-configuration --bootstrap=tests/bootstrap.php --dont-report-useless-tests - - name: 'PHPCS: check code style without cache, no parallel' if: ${{ matrix.custom_ini == false && matrix.php != '7.4' }} run: php bin/phpcs --no-cache --parallel=1 diff --git a/.gitignore b/.gitignore index 9cc808cd28..a6c95c29f6 100644 --- a/.gitignore +++ b/.gitignore @@ -1,6 +1,7 @@ /CodeSniffer.conf /phpcs.xml /phpunit.xml +.phpunit.result.cache .idea/* /vendor/ composer.lock diff --git a/CHANGELOG.md b/CHANGELOG.md index ac65631c91..38969e8fb2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -36,6 +36,9 @@ The file documents changes to the PHP_CodeSniffer project. - PSR2.Methods.FunctionCallSignature - PSR2.Methods.FunctionClosingBrace - Thanks to Atsushi Okui (@blue32a) for the patch +- Support for PHPUnit 8 and 9 to the test suite. + - Test suites for external standards which run via the PHPCS native test suite can now run on PHPUnit 4-9 (was 4-7). + - If any of these tests use the PHPUnit `setUp()`/`tearDown()` methods or overload the `setUp()` in the `AbstractSniffUnitTest` test case, they will need to be adjusted. See the [PR details for further information](https://github.com/PHPCSStandards/PHP_CodeSniffer/pull/59/commits/26384ebfcc0b1c1651b0e1e40c9b6c8c22881832). ### Changed - Changes have been made to the way PHPCS handles invalid sniff properties being set in a custom ruleset diff --git a/composer.json b/composer.json index 94c3a2440d..7abe62e21f 100644 --- a/composer.json +++ b/composer.json @@ -41,7 +41,7 @@ "ext-simplexml": "*" }, "require-dev": { - "phpunit/phpunit": "^4.0 || ^5.0 || ^6.0 || ^7.0" + "phpunit/phpunit": "^4.0 || ^5.0 || ^6.0 || ^7.0 || ^8.0 || ^9.0" }, "replace": { "squizlabs/php_codesniffer": "> 2.0" @@ -61,10 +61,6 @@ "Composer\\Config::disableProcessTimeout", "@php ./vendor/phpunit/phpunit/phpunit tests/AllTests.php --no-coverage" ], - "test-php8": [ - "Composer\\Config::disableProcessTimeout", - "@php ./vendor/phpunit/phpunit/phpunit tests/AllTests.php --no-configuration --bootstrap=tests/bootstrap.php --dont-report-useless-tests --no-coverage" - ], "coverage": [ "Composer\\Config::disableProcessTimeout", "@php ./vendor/phpunit/phpunit/phpunit tests/AllTests.php -d max_execution_time=0" @@ -82,7 +78,6 @@ "cs": "Check for code style violations.", "cbf": "Fix code style violations.", "test": "Run the unit tests without code coverage.", - "test-php8": "Run the unit tests without code coverage on PHP 8.1 or higher.", "coverage": "Run the unit tests with code coverage.", "build": "Create PHAR files for PHPCS and PHPCBF.", "check-all": "Run all checks (phpcs, tests)." diff --git a/phpunit.xml.dist b/phpunit.xml.dist index 34b4afcded..68b5bac47f 100644 --- a/phpunit.xml.dist +++ b/phpunit.xml.dist @@ -1,5 +1,15 @@ - + tests/AllTests.php diff --git a/tests/AllTests.php b/tests/AllTests.php index 5ee125f437..4fa41d9d9c 100644 --- a/tests/AllTests.php +++ b/tests/AllTests.php @@ -18,7 +18,7 @@ $phpunit7 = false; if (class_exists('\PHPUnit\Runner\Version') === true) { $version = \PHPUnit\Runner\Version::id(); - if ($version[0] === '7') { + if (version_compare($version, '7.0', '>=') === true) { $phpunit7 = true; } }