WordPress.org

Make WordPress Core

Opened 8 months ago

Last modified 4 months ago

#39265 new enhancement

Missing @covers and @uses in the comments block in phpunt test for wordpress

Reported by: pbearne Owned by:
Milestone: Awaiting Review Priority: normal
Severity: normal Version: 4.8
Component: Build/Test Tools Keywords:
Focuses: Cc:

Description

PHPUnit has a comment block notation @cover and @uses that we are not using so we can create coverage report to see how much of WordPress has test

https://thephp.cc/news/2014/03/phpunit-4-0-code-coverage-improvements
http://blog.teamlazerbeez.com/2009/08/18/phpunit-annotations/#covers
https://phpunit.de/manual/current/en/appendixes.annotations.html#appendixes.annotations.covers

@covers tells PHPUnit that this test what the test function is testing

    /**
     * @covers Class_Name::function_name
     */

@uses tells PHPunit that the test is using another function to help test the @covers function but this test is not testing it so it should ignore it for the covers report

     /**
     * @covers Class_Name::function_name
     * @uses   Other function
     */

note: if the @ covers function that is not in a class and in global namespace then the notation is like this

     /**
     * @covers ::function_name
     */

This ticket is place holder for how and what the changes need to be so that we can encourage the fixing of this.

Change History (6)

This ticket was mentioned in Slack in #core by pbearne. View the logs.


5 months ago

#2 @jnylen0
5 months ago

While I am definitely a proponent of thorough unit tests, I am -1 on adding annotations for code coverage, and in general I think the @covers annotation is not a good idea. I will try to explain why.

Someone will have to update every single test to indicate which functions are covered. This will be a manual process requiring an enormous amount of effort, not only now, but in perpetuity. I can't speak for everyone but I personally don't want to do this in my patches.

It is virtually guaranteed that tests will not be consistently updated with @covers annotations in the future. (It's hard enough to ensure that tests are written in the first place.) Even if we do a huge effort now to add them, the coverage reports will become essentially useless in the future due to attrition.

Per-function coverage is not a very useful thing to report on. Consider: @covers wp_update_post. This is a very long function with a lot of complicated logic. Line-by-line coverage reporting is far more useful.

I don't want to read lots of lines of @covers and other annotations in the test code. I think we already have too many of these annotations and they are just visual noise, because the information about which functions are called is already present in the test code itself.

I would be far more interested in an effort to provide automated code coverage reports on a line-by-line basis, perhaps with results split by WordPress component or something like that. I do think this would be really valuable, and once it's set up, there's far less ongoing maintenance required.

#3 @pbearne
5 months ago

Hi @jnylen0

How would we go about providing automated code coverage reports on a line-by-line basis? This would good but can it be done?

A good test will only be testing one function so I expect to see just one @covers per test.

We went through the code base and add comments to actions and filters this a smaller task and we already have volunteers to do the heavy lifting who just waiting for the OK to get started

Paul

#4 @jnylen0
5 months ago

Most of https://phpunit.de/manual/current/en/code-coverage-analysis.html is dedicated to setting up automated code coverage.

Reading closer, the @covers annotation is a supplement to this automated coverage, and I don't think you can generate a report from manual annotations only.

@jorbin has already set this up: https://daily.jorb.in/2015/10/automated-code-coverage-of-wordpress-core-php-code/

The basic way to run it is as follows, but I get a mysterious error when I do so:

$ phpunit --coverage-html coverage-dir
Installing...
Running as single site... To run multisite, use -c tests/phpunit/multisite.xml
Not running ajax tests. To execute these, use --group ajax.
Not running ms-files tests. To execute these, use --group ms-files.
Not running external-http tests. To execute these, use --group external-http.
PHPUnit 5.7.15 by Sebastian Bergmann and contributors.

Cannot modify header information - headers already sent by (output started at /home/james/wp/wordpress-develop-official/tests/phpunit/includes/bootstrap.php:62)

#5 @pbearne
5 months ago

So the end point is to get coverage reports so if this work then we are good

The error is due to print statements e.g. "Installing..." there is a bug ticket for that we need away to turn it off for coverage reports.

Last edited 5 months ago by pbearne (previous) (diff)

#6 @tloureiro
4 months ago

Hey, @jnylen0 what version of phpunit are you using?
I got the same bug in version 5.1.3. Downloading and using the phpunit 5.7 version .phar worked for me. The report was generated successfully and it looks like this:
https://ibb.co/e4UNAk

Also, I couldn't see any results for code coverage in @jorbin's page (https://codecov.io/gh/aaronjorbin/wordpress.codecov)

Note: See TracTickets for help on using tickets.