WordPress.org

Make WordPress Core

Opened 3 years ago

Closed 22 months ago

#43067 closed defect (bug) (fixed)

Code coverage cannot be run

Reported by: jipmoors Owned by: johnbillion
Milestone: 5.0 Priority: normal
Severity: normal Version:
Component: Build/Test Tools Keywords: needs-patch
Focuses: Cc:

Description

The current PHPUnit configuration has been configured as follows:

<filter>
    <whitelist processUncoveredFilesFromWhitelist="true">
        <directory suffix=".php">src</directory>
    </whitelist>
</filter>

This means that all the files in the src/ folder are included when running a coverage report.

Reference to documentation:
https://phpunit.de/manual/current/en/code-coverage-analysis.html#code-coverage-analysis.whitelisting-files

Please note that the loading of sourcecode files that is performed when processUncoveredFilesFromWhitelist="true" is set can cause problems when a sourcecode file contains code outside the scope of a class or function, for instance.

Changing it to the following configuration allows for coverage to be run.

<filter>
    <whitelist addUncoveredFilesFromWhitelist="true">
        <directory suffix=".php">src</directory>
    </whitelist>
</filter>

See tweet by Sebastian Bergmann (creator of PHPUnit): https://twitter.com/s_bergmann/status/495154142319747073

This was added in https://core.trac.wordpress.org/changeset/37449

Props for https://profiles.wordpress.org/schlessera for helping to find the problem and the solution.

Attachments (1)

make_the_code_coverage_runnable.diff (546 bytes) - added by jipmoors 3 years ago.

Download all attachments as: .zip

Change History (16)

#1 @johnbillion
3 years ago

  • Milestone changed from Awaiting Review to 5.0
  • Version trunk deleted

LGTM.

Also, third party code should be excluded from the whitelist (with <exclude>) to avoid unnecessary coverage reports for those files. The list will probably look similar to the list used for excluding PHPCS sniffs: https://github.com/WordPress/wordpress-develop/blob/b9b2f2a79d3506a74e3c3ce34de69ed069864dfe/phpcs.xml.dist#L16 . Can be done in another ticket.

#2 @swissspidy
3 years ago

Would this also solve #39237? 🤔

#3 @schlessera
3 years ago

@swissspidy Yes, seems to be the exact same issue at first glance.

#4 @schlessera
3 years ago

This should be a very straight-forward change:

  • goes from broken to not-broken with a single declarative change
  • only affects build tools

Can we have this committed to trunk so that everyone can easily run code coverage?

#6 @swissspidy
3 years ago

I'm still not able to run coverage with this patch because of #39237. So I'm unable to test the patch properly.

#7 @netweb
3 years ago

  • Keywords needs-patch added

This ticket needs a refreshed patch, it needs to exclude the 3rd party code as referenced by @johnbillion in comment:1

That said, running phpunit --coverage-html some/folder works for me:

❯ phpunit --coverage-html tests/phpunit/build/logs/
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.26 by Sebastian Bergmann and contributors.

.............................................................   61 / 8529 (  0%)
.............................................................  122 / 8529 (  1%)
...
............................................................. 8479 / 8529 ( 99%)
...................................SSSSSSSSSSSSSS.            8529 / 8529 (100%)

You should really fix these slow tests (>150ms)...
 1. 10962ms to run WP_Test_REST_Users_Controller:test_get_items_pagination_headers
 2. 8505ms to run WP_Test_REST_Posts_Controller:test_get_items_pagination_headers
 3. 6038ms to run Tests_XMLRPC_mt_getRecentPostTitles:test_no_editable_posts
 4. 5615ms to run WP_Test_REST_Users_Controller:test_get_items_per_page
 5. 4998ms to run WP_Test_REST_Users_Controller:test_get_items_page
 6. 4707ms to run Tests_WP_Customize_Manager:test_import_theme_starter_content
 7. 4433ms to run Tests_Import_Import:test_double_import
 8. 4062ms to run WP_Test_REST_Schema_Initialization:test_build_wp_api_client_fixtures
 9. 3858ms to run Tests_Import_Import:test_small_import
 10. 3366ms to run WP_Test_REST_Categories_Controller:test_get_terms_pagination_headers
...and there are 6931 more above your threshold hidden from view

Time: 58.12 minutes, Memory: 1104.14MB

OK, but incomplete, skipped, or risky tests!
Tests: 8529, Assertions: 35560, Skipped: 34, Risky: 8.

Generating code coverage report in HTML format ... done
  • https://cldup.com/RwLVWNDrXo.png

#8 @johnbillion
3 years ago

  • Owner set to johnbillion
  • Status changed from new to accepted

Yep I've also been able to generate a code coverage report (and the test run took just over an hour). @swissspidy When running the test suite with code coverage active, it takes a while before anything happened (over a minute for me).

@netweb Third party code exclusion is covered in #43240.

#9 @swissspidy
3 years ago

Perhaps I should have clarified. I'm still running into #39237. I thought this patch here might fix that also, but unfortunately it doesn't.

It's not a matter of waiting. I'm getting a Cannot modify header information - headers already sent by (output started at tests/phpunit/includes/bootstrap.php:70) error straight away.

But again, this is something that can be addressed in #39237.

#10 @johnbillion
2 years ago

  • Resolution set to fixed
  • Status changed from accepted to closed

In 42665:

Build/Test Tools: Allow code coverage reports to be generated.

This corrects the attribute needed to allow PHPUnit to generate coverage reports, and also adds a blacklist of files and directories that are excluded from coverage reports.

Props schlessera, jipmoors

Fixes #43067, #43240

#11 @johnbillion
2 years ago

In 42666:

Build/Test Tools: Allow code coverage reports to be generated.

This corrects the attribute needed to allow PHPUnit to generate coverage reports, and also adds a blacklist of files and directories that are excluded from coverage reports.

Props schlessera, jipmoors

See #43067, #43240

Merges [42665] to the 4.9 branch.

#12 @jorbin
22 months ago

  • Milestone changed from 5.0 to 5.1

#13 @netweb
22 months ago

  • Milestone changed from 5.1 to 5.0

Moving back to the 5.0 milestone as the 4.9 branch [42666] commit is already included in the 5.0 branch

#14 @pento
22 months ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

👍🏻

#15 @pento
22 months ago

  • Resolution set to fixed
  • Status changed from reopened to closed
Note: See TracTickets for help on using tickets.