slevomat / coding-standard

Slevomat Coding Standard for PHP_CodeSniffer provides many useful sniffs
MIT License
1.39k stars 171 forks source link

PHPUnit tweaks #1628

Closed jrfnl closed 1 year ago

jrfnl commented 1 year ago

.gitignore: ignore PHPUnit cache file

PHPUnit config: display errors/warnings/notices etc

The config as it was, was hiding 6 Undefined array key/Trying to access array offset on null notices.

As any notice caused by a sniff will stop a PHPCS scan of a file dead, these should be discovered when running the tests and then fixed. Better yet: the tests should fail on them. This change in the configuration makes it so.

Note: this does mean you now have a bug to fix.

Also see: squizlabs/PHP_CodeSniffer#3844

PHPUnit config: upgrade to 10.3 schema

This upgrades the XML configuration to one accepted by PHPUnit 10.3 and higher, which is in line with the PHPUnit requirements in the composer.json file.

Note: the configuration will not validate for PHPUnit < 10.3 and will throw warnings, but as code coverage in CI is only run with PHPUnit 10 and the warnings are about that part of the configuration, this is nothing to worry about. Moreover, on PHPUnit 8.x/9.x, those warnings won't fail the build and the tests will still be run.

GH Actions: turn error reporting on

The default ini file used by the SetupPHP action is the production one, which turns error_reporting/display off.

For the purposes of CI, I'd recommend running with error_reporting=-1 and display_errors=On to ensure all PHP notices are shown.

Also see: shivammathur/setup-php#469

Composer: no need to allow PHPUnit 7.x

PHPUnit 8.x has a minimum supported version of PHP 7.2, so there is no need to include PHPUnit 7.x in the allowed versions.

kukulich commented 1 year ago

Note: this does mean you now have a bug to fix.

@jrfnl I'm sorry I'm not able to reproduce any bug.

jrfnl commented 1 year ago

@kukulich I'll have a look later again later tonight and provide additional info.

I also noticed (I should have known) that the PHPUnit config is causing build failures as PHPUnit 10.x no longer makes a distinction between PHP warnings and PHPUnit warnings and will fail the build on both.

I'll make some tweaks to get that sorted later too.

jrfnl commented 1 year ago

@kukulich I've added an extra commit to upgrade the PHPUnit configuration to be valid against the PHPUnit 10.3+ schema and another commit to fix another CI issue (wrong error_reporting settings) - I've updated the PR description to match.

Running the tests against PHP 8.2.8 with the PHPUnit 10.3.5 PHAR on Windows 10 shows me:

1 test triggered 6 PHP warnings:

1) path\to\SlevomatCodingStandard\Sniffs\Commenting\InlineDocCommentDeclarationSniff.php:81
Undefined array key ""

Triggered by:

* SlevomatCodingStandard\Helpers\AttributeHelperTest::testIsValidAttributeAttributeAtEndOfFile
  path\to\tests\Helpers\AttributeHelperTest.php:38

2) path\to\SlevomatCodingStandard\Sniffs\Commenting\InlineDocCommentDeclarationSniff.php:81
Trying to access array offset on value of type null

Triggered by:

* SlevomatCodingStandard\Helpers\AttributeHelperTest::testIsValidAttributeAttributeAtEndOfFile
  path\to\tests\Helpers\AttributeHelperTest.php:38

3) path\to\SlevomatCodingStandard\Sniffs\Commenting\InlineDocCommentDeclarationSniff.php:92
Undefined array key ""

Triggered by:

* SlevomatCodingStandard\Helpers\AttributeHelperTest::testIsValidAttributeAttributeAtEndOfFile
  path\to\tests\Helpers\AttributeHelperTest.php:38

4) path\to\SlevomatCodingStandard\Sniffs\Commenting\InlineDocCommentDeclarationSniff.php:92
Trying to access array offset on value of type null

Triggered by:

* SlevomatCodingStandard\Helpers\AttributeHelperTest::testIsValidAttributeAttributeAtEndOfFile
  path\to\tests\Helpers\AttributeHelperTest.php:38

5) path\to\SlevomatCodingStandard\Sniffs\Commenting\InlineDocCommentDeclarationSniff.php:99
Undefined array key ""

Triggered by:

* SlevomatCodingStandard\Helpers\AttributeHelperTest::testIsValidAttributeAttributeAtEndOfFile
  path\to\tests\Helpers\AttributeHelperTest.php:38

6) path\to\SlevomatCodingStandard\Sniffs\Commenting\InlineDocCommentDeclarationSniff.php:99
Trying to access array offset on value of type null

Triggered by:

* SlevomatCodingStandard\Helpers\AttributeHelperTest::testIsValidAttributeAttributeAtEndOfFile
  path\to\tests\Helpers\AttributeHelperTest.php:38

And when I then change the stopOnDefect="true" attribute (pre-existing setting in the PHPUnit config file) to stopOnDefect="false" (or just remove it as false is the default), I get the following:

There were 19 failures:

1) SlevomatCodingStandard\Sniffs\Classes\ClassStructureSniffTest::testNoErrors
No errors expected, but 1 errors found.
Failed asserting that an array is empty.

path\to\SlevomatCodingStandard\Sniffs\TestCase.php:77
path\to\tests\Sniffs\Classes\ClassStructureSniffTest.php:31

2) SlevomatCodingStandard\Sniffs\Classes\ClassStructureSniffTest::testErrors
Failed asserting that 27 is identical to 28.

path\to\tests\Sniffs\Classes\ClassStructureSniffTest.php:38

3) SlevomatCodingStandard\Sniffs\Classes\RequireMultiLineMethodSignatureSniffTest::testErrors
Failed asserting that 1 is identical to 4.

path\to\tests\Sniffs\Classes\RequireMultiLineMethodSignatureSniffTest.php:45

4) SlevomatCodingStandard\Sniffs\Classes\RequireSingleLineMethodSignatureSniffTest::testNoErrors
No errors expected, but 6 errors found.
Failed asserting that an array is empty.

path\to\SlevomatCodingStandard\Sniffs\TestCase.php:77
path\to\tests\Sniffs\Classes\RequireSingleLineMethodSignatureSniffTest.php:29

5) SlevomatCodingStandard\Sniffs\Commenting\DocCommentSpacingSniffTest::testDefaultSettingsNoErrors
No errors expected, but 1 errors found.
Failed asserting that an array is empty.

path\to\SlevomatCodingStandard\Sniffs\TestCase.php:77
path\to\tests\Sniffs\Commenting\DocCommentSpacingSniffTest.php:48

6) SlevomatCodingStandard\Sniffs\Commenting\DocCommentSpacingSniffTest::testDefaultSettingsErrors
Failed asserting that 17 is identical to 12.

path\to\tests\Sniffs\Commenting\DocCommentSpacingSniffTest.php:55

7) SlevomatCodingStandard\Sniffs\ControlStructures\BlockControlStructureSpacingSniffTest::testDefaultSettingsErrors
Failed asserting that 1 is identical to 26.

path\to\tests\Sniffs\ControlStructures\BlockControlStructureSpacingSniffTest.php:20

8) SlevomatCodingStandard\Sniffs\ControlStructures\EarlyExitSniffTest::testErrors
Failed asserting that 42 is identical to 74.

path\to\tests\Sniffs\ControlStructures\EarlyExitSniffTest.php:20

9) SlevomatCodingStandard\Sniffs\ControlStructures\JumpStatementsSpacingSniffTest::testDefaultSettingsErrors
Failed asserting that 12 is identical to 29.

path\to\tests\Sniffs\ControlStructures\JumpStatementsSpacingSniffTest.php:20

10) SlevomatCodingStandard\Sniffs\ControlStructures\JumpStatementsSpacingSniffTest::testAtTheBeginnningOfFile
Failed asserting that 0 is identical to 1.

path\to\tests\Sniffs\ControlStructures\JumpStatementsSpacingSniffTest.php:299

11) SlevomatCodingStandard\Sniffs\ControlStructures\RequireMultiLineConditionSniffTest::testNoErrors
No errors expected, but 2 errors found.
Failed asserting that an array is empty.

path\to\SlevomatCodingStandard\Sniffs\TestCase.php:77
path\to\tests\Sniffs\ControlStructures\RequireMultiLineConditionSniffTest.php:13

12) SlevomatCodingStandard\Sniffs\ControlStructures\RequireMultiLineConditionSniffTest::testErrors
Failed asserting that 0 is identical to 5.

path\to\tests\Sniffs\ControlStructures\RequireMultiLineConditionSniffTest.php:20

13) SlevomatCodingStandard\Sniffs\ControlStructures\UselessIfConditionWithReturnSniffTest::testErrors
Failed asserting that two strings are equal.
--- Expected
+++ Actual
@@ @@
 };\n
 \n
 function ($bool) {\n
-       if ($bool) {\n
-               return true;\n
-       } else {\n
-               return false;\n
-       }\n
+       return $bool;\n
 };\n
 \n
 function ($e, $number) {\n
@@ @@
 \n
 function () {\n
        if (doSomething()) {\n
-               if (doSomethingElse()) {\n
-                       return true;\n
-               }\n
-\n
-               return false;\n
+               return doSomethingElse();\n
        }\n
 };\n
 \n

path\to\SlevomatCodingStandard\Sniffs\TestCase.php:155
path\to\tests\Sniffs\ControlStructures\UselessIfConditionWithReturnSniffTest.php:35

14) SlevomatCodingStandard\Sniffs\ControlStructures\UselessTernaryOperatorSniffTest::testNoErrors
No errors expected, but 1 errors found.
Failed asserting that an array is empty.

path\to\SlevomatCodingStandard\Sniffs\TestCase.php:77
path\to\tests\Sniffs\ControlStructures\UselessTernaryOperatorSniffTest.php:13

15) SlevomatCodingStandard\Sniffs\ControlStructures\UselessTernaryOperatorSniffTest::testErrors
Failed asserting that two strings are equal.
--- Expected
+++ Actual
@@ @@
 };\n
 \n
 function ($bool) {\n
-       $b = $bool ? true : false;\n
+       $b = $bool;\n
 };\n
 \n
 function ($bool) {\n
-       $c = ($bool ? false : true);\n
+       $c = (!$bool);\n
 };\n
 \n
 function ($array) {\n

path\to\SlevomatCodingStandard\Sniffs\TestCase.php:155
path\to\tests\Sniffs\ControlStructures\UselessTernaryOperatorSniffTest.php:36

16) SlevomatCodingStandard\Sniffs\Functions\RequireMultiLineCallSniffTest::testErrors
Failed asserting that 8 is identical to 15.

path\to\tests\Sniffs\Functions\RequireMultiLineCallSniffTest.php:20

17) SlevomatCodingStandard\Sniffs\Namespaces\ReferenceUsedNamesOnlySniffTest::testWithCollision
Expected error on line 7, but none found.
Failed asserting that false is true.

path\to\SlevomatCodingStandard\Sniffs\TestCase.php:89
path\to\tests\Sniffs\Namespaces\ReferenceUsedNamesOnlySniffTest.php:1192

18) SlevomatCodingStandard\Sniffs\Namespaces\UnusedUsesSniffTest::testFixableUnusedUses
Failed asserting that two strings are equal.
--- Expected
+++ Actual
@@ @@
 use NewNamespace\ObjectPrototype as NewObject;\n
 use R\S;\n
 use T;\n
+use Test_Invalid;\n
 use function LoremIpsum\UsedFunction;\n
 use const LoremIpsum\USED_CONSTANT;\n
 use Lorem\FirstInterface;\n

path\to\SlevomatCodingStandard\Sniffs\TestCase.php:155
path\to\tests\Sniffs\Namespaces\UnusedUsesSniffTest.php:272

19) SlevomatCodingStandard\Sniffs\Operators\DisallowIncrementAndDecrementOperatorsSniffTest::testErrors
Failed asserting that 2 is identical to 6.

path\to\tests\Sniffs\Operators\DisallowIncrementAndDecrementOperatorsSniffTest.php:20

--

1 test triggered 6 PHP warnings:

1) path\to\SlevomatCodingStandard\Sniffs\Commenting\InlineDocCommentDeclarationSniff.php:81
Undefined array key ""

Triggered by:

* SlevomatCodingStandard\Helpers\AttributeHelperTest::testIsValidAttributeAttributeAtEndOfFile
  path\to\tests\Helpers\AttributeHelperTest.php:38

2) path\to\SlevomatCodingStandard\Sniffs\Commenting\InlineDocCommentDeclarationSniff.php:81
Trying to access array offset on value of type null

Triggered by:

* SlevomatCodingStandard\Helpers\AttributeHelperTest::testIsValidAttributeAttributeAtEndOfFile
  path\to\tests\Helpers\AttributeHelperTest.php:38

3) path\to\SlevomatCodingStandard\Sniffs\Commenting\InlineDocCommentDeclarationSniff.php:92
Undefined array key ""

Triggered by:

* SlevomatCodingStandard\Helpers\AttributeHelperTest::testIsValidAttributeAttributeAtEndOfFile
  path\to\tests\Helpers\AttributeHelperTest.php:38

4) path\to\SlevomatCodingStandard\Sniffs\Commenting\InlineDocCommentDeclarationSniff.php:92
Trying to access array offset on value of type null

Triggered by:

* SlevomatCodingStandard\Helpers\AttributeHelperTest::testIsValidAttributeAttributeAtEndOfFile
  path\to\tests\Helpers\AttributeHelperTest.php:38

5) path\to\SlevomatCodingStandard\Sniffs\Commenting\InlineDocCommentDeclarationSniff.php:99
Undefined array key ""

Triggered by:

* SlevomatCodingStandard\Helpers\AttributeHelperTest::testIsValidAttributeAttributeAtEndOfFile
  path\to\tests\Helpers\AttributeHelperTest.php:38

6) path\to\SlevomatCodingStandard\Sniffs\Commenting\InlineDocCommentDeclarationSniff.php:99
Trying to access array offset on value of type null

Triggered by:

* SlevomatCodingStandard\Helpers\AttributeHelperTest::testIsValidAttributeAttributeAtEndOfFile
  path\to\tests\Helpers\AttributeHelperTest.php:38

FAILURES!
Tests: 1382, Assertions: 7560, Failures: 19, Warnings: 6.

I get the same results whether I run the tests via a local PHPUnit PHAR file, via the bin/phing tests-without-code-coverage command or directly calling bin/phpunit --no-coverage.

I made sure to run composer update prior to running the tests just to be sure that I was using the latest versions of all dependencies.

Not sure why you are not seeing the issues. Or why I am when I shouldn't ?

I'm also mystified why these issues aren't showing in CI, not even with the additional commit turning error_reporting on. I wonder whether there is something in PHING which is interfering or changing the ini settings back, though if that were the case, I would expect the test run via phing locally to pass (while it failed) ?

On that note, might it be an idea to convert the PHING checks to Composer scripts ?

kukulich commented 1 year ago

Thanks. I will merge the PR now and try to fix the warnings later.