squizlabs / PHP_CodeSniffer

PHP_CodeSniffer tokenizes PHP files and detects violations of a defined set of coding standards.
BSD 3-Clause "New" or "Revised" License
10.67k stars 1.48k forks source link

[2.9.1] No Progress output: differences in local vs travis testing??? #1797

Closed photodude closed 6 years ago

photodude commented 6 years ago

I'm trying to run a test of our coding standard on Travis CI. our ruleset specifies core sniffs and adds some custom sniffs. We also have a secondary ruleset that lists some excluded folders and some rules that are not yet implemented in some projects. Running locally I get a long list of errors, but on Travis with the secondary ruleset it seems like nothing is getting checked. We get no progress in the checks.

I know locally with the secondary rulesets we get errors and things are working as I have run the autofixers and opened PRs with those corrections.

Using PHPCS 2.9.1 and PHP 5.6

gsherwood commented 6 years ago

I've got no idea, but the things I'd check are:

photodude commented 6 years ago

Got a clue, with -v the following was listed after the sniffs were registered. Creating file list... DONE (0 files in queue)

So, for some odd reason I'm losing the directory list with the secondary ruleset on travis but not when I'm running with the primary ruleset.

gsherwood commented 6 years ago

Try -vv and see how the ruleset is being processed. This will tell you what exclude rules are being added. Maybe that will help.

photodude commented 6 years ago

$TRAVIS_BUILD_DIR is what I expect, /home/travis/build/photodude/joomla-cms the same as I get with . *note: this travis directory has issues with our exclude `build/rule. see explanation below** I've got the -vv report still gettingCreating file list... DONE (0 files in queue)` Nothing is unusual in the directories excluded nor the excluded rules. https://travis-ci.org/photodude/joomla-cms/builds/317875966#L1480

I'll rerun with just the primary rule set and -v then -vv to show the difference.

photodude commented 6 years ago

So with the primary ruleset I get the following expected result

Registering sniffs in the Joomla standard... DONE (62 sniffs registered)
Creating file list... DONE (6092 files in queue)
Changing into directory /home/travis/build/photodude/joomla-cms
Processing
Joomla ruleset Joomla-CMS ruleset
sniffs 62 62
file list 6092 0
directory /home/travis/build/photodude/joomla-cms /home/travis/build/photodude/joomla-cms

I'm going to try renaming the secondary ruleset to something that does not have a hyphen in it; on the off chance that the ruleset naming is partly the issue. *note: this travis directory has issues with our exclude `build/` rule. see explanation below**

photodude commented 6 years ago

Renamed the secondary ruleset to remove the hyphen and still got Creating file list... DONE (0 files in queue)

https://travis-ci.org/photodude/joomla-cms/builds/317880581

gsherwood commented 6 years ago

Sorry, I really have no idea why it would be different when running in the Travis environment.

photodude commented 6 years ago

The only thing I can think of that is different between my local set up and the Travis set up; is where the secondary ruleset is. On my local the primary and secondary ruleset are in standards folder of the PHPCS install. On Travis both the primary and secondary rulesets are listed in the --config-set installed_paths on the CMD.

photodude commented 6 years ago

I reset my local so I had to use --config-set installed_paths that didn't seem to make any difference. My local is still running correctly. I am doing a specified directory in my local, I'll try seeing if I can specify some directory on Travis and if that makes a difference.

photodude commented 6 years ago

@gsherwood Is there any way to get PHPCS to report how it's building the file list for the run? As best I can tell, somehow with the secondary ruleset the directory parsing for building the file list is acting up.

gsherwood commented 6 years ago

Is there any way to get PHPCS to report how it's building the file list for the run?

No. You'd have to put debug code in to see what's happening, which is going to be pretty hard if you can only get it to fail with travis.

jrfnl commented 6 years ago

The issue sounds similar to #1391 - could there be an exclude pattern which gets triggered ?

photodude commented 6 years ago

@jrfnl the secondary ruleset is all exclude patterns. It's only purpose is to modify the standard to mute errors where past projects were not enforcing the standard. the exclude patterns used come in three types

  1. exclude rules
  2. exclude files and folders
  3. exclude some files or folders within select rules.

both the custom ruleset and the secondard ruleset works locally, but on Travis we get zero files in the queue with the secondary ruleset.

jrfnl commented 6 years ago

@photodude I suspect that one of the exclude patterns for files/folders is being triggered on Travis in that case. You could try to make the exclude patterns more specific - they are regexes after all -.

photodude commented 6 years ago

This is what I have so far, I'm not sure it can be simpler. Note: the */vendor/* and */fof/* rules were added due to some odd things on my local (win10 box).


    <!-- Exclude folders not containing production code -->
    <exclude-pattern type="relative">build/*</exclude-pattern>
    <exclude-pattern type="relative">docs/*</exclude-pattern>
    <exclude-pattern type="relative">cache/*</exclude-pattern>
    <exclude-pattern type="relative">tmp/*</exclude-pattern>
    <exclude-pattern type="relative">logs/*</exclude-pattern>

    <!-- Exclude 3rd party libraries and Framework code. -->
    <exclude-pattern type="relative">libraries/compat/password/*</exclude-pattern>
    <exclude-pattern type="relative">libraries/fof/*</exclude-pattern>
    <exclude-pattern type="relative">libraries/idna_convert/*</exclude-pattern>
    <exclude-pattern type="relative">libraries/php-encryption/*</exclude-pattern>
    <exclude-pattern type="relative">libraries/phputf8/*</exclude-pattern>
    <exclude-pattern type="relative">libraries/simplepie/*</exclude-pattern>
    <exclude-pattern type="relative">libraries/phpass/*</exclude-pattern>
    <exclude-pattern type="relative">libraries/vendor/*</exclude-pattern>
    <exclude-pattern type="relative">libraries/joomla/*</exclude-pattern>
    <exclude-pattern>*/vendor/*</exclude-pattern>
    <exclude-pattern>*/fof/*</exclude-pattern>

    <!-- Exclude the restore_finalisation until we can deal with nested class definitions -->
    <exclude-pattern type="relative">administrator/components/com_joomlaupdate/restore_finalisation.php</exclude-pattern>
    <exclude-pattern type="relative">administrator/components/com_joomlaupdate/restore.php</exclude-pattern>
    <exclude-pattern type="relative">configuration.php</exclude-pattern>
    <exclude-pattern type="relative">installation/template/index.php</exclude-pattern>
    <exclude-pattern type="relative">plugins/captcha/recaptcha/recaptchalib.php</exclude-pattern>

    <!-- Exclude some test related files that don't actually include PHP code -->
    <exclude-pattern type="relative">tests/unit/suites/libraries/joomla/model/stubs/barbaz.php</exclude-pattern>
    <exclude-pattern type="relative">tests/unit/suites/libraries/joomla/view/layouts1/fringe/division.php</exclude-pattern>
    <exclude-pattern type="relative">tests/unit/suites/libraries/joomla/view/layouts1/olivia.php</exclude-pattern>
    <exclude-pattern type="relative">tests/unit/suites/libraries/joomla/view/layouts1/peter.php</exclude-pattern>
    <exclude-pattern type="relative">tests/unit/suites/libraries/joomla/view/layouts2/fauxlivia.php</exclude-pattern>
    <exclude-pattern type="relative">tests/unit/suites/libraries/joomla/view/layouts2/olivia.php</exclude-pattern>
    <exclude-pattern type="relative">tests/unit/suites/libraries/legacy/controller/stubs/component1/controller.json.php</exclude-pattern>
    <exclude-pattern type="relative">tests/unit/suites/libraries/legacy/controller/stubs/component2/controller.php</exclude-pattern>

    <!-- Exclude the RoboFile.php -->
    <exclude-pattern type="relative">RoboFile.php</exclude-pattern>
jrfnl commented 6 years ago

@photodude Could you try to do a run with the exclude pattern for build commented out ? That just sounds like the most suspect one.

If the Travis build runs correctly with that pattern commented out, the next thing would be to try to make that pattern more specific to prevent the issue.

photodude commented 6 years ago

Bingo, it's the <exclude-pattern type="relative">build/*</exclude-pattern> rule that was preventing all testing on Travis. Now I need to figure out how to properly exclude that folder without messing up Travis. Things look good there now

Of course I've been told that we will not be going back to using Travis and are working to implement testing with Docker and Drone. Hopfully we don't see this issue in the docker/drone setup.

Thanks @jrfnl for your assistance in solving the issue of why Travis was different from the local.

jrfnl commented 6 years ago

@photodude Glad you're well on your way to fixing it now. Good start of the new year I'd say :grinning:

photodude commented 6 years ago

The following change seems to fully resolve the issue with build being excluded in the ruleset but part of both the travis "build" directory name (see path listed above ^ ) and a folder in the project.

<exclude-pattern type="relative">^build/*</exclude-pattern>

This change seems to works with and without the type="relative" so that may or may not be necessary.

Now I just need to fix the indenting issue we have with control structures that are inside of closures (see https://gitter.im/squizlabs/PHP_CodeSniffer?at=5a48504668d092bb6208e1d6 ) Fixed the indent issue inside closures with using if ($phpcsFile->hasCondition($stackPtr, T_CLOSURE) === true) {