Maniphest T419073

mwext-phpunit-coverage-patch no longer runs any tests for modified files
Closed, ResolvedPublic
Actions

Assigned To

Authored By

	Dreamy_Jazz
	Mar 4 2026, 9:37 PM

Description

Summary

mwext-phpunit-coverage-patch is no longer running any tests, even with the filters set

Technical details

For example, https://integration.wikimedia.org/ci/job/mwext-phpunit-coverage-patch/63605/console#console-section-10 for CheckUser has tests which should have been executed but instead PHPUnit says "No tests executed!"

21:31:51 + exec phpunit-patch-coverage check --command 'php -d extension=pcov.so -d pcov.enabled=1 -d pcov.directory=/workspace/src/extensions/CheckUser -d pcov.exclude='\''@(tests|vendor)@'\'' -d pcov.initial.files=3000 "$MW_INSTALL_PATH"/vendor/bin/phpunit -c /workspace/src/phpunit.xml.dist' --html /workspace/log/coverage.html --test-dir tests/phpunit
21:31:51 Finding coverage difference in 38b12bdb535db0617a4b1e78051ca027c36e3468
21:31:51 $ php -d extension=pcov.so -d pcov.enabled=1 -d pcov.directory=/workspace/src/extensions/CheckUser -d pcov.exclude='@(tests|vendor)@' -d pcov.initial.files=3000 "$MW_INSTALL_PATH"/vendor/bin/phpunit -c /workspace/src/phpunit.xml.dist --coverage-clover /tmp/cloverVp3a0Y --filter '/GlobalBlockCheckTest|GlobalBlockCheckTest/'
21:31:51 Using PHP 8.3.30
21:31:51 Running with MediaWiki settings because there might be integration tests
21:31:57 PHPUnit 9.6.34 by Sebastian Bergmann and contributors.
21:31:57 
21:31:57 No tests executed!
21:31:57 
21:31:59 Generating code coverage report in Clover XML format ... done [00:01.691]
21:31:59 $ php -d extension=pcov.so -d pcov.enabled=1 -d pcov.directory=/workspace/src/extensions/CheckUser -d pcov.exclude='@(tests|vendor)@' -d pcov.initial.files=3000 "$MW_INSTALL_PATH"/vendor/bin/phpunit -c /workspace/src/phpunit.xml.dist --coverage-clover /tmp/clover2QgNsY --filter '/GlobalBlockCheckTest|GlobalBlockCheckTest/'
21:31:59 Using PHP 8.3.30
21:31:59 Running with MediaWiki settings because there might be integration tests
21:32:04 PHPUnit 9.6.34 by Sebastian Bergmann and contributors.
21:32:04 
21:32:05 No tests executed!
21:32:05 
21:32:06 Generating code coverage report in Clover XML format ... done [00:01.580]
21:32:07 No coverage changes found.
21:32:07 INFO:quibble.commands:<<< Finish: User commands: mwext-phpunit-coverage-patch, in 16.031 s

The same thing happens for tests in GlobalBlocking: https://integration.wikimedia.org/ci/job/mwext-phpunit-coverage-patch/63604/console#console-section-14

Acceptance criteria

mwext-phpunit-coverage-patch actually runs the tests

Details

Related Changes in Gerrit:

Subject	Repo	Branch	Lines +/-
jjb: update MediaWiki coverage jobs	integration/config	master	+3 -3
dockerfiles: [quibble-coverage] keep comments in phpunit suite edit	integration/config	master	+190 -44
jjb: Generate PHPUnit config before running tests	integration/config	master	+9 -4
dockerfiles: [quibble-coverage] Use local PHPUnit config	integration/config	master	+15 -4

Customize query in gerrit

Related Objects

Mentioned In: T420906: MediaWiki code coverage reports include coverage for the Vector skin
T419107: The PHPUnit config override does not appear to be auto-generated
T345481: Migrate Parser and extension tests away from deprecated PHPUnit TestSuite subclassing
Mentioned Here: T345481: Migrate Parser and extension tests away from deprecated PHPUnit TestSuite subclassing

Event Timeline

Dreamy_Jazz created this task.Mar 4 2026, 9:37 PM

Restricted Application added a subscriber: Aklapper. · View Herald TranscriptMar 4 2026, 9:37 PM

Dreamy_Jazz renamed this task from mwext-phpunit-coverage-patch no longer actually running any tests to mwext-phpunit-coverage-patch no longer runs any tests for modified files.Mar 4 2026, 9:38 PM

Dreamy_Jazz updated the task description. (Show Details)

Dreamy_Jazz mentioned this in T345481: Migrate Parser and extension tests away from deprecated PHPUnit TestSuite subclassing.Mar 4 2026, 10:08 PM

Jdforrester-WMF added projects: Continuous-Integration-Config, phpunit-patch-coverage.Mar 4 2026, 10:24 PM

Jdforrester-WMF subscribed.

Yes, most probably caused by changes in T345481. I have a few patches up to fix this, all linked to the other task, except for some documentation changes that simply use the same topic in gerrit: https://gerrit.wikimedia.org/r/q/topic:%22phpunit-ref%22

Change #1248112 had a related patch set uploaded (by Daimona Eaytoy; author: Daimona Eaytoy):

[integration/config@master] dockerfiles: [quibble-coverage] Use local PHPUnit config

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

Change #1248113 had a related patch set uploaded (by Daimona Eaytoy; author: Daimona Eaytoy):

[integration/config@master] jjb: Generate PHPUnit config before running tests

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

Change #1248112 merged by jenkins-bot:

[integration/config@master] dockerfiles: [quibble-coverage] Use local PHPUnit config

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

With @Daimona's new fixes for CI, I re-triggered the job for PS4 of your patch.

Before: https://integration.wikimedia.org/ci/job/mwext-phpunit-coverage-patch/63609/console
After: https://integration.wikimedia.org/ci/job/mwext-phpunit-coverage-patch/63619/console

It now runs, covering 16 tests.

Note that the output is not ideal ('└─── secondsNo coverage changes found.'), but that's unrelated to this issue.

Change #1248113 merged by jenkins-bot:

[integration/config@master] jjb: Generate PHPUnit config before running tests

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

Maintenance_bot removed a project: Patch-For-Review.Mar 5 2026, 1:30 AM

Thanks!

It's broken again even when the associated PHPUnit test jobs pass such as https://integration.wikimedia.org/ci/job/mwext-phpunit-coverage-patch/63700/console#console-section-13:

12:37:39 Error in bootstrap script: RuntimeException:
12:37:39 The PHPUnit config override does not appear to be auto-generated. Generate it manually by running `composer phpunit:config`, or automatically by running tests via `composer phpunit`.
12:37:39 #0 /workspace/src/vendor/phpunit/phpunit/src/Util/FileLoader.php(66): include_once()
12:37:39 #1 /workspace/src/vendor/phpunit/phpunit/src/Util/FileLoader.php(49): PHPUnit\Util\FileLoader::load()
12:37:39 #2 /workspace/src/vendor/phpunit/phpunit/src/TextUI/Command.php(567): PHPUnit\Util\FileLoader::checkAndLoad()
12:37:39 #3 /workspace/src/vendor/phpunit/phpunit/src/TextUI/Command.php(347): PHPUnit\TextUI\Command->handleBootstrap()
12:37:39 #4 /workspace/src/vendor/phpunit/phpunit/src/TextUI/Command.php(114): PHPUnit\TextUI\Command->handleArguments()
12:37:39 #5 /workspace/src/vendor/phpunit/phpunit/src/TextUI/Command.php(99): PHPUnit\TextUI\Command->run()
12:37:39 #6 /workspace/src/vendor/phpunit/phpunit/phpunit(107): PHPUnit\TextUI\Command::main()
12:37:39 #7 /workspace/src/vendor/bin/phpunit(122): include('...')
12:37:39 #8 {main}
12:37:39 
12:37:39 In CloverXml.php line 70:
12:37:39                                      
12:37:39   String could not be parsed as XML  
12:37:39                                      
12:37:39 
12:37:39 check [--sha1 [SHA1]] [--test-dir TEST-DIR] [--html [HTML]] [--command COMMAND]
12:37:39 
12:37:39 INFO:quibble.commands:<<< Finish: User commands: mwext-phpunit-coverage-patch, in 0.951 s

Duhhhhh that's because phpunit-suite-edit strips all comments, and we're using the "this file is autogenerated ..." comment as a quick indicator.

And this is also documented BTW. If someone knows how to fix this, please go ahead... I don't speak sufficient python

In T419073#11677494, @Daimona wrote:

And this is also documented BTW. If someone knows how to fix this, please go ahead... I don't speak sufficient python

Looking at https://docs.python.org/3/library/xml.etree.elementtree.html it seems like we should be able to jump to use xml.etree.ElementTree.canonicalize with the with_comments option set true, maybe? (We're running Python 3.9.2 in the images, and from the docs it was added in Python 3.8.)

Using https://stackoverflow.com/questions/66666821/python-how-to-keep-the-xml-comment-exist-after-write-a-new-value-using-python, I was able to make it preserve comments with minimal changes... Except it still doesn't preserve the top-level comment, which is also the one we need.

Daimona mentioned this in T419107: The PHPUnit config override does not appear to be auto-generated.Mar 5 2026, 3:27 PM

DLynch subscribed.Mar 6 2026, 7:34 PM

VPuffetMichel subscribed.Mar 6 2026, 7:49 PM

Viktoria_Hillerud_WMSE subscribed.Mar 9 2026, 8:19 AM

Clarification on the status of this task: The cause has been identified: the XML-parsing python code in phpunit-suite-edit strips away all comments, including the marker that lets us recognize the config file as autogenerated. The fix would be as simple as preserving comments in the output XML file. Except that for whatever reason this seems unusually hard to do in python ¯\_(ツ)_/¯

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

[integration/config@master] dockerfiles: [quibble-coverage] keep XML comments

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

gerritbot added a project: Patch-For-Review.Mar 9 2026, 6:30 PM

@Daimona is there a week when you are not a hero? I can't believe stripping comments was even documented inside the phpunit-suite-edit script! With that root cause finally found, the python sauce is very quite straightforward. We need an XMLParser that is given a TreeObject that inserts comments instead of ignoring them entirely.

Side note: overall those scripts should probably be ported to PHP, made a standalone library and shipped in mediawiki/core via a require-dev. I think we did not do that because they are tightly tied to the CI environment/image etc and it is unlikely to be of any use outside of that scope. But we could at least add some tests to cover them!

In T419073#11689408, @hashar wrote:

With that root cause finally found, the python sauce is very quite straightforward. We need an XMLParser that is given a TreeObject that inserts comments instead of ignoring them entirely.

Note, we discussed this in gerrit, but sadly that method skips top-level comments, and that's exactly what we have here... Obviously I would prefer if the python parser just parsed the darn top-level comments, but if that really isn't possible for whatever reason, I suppose we could move the comment in the file...

In T419073#11689480, @hashar wrote:

Side note: overall those scripts should probably be ported to PHP, made a standalone library and shipped in mediawiki/core via a require-dev. I think we did not do that because they are tightly tied to the CI environment/image etc and it is unlikely to be of any use outside of that scope. But we could at least add some tests to cover them!

This also sounds doable. It isn't much code, especially once we drop some older-PHPUnit stuff.

I ported the script to use xml.dom.minidom https://gerrit.wikimedia.org/r/c/integration/config/+/1249372 (patchset 9). I'll look at finding a way to validate it against an actual coverage report and I guess then rebuild the image and update the Jenkins jobs.

hashar added a project: Essential-Work.Mar 10 2026, 3:48 PM

Change #1249372 merged by jenkins-bot:

[integration/config@master] dockerfiles: [quibble-coverage] keep comments in phpunit suite edit

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

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

[integration/config@master] jjb: update MediaWiki coverage jobs

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

Mentioned in SAL (#wikimedia-releng) [2026-03-10T17:11:38Z] <hashar> Updated MediaWiki coverage jobs so that they now keep "Generate a local configuration by running composer phpunit:config" message # T419073

Change #1250009 merged by jenkins-bot:

[integration/config@master] jjb: update MediaWiki coverage jobs

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

I got a successful build for mwext-phpunit-coverage-patch job. It was for https://gerrit.wikimedia.org/r/c/mediawiki/extensions/WikiLambda/+/1240009 and the console shows:

+-----------------------------------------------+--------+--------+
| Filename                                      | Old %  | New %  |
+-----------------------------------------------+--------+--------+
| includes/HookHandler/PageRenderingHandler.php |  67.00 |  68.00 |
+-----------------------------------------------+--------+--------+

Which I guess means the issue has been solved 🎉

I am leaving the task open, feel free to mark it resolved if there are other confirmations it is indeed solved.

Huge thanks for the debugging and for having found the root cause!

Confirming, this looks fixed. Thank you all, and especially @Daimona and @hashar.

Maintenance_bot removed a project: Patch-For-Review.Mar 10 2026, 5:33 PM

Echoing thanks!

Thanks to you @hashar! The cause was really easy to identify; rewriting the python script, not as much, and that was you :)

Well I usually find the root cause investigation to be the hardest part :-] The good outcome is the script is slightly better. Eventually one day we will have to make it more robust, find it a better place than having it embedded in a docker image that needs a rebuild, but that will be for another day.

Viktoria_Hillerud_WMSE unsubscribed.Mar 11 2026, 9:40 AM

Umherirrender mentioned this in T420906: MediaWiki code coverage reports include coverage for the Vector skin.Tue, Mar 24, 8:01 PM

mwext-phpunit-coverage-patch no longer runs any tests for modified filesClosed, ResolvedPublicActions