47 current users in https://libup.wmcloud.org/library/composer/ockcyp/covers-validator?branch=main as it's part of our standard set.

Change #1155731 had a related patch set uploaded (by Jforrester; author: Jforrester):

[mediawiki/tools/cookiecutter-library@master] Adjust the version of devDependencies set to 2025-06-11 standards

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

[If] the check of @covers should be kept in libraries, another tool is needed.

Note to self: In MediaWiki core and extensions, we use MediaWikiCoversValidator which is part of the default MediaWikiUnitTestCase and MediaWikiIntegrationTestCase base classes. Thus not affected.

I made https://github.com/oradwell/covers-validator/issues/44 to get the ball rolling: I don't think the new release is going to tag itself after 3 years of inactivity. Also, I'd find it useful to list what options are available. I suppose:

• Ditch the library and rely on the native behaviour of PHPUnit crashing when run with a coverage driver and an invalid @covers is encountered
• Replace the library with an in-house PHPCS sniff
• Fork the library, make it work with PHPUnit 10, and switch to our version
• Hope that the library is updated by its maintainer

Anything else?

In T395743#10915539, @Daimona wrote:

Ditch the library and rely on the native behaviour of PHPUnit crashing when run with a coverage driver and an invalid @covers is encountered
[…]

This is what we did historically. We didn't do this because the trailing feedback often left coverage reports broken/outdated in ways that require several non-obvious discovery factors. The main issue here is that (afaik) we don't enable coverage in the "test" build, but in the post-merge "coverage" build, which means it isn't seen until weeks/months later until someone 1) notices that date at the bottom of the coverage report is from long ago, 2) knows that a more recent commit exists in that lib (not a given, since most repos are stable), and 3) knows to look at a recent merged patch to examine its post-merge build. And then, the cycle may repeat as it may not pass in one go without pre-merge feedback.

But... for libraries (as opposed to MediaWiki core and extensions), they're small enough that we can simply enable coverage by default in the composer test command. I've done this for a number of side projects already, and have satisfied with it. Especially because:

• The overhead nowadays is quite small, especially with pcov instead of xdebug the overhead only ~15% (on 10 seconds, that adds 1s).
• PHPUnit gracefully skips coverage for local development when pcov or xdebug are not installed or disabled, and
• PHPUnit prints a helpful debug line to stdout explaining how to install/enable these.

To do this:

• CI job for composer-test and postmerge coverage publish should use the same docker image (or have pcov in both).
• Update composer.json to run phpunit with coverage in the "test" command.

Looking at https://gerrit.wikimedia.org/r/c/mediawiki/libs/Minify/+/1155363

• test: composer-package-php81 runs docker-registry.wikimedia.org/releng/composer-package-php81:8.1.32
• postmerge: phpunit-coverage-php81-publish runs docker-registry.wikimedia.org/releng/composer-php81:8.1.32

Those aren't identical but they both inherit the same composer-scratch and php81 base, so no differences in packages there that I can see.

However... that doesn't help MediaWiki core and extensions, and the absolute overhead would be quite significant there given the size of those projects (15% on 16 minutes is an extra 2-3 minutes). But.. that's okay because we use MediaWikiCoversValidator there instead. So.. I guess that means we have a path forward here?

Agreed, running libraries' regular test with coverage enabled seems like the way forward.

Change #1160183 had a related patch set uploaded (by Krinkle; author: Krinkle):

[mediawiki/libs/Minify@master] build: Switch from covers-validator to enabling phpunit coverage

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

Change #1160184 had a related patch set uploaded (by Krinkle; author: Krinkle):

[mediawiki/libs/Minify@master] WIP: Demo invalid covers

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

In T395743#10924315, @gerritbot wrote:

Change #1160184 had a related patch set uploaded (by Krinkle; author: Krinkle):

[mediawiki/libs/Minify@master] WIP: Demo invalid covers

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

php81 - php84 all say Warning: No code coverage driver available.

Because:

# Disable pcov by default, although the performance overhead should be minimal
RUN phpdismod pcov

- job-template:
    name: 'phpunit-coverage-{php}-publish'
…
             -d extension=pcov.so \
             -d pcov.enable=1 \

Change #1160197 had a related patch set uploaded (by Krinkle; author: Jforrester):

[mediawiki/libs/Minify@master] [DNM] composer: Demonstrate how to enable pcov inline for phpunit

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

In T395743#10929362, @gerritbot wrote:

Change #1160197 had a related patch set uploaded (by Krinkle; author: Jforrester):

[mediawiki/libs/Minify@master] [DNM] composer: Demonstrate how to enable pcov inline for phpunit

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

This confirms that it can work at the last mile as well. It does currenlty hardcode WMF CI, but this shows it can work at least (Locally that yields PHP Startup: Unable to load dynamic library 'pcov.so', incl when ppl run tests with xdebug in PhpStorm.).

I guess the question now is, where and how widely we want to enable this in between these two extremes.

I think the main area where we don't want to enable it is, is Quibble jobs, which cummulate the most runtime and thus are most sensitive to small increases (ref T225730). But, I realized just now that those don't actually inherit from the php8X base images. They install PHP and extensions themselves. (Today I learned!)

Today:

• dockerfiles/php81
  • install php81
  • install pcov
  • globally disable pcov
• dockerfiles/composer-php81
  • inherit dockerfiles/php81
• jjb: phpunit-coverage-{php}-publish
  • use dockerfiles/composer-php81
  • runtime re-enable pcov

• dockerfiles/quibble-bullseye-php81
  • install php81
  • install pcov
  • globally disable pcov
• quibble-bullseye-php81-coverage
  • inherit dockerfiles/quibble
  • runtime re-enable pcov
• jjb: mwcore-phpunit-coverage-patch
  • runtime re-enable pcov

I've looked at JJB templates and don't see a clean point to inject it, because composer will spawn sub processes, so enabling it for one command isn't enough, you really want it enabled in php.ini. Modifying php.ini mid-job would require sudo as well, whereas Jenkins runs as nobody.

That leaves two fairly good options:

1. Enable pcov in the composer* docker images.

That would give us:

• dockerfiles/php81 (unchanged)
  • install pcov
  • globall disable pcov
• dockerfiles/composer-php
  • inherit php81
  • Add: Undo the above by globally re-enabling pcov
• dockerfiles/quibble (unchanged)

2. Enable pcov in the php# docker images.

Most PHP extenisons, we install in the php# image. In a way, we do that with pcov today as well, except we then immediately disable it. If there are derivatives where we need pcov disabled, we could move these steps down one layer.

• php81
  • install pcov
  • Remove: globally disable pcov
• composer-php (unchanged)
  • inherit php81
• something-else-php-that-wants-pcov-disabled
  • Add: globally disable pcov
• quibble (unchanged)

Are there derivates where where we need pcov disabled? E.g. some kind of conflict like enabling xdebug as well, or a slow non-coverage job that doesn't need it? If there are a lot of those, option 1 might be the quicker fix, and would naturally contain the change to just composer standalone jobs.

Enable pcov in the php# docker images.

This gets my vote. The current comment is # Disable pcov by default, although the performance overhead should be minimal, re-written from when it was xdebug where it was very much not minimal.

In T395743#10929464, @Krinkle wrote:

I think the main area where we don't want to enable it is, is Quibble jobs, which cummulate the most runtime and thus are most sensitive to small increases (ref T225730). But, I realized just now that those don't actually inherit from the php8X base images. They install PHP and extensions themselves. (Today I learned!)

This alleviates the concerns I would've had. I certainly wouldn't want to make CI slower for all.

I do, however, wonder if we have an idea of what changes would be needed to the upstream library: is it "just" a matter of bumping the version requirement and maybe updating a couple function calls, or would it require a major rewrite? Same for making a PHPCS sniff for it.

To me, the main drawback of just running PHPUnit with coverage is that it makes it more difficult for developers to run tests, and especially to run the same tests as CI. Loading a PHP extension is not something we can easily hardcode in a composer script, as you pointed out. And even then, people need to install it beforehand. In other words, it adds semi-permanent complexity and it makes me wonder if we should just bite the bullet and make a one-off investment into a tool equivalent to the existing library, be it a fork of the library or a PHPCS sniff; or a PR upstream, but who knows if the maintainer will be there to approve and release it.

Are there derivates where where we need pcov disabled? E.g. some kind of conflict like enabling xdebug as well

If we have anything in CI that runs with xdebug enabled, that's a bug that needs to be fixed immediately and will make me really sad.

@Daimona I agree, which is why we shouldn't hardcode the PHP ini in composer.json. That was just to show that it "works". Passing --coverage by itself to phpunit is fine to do in composer.json. It doesn't impose any requirement on local dev to have pcov or debug installed or enabled. It means it'll generate coverage if you have that, and won't if you don't.

In T395743#10935818, @Krinkle wrote:

@Daimona I agree, which is why we shouldn't hardcode the PHP ini in composer.json. That was just to show that it "works". Passing --coverage by itself to phpunit is fine to do in composer.json. It doesn't impose any requirement on local dev to have pcov or debug installed or enabled. It means it'll generate coverage if you have that, and won't if you don't.

That's true, but the other point I was making is that it makes local tests different from CI, if you don't have a coverage driver available. Maybe this is not a big deal, and to clarify, I do not oppose using just --coverage to replace the library. However, it's not clear to me if the difficulty of updating the library (or forking it, or making a PHPCS sniff) is enough to justify the change.

Change #1162106 had a related patch set uploaded (by Krinkle; author: Krinkle):

[integration/config@master] dockerfiles: Enable pcov in php81-84 base images

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

Change #1162106 merged by jenkins-bot:

[integration/config@master] dockerfiles: Enable pcov in php81-84 base images

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

In T395743#10935851, @Daimona wrote:

In T395743#10935818, @Krinkle wrote:

[…] It doesn't impose any requirement on local dev to have pcov or debug installed or enabled. It means it'll generate coverage if you have that, and won't if you don't.

That's true, but the other point I was making is that it makes local tests different from CI, if you don't have a coverage driver available. […]

I see what you mean. It's a bit of both.

This change also brings us closer to replicating CI results. Rather than emulating how PHPUnit will validate @covers in a post-merge job in WMF CI, this is now done much earlier on-patch ("shift left"), with the option to also run that locally as part of composer test. Previously you had both install pcov and run composer cover instead of composer test. Now it is only the former.

At the same time, you're right that it does mean covers annotations aren't validated anymore locally by default if you don't have pcov or xdebug, whereas they are still validated in WMF CI, which creates an inconsistency. By comparison, the emulation would be consistent local and in WMF CI, despite being in turn inconsistent between "composer test" and "composer cover" and thus inconsistent between local/CI test and post-merge CI.

I'm open to the PHPCS route, that makes a lot of sense (recommended in PHPUnit issue). Although realistically, given no deep modelling of the codebase in PHPCS, I'd expect Phan to be better even positioned for this.

Change #1162179 had a related patch set uploaded (by Krinkle; author: Krinkle):

[integration/config@master] dockerfiles: Re-disable pcov in php81-84 base images

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

I got a bit into Phan plugins and it seems like that can work indeed. Looking at how `UndeclaredClassa and UndeclaredClassMethod` work upstream in Phan (for example, in Phan/AST/UnionTypeVisitor.php) there is a built-in method for this.

if (!$code_base->hasMethodWithFQSEN($method_fqsen)) {
  throw new CodeBaseException( …, "Method with name $name does not exist for class …");

            $parent_class_fqsen = $class->getParentClassFQSEN();
            if (!$code_base->hasClassWithFQSEN($parent_class_fqsen)) {
                throw new IssueException(
                    Issue::fromType(Issue::UndeclaredClass)(
…
          } catch (CodeBaseException $exception) {
            $exception_fqsen = $exception->getFQSEN();
            $this->emitIssueWithSuggestion(
                Issue::UndeclaredClassMethod,

The part I'm unsure about is what kind of plugin/visitor we'd need. There's a ton of choices at https://github.com/phan/phan/tree/5.4.6/src/Phan/PluginV3.

There's two internal plugins that upstream has, that do part of what we'd want. For example PHPUnitNotDeadCodePlugin.php has logic for (presumably, efficiently) iterating only classes that extend PHPUnit\Framework\TestCase, and logic for iterating only methods that are test cases (i.e. testFoo and not other helper methods), and logic for accessing doc block comments from those methods. And then there's PHPDocRedundantPlugin.php which similarly analyzes a doc block.

Combined with the CodeBase methods, it seems like the relevant puzzle pieces are there for a simple plugin to be added to mediawiki/tools/phan.

Change #1162196 had a related patch set uploaded (by Krinkle; author: Krinkle):

[mediawiki/tools/phan@master] WIP: Add InvalidCoversPlugin

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

Change #1162179 merged by jenkins-bot:

[integration/config@master] dockerfiles: Re-disable pcov in php81-84 base images

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

In T395743#10935934, @Krinkle wrote:

I'm open to the PHPCS route, that makes a lot of sense (recommended in PHPUnit issue). Although realistically, given no deep modelling of the codebase in PHPCS, I'd expect Phan to be better even positioned for this.

PHPCS typically does not load other classes to verify the code, that would be new to interact with the autoloader like this. It may work, but even MediaWikiCoversValidator does not parse the annotation by itself, it is using a internal util class from PHPUnit. With PHPUnit 10 the covers can be written as PHP attribute, maybe that is easier to parse and validate.
Most repos (including mediawiki core, extensions and skins, but also libraries) does not run phan over the tests folders, that would be new to make everything pass.

In T395743#10935934, @Krinkle wrote:

This change also brings us closer to replicating CI results. Rather than emulating how PHPUnit will validate @covers in a post-merge job in WMF CI, this is now done much earlier on-patch ("shift left"), with the option to also run that locally as part of composer test. Previously you had both install pcov and run composer cover instead of composer test. Now it is only the former.

I see, I did not consider this. However, I would still consider it preferable, because in this case, avoiding those inconsistencies is up to the covers-validator library, and developers don't have to worry about it.

I'd expect Phan to be better even positioned for this.

One thing to keep in mind (also pointed out by @Umherirrender above) is that we don't run phan on tests by default (because it would slow things down for little added value). For MW core, I might oppose changing that given the size of the test suites. I experimented with this a while back, in r650235, and I didn't like it. This task is about libraries though, so that's probably fine. In fact, several libraries already do that! But I suppose we would still need to analyze the tests/ directory by default in library mode.

In T395743#10935938, @Krinkle wrote:

The part I'm unsure about is what kind of plugin/visitor we'd need. There's a ton of choices at https://github.com/phan/phan/tree/5.4.6/src/Phan/PluginV3.

There's also an overview in https://github.com/phan/phan/blob/5f5153b19a91bfc8ed5aa515de317a72a73a8194/src/Phan/PluginV3.php#L9-L134, in case you didn't see it. PostAnalyzeNodeCapability (current implementation) is the most powerful as you can arbitrarily visit any node type. For @covers, I thought we could use a combination of AnalyzeClassCapability and AnalyzeMethodCapability, but it's interesting to see how PHPUnitNotDeadCodePlugin, which you mentioned, doesn't do that.

In T395743#10936071, @Umherirrender wrote:

PHPCS typically does not load other classes to verify the code, that would be new to interact with the autoloader like this.

Ah, I didn't realize that this was the case! I thought PHPCS had at least a list of classes. But with that not being the case, I agree that it's probably not the best tool.

With PHPUnit 10 the covers can be written as PHP attribute, maybe that is easier to parse and validate.

Good point, and something to keep in mind whatever we end up doing. Especially because 1) libraries can already switch to PHPUnit 10, and 2) metadata (like @covers) in comments has been deprecated in PHPUnit 11, and PHPUnit 12 allows it in attributes only.

Also a heads-up that the library maintainer replied acknowledging the issue and saying they'll take a look: https://github.com/oradwell/covers-validator/issues/44. Maybe let's wait a bit and see how that goes?

Change #1160183 abandoned by Krinkle:

[mediawiki/libs/Minify@master] build: Switch from covers-validator to enabling phpunit coverage

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

Change #1160184 abandoned by Krinkle:

[mediawiki/libs/Minify@master] WIP: Demo invalid covers

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

Change #1160197 abandoned by Jforrester:

[mediawiki/libs/Minify@master] [DNM] composer: Demonstrate how to enable pcov inline for phpunit

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

In T395743#10936335, @Daimona wrote:

Also a heads-up that the library maintainer replied acknowledging the issue and saying they'll take a look: https://github.com/oradwell/covers-validator/issues/44. Maybe let's wait a bit and see how that goes?

I traced where the relevant bits of code moved to inside PHPUnit over PHPUnit versions 10, 11, and 12.

Posted at https://github.com/oradwell/covers-validator/issues/44#issuecomment-3070751074

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

[integration/config@master] jjb: update php jobs for pcov enabled then disabled

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

Change #1169615 merged by jenkins-bot:

[integration/config@master] jjb: update php jobs for pcov enabled then disabled

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

Change #1162196 abandoned by Krinkle:

[mediawiki/tools/phan@master] WIP: Add InvalidCoversPlugin

Reason:

Made a PR instead at https://github.com/oradwell/covers-validator/issues/44

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

Change #1187957 had a related patch set uploaded (by Krinkle; author: Krinkle):

[mediawiki/libs/Minify@master] [WIP] build: Demonstrate PHPUnit 10 with patched ockcyp/covers-validator

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

Change #1187957 abandoned by Krinkle:

[mediawiki/libs/Minify@master] [WIP] build: Demonstrate PHPUnit 10 with patched ockcyp/covers-validator

Reason:

Upstream has merged my patch :)

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

Change #1194203 had a related patch set uploaded (by Jforrester; author: Jforrester):

[mediawiki/tools/phan@master] build: Upgrade ockcyp/covers-validator to 1.7.0 for PHPUnit 10 support

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

jforrester opened https://gitlab.wikimedia.org/repos/ci-tools/libup-config/-/merge_requests/94

releases: Bump covers-version to 1.7.0 and raise priority

Change #1194203 merged by jenkins-bot:

[mediawiki/tools/phan@master] build: Upgrade ockcyp/covers-validator to 1.7.0 for PHPUnit 10 support

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

daimona merged https://gitlab.wikimedia.org/repos/ci-tools/libup-config/-/merge_requests/94

releases: Bump covers-validator to 1.7.0 and raise priority

OK, so LibUp has a bunch of repos to update, but otherwise this looks done. Thank you, all, especially @Krinkle for getting the upstream patches made, landed, and released!

Change #1194634 had a related patch set uploaded (by Jforrester; author: Jforrester):

[wikipeg@master] build: Upgrade ockcyp/covers-validator to 1.7.0 for PHPUnit 10 support

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

Change #1194637 had a related patch set uploaded (by Jforrester; author: Jforrester):

[mediawiki/libs/Shellbox@master] build: Upgrade ockcyp/covers-validator to 1.7.0 for PHPUnit 10 support

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

Change #1194634 merged by jenkins-bot:

[wikipeg@master] build: Upgrade ockcyp/covers-validator to 1.7.0 for PHPUnit 10 support

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

Change #1194637 merged by jenkins-bot:

[mediawiki/libs/Shellbox@master] build: Upgrade ockcyp/covers-validator to 1.7.0 for PHPUnit 10 support

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

Change #1155731 merged by jenkins-bot:

[mediawiki/tools/cookiecutter-library@master] Adjust the version of devDependencies set to 2025-10-08 standards

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