Page MenuHomePhabricator

Figure out some way to not use addUncoveredFilesFromWhitelist="true" when running only some PHPUnit tests
Closed, ResolvedPublic

Description

In ee753ba4 we started using <whitelist addUncoveredFilesFromWhitelist="true">. This is good for ignoring files we don't want to have coverage reports for, and including files that nothing touches yet. But unfortunately it also means that even if you run code coverage for only a single test, like

$ php phpunit.php includes/api/ApiQuerySiteinfoTest.php --coverage-html ../../docs/coverage/

it will go through every file instead of just the one that was touched. This is the difference on my machine between ~45 seconds and 9 seconds for the generation of the coverage files. It would be good if we could somehow change suite.xml to have addUncoveredFilesFromWhitelist="false" for when you're generating a few files, and true only for when you want to generate the full coverage report. If we assume that individuals don't often want to locally run the full coverage report, a simple solution would be to change it to false in the repo and have the bots that do the full coverage report patch it.

Event Timeline

Restricted Application added a subscriber: Aklapper. · View Herald TranscriptApr 12 2018, 2:00 PM
Legoktm added a subscriber: Legoktm.

a simple solution would be to change it to false in the repo and have the bots that do the full coverage report patch it.

This sounds like a good idea. We'd need to alter the CI config to do that.

This is a suboptimal solution, by the way, because it still generates coverage reports for each file that's executed, which in my case still takes several seconds. The really best solution would be to only generate coverage reports for the files that are actually executed, i.e., the ones provided on the command line. This takes a fraction of a second. It seems like the best way to go would be to have phpunit.php generate a temporary configuration file and tell this phpunit invocation to use that config file (with -c or equivalent). Presumably you'd want to parse the existing config file, alter the <whitelist> element to contain the contents we want, and then write it out again to a temporary file. This also has the advantage that if someone actually wants to run all the tests locally, it will generate coverage reports for all files, as desired, without having to alter the config.

In the meantime I'm manually editing suite.xml, but it's tiresome to be careful not to check in the change. I don't want to use git update-index --assume-unchanged because it's easy to forget that I did it.

It's worth noting that it seems when I change suite.xml to only one file, it places the generated file directly in the root of the coverage directory, not in a subdirectory. This makes sense, but differs from current behavior.

Change 520459 had a related patch set uploaded (by Kosta Harlan; owner: Kosta Harlan):
[mediawiki/core@master] (WIP) Speed up code coverage generation for local development

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

Regarding slow local use, indeed, it isn't meant to be used as-is locally. The documentation at https://www.mediawiki.org/wiki/Manual:PHP_unit_testing/Code_coverage always recommends editing it to reduce the scope or disable the flag.

Inverting this to instead have CI be the one to edit it makes a lot of sense and is a huge usability/discoverability bonus.

Change 520459 merged by jenkins-bot:
[mediawiki/core@master] Speed up code coverage generation for local development

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

Change 523961 had a related patch set uploaded (by Kosta Harlan; owner: Kosta Harlan):
[mediawiki/core@master] Exclude extensions/skins test directories from coverage analysis

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

Change 523961 merged by jenkins-bot:
[mediawiki/core@master] Exclude extensions/skins test directories from coverage analysis

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

Simetrical closed this task as Resolved.Aug 12 2019, 9:50 AM
Simetrical claimed this task.

This is fixed now, right? Yay! \o/