Pre-push git hooks false positives

[patpilus]

Q	A
Version	1.2.0; 0.22.0;
Last working version	0.19.1
Bug?	yes
New feature?	no
Question?	no
Documentation?	no

My configuration Configuration contains some custom grumphp tasks to align with plugin-based project structure, but it worked fine up to 0.19.1 version (inclusive).

# grumphp.yml
parameters:
    plugins_path: "custom/plugins"
    code_coverage_relative_path: "build/phpunit"

grumphp:
    process_timeout: 500
    ascii:
        succeeded:
            - './bin/resources/grumphp/successful_ascii.txt'
            - './bin/resources/grumphp/successful_ascii2.txt'
        failed:
            - './bin/resources/grumphp/failed_ascii.txt'
            - './bin/resources/grumphp/failed_ascii2.txt'
    tasks:
        clover_coverage_combined:
            input_coverages_extension: cov
            clover_file: build/phpunit-merged/coverage.xml
            html_output_path: build/phpunit-merged
            level: 70
            metadata:
                priority: -1
        composer: ~
        jsonlint: ~
        phpcpd:
            exclude: ['vendor', 'var', 'custom/plugins', 'src/Migrations']
        phpcs:
            whitelist_patterns:
                - /^src\/(.*)/
            ignore_patterns:
                - custom/plugins/*/vendor
        phpcsfixer:
            allow_risky: ~
            cache_file: ~
            config: ~
            rules: ['no_unused_imports']
            using_cache: ~
            config_contains_finder: true
            verbose: true
            diff: false
            triggered_by: ['php']
        phplint: ~
        phpparser:
            ignore_patterns:
                - 'bundles.php'
                - 'index.php'
                - 'vendor'
                - 'var'
            visitors:
                declare_strict_types: ~
                forbidden_function_calls:
                    blacklist:
                        - 'var_dump'
                        - 'print_r'
                        - 'echo'
                        - 'dump'
                        - 'dd'
                no_exit_statements: ~
        phpstan:
            level: 7
            configuration: phpstan.neon
            ignore_patterns: ['custom/plugins']
        phpstan_custom_plugins: ~
        phpunit:
            testsuite: unit
            always_execute: true
        phpunit_custom_plugins:
        securitychecker: ~
        yamllint:
            parse_custom_tags: true

    testsuites:
        pre-commit:
            tasks:
                - phpcs
                - phpcsfixer
                - phplint
                - phpparser
                - phpstan
                - phpstan_custom_plugins
                - jsonlint
                - yamllint
        full:
            tasks:
                - phpunit
                - phpunit_custom_plugins
                - composer
                - phpcs
                - phpcsfixer
                - phpcpd
                - phplint
                - phpparser
                - phpstan
                - phpstan_custom_plugins
                - jsonlint
                - securitychecker
                - yamllint

services:
    task.phpunitcustomplugins:
        class: MyProject\Tests\Utility\GrumPhpTasks\PhpunitCustomPlugins
        arguments:
            - '@process_builder'
            - '@formatter.raw_process'
        tags:
            - {name: grumphp.task, task: phpunit_custom_plugins}

    task.phpstancustomplugins:
        class: MyProject\Tests\Utility\GrumPhpTasks\PhpstanCustomPlugins
        arguments:
            - '@process_builder'
            - '@formatter.raw_process'
        tags:
            - {name: grumphp.task, task: phpstan_custom_plugins}

    task.combinedclovercoverage:
        class: MyProject\Tests\Utility\GrumPhpTasks\CombinedCloverCoverage
        arguments:
            - '@MyProject\Tests\Utility\Service\CodeCoverageCollector'
            - '@filesystem'
        tags:
            - {name: grumphp.task, task: clover_coverage_combined}

    MyProject\Tests\Utility\Service\CodeCoveragePathsSeeker:
        arguments:
            - '%plugins_path%'
            - '%code_coverage_relative_path%'

    MyProject\Tests\Utility\Service\CodeCoverageCollector:
        arguments:
            - '@MyProject\Tests\Utility\Service\CodeCoveragePathsSeeker'

Your actions:

1. Add pre-push git hook. Hook script content looks like this:

   docker-compose up -d app
   docker-compose exec -T app vendor/bin/grumphp run --testsuite=full

2. Push to git repository. phpunit task is marked as done and the other tasks are skipped. git push action is made without the guarantee that all grumphp tasks are passing.

Result: Check can be false positive (only one task out of 13 was actually launched):

[veewee]

Hello @patpilus

Thanks for reporting!

Can you try to narrow the problem down to a smaller config? Besides that, what is the app container refrering to?

If tasks are being skipped, this is mostly because the files provided to the tool result in an empty list after filtering the files that can be used by the task. For example:

https://github.com/phpro/grumphp/blob/7484913326228044c330e9ddadc11a535b7b9867/src/Task/PhpCpd.php#L55-L59

To debug, I suggest to disable parallel execution:

grumphp:
    parallel:
        enabled: false

Next, you can run grumphp with the -vvv flag, meaning it will list you all detected file locations and a list of files that are automatically being detected from git. This information can help you track back to what is failing exactly.

[patpilus]

Hey @veewee! Thanks for your response and apologise for the delayed one from my side. Finally I have some time to debug this issue.

You're seems to be right with the reason of skipping the tasks - the list of files that are automatically being detected from git is empty (grumphp run with -vvv flag).

I've reduced the config to almost bare minimum, but the problem still exists (checked on newest 1.2.0 grumphp version). Now the grumphp.yaml file looks like this:

grumphp:
    parallel:
        enabled: false
    tasks:
        composer: ~
        phpcsfixer: ~
        phpunit:
            testsuite: unit
            always_execute: true

    testsuites:
        pre-commit:
            tasks:
                - composer
                - phpcsfixer
        full:
            tasks:
                - phpunit
                - composer
                - phpcsfixer

The app container is based on official php:7.4-apache-buster Docker image with additionally xdebug enabled.

Steps to reproduce are:

1. create new branch, eg. git checkout -b test-branch
2. modify at least one file and add it: git add .
3. git commit -m 'whatever'
4. git push origin test-branch

In the above scenario pre-commit testsuite is launched correctly and the list of files is given, however for the full testsuite (which is being launched pre-push), the list is empty and all tasks except phpunit are skipped:

What is weird, when executing git push once again it works correct (the list of files is given and all tasks are being launched).

Pre-commit git hook looks exactly like below:

#!/usr/bin/env sh

docker-compose -f docker-compose-test.yml up -d app
docker-compose -f docker-compose-test.yml exec -T app vendor/bin/grumphp run --testsuite=pre-commit -vvv

Pre-push git hook:

#!/usr/bin/env sh

docker-compose -f docker-compose-test.yml up -d app
docker-compose -f docker-compose-test.yml exec -T app vendor/bin/grumphp run --testsuite=full -vvv

Do you have any idea what may cause the list of detected file locations to be empty in such case?

[veewee]

I really don't have a clue about this to be honest. You could try piping in git ls-files manually from within your hook? (if docker-compose accepts that kind of input) That way the files are provided from the host.

[patpilus]

@veewee I've added git ls-files to the hook and surprisingly it always returns a valid list of files tracked by git. It never returned empty list... 🤔

As a side effect it was possible for me to create a workaround that will prevent from running grumphp on empty fileset. I added git ls-files --error-unmatch > /dev/null before running grumphp testsuite. It will throw error if the list is empty (redundant output is silenced).

Pre-push git hook now looks like below:

#!/usr/bin/env sh

set -e

docker-compose -f docker-compose-test.yml up -d app

docker-compose -f docker-compose-test.yml exec -T app git ls-files --error-unmatch > /dev/null \
    && docker-compose -f docker-compose-test.yml exec -T app vendor/bin/grumphp run --testsuite=full

For the reason still unknown, removing git ls-files from the hook causes the issue to re-appear... 🤔

I can try to create public skeleton project on github with this issue reproducible to help with debugging. What do you think?

[veewee]

That would be nice. We could use that skeleton as a base to test some things. I am wondering : does it behave in a similar way without the docker setup?

[patpilus]

Alright, I'll let you know when it's done.

I don't even have grumphp installed locally so I don't know to be honest, but I can try to verify it in my spare time.

[veewee]

Hello @patpilus ,

did you make any progress in creating the skeleton?

Closing this one for now. If it is still an issue, feel free to reopen.