From 5499e6e12a1527c2ab263af7dcb8bd58a3a8386c 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 b5bd61813c..8a12348b7d 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 bf5c6708b599f16ca7bb48bf27c7ca6060a20d59 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 934d8e2890..75d6857074 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 5d97c6e35621bb01732c918b77e5a8bcb4a6cf2c 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 75d6857074..a201f47ec3 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 8a12348b7d..04ddf53dac 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 66f4eea3ea..7e2745fd37 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 4339e24633654f74d02b3cfb51228c9026523852 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 fb816ad586..2d1560acc1 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 a201f47ec3..d5d3c3d520 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 2586535b3f31eb5c3b3159964789ede3ee90d692 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 0f937cc8b0..60fd127523 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 0e26bf8fdd62cad9988ad3e82fb18df9f6aff0b3 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 8b138e0b01..0fed0831e2 100644 --- a/tests/Core/Ruleset/RuleInclusionAbsoluteLinuxTest.php +++ b/tests/Core/Ruleset/RuleInclusionAbsoluteLinuxTest.php @@ -98,7 +98,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 f8e3255b88..72b041e7a9 100644 --- a/tests/Core/Ruleset/RuleInclusionAbsoluteWindowsTest.php +++ b/tests/Core/Ruleset/RuleInclusionAbsoluteWindowsTest.php @@ -99,7 +99,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 8a1bfd9ce5..897aaab274 100644 --- a/tests/Core/Ruleset/RuleInclusionTest.php +++ b/tests/Core/Ruleset/RuleInclusionTest.php @@ -113,7 +113,6 @@ public function tearDown() */ public function testHasSniffCodes() { - $this->assertObjectHasAttribute('sniffCodes', self::$ruleset); $this->assertCount(48, self::$ruleset->sniffCodes); }//end testHasSniffCodes() @@ -358,7 +357,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]); @@ -448,7 +446,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 09af5863412f87ae5f22a1d22ed515fe36b229da 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 897aaab274..5cbd90a6c9 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 { @@ -358,7 +359,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); @@ -449,7 +453,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 9dbfc7bc88..31e4d67b1d 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. @@ -135,8 +136,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 0421e80b627da45eed95e7b2d6325717e6458e90 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 ++++++++---- .../Autoloader/DetermineLoadedClassTest.php | 6 ++++-- tests/Core/Filters/Filter/AcceptTest.php | 12 ++++++++---- .../Ruleset/RuleInclusionAbsoluteLinuxTest.php | 12 ++++++++---- .../RuleInclusionAbsoluteWindowsTest.php | 12 ++++++++---- tests/Core/Ruleset/RuleInclusionTest.php | 18 ++++++++++++------ tests/Core/Ruleset/SetSniffPropertyTest.php | 6 ++++-- tests/Core/Sniffs/AbstractArraySniffTest.php | 8 +++++--- .../Core/Tokenizer/HeredocNowdocCloserTest.php | 6 ++++-- tests/Standards/AbstractSniffUnitTest.php | 6 ++++-- 11 files changed, 74 insertions(+), 40 deletions(-) diff --git a/src/Standards/Generic/Tests/Debug/ESLintUnitTest.php b/src/Standards/Generic/Tests/Debug/ESLintUnitTest.php index 0a079d1489..55f985c122 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 4d4f546995..aaefb687be 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 c0f38fa6f1..a27018895c 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 2c9f57cd39..c5e9a5cbfc 100644 --- a/tests/Core/Filters/Filter/AcceptTest.php +++ b/tests/Core/Filters/Filter/AcceptTest.php @@ -36,9 +36,11 @@ class AcceptTest extends TestCase /** * Initialize the test. * + * @before + * * @return void */ - public function setUp() + public function skipOnPEAR() { if ($GLOBALS['PHP_CODESNIFFER_PEAR'] === true) { // PEAR installs test and sniff files into different locations @@ -47,15 +49,17 @@ public function setUp() $this->markTestSkipped('Test cannot run from a PEAR install'); } - }//end setUp() + }//end skipOnPEAR() /** * Initialize the config and ruleset objects based on the `AcceptTest.xml` ruleset file. * + * @beforeClass + * * @return void */ - public static function setUpBeforeClass() + public static function initializeConfigAndRuleset() { if ($GLOBALS['PHP_CODESNIFFER_PEAR'] === true) { // This test will be skipped. @@ -66,7 +70,7 @@ public static function setUpBeforeClass() 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 0fed0831e2..356b0c52b2 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() { if ($GLOBALS['PHP_CODESNIFFER_PEAR'] === true) { // PEAR installs test and sniff files into different locations @@ -74,19 +76,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 72b041e7a9..3a8da7bfec 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'); @@ -73,21 +75,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 5cbd90a6c9..faea961e82 100644 --- a/tests/Core/Ruleset/RuleInclusionTest.php +++ b/tests/Core/Ruleset/RuleInclusionTest.php @@ -42,9 +42,11 @@ class RuleInclusionTest extends TestCase /** * Initialize the test. * + * @before + * * @return void */ - public function setUp() + public function skipOnPEAR() { if ($GLOBALS['PHP_CODESNIFFER_PEAR'] === true) { // PEAR installs test and sniff files into different locations @@ -53,15 +55,17 @@ public function setUp() $this->markTestSkipped('Test cannot run from a PEAR install'); } - }//end setUp() + }//end skipOnPEAR() /** * Initialize the config and ruleset objects based on the `RuleInclusionTest.xml` ruleset file. * + * @beforeClass + * * @return void */ - public static function setUpBeforeClass() + public static function initializeConfigAndRuleset() { if ($GLOBALS['PHP_CODESNIFFER_PEAR'] === true) { // This test will be skipped. @@ -92,19 +96,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/Ruleset/SetSniffPropertyTest.php b/tests/Core/Ruleset/SetSniffPropertyTest.php index 31e4d67b1d..2057e0609b 100644 --- a/tests/Core/Ruleset/SetSniffPropertyTest.php +++ b/tests/Core/Ruleset/SetSniffPropertyTest.php @@ -26,9 +26,11 @@ class SetSniffPropertyTest extends TestCase /** * Initialize the test. * + * @before + * * @return void */ - public function setUp() + public function skipOnPEAR() { if ($GLOBALS['PHP_CODESNIFFER_PEAR'] === true) { // PEAR installs test and sniff files into different locations @@ -37,7 +39,7 @@ public function setUp() $this->markTestSkipped('Test cannot run from a PEAR install'); } - }//end setUp() + }//end skipOnPEAR() /** diff --git a/tests/Core/Sniffs/AbstractArraySniffTest.php b/tests/Core/Sniffs/AbstractArraySniffTest.php index 20c28d60b2..1648399f4c 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 6e32240700..e7c9d4a95e 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 c050a0c2af..88e3915860 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 01b17aea03bbd106badfc27037b3f679eba3e2fc 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. * 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` workflow: removing work-arounds which were in place related to running PHPUnit 7.x on PHP 8.x. * `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. Related to 3395 --- .github/workflows/test.yml | 19 +------------------ .gitignore | 1 + composer.json | 2 +- phpunit.xml.dist | 12 +++++++++++- tests/AllTests.php | 2 +- 5 files changed, 15 insertions(+), 21 deletions(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 53bd6bf782..63fa0cea8e 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -114,37 +114,20 @@ jobs: # Install dependencies and handle caching in one go. # @link https://github.com/marketplace/actions/install-composer-dependencies - - 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-reqs - 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 99658952b8..53f6f801e4 100644 --- a/.gitignore +++ b/.gitignore @@ -4,3 +4,4 @@ .idea/* /vendor/ composer.lock +.phpunit.result.cache diff --git a/composer.json b/composer.json index 37f41a0b80..962edee236 100644 --- a/composer.json +++ b/composer.json @@ -32,7 +32,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" }, "bin": [ "bin/phpcs", 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 9d099c1e3d..53535ed108 100644 --- a/tests/AllTests.php +++ b/tests/AllTests.php @@ -24,7 +24,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; } }