Page MenuHomePhabricator

Test failures on PHPUnit 9.5: "This test does not have a @covers annotation but is expected to have one"
Closed, ResolvedPublic

Description

PHPUnit tests are failing on master in the Scribunto extension: "This test does not have a @covers annotation but is expected to have one"

As seen on https://gerrit.wikimedia.org/r/c/mediawiki/extensions/Scribunto/+/838283: https://integration.wikimedia.org/ci/job/quibble-vendor-mysql-php74-phpunit-standalone-docker/1464/console

Caused by the PHPUnit 9.5 upgrade in MediaWiki core (T243600).

Event Timeline

matmarex triaged this task as Unbreak Now! priority.Oct 8 2022, 7:40 PM

This is blocking merges in Scribunto, and extensions that depend on it.

Change 840335 had a related patch set uploaded (by Reedy; author: Reedy):

[mediawiki/extensions/Scribunto@master] Add more @covers

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

Legoktm added a subscriber: Legoktm.

The relevant code in PHPUnit is:

if ($this->forceCoversAnnotation && !$error && !$failure && !$warning && !$incomplete && !$skipped && !$risky) {
    $annotations = TestUtil::parseTestMethodAnnotations(
        get_class($test),
        $test->getName(false)
    );

    if (!isset($annotations['class']['covers']) &&
        !isset($annotations['method']['covers']) &&
        !isset($annotations['class']['coversNothing']) &&
        !isset($annotations['method']['coversNothing'])) {
        $this->addFailure(
            $test,
            new MissingCoversAnnotationException(
                'This test does not have a @covers annotation but is expected to have one'
            ),
            $time
        );

        $risky = true;
    }
}

Since we set forceCoversAnnotation=true in core's phpunit.xml.dist, it seems like we need to put @coversNothing on everything apparently...

Since we set forceCoversAnnotation=true in core's phpunit.xml.dist, it seems like we need to put @coversNothing on everything apparently...

I'm guessing that's a side effect of --migrate-configuration being run...

Since we set forceCoversAnnotation=true in core's phpunit.xml.dist, it seems like we need to put @coversNothing on everything apparently...

I'm guessing that's a side effect of --migrate-configuration being run...

Looking at when I ran it against XMPReader, it didn't add it...

I feel like it's aspirational that we do have it, but I'm curious how widespread the damage may or may not be.

Change 840335 merged by jenkins-bot:

[mediawiki/extensions/Scribunto@master] Add more @covers

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

Since we set forceCoversAnnotation=true in core's phpunit.xml.dist, it seems like we need to put @coversNothing on everything apparently...

I believe it's a good thing. That way the intention is explicit, and you don't forget to add @covers where you really need it. We also have a phpcs sniff for that: MediaWiki.Commenting.MissingCovers.MissingCovers. It's not catching all cases though apparently, because the sniff is not disabled for Scribunto, yet it didn't catch these tests.

matmarex assigned this task to Reedy.

Thanks for the quick fix!