[wp-trac] [WordPress Trac] #56706: Tests: `parent::set_up()` calls `wp_list_pluck()`, causing inaccurate coverage.

Sat Oct 1 02:22:45 UTC 2022

#56706: Tests: `parent::set_up()` calls `wp_list_pluck()`, causing inaccurate
coverage.
---------------------------------------+-----------------------
 Reporter:  costdev                    |       Owner:  costdev
     Type:  defect (bug)               |      Status:  assigned
 Priority:  normal                     |   Milestone:  6.1
Component:  Build/Test Tools           |     Version:
 Severity:  normal                     |  Resolution:
 Keywords:  has-screenshots has-patch  |     Focuses:
---------------------------------------+-----------------------
Description changed by costdev:

Old description:

> == How was this discovered?
>
> While adding new tests for `wp_list_pluck()`, I noticed that the coverage
> report included coverage that should not have been possible.
>
> == Steps to reproduce
>
> 1. Add the following to your PHPUnit configuration file (either
> `phpunit.xml.dist`, or copy this to `phpunit.xml`.
>
> {{{#!xml
> <coverage includeUncoveredFiles="true" processUncoveredFiles="false"
> pathCoverage="true" cacheDirectory="./tests/phpunit/build/logs/phpunit-
> cache">
>     <include>
>         <directory suffix=".php">src/wp-includes/class-wp-list-
> util.php</directory>
>     </include>
>     <report>
>         <text outputFile="php://stdout" showOnlySummary="true"/>
>         <html outputDirectory="./tests/phpunit/build/logs/coverage-
> html"/>
>     </report>
> </coverage>
> }}}
>
> 2. Add the following test method to
> `tests/phpunit/tests/functions/wpListPluck.php`.
>
> {{{#!php
> <?php
>
> /**
>  * @covers WP_List_Util::pluck
>  */
> public function test_set_up_causes_wp_list_util_pluck_coverage_bug() {
>         $this->assertTrue( true );
> }
> }}}
>
> 3. Run:
> {{{
> XDEBUG_MODE=coverage phpunit --filter
> test_set_up_causes_wp_list_util_pluck_coverage_bug
> }}}
>
> 4. Navigate to:
> {{{
> http://localhost/wordpress-develop/tests/phpunit/build/logs/coverage-html
> /class-wp-list-util.php.html
> }}}
>
> 5. Click `pluck`.
> 6. 🐞 Observe that `$newlist[ $key ] = $value->$field;` (Line 168) is
> unexpectedly covered.
>
> == What's happening?
>
> Huge thanks to @jrf for research on this!
>
> - `WP_UnitTestCase_Base::set_up()` calls
> `WP_UnitTestCase_Base::reset_post_statuses()`.
> - Which calls `get_post_stati()`.
> - Which calls `wp_filter_object_list()`.
> - Which calls `WP_List_Util::pluck()`. 💥
>
> Since code called in `WP_UnitTestCase_Base::set_up()` is included in
> coverage reports, this presents an inaccurate coverage report for
> `wp_list_pluck()/WP_List_Util::pluck()`.
>
> == Solutions
>
> The real solution involves ensuring that the code under test is not
> called in `WP_UnitTestCase_Base::set_up()`. However, this is best done
> after a wider discussion on improving the test suite.
>
> For now, removing `parent::set_up()` from
> `Tests_Functions_wpListPluck::set_up()` will prevent inaccurate coverage
> and confusion for contributors. To allow for testing deprecation and
> incorrect usage notices, `$this->expectDeprecated()` should be added
> instead.

New description:

 == How was this discovered?

 While adding new tests for `WP_List_Util::pluck()`, I noticed that the
 coverage report included coverage that should not have been possible.

 == Steps to reproduce

 1. Add the following to your PHPUnit configuration file, either
 `phpunit.xml.dist`, or copy this to `phpunit.xml`.

 {{{#!xml
 <coverage includeUncoveredFiles="true" processUncoveredFiles="false"
 pathCoverage="true" cacheDirectory="./tests/phpunit/build/logs/phpunit-
 cache">
     <include>
         <directory suffix=".php">src/wp-includes/class-wp-list-
 util.php</directory>
     </include>
     <report>
         <text outputFile="php://stdout" showOnlySummary="true"/>
         <html outputDirectory="./tests/phpunit/build/logs/coverage-html"/>
     </report>
 </coverage>
 }}}

 2. Add the following test method to
 `tests/phpunit/tests/functions/wpListPluck.php`.

 {{{#!php
 <?php

 /**
  * @covers WP_List_Util::pluck
  */
 public function test_set_up_causes_wp_list_util_pluck_coverage_bug() {
         $this->assertTrue( true );
 }
 }}}

 3. Run:
 {{{
 XDEBUG_MODE=coverage phpunit --filter
 test_set_up_causes_wp_list_util_pluck_coverage_bug
 }}}

 4. Navigate to:
 {{{
 http://localhost/wordpress-develop/tests/phpunit/build/logs/coverage-html
 /class-wp-list-util.php.html
 }}}

 5. Click `pluck`.
 6. 🐞 Observe that parts of this method are unexpectedly covered.

 == What's happening?

 Huge thanks to @jrf for research on this!

 - `WP_UnitTestCase_Base::set_up()` calls
 `WP_UnitTestCase_Base::reset_post_statuses()`.
 - Which calls `get_post_stati()`.
 - Which calls `wp_filter_object_list()`.
 - Which calls `WP_List_Util::pluck()`. 💥

 Since code called in `WP_UnitTestCase_Base::set_up()` is included in
 coverage reports, this presents an inaccurate coverage report for
 `WP_List_Util::pluck()`.

 == Solutions

 The real solution involves ensuring that the code under test is not called
 in `WP_UnitTestCase_Base::set_up()`. However, this is best done after a
 wider discussion on improving the test suite.

 For now, removing `parent::set_up()` from
 `Tests_Functions_wpListPluck::set_up()` will prevent inaccurate coverage
 and confusion for contributors. To allow for testing deprecation and
 incorrect usage notices, `$this->expectDeprecated()` should be added
 instead.

--

-- 
Ticket URL: <https://core.trac.wordpress.org/ticket/56706#comment:1>
WordPress Trac <https://core.trac.wordpress.org/>
WordPress publishing platform