Page MenuHomePhabricator

Add rule to require use of @covers in PHPUnit tests
Closed, ResolvedPublic

Description

Given our convention of whitelisted code coverage tracking in PHPUnit, it doesn't make sense for a test to exist without any @covers annotation. This is a common source of issue for MediaWiki core where code coverage is quite low (<20%), but "actual" coverage I believe is at least somewhat higher given how many tests we have that are simply missing the annotation for the few relevant methods and/or classes.

Notes:

  • How to identify whether a class or file is a PHPUnit test case?
    • Maybe something about directory structure and file name? (tests/phpunit/**/*Test.php)
    • Maybe it's possible to test the class hierarchy? (subclass of PHPUnit TestCase)
    • Maybe just check the class name only? (name ends with "Test")
  • How to identify whether a method is a PHPUnit test?
    • Method name starts with "test".
  • How to enforce the rule?
    • Method must have a doc block with at least one @covers annotation.
    • Or: Method must be in a class that has a doc block with at least one @covers annotation.
    • Or: There's a @coversNothing annotation

Details

Related Gerrit Patches:
mediawiki/tools/codesniffer : masterAdd sniff to find tests without @covers tags

Event Timeline

Krinkle created this task.Oct 26 2017, 5:24 PM
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptOct 26 2017, 5:24 PM
Tgr added a subscriber: Tgr.Oct 26 2017, 7:01 PM

A lot of extensions do not follow the tests/phpunit directory structure.

pmiazga added a subscriber: pmiazga.

There is also a @coversNothing annotation which may be useful in some high level/integration tests.

WordPoints did simiar sniff: MissingCoversSniff.php

A lot of extensions do not follow the tests/phpunit directory structure.

IMHO checking that function matches test.+ and classname matches .+Test should be good enough. If we want to check only files located inside tests/phpunit, IMHO thats fine. I wouldn't worry about extensions not following the tests/phpunit` directory structure as

  • it's super easy to fix this thing - just move the tests to follow the mw directory structure
  • I don't think those extensions have @covers annotations so anyway someone would have to fix those unittests
phuedx awarded a token.Feb 2 2018, 1:35 PM
Legoktm claimed this task.Feb 7 2018, 7:39 PM
Legoktm updated the task description. (Show Details)

Change 409444 had a related patch set uploaded (by Legoktm; owner: Legoktm):
[mediawiki/tools/codesniffer@master] Add sniff to find tests without @covers tags

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

Change 409444 merged by jenkins-bot:
[mediawiki/tools/codesniffer@master] Add sniff to find tests without @covers tags

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

Umherirrender closed this task as Resolved.Feb 14 2018, 7:19 PM
Umherirrender triaged this task as Medium priority.