Page MenuHomePhabricator

[EPIC] Encourage developers to increase code coverage
Open, NormalPublic

Description

As a developer I want to develop on software that doesn't break in unexpected ways so that I can feel confident in the changes I am making to the codebase.

Possible parts of a solution
  1. Generate a code coverage report, i.e. increase/decrease in code coverage, in the pre-review hook and warn the developer if their patch decreases code coverage (or doesn't increase it, maybe)
  2. Tooling:
  3. Dashboards:

Related Objects

Event Timeline

There are a very large number of changes, so older changes are hidden. Show Older Changes
phuedx raised the priority of this task from to Needs Triage.May 25 2015, 12:52 PM
phuedx added subscribers: phuedx, Aklapper.
phuedx updated the task description. (Show Details)May 25 2015, 1:37 PM
phuedx set Security to None.
Jdlrobson moved this task from Needs triage to Product triage on the Gather board.May 25 2015, 9:19 PM
phuedx added a subscriber: hashar.May 29 2015, 4:01 PM

@hashar: What would we have to do – hopefully just enable, right? – to get code coverage reports for our PHP and JavaScript unit tests?

+ Release-Engineering-Team because that sounds Epic worthy.

At first we would need a place to push temporary materials tool. A lot of disk (maybe Swift) and a place under wmfusercontent.org . The reason is you we don't want random code under wikimedia.org domain. That is more or less T72945: Preview generated documentation in test pipeline for review.

We have code coverage report for MediaWiki core at https://integration.wikimedia.org/ci/job/mediawiki-core-code-coverage/ . It takes more than hour on a production machine. But then that is running the whole PHPUnit test suite. Running only the extension test would be way faster but might well be too slow.

I have no idea whether PHPUnit supports HHVM XHPROF profiler to generate code coverage. On Zend that is based on XDebug. No task for that. One sure thing is that we still run a stalled PHPUnit 3.7.x which certainly does not support coverage under HHVM. The Jenkins jobs thus have to be migrated to use a composer entry point so we can get a later PHPUnit version. A lot of work have been toward that goal but it is non trivial.

Then we need to come up with a standard place to output coverage report to so we can push them to the web host that is going to host them. I have described it on https://www.mediawiki.org/wiki/Continuous_integration/Entry_points#Testing_PHP proposing the composer.json to have:

"scripts": {
    "test": [
        "phpcs $PHPCS_ARGS",
        "phpunit $PHPUNIT_ARGS"
    ]
}

This way we could use env variable in Jenkins to pass well known coverage options.

So yeah doable, but some prerequisites sorry :-(

Tgr added a subscriber: Tgr.Jun 2 2015, 12:24 AM

At first we would need a place to push temporary materials tool. A lot of disk (maybe Swift) and a place under wmfusercontent.org . The reason is you we don't want random code under wikimedia.org domain. That is more or less T72945: Preview generated documentation in test pipeline for review.

We already have a system for storing artifacts in Jenkins and linking them from Gerrit. A coverage report is just another artifact.

We have code coverage report for MediaWiki core at https://integration.wikimedia.org/ci/job/mediawiki-core-code-coverage/ . It takes more than hour on a production machine. But then that is running the whole PHPUnit test suite. Running only the extension test would be way faster but might well be too slow.

FWIW Gather tests take 135s with and 50s without coverage on my vagrant box. https://gerrit.wikimedia.org/r/#/c/209032/ runs the complete MediaWiki + extensions test suite in 50s so the time for generating coverage should be trivial.

I have no idea whether PHPUnit supports HHVM XHPROF profiler to generate code coverage. On Zend that is based on XDebug. No task for that.

It's claimed to be unnecessary, see sebastianbergmann/php-code-coverage#324. XDebug support was added in HHVM 3.4 and Wikimedia uses 3.6 so we are probably okay (see also hhvm#3704 which lists remaining incompatibilities, code coverage is not mentioned).

I do run into hhvm#5099 when trying to generate coverage, though.

In any case, not sure why this would be a blocker, given that we execute unit tests with both Zend and HHVM. We can just use Zend for coverage report.

Tgr added a project: Epic.Jun 4 2015, 10:40 PM
greg triaged this task as Normal priority.Jun 5 2015, 6:44 PM

Change 220025 had a related patch set uploaded (by Jdlrobson):
Add thresholds for QUnit coverage to ensure we don't get lazy

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

It seems a cheap way to do this would be to use the existing grunt qunit:cov command and then reject patches that push test coverage below a certain threshold (see patch above). This can be tweaked to not require any kind of storage and possibly plugged into Jenkins - thoughts @hashar ?

Change 220025 merged by jenkins-bot:
Add thresholds for QUnit coverage to ensure we don't get lazy

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

It seems a cheap way to do this would be to use the existing grunt qunit:cov command and then reject patches that push test coverage below a certain threshold (see patch above). This can be tweaked to not require any kind of storage and possibly plugged into Jenkins - thoughts @hashar ?

I'm not in favour of straight up rejection – you'll note that I said "warn the developer". I think that this patch jumps over a lot of discussion that we should be having as a team. We've only just started to have those discussions around the Q1 planning.

Jdlrobson moved this task from Backlog to Epics on the MobileFrontend board.Aug 4 2015, 6:37 PM
greg added a subscriber: greg.Sep 4 2015, 6:03 PM

Should we use this task as the main tracking task across all projects? I made T111546: [EPIC] Add a voting test code coverage report to gate pipeline to track that, but the more I think about it/look at the blockers of this task already, I think *this* task is it.

Is it OK for me/RelEng to "take" ownership of this task (basically: removing the reading-team related projects, and have them create any reading-specific tasks as blockers of this)? I'll just merge that other task (T111546) into this one, if that's OK.

Better to merge / decline T111546: [EPIC] Add a voting test code coverage report to gate pipeline imho. Until we get a coverage build that is fast enough, I don't see it being added to gate (it takes 2 hours currently).

Is it OK for me/RelEng to "take" ownership of this task (basically: removing the reading-team related projects, and have them create any reading-specific tasks as blockers of this)? I'll just merge that other task (T111546) into this one, if that's OK.

Fine by me. There'll be reading-web-specific tasks that describe/track the work that we'll be doing so the beginning and end states will be almost the same, except that we'll have Release-Engineering-Team pushing us to do better too (!!!).

jayvdb added a subscriber: jayvdb.Oct 15 2015, 9:05 AM
greg moved this task from INBOX to Epics on the Release-Engineering-Team board.Mar 11 2016, 10:09 PM
hashar removed a subscriber: hashar.May 6 2019, 7:57 AM

Change 526421 had a related patch set uploaded (by Kosta Harlan; owner: Kosta Harlan):
[mediawiki/core@master] (wip) Provide command to adjust phpunit.xml for code coverage

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

Change 526421 merged by jenkins-bot:
[mediawiki/core@master] Provide command to adjust phpunit.xml for code coverage

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

MobileFrontend now publishes JS code coverage reports: https://doc.wikimedia.org/MobileFrontend/master/js/lcov-report