The php72 test-report job is always successful

[gmponos]

Is this correct?

https://github.com/doctrine/coding-standard/pull/205/checks?check_run_id=788441821

As you can see on this job there are differences but the job is success.

Is this addressed here? https://github.com/doctrine/coding-standard/issues

[greg0ire]

It does look weird… can you try doing an obvious mistake in one of your PRs and see if it is caught by that job?

[gmponos]

Not sure exactly what you call "obvious".. check this PR where I'm adding a new sniff and many files of this repo do not comply with it.

Right from the first job you'll notice the PHP 72 that is successful.

[greg0ire]

Link for the lazy: https://github.com/doctrine/coding-standard/pull/206/checks?check_run_id=791333879

I think the issue would have to do with this line:

coding-standard/Makefile

Line 9 in 87308f5

@vendor/bin/phpcs `find tests/input/* | sort` --report=summary --report-file=phpcs.log; diff -u tests/expected_report.txt phpcs.log; if [ $$? -ne 0 ] && [ $(PHP_74_OR_NEWER) -eq 1 ]; then git apply -R tests/php-compatibility.patch; exit 1; fi

I think it is equivalent to

vendor/bin/phpcs `find tests/input/* | sort` --report=summary --report-file=phpcs.log
diff -u tests/expected_report.txt phpcs.log
if [ $$? -ne 0 ] && [ $(PHP_74_OR_NEWER) -eq 1 ]
then
    git apply -R tests/php-compatibility.patch
    exit 1
fi

This does not make much sense… maybe it should really be

vendor/bin/phpcs `find tests/input/* | sort` --report=summary --report-file=phpcs.log
diff -u tests/expected_report.txt phpcs.log
if [ $(PHP_74_OR_NEWER) -eq 1 ]
then
    git apply -R tests/php-compatibility.patch
fi


if [ $$? -ne 0 ]
then
    exit 1
fi

It was introduced as such in f9d89e0 as part of #144

@lcobucci can you please advise?

[lcobucci]

@greg0ire oh my, that's indeed a terrible mistake.

It seems to be failing since #148.

I believe it should have been something like this:

if [ $$? -ne 0 ]; then
    if  [ $(PHP_74_OR_NEWER) -eq 1 ]; then
        git apply -R tests/php-compatibility.patch;
    fi

    exit 1;
fi

Fixed in #210

[lcobucci]

@gmponos thanks for reporting this issue 👍