Hello,
Too long, dont want to read: https://integration.wikimedia.org/cover/mediawiki-core/master/php/
Jenkins is nowadays producing a nightly coverage report. The idea is to run our full PHPUnit tests, track which lines in MediaWiki core have been executed and show up metrics regarding code not being executed. That is made possible by using the XDebug php extensions and PHPUnit code coverage feature.
In June I enforced a PHPUnit feature which force us to mention which MediaWiki function is covered by a test method [FORCE COVER]. The option is forceCoversAnnotation and is described in PHPUnit as:
forceCoversAnnotation Code Coverage will only be recorded for tests that use the @covers annotation documented in the section called “@covers”.
Jenkins build the coverage report every day at 1am UTC and it takes roughly 40 minutes to generate. You can access the report by browsing:
https://integration.wikimedia.org
Then on the top menu "Coverage" and pick the MediaWiki core (PHP) link:
https://integration.wikimedia.org/cover/mediawiki-core/master/php/
A first step to enhance our coverage would be to add '@covers' statements to our unit test methods. The tests for global functions are good candidates: tests/phpunit/includes/GlobalFunctions
Now I am wondering where on mw.org I can document our coverage system. Should we get a page under [[Continuous integration]] or maybe another section in [[Manual:PHP unit testing]] ??
[FORCE COVER] https://gerrit.wikimedia.org/r/66125 [BUG 45594] https://bugzilla.wikimedia.org/45594
On Fri, Oct 18, 2013 at 5:52 AM, Antoine Musso hashar+wmf@free.fr wrote:
track which lines in MediaWiki core have been executed
Does it count the whole line as covered for something like
$var = $alwaysTrue ? $something : $something_else;
or just the "true" case?
In June I enforced a PHPUnit feature which force us to mention which MediaWiki function is covered by a test method [FORCE COVER].
Why is this desirable?
On Fri, Oct 18, 2013 at 8:23 AM, Brad Jorsch (Anomie) <bjorsch@wikimedia.org
wrote:
On Fri, Oct 18, 2013 at 5:52 AM, Antoine Musso hashar+wmf@free.fr wrote:
In June I enforced a PHPUnit feature which force us to mention which
MediaWiki function is covered by a test method [FORCE COVER].
Why is this desirable?
In my experience this is desirable in order to only mark the particular code you are testing as covered. So for example if you are testing database functions, you don't want it to mark wfRunHooks and friends as "covered" when it just happened to be called but wasn't specifically tested.
Erik B.
On Fri, Oct 18, 2013 at 12:13 PM, Erik Bernhardson ebernhardson@wikimedia.org wrote:
On Fri, Oct 18, 2013 at 8:23 AM, Brad Jorsch (Anomie) <bjorsch@wikimedia.org
wrote:
On Fri, Oct 18, 2013 at 5:52 AM, Antoine Musso hashar+wmf@free.fr wrote:
In June I enforced a PHPUnit feature which force us to mention which
MediaWiki function is covered by a test method [FORCE COVER].
Why is this desirable?
In my experience this is desirable in order to only mark the particular code you are testing as covered. So for example if you are testing database functions, you don't want it to mark wfRunHooks and friends as "covered" when it just happened to be called but wasn't specifically tested.
On the other hand, you *did* test part of wfRunHooks, even if indirectly.
different definitions of test ;-) code touched seems like a much less useful metric than code specifically tested, but i could be convinced otherwise.
On Fri, Oct 18, 2013 at 9:31 AM, Brad Jorsch (Anomie) <bjorsch@wikimedia.org
wrote:
On Fri, Oct 18, 2013 at 12:13 PM, Erik Bernhardson ebernhardson@wikimedia.org wrote:
On Fri, Oct 18, 2013 at 8:23 AM, Brad Jorsch (Anomie) <
bjorsch@wikimedia.org
wrote:
On Fri, Oct 18, 2013 at 5:52 AM, Antoine Musso hashar+wmf@free.fr
wrote:
In June I enforced a PHPUnit feature which force us to mention which
MediaWiki function is covered by a test method [FORCE COVER].
Why is this desirable?
In my experience this is desirable in order to only mark the particular code you are testing as covered. So for example if you are testing database functions, you don't want it to mark wfRunHooks and friends as "covered" when it just happened to be called but wasn't specifically
tested.
On the other hand, you *did* test part of wfRunHooks, even if indirectly.
-- Brad Jorsch (Anomie) Software Engineer Wikimedia Foundation
Wikitech-l mailing list Wikitech-l@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikitech-l
True, but there's two sides of this.
When needing the explicit annotation, it will keep out stuff that shouldn't be included (I agree on the wfRunHooks example).
However, depending on how it's implemented, it might also remove the very essence of line coverage. We wouldn't want to disable automatic coverage entirely (e.g. @cover Foo to mark Foo as 100% covered). It's still useful to have it provide the line-by-line coverage of an individual method to see what hasn't been covered yet.
Anyway, it looks like it's implemented with this in mind. It's not a boolean flag to disable the covering detection. It's more like a filter to to keep everything else out. Great :)
-- Krinkle
On 2013-10-18, at 18:38, Erik Bernhardson ebernhardson@wikimedia.org wrote:
different definitions of test ;-) code touched seems like a much less useful metric than code specifically tested, but i could be convinced otherwise.
On Fri, Oct 18, 2013 at 9:31 AM, Brad Jorsch (Anomie) <bjorsch@wikimedia.org
wrote:
On Fri, Oct 18, 2013 at 12:13 PM, Erik Bernhardson ebernhardson@wikimedia.org wrote:
On Fri, Oct 18, 2013 at 8:23 AM, Brad Jorsch (Anomie) <
bjorsch@wikimedia.org
wrote:
On Fri, Oct 18, 2013 at 5:52 AM, Antoine Musso hashar+wmf@free.fr
wrote:
In June I enforced a PHPUnit feature which force us to mention which
MediaWiki function is covered by a test method [FORCE COVER].
Why is this desirable?
In my experience this is desirable in order to only mark the particular code you are testing as covered. So for example if you are testing database functions, you don't want it to mark wfRunHooks and friends as "covered" when it just happened to be called but wasn't specifically
tested.
On the other hand, you *did* test part of wfRunHooks, even if indirectly.
-- Brad Jorsch (Anomie) Software Engineer Wikimedia Foundation
Wikitech-l mailing list Wikitech-l@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikitech-l
Wikitech-l mailing list Wikitech-l@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikitech-l
On 10/18/2013 05:23 PM, Brad Jorsch (Anomie) wrote:
On Fri, Oct 18, 2013 at 5:52 AM, Antoine Musso hashar+wmf@free.fr wrote:
track which lines in MediaWiki core have been executed
Does it count the whole line as covered for something like
$var = $alwaysTrue ? $something : $something_else;
or just the "true" case?
The whole line. This is a limitation of the PHP extension Xdebug. See http://phpunit.de/manual/current/en/code-coverage-analysis.html#code-coverag...
Based on my experience, there will also be some false negatives.
Le 18/10/13 19:20, Jan Zerebecki a écrit :
The whole line. This is a limitation of the PHP extension Xdebug. See http://phpunit.de/manual/current/en/code-coverage-analysis.html#code-coverag...
Based on my experience, there will also be some false negatives.
And also note that line coverage means that a function claimed as 100% covered does not necessary has all code paths covered or all contexts. That is specially true with globals / RequestContext varying.
An example of a function that should print only one message:
/** * put a single message error. */ function oneMessage( $x, $y ) { $i = 0; if ( $x ) { $i++; } if ( $y ) { print "$y has $i"; } }
You will get full line coverage by testing:
oneError( true, "message"); // prints 'message has 1' oneError( false, '' ); // prints nothing
But you are not actually covering all the possible code paths:
oneError( false, 'Oh yeah' ); // print 'Oh yeah has 0' oneError( true, '' ); // prints nothing
It could potentially be done by dumping the PHP code AST and record the opcodes being traversed by each test, but I am not aware of any tool to do that (yet?).
cheers,
2013/10/18 Antoine Musso hashar+wmf@free.fr:
Hello,
Too long, dont want to read: https://integration.wikimedia.org/cover/mediawiki-core/master/php/
This is very cool, apart from the numbers of course.
Is it hard to setup for extensions as well? Lots of development happens in extensions nowdays. I read from the linked bug that it is currently not possible due to bug in PHP.
-Niklas
Le 18/10/13 18:42, Niklas Laxström a écrit :
Too long, dont want to read: https://integration.wikimedia.org/cover/mediawiki-core/master/php/
This is very cool, apart from the numbers of course.
Is it hard to setup for extensions as well? Lots of development happens in extensions nowdays. I read from the linked bug that it is currently not possible due to bug in PHP.
I haven't looked at how to do it for extensions. All the core hooks and methods should ideally be covered by the core test suite itself, so we could discard them in the report. phpunit configuration has a <filter> which would let us get rid of core (aka includes, languages..):
http://phpunit.de/manual/3.7/en/appendixes.configuration.html#appendixes.con...
(yeah long url sorry)
Hey,
For those interested in having coverage reports for their extensions, you can look at the source of any of the extensions listed here:
https://coveralls.io/r/wikimedia
These extensions allow running their tests by executing "phpunit" in their root directory. That makes creating the coverage for those as simple as running "phpunit --coverage-html". All those extensions have their tests run on TravisCI, which on successful build submits the coverage report to the coveralls.io service, which keeps track of coverage changes over time.
One thing that is lacking in this setup is having the project risk and CRAP reports, which are in themselves also very useful. I'd be very cool if someone set something generally usable up for getting those reports for extensions.
Cheers
-- Jeroen De Dauw http://www.bn2vs.com Don't panic. Don't be evil. ~=[,,_,,]:3 --
Mobile is definitely interested in running this for MobileFrontend. I'll read up as well.
On Fri, Oct 18, 2013 at 1:34 PM, Antoine Musso hashar+wmf@free.fr wrote:
Le 18/10/13 18:42, Niklas Laxström a écrit :
Too long, dont want to read: https://integration.wikimedia.org/cover/mediawiki-core/master/php/
This is very cool, apart from the numbers of course.
Is it hard to setup for extensions as well? Lots of development happens in extensions nowdays. I read from the linked bug that it is currently not possible due to bug in PHP.
I haven't looked at how to do it for extensions. All the core hooks and methods should ideally be covered by the core test suite itself, so we could discard them in the report. phpunit configuration has a <filter> which would let us get rid of core (aka includes, languages..):
http://phpunit.de/manual/3.7/en/appendixes.configuration.html#appendixes.con...
(yeah long url sorry)
-- Antoine "hashar" Musso
Wikitech-l mailing list Wikitech-l@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikitech-l
Hey,
Thanks for setting this up hashar! It's a very useful metric to have.
I'd like to bring some attention to the "dashboard" link at the top of coverage pages, which leads to for instance
https://integration.wikimedia.org/cover/mediawiki-core/master/php/index.dash...
This page is very useful in finding the things most in need of attention. It also lists classes by complexity, in this particular case making it very clear that classes such as Parser are doing way to much stuff.
Cheers
-- Jeroen De Dauw http://www.bn2vs.com Don't panic. Don't be evil. ~=[,,_,,]:3 --
Le 18/10/13 18:52, Jeroen De Dauw a écrit :
Hey,
Thanks for setting this up hashar! It's a very useful metric to have.
I'd like to bring some attention to the "dashboard" link at the top of coverage pages, which leads to for instance
https://integration.wikimedia.org/cover/mediawiki-core/master/php/index.dash...
This page is very useful in finding the things most in need of attention. It also lists classes by complexity, in this particular case making it very clear that classes such as Parser are doing way to much stuff.
Good catch, I like the top project risks:
Parser (840552) Language (558331) Title (519190) User (442890) EditPage (320922)
Parser / Language we already know about it. Title is the subject of an RFC to refactor it:
https://www.mediawiki.org/wiki/Requests_for_comment/TitleValue
User will follow I guess.
EditPage is on the same vein as Parser and I usually refer to both of them as our technical debt that almost everyone is too afraid to edit.
If we manage to properly cover the public methods of those classes, that will most probably makes later refactoring easier to handle.
On 10/18/2013 05:52 AM, Antoine Musso wrote:
Hello,
Too long, dont want to read: https://integration.wikimedia.org/cover/mediawiki-core/master/php/
Uh, what does test coverage on tests/ directory even mean? Who <s>watches the watchers</s> tests the tests?
— Victor.
Hey,
Uh, what does test coverage on tests/ directory even mean? Who
<s>watches the watchers</s> tests the tests?
Good point. Non-production code typically ought to be excluded. This is pretty simple to do with some PHPUnit config:
https://github.com/wikimedia/mediawiki-extensions-Diff/blob/master/phpunit.x...
The relevant section there is "filter". This makes sure only the files in the "src" directory get held into account for the coverage report. This config also makes sure that all such files get held into account, and prevents the ones with no coverage at all from being omitted (which is the default behavior).
Cheers
-- Jeroen De Dauw http://www.bn2vs.com Don't panic. Don't be evil. ~=[,,_,,]:3 --
On 10/18/2013 07:31 PM, Jeroen De Dauw wrote:
Uh, what does test coverage on tests/ directory even mean? Who <s>watches the watchers</s> tests the tests?
Good point. Non-production code typically ought to be excluded. This is pretty simple to do with some PHPUnit config:
https://gerrit.wikimedia.org/r/#/c/90584/
Thank you!
On 10/18/2013 11:52 AM, Antoine Musso wrote:
Now I am wondering where on mw.org I can document our coverage system. Should we get a page under [[Continuous integration]] or maybe another section in [[Manual:PHP unit testing]] ??
Its specific to PHPUnit so I went ahead and added https://www.mediawiki.org/wiki/Manual:PHP_unit_testing/Writing_unit_tests#Co... .
Le 18/10/13 20:18, Jan Zerebecki a écrit :
Its specific to PHPUnit so I went ahead and added https://www.mediawiki.org/wiki/Manual:PHP_unit_testing/Writing_unit_tests#Co...
Excellent! And I think that is the first time ever I validate a diff on mediawiki.org \O/
Thank you!
wikitech-l@lists.wikimedia.org