Page MenuHomePhabricator

Re-start Wikibase test coverage reporting
Open, Needs TriagePublic

Description

Following on from T287913: Enable extension-coverage on most WMF deployed and all MW bundled extensions

Wikibase shows on https://doc.wikimedia.org/cover-extensions/ at 19% coverage. But if you click in to https://doc.wikimedia.org/cover-extensions/Wikibase/ you get Empty directory!

Seems this might be worth fixing

Event Timeline

Restricted Application added a subscriber: Aklapper. · View Herald Transcript

Related to T242617, I believe – it looks like the extension-coverage job for Wikibase was removed from CI. (I don’t remember the details.)

I wonder if it's worth deleting the Wikibase directory so we don't get that "false positive" showing in the coverage list. Whenever it gets re-enabled, it will get automaticall recreated etc

Related to T242617, I believe – it looks like the extension-coverage job for Wikibase was removed from CI. (I don’t remember the details.)

From T242444: WikibaseQualityConstraints coverage reports broken

Wikibase fails T242617 and got removed by https://gerrit.wikimedia.org/r/583671

Mentioned in SAL (#wikimedia-releng) [2021-08-10T16:06:42Z] <James_F> Ran sudo -u doc-uploader rm -rf /srv/doc/cover-extensions/Wikibase on doc1001 for T288396

I wonder if it's worth deleting the Wikibase directory so we don't get that "false positive" showing in the coverage list. Whenever it gets re-enabled, it will get automaticall recreated etc

Done.

Jdforrester-WMF renamed this task from Fix Wikibase test coverage to Re-start Wikibase test coverage reporting.Aug 10 2021, 4:07 PM

The way extension coverage works was not obvious to me and took a bit of time to figure out.

  • The Jenkins job mwext-phpunit-coverage-docker-publish runs quibble with --commands=mwext-phpunit-coverage
  • That is not an actual quibble command. Quibble interprets unknown commands as shell commands and runs them.
  • The relevant Dockerfile installs mwext-phpunit-coverage.sh from the integration/config repo as /usr/local/bin/mwext-phpunit-coverage
  • This shell script calls phpunit-suite-edit, which is a python script that edits core's suite.xml, removing the original coverage filter, and instead covering src/, includes/ and maintenance/ of the extension directory.
  • Then it runs phpunit with the positional argument $ext/tests/phpunit. This directory doesn't exist on Wikibase. Wikibase has an onUnitTestsList hook to declare the directories that actually have tests in them.

If you omit the $ext/tests/phpunit argument, it runs 36438 tests (presumably all tests from all installed extensions). Actually it's unclear to me how CI can ever run only the tests from a specific extension. It looks as if non-coverage test jobs will always run tests from extension dependencies.

I think core's ExtensionsTestSuite should create one suite per extension, under a top-level extensions suite. Then CI could just run that suite, instead of trying to guess the path.

If I fix the test path, I still get "Warning: Incorrect filter configuration, code coverage will not be processed". This is probably because src/, includes/ and maintenance/ are all non-existent, and so no files are included in the coverage filter. Perhaps CI could get the list of directories to cover from some configuration file in the extension, instead of guessing.

I don't know why mwext-phpunit-coverage is a shell script instead of being part of quibble. Maybe @hashar can explain why it was done that way. I couldn't find an explanation in T195918.

I think core's ExtensionsTestSuite should create one suite per extension, under a top-level extensions suite. Then CI could just run that suite, instead of trying to guess the path.

Alternatively, you can just specify extensions/Wikibase as the path. It seems to work.

T288396#8979619 is a very nice dig :)

If you omit the $ext/tests/phpunit argument, it runs 36438 tests (presumably all tests from all installed extensions).

Actually it's unclear to me how CI can ever run only the tests from a specific extension. It looks as if non-coverage test jobs will always run tests from extension dependencies.

When a patch is triggered for a MediaWiki extension, Quibble invokes composer run phpunit:entrypoint -- --testsuite extensions. The suite is defined in mediawiki/core tests/phpunit/suite.xml and finds tests extensions registered via the onUnitTestsList hook.

As a result, Quibble triggers PHPUnit tests for every single extensions that are dependencies. It is a brute force approach to run the cross extensions integration tests. There are a lot of issues: we run way too many tests and given the dependencies are not bi directional, some integration tests are not run when an extension is not defined as a dependency. But I digress.

The coverage is rendered with solely for tests of the triggering extension (and thus the coverage reports lacks any tests exercised by other repositories). It is probably not a big issue.


Alternatively, you can just specify extensions/Wikibase as the path. It seems to work.

That is the root cause I guess, congratulation. tests/phpunit suffix probably rely on the convention used for most extension but clashes with Wikibase (and possibly other extensions). The path suffix comes from 2018 patch Only run that extension's tests in coverage job https://gerrit.wikimedia.org/r/c/integration/config/+/405393

The scripts can be altered:

dockerfiles/quibble-buster-php74-coverage/mwext-phpunit-coverage.sh
- "$MW_INSTALL_PATH/extensions/$EXT_NAME/tests/phpunit" &
+ "$MW_INSTALL_PATH/extensions/$EXT_NAME" &
dockerfiles/quibble-buster-php74-coverage/mwskin-phpunit-coverage.sh
- "$MW_INSTALL_PATH/extensions/$SKIN_NAME/tests/phpunit" &
+ "$MW_INSTALL_PATH/extensions/$SKIN_NAME" &

If I fix the test path, I still get "Warning: Incorrect filter configuration, code coverage will not be processed". This is probably because src/, includes/ and maintenance/ are all non-existent, and so no files are included in the coverage filter. Perhaps CI could get the list of directories to cover from some configuration file in the extension, instead of guessing.

MediaWiki core has composer run phpunit:coverage-edit which invokes includes/composer/ComposerPhpunitXmlCoverageEdit.php which looks similar to the CI script phpunit-suite-edit.py uses the same wrong assumption. Looks like it can be replaced by the MediaWiki core equivalent code. I have found T235031: phpunit:coverage-edit - Add configuration flag so it can replace phpunit-suite-edit.py.

Change 935410 had a related patch set uploaded (by Hashar; author: Tim Starling):

[integration/config@master] dockerfiles: use extension base path for coverage report

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

MediaWiki core has composer run phpunit:coverage-edit which invokes includes/composer/ComposerPhpunitXmlCoverageEdit.php which looks similar to the CI script phpunit-suite-edit.py uses the same wrong assumption. Looks like it can be replaced by the MediaWiki core equivalent code. I have found T235031: phpunit:coverage-edit - Add configuration flag so it can replace phpunit-suite-edit.py.

It's all very unsatisfying. I don't want to touch it because I don't like any part of it. I'd end up patching MediaWiki, PHPUnit, Quibble and integration/config/dockerfiles. A month later, my PHPUnit PRs would be rejected and we'd be back to square one. Maybe you can slip something like if args.cover_extension == 'Wikibase': into phpunit-suite-edit.py and I can pretend not to see it.

Change 935410 merged by jenkins-bot:

[integration/config@master] dockerfiles: use extension base path for coverage report

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

Change 936768 had a related patch set uploaded (by Hashar; author: Hashar):

[integration/config@master] Add experimental coverage job for Wikibase

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

Change 936768 merged by jenkins-bot:

[integration/config@master] Add experimental coverage job for Wikibase

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

Change 936770 had a related patch set uploaded (by Hashar; author: Hashar):

[mediawiki/extensions/Wikibase@master] (DO NOT SUBMIT) test coverage

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

I gave it a try with a new job mwext-phpunit-coverage-docker which has the new image version.

The new job does run Phpunit with just the extension:

php -d extension=pcov.so -d pcov.enabled=1 -d pcov.directory=/workspace/src/extensions/Wikibase \
  -d 'pcov.exclude=@(tests|vendor)@'
   --coverage-clover /workspace/log/clover.xml
   --coverage-html /workspace/cover
   --log-junit /workspace/log/junit.xml
      /workspace/src/extensions/Wikibase

I have send a dummy change https://gerrit.wikimedia.org/r/c/mediawiki/extensions/Wikibase/+/936770 which removes repo/tests/phpunit/includes/ArrayValueCollectorTest.php and it fails a test https://integration.wikimedia.org/ci/job/mwext-phpunit-coverage-docker/4/console#console-section-11

00:07:57.819 1) Wikibase\Repo\Tests\Api\PermissionsTest::testCreateItem with data set "normal permissions, no error" (null, null)
00:07:57.819 API did not return expected error code. Got error message ApiUsageException: failed-save: The save has failed. in /workspace/src/includes/api/ApiBase.php:1543

Which looks unrelated. If I retry with a change which does not alter anything, the same issue occurs so something is off.

Change 936770 abandoned by Hashar:

[mediawiki/extensions/Wikibase@master] (DO NOT SUBMIT) test coverage

Reason:

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

Change 960553 had a related patch set uploaded (by Hashar; author: Hashar):

[integration/config@master] jjb: rm Docker image override for mwext-phpunit-coverage-docker

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

Change 960553 merged by jenkins-bot:

[integration/config@master] jjb: rm Docker image override for mwext-phpunit-coverage-docker

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