Page MenuHomePhabricator

Can PHPUnit @covers tags cover entire files
Closed, ResolvedPublic

Event Timeline

Legoktm created this task.Dec 23 2017, 12:13 AM
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptDec 23 2017, 12:13 AM

My answer to the question asked is: yes. Why not?

The more relevant question is: Which tools complain about @covers tags that point to ….php file names instead of (full qualified) class names? I don't know the answer to this question.

Whenever I stumble across a @covers ….php (and Wikibase does have a few) I can see why it makes sense to have it, e.g. because there is no class, but just a .php file. In my opinion tools that build actual coverage metrics should either be able to understand these tags, or simply ignore them. To my knowledge, all tools do this.

So what is the issue this ticket aims to fix? A too rigorous validation of these tags?

There is work to fix T171899

For EventLogging the @covers contains a whole file, which was detected as error (but that was without .php).

My answer to the question asked is: yes. Why not?

The more relevant question is: Which tools complain about @covers tags that point to ….php file names instead of (full qualified) class names? I don't know the answer to this question.

PHPUnit. Did you read https://phpunit.de/manual/4.8/en/appendixes.annotations.html#appendixes.annotations.covers.tables.annotations which I linked above?

Whenever I stumble across a @covers ….php (and Wikibase does have a few) I can see why it makes sense to have it, e.g. because there is no class, but just a .php file. In my opinion tools that build actual coverage metrics should either be able to understand these tags, or simply ignore them. To my knowledge, all tools do this.

PHPUnit does not, it dies when trying to generate coverage. Also, Wikibase does not have any according to codesearch - just ORES, Cognate, and WikibaseMediaInfo.

So what is the issue this ticket aims to fix? A too rigorous validation of these tags?

Removal of the tags most likely, so we can move forward with T171899: Validate @covers tags during normal test runs

Change 400142 had a related patch set uploaded (by Legoktm; owner: Legoktm):
[mediawiki/extensions/WikibaseMediaInfo@master] Remove invalid @covers tag

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

Change 400143 had a related patch set uploaded (by Legoktm; owner: Legoktm):
[mediawiki/extensions/ORES@master] Remove invalid @covers tag

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

Change 400144 had a related patch set uploaded (by Legoktm; owner: Legoktm):
[mediawiki/extensions/Cognate@master] Remove invalid @covers tag

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

Restricted Application added a project: Machine Learning Platform. · View Herald TranscriptDec 25 2017, 3:32 AM

Change 400142 merged by jenkins-bot:
[mediawiki/extensions/WikibaseMediaInfo@master] Remove invalid @covers tag

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

Change 400144 merged by jenkins-bot:
[mediawiki/extensions/Cognate@master] Remove invalid @covers tag

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

Change 400143 merged by jenkins-bot:
[mediawiki/extensions/ORES@master] Remove invalid @covers tag

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

PHPUnit. Did you read https://phpunit.de/[…]

PHPUnit is not the only tool that can understand @covers tags.

PHPUnit […] dies […]

So report this as a bug upstream?

PHPUnit. Did you read https://phpunit.de/[…]

PHPUnit is not the only tool that can understand @covers tags.

Like? Regardless, PHPUnit is the tool we use for coverage, so it seems like the one that actually matters here.

PHPUnit […] dies […]

So report this as a bug upstream?

We already tried that: https://github.com/sebastianbergmann/phpunit/issues/1760

Legoktm closed this task as Resolved.Dec 25 2017, 6:49 PM