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
| Status | Subtype | Assigned | Task | ||
|---|---|---|---|---|---|
| Resolved | • Jhernandez | T104318 [GOAL]: Testing practices & expose test coverage | |||
| Open | None | T100294 [EPIC] Encourage developers to increase code coverage | |||
| Resolved | Legoktm | T71685 Generate PHP code coverage reports for extensions | |||
| Open | None | T185211 Generate coverage report for Wikidata extensions | |||
| Open | None | T242617 Wikibase post-merge builds are failing | |||
| Open | None | T288396 [REPO][CLIENT][SW] Re-start Wikibase test coverage reporting |
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
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
The way extension coverage works was not obvious to me and took a bit of time to figure out.
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 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:
- "$MW_INSTALL_PATH/extensions/$EXT_NAME/tests/phpunit" & + "$MW_INSTALL_PATH/extensions/$EXT_NAME" &
- "$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
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
Change 936768 had a related patch set uploaded (by Hashar; author: Hashar):
[integration/config@master] Add experimental coverage job for Wikibase
Change 936768 merged by jenkins-bot:
[integration/config@master] Add experimental coverage job for Wikibase
Change 936770 had a related patch set uploaded (by Hashar; author: Hashar):
[mediawiki/extensions/Wikibase@master] (DO NOT SUBMIT) test coverage
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/WikibaseI 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:
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
Change 960553 merged by jenkins-bot:
[integration/config@master] jjb: rm Docker image override for mwext-phpunit-coverage-docker
Change #1017888 had a related patch set uploaded (by Lucas Werkmeister (WMDE); author: Lucas Werkmeister (WMDE)):
[integration/config@master] Revert "layout.yaml: Add extension-coverage to Wikibase extension"
Change #1017888 merged by jenkins-bot:
[integration/config@master] Revert "layout.yaml: Add extension-coverage to Wikibase extension"