Page MenuHomePhabricator

Parsoid/PHP post-merge doc publishing task needs updating to use PHP 7.2
Closed, ResolvedPublic

Description

This error has been a common sight in post-merge messages.

phpunit-coverage-docker-publish https://integration.wikimedia.org/ci/job/phpunit-coverage-docker-publish/749/console : FAILURE in 22s

Specifically:

- This package requires php >=7.2.0 but your PHP version (7.0.33) does not satisfy that requirement

Parsoid requires PHP 7.2 but the doc publishing seems to be based on 7.0 and this is making the task unhappy. As @Krinkle put it: "Might need to fix something somewhere :)".

Event Timeline

ssastry created this task.Mar 26 2019, 8:06 PM
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptMar 26 2019, 8:06 PM
ssastry triaged this task as Normal priority.Mar 26 2019, 8:07 PM
ssastry moved this task from Backlog to Deployment on the Parsoid-PHP board.

Change 517129 had a related patch set uploaded (by Jforrester; owner: Jforrester):
[integration/config@master] jjb: Move phpunit-coverage-docker-publish from php to php72

https://gerrit.wikimedia.org/r/517129

Change 517129 merged by jenkins-bot:
[integration/config@master] jjb: Move phpunit-coverage-docker-publish from php to php72

https://gerrit.wikimedia.org/r/517129

Change 517132 had a related patch set uploaded (by Jforrester; owner: Jforrester):
[integration/config@master] jjb: Move phpunit-coverage-docker-publish from composer to composer-php72

https://gerrit.wikimedia.org/r/517132

Change 517132 merged by jenkins-bot:
[integration/config@master] jjb: Move phpunit-coverage-docker-publish from composer to composer-php72

https://gerrit.wikimedia.org/r/517132

So, good news – it now runs the test suite:

https://integration.wikimedia.org/ci/job/phpunit-coverage-docker-publish/1106/console

… bad news, it fails to push the new version to the public server for some reason?

hashar added a subscriber: hashar.Jun 21 2019, 7:34 AM

PHPUnit failed somehow:

00:02:09.049 Generating code coverage report in HTML format ... done
00:02:13.226 Build step 'Execute shell' marked build as failure

The post build script thus does not execute as intended:

00:02:13.246 [PostBuildScript] - Execution post build scripts.
00:02:13.246 [PostBuildScript] Build is not success : do not execute script

From https://doc.wikimedia.org/cover/mediawiki-services-parsoid/ : the last working coverage report was on Feb 14th 2019 18:06:51 UTC. Surely we should have noticed this task earlier. At that date there were not tests at all so the PHPUnit was working fine and the last published report is empty.

7f09b0f3d8c7ca349dc35908c61e50d82170fc66 bumped the PHP requirement to php 7.1 which thus caused the build to fail and triggered the filling of this task. Tests later got added but would never work until we finally switched the container to php 7.2.


The reason for the build failure is that PHPUnit converts warnings to exception (which is good):

phpunit.xml.dist
<phpunit
    convertWarningsToExceptions="true"
    ...
>

There is no warnings when running the testsuite:

Tests: 8725, Assertions: 9457, Skipped: 1.

But with coverage report ./vendor/bin/phpunit --coverage-clover coverage/clover.xml:

Tests: 8725, Assertions: 9457, Warnings: 8248, Skipped: 1.
                               ^^^^^^^^^^^^^^

An example:

tests/phpunit/Parsoid/Utils/UtilTest.php:        * @covers decodeWtEntities
tests/phpunit/Parsoid/Utils/UtilTest.php:               $actual = Util::decodeWtEntities( $wt );

Which leads to:

Trying to @cover or @use not existing class or interface "decodeWtEntities".

for MediaWiki core, we validated the @covers tags when running tests via a trait: tests/phpunit/MediaWikiCoversValidator.php. I don't think PHPUnit has a way to validate the tags without actually generating the coverage.

So:

  • the @covers tags have to be fixed to point to the proper class
  • some code needs to be added to validate the coverage tags when running tests.

Fun things:

https://github.com/sebastianbergmann/phpunit/issues/1760 "Do not error out on invalid @covers tag" which apparently is intentional and the @covers tags are supposed to be right in the first place. Upstream asked for it to be validated by another tool.

https://github.com/sebastianbergmann/phpunit/issues/1758 "Verifying correctness of @covers tags" got rejected:

I agree that this functionality is desirable but it does not belong in PHPUnit but rather a separate tool, for instance implemented as a sniff for PHP_CodeSniffer.

So I guess we could use https://packagist.org/packages/ockcyp/covers-validator

Jdforrester-WMF closed this task as Resolved.Jun 21 2019, 5:57 PM
Jdforrester-WMF claimed this task.

OK, the task as filed is indeed fixed. Will put in a follow-up.