Behat code coverage improvements

In #234 we added Behat code coverage support which works great except for a few minor issues.

Incorrect autoload require statement here:

wp-cli-tests/utils/generate-coverage.php

Lines 16 to 18 in 16ee1c9

    
           if ( ! class_exists( 'SebastianBergmann\CodeCoverage\Filter' ) ) { 
        
           	require "{$root_folder}/vendor/autoload.php"; 
        
           }

The path is wrong.

Class mismatch

Sometimes SebastianBergmann\CodeCoverage\ProcessedCodeCoverageData is loaded but SebastianBergmann\CodeCoverage\Data\ProcessedCodeCoverageData is expected (or the other way around?). That means somehow some older versions of that class are loaded. Needs investigating.

Very slow tests

Behat tests with coverage reporting take much longer to process. In the case of entity-command the job is even cancelled after 6 hours because it's still not finished.

We should look into improving this. Maybe pcov is faster than xdebug. Or we split up the job into multiple ones to run in parallel (see #189)

Another one in doctor-command:

Fatal error: Uncaught Error: Call to undefined method SebastianBergmann\CodeCoverage\Filter::includeDirectory() in vendor/wp-cli/wp-cli-tests/utils/generate-coverage.php:21

In that package, SebastianBergmann\CodeCoverage\Filter::addDirectoryToWhitelist() was renamed to SebastianBergmann\CodeCoverage\Filter::includeDirectory() at some point.

doctor-command uses a very old version (phpunit/php-code-coverage (4.0.8)) because it still supports PHP 5.6 (`"php": "5.6" in composer.json)

Another thing I think is not working:

wp-cli-tests/src/Context/FeatureContext.php

Line 697 in 16ee1c9

    
           return preg_replace( '/(^wp )|( wp )|(\/wp )/', '$1$2$3--require={SRC_DIR}/utils/generate-coverage.php ', $cmd );

Putting the --require first seems to break alias commands, because aliases are only extracted from $argv[0] so they must be the first thing after wp

https://github.com/wp-cli/wp-cli/blob/279794bce3f5b92d7fd92e4559061921f433bf64/php/WP_CLI/Runner.php#L1053-L1056

I noticed when testing failures like:

Warning: No WordPress installation found. If the command '@all option get home' is in a plugin or theme, pass --path=`path/to/wordpress`.
Error: '@all' is not a registered wp command. See 'wp help' for available commands.

So maybe that should go at the end of the command?

I knew the regex approach was risky 😬

At the end of the command there could be pipes etc. too, so need to be careful there as well.

Maybe we could use one of the special env vars we support to load this really early?

Maybe we could use one of the special env vars we support to load this really early?

I was thinking of WP_CLI_EARLY_REQUIRE here but there's a test in wp-cli/wp-cli that actually uses that var, so that wouldn't work.

We could probably figure out how to handle pipes, for example if we split on pipe characters and then applied the preg_replace which already checks that it is a wp command, then put it back together. Here is a quick test

<?php
$commands = [
    'COLUMNS=80 wp help test-wordwrap my_command | awk \'{print length, $0}\' | sort -nr | head -1 | cut -f1 -d" "',
    'wp plugin list | grep active',
    'some_command | wp eval "echo get_option(\'siteurl\');" | another_command',
    'echo "Testing" | wp db query "SELECT * FROM wp_options" | cut -d" " -f2'
];

foreach ($commands as $cmd) {
    $parts = explode('|', $cmd);

    foreach ($parts as &$part) {
        if (preg_match('/(^wp )|( wp )|(\/wp )/', $part)) {

            $part = preg_replace('/(^wp )|( wp )|(\/wp )/', '$1$2$3', $part);
            $part .= ' --require={SRC_DIR}/utils/generate-coverage.php';
        }
    }

    $updated_cmd = implode('|', $parts);
    echo "Original: $cmd\n";
    echo "Updated:  $updated_cmd\n\n";
}

with output:

Original: COLUMNS=80 wp help test-wordwrap my_command | awk '{print length, $0}' | sort -nr | head -1 | cut -f1 -d" "
Updated:  COLUMNS=80 wp help test-wordwrap my_command  --require={SRC_DIR}/utils/generate-coverage.php| awk '{print length, $0}' | sort -nr | head -1 | cut -f1 -d" "

Original: wp plugin list | grep active
Updated:  wp plugin list  --require={SRC_DIR}/utils/generate-coverage.php| grep active

Original: some_command | wp eval "echo get_option('siteurl');" | another_command
Updated:  some_command | wp eval "echo get_option('siteurl');"  --require={SRC_DIR}/utils/generate-coverage.php| another_command

Original: echo "Testing" | wp db query "SELECT * FROM wp_options" | cut -d" " -f2
Updated:  echo "Testing" | wp db query "SELECT * FROM wp_options"  --require={SRC_DIR}/utils/generate-coverage.php| cut -d" " -f2

Maybe we could use one of the special env vars we support to load this really early?

I was thinking of WP_CLI_EARLY_REQUIRE here but there's a test in wp-cli/wp-cli that actually uses that var, so that wouldn't work.

Thinking again... We could add support for WP_CLI_EARLY_REQUIRE to accept a comma-separated list of files to include. That feels less fragile than the regex approach.

Interestingly, the following framework test now seems to be failing after #242:

https://github.com/wp-cli/wp-cli/blob/d323e5dee744444c8605f52303bdf37b5e65e717/features/bootstrap.feature#L177-L254

Sounds like an autoloading change.

The wp-cli/wp-cli-bundle coverage tests only seem to be failing because old PHP 5.6 compatible dependencies are pulled in

PHP Fatal error: Uncaught Error: Call to undefined method SebastianBergmann\CodeCoverage\Filter::includeFiles() in vendor/wp-cli/wp-cli-tests/utils/generate-coverage.php:61

-> the method used to be called addFilesToWhitelist()

The former should be fixed by wp-cli/wp-cli#6069

For the latter I think we should just wait until we drop the 5.6 requirement.

Two more failures:

config-command: https://github.com/wp-cli/config-command/actions/runs/13868320710/job/38811601871#step:11:78

The command that's being called in the end is EDITOR='ex -i NONE -c q!' wp config edit; --require=vendor/wp-cli/wp-cli-tests/utils/generate-coverage.php

I think we should simply remove that semicolon from the test here: https://github.com/wp-cli/config-command/blob/9055fe048b63aed70cbce64067a71d9ac0cec027/features/config-edit.feature#L6

core-command: https://github.com/wp-cli/core-command/actions/runs/13868325394/job/38811610524#step:11:96

Test in question: https://github.com/wp-cli/core-command/blob/f114e2f8347ce48a839d33c7220e47a5e16727c3/features/core-download.feature#L31-L36

The command that's being called in the end is WP_CLI_STRICT_ARGS_MODE=1 wp core download --path=inner --require=vendor/wp-cli/wp-cli-tests/utils/generate-coverage.php

It uses WP_CLI_STRICT_ARGS_MODE , which tells WP-CLI to treat any arguments before the command as global, and after the command as local.

So the correct order in this case would be to use WP_CLI_STRICT_ARGS_MODE=1 wp --require=vendor/wp-cli/wp-cli-tests/utils/generate-coverage.php core download --path=inner

Maybe we should refine our existing regex solution to move it back to the beginning but handle aliases more clearly.

Then we could also add such a test scenario in this repo here to catch these situations earlier.

Hmm yea. I will say this part was much nicer using an env var like WP_CLI_EARLY_REQUIRE (before it didn't actually work 😅) Do you think it is worth adding a similar WP_CLI_REQUIRE for this purpose? I know we support require via config files as well but there didn't seem to be a clean way to do that conditionally for just the test runs. Doing something like that feels more right than trying to parse out random strings like this and stick something in exactly the right spot.

That is a good idea, it makes sense to me 👍 It should be possible to have parity like that.

config-command and core-command are now green again 🎉

https://github.com/wp-cli/config-command/actions/runs/13931082007

https://github.com/wp-cli/core-command/actions/runs/13931080335

@swissspidy

@swissspidy (not sure if this link will maintain) but do you happen to know why sometimes it says a line isn't covered by tests when it clearly is. Simple example being: https://github.com/wp-cli/rewrite-command/pull/69/files#diff-1aa1c0fcc9fb4a3ea9f9a7967b93735391117a3b0df903af0f358f44605b0f59R362

Great question...

This is the coverage file for the Warn the user when --skip-plugins or --skip-themes is used scenario when I run the tests with coverage locally:

Coverage XML

<?xml version="1.0" encoding="UTF-8"?>
<coverage generated="1742636161">
  <project timestamp="1742636161" name="Manage WordPress rewrites - Warn the user when --skip-plugins or --skip-themes is used">
    <file name="/Users/pascalb/Workspace/WordPress/wp-cli-dev/rewrite-command/rewrite-command.php">
      <line num="3" type="stmt" count="0"/>
      <line num="4" type="stmt" count="0"/>
      <line num="7" type="stmt" count="0"/>
      <line num="8" type="stmt" count="0"/>
      <line num="9" type="stmt" count="0"/>
      <line num="12" type="stmt" count="0"/>
      <metrics loc="13" ncloc="13" classes="0" methods="0" coveredmethods="0" conditionals="0" coveredconditionals="0" statements="6" coveredstatements="0" elements="6" coveredelements="0"/>
    </file>
    <file name="/Users/pascalb/Workspace/WordPress/wp-cli-dev/rewrite-command/src/Rewrite_Command.php">
      <class name="Rewrite_Command" namespace="global">
        <metrics complexity="47" methods="5" coveredmethods="0" conditionals="0" coveredconditionals="0" statements="107" coveredstatements="14" elements="112" coveredelements="14"/>
      </class>
      <line num="58" type="method" name="flush" visibility="public" complexity="6" crap="6.97" count="1"/>
      <line num="60" type="stmt" count="1"/>
      <line num="62" type="stmt" count="1"/>
      <line num="63" type="stmt" count="0"/>
      <line num="66" type="stmt" count="1"/>
      <line num="67" type="stmt" count="0"/>
      <line num="70" type="stmt" count="1"/>
      <line num="72" type="stmt" count="1"/>
      <line num="74" type="stmt" count="1"/>
      <line num="75" type="stmt" count="1"/>
      <line num="77" type="stmt" count="0"/>
      <line num="114" type="method" name="structure" visibility="public" complexity="15" crap="240" count="0"/>
      <line num="115" type="stmt" count="0"/>
      <line num="119" type="stmt" count="0"/>
      <line num="120" type="stmt" count="0"/>
      <line num="121" type="stmt" count="0"/>
      <line num="122" type="stmt" count="0"/>
      <line num="125" type="stmt" count="0"/>
      <line num="127" type="stmt" count="0"/>
      <line num="128" type="stmt" count="0"/>
      <line num="129" type="stmt" count="0"/>
      <line num="130" type="stmt" count="0"/>
      <line num="132" type="stmt" count="0"/>
      <line num="135" type="stmt" count="0"/>
      <line num="138" type="stmt" count="0"/>
      <line num="140" type="stmt" count="0"/>
      <line num="141" type="stmt" count="0"/>
      <line num="142" type="stmt" count="0"/>
      <line num="144" type="stmt" count="0"/>
      <line num="147" type="stmt" count="0"/>
      <line num="149" type="stmt" count="0"/>
      <line num="150" type="stmt" count="0"/>
      <line num="151" type="stmt" count="0"/>
      <line num="153" type="stmt" count="0"/>
      <line num="157" type="stmt" count="0"/>
      <line num="159" type="stmt" count="0"/>
      <line num="163" type="stmt" count="0"/>
      <line num="164" type="stmt" count="0"/>
      <line num="165" type="stmt" count="0"/>
      <line num="166" type="stmt" count="0"/>
      <line num="167" type="stmt" count="0"/>
      <line num="168" type="stmt" count="0"/>
      <line num="169" type="stmt" count="0"/>
      <line num="173" type="stmt" count="0"/>
      <line num="174" type="stmt" count="0"/>
      <line num="176" type="stmt" count="0"/>
      <line num="218" type="method" name="list_" visibility="public" complexity="19" crap="380" count="0"/>
      <line num="219" type="stmt" count="0"/>
      <line num="221" type="stmt" count="0"/>
      <line num="222" type="stmt" count="0"/>
      <line num="223" type="stmt" count="0"/>
      <line num="224" type="stmt" count="0"/>
      <line num="227" type="stmt" count="0"/>
      <line num="229" type="stmt" count="0"/>
      <line num="230" type="stmt" count="0"/>
      <line num="231" type="stmt" count="0"/>
      <line num="232" type="stmt" count="0"/>
      <line num="233" type="stmt" count="0"/>
      <line num="234" type="stmt" count="0"/>
      <line num="235" type="stmt" count="0"/>
      <line num="237" type="stmt" count="0"/>
      <line num="238" type="stmt" count="0"/>
      <line num="239" type="stmt" count="0"/>
      <line num="240" type="stmt" count="0"/>
      <line num="241" type="stmt" count="0"/>
      <line num="242" type="stmt" count="0"/>
      <line num="246" type="stmt" count="0"/>
      <line num="247" type="stmt" count="0"/>
      <line num="248" type="stmt" count="0"/>
      <line num="249" type="stmt" count="0"/>
      <line num="250" type="stmt" count="0"/>
      <line num="251" type="stmt" count="0"/>
      <line num="252" type="stmt" count="0"/>
      <line num="253" type="stmt" count="0"/>
      <line num="256" type="stmt" count="0"/>
      <line num="257" type="stmt" count="0"/>
      <line num="258" type="stmt" count="0"/>
      <line num="260" type="stmt" count="0"/>
      <line num="265" type="stmt" count="0"/>
      <line num="267" type="stmt" count="0"/>
      <line num="268" type="stmt" count="0"/>
      <line num="269" type="stmt" count="0"/>
      <line num="271" type="stmt" count="0"/>
      <line num="274" type="stmt" count="0"/>
      <line num="279" type="stmt" count="0"/>
      <line num="280" type="stmt" count="0"/>
      <line num="282" type="stmt" count="0"/>
      <line num="283" type="stmt" count="0"/>
      <line num="286" type="stmt" count="0"/>
      <line num="287" type="stmt" count="0"/>
      <line num="288" type="stmt" count="0"/>
      <line num="289" type="stmt" count="0"/>
      <line num="293" type="stmt" count="0"/>
      <line num="294" type="stmt" count="0"/>
      <line num="297" type="stmt" count="0"/>
      <line num="300" type="stmt" count="0"/>
      <line num="328" type="method" name="apache_modules" visibility="private" complexity="3" crap="4.94" count="1"/>
      <line num="329" type="stmt" count="1"/>
      <line num="330" type="stmt" count="1"/>
      <line num="331" type="stmt" count="0"/>
      <line num="333" type="stmt" count="0"/>
      <line num="336" type="stmt" count="0"/>
      <line num="339" type="stmt" count="0"/>
      <line num="350" type="method" name="check_skip_plugins_themes" visibility="private" complexity="4" crap="5.40" count="1"/>
      <line num="351" type="stmt" count="1"/>
      <line num="352" type="stmt" count="1"/>
      <line num="353" type="stmt" count="0"/>
      <line num="355" type="stmt" count="1"/>
      <line num="356" type="stmt" count="0"/>
      <line num="358" type="stmt" count="1"/>
      <line num="359" type="stmt" count="1"/>
      <line num="361" type="stmt" count="0"/>
      <line num="362" type="stmt" count="0"/>
      <metrics loc="367" ncloc="200" classes="1" methods="5" coveredmethods="0" conditionals="0" coveredconditionals="0" statements="108" coveredstatements="14" elements="113" coveredelements="14"/>
    </file>
    <metrics files="2" loc="380" ncloc="213" classes="1" methods="5" coveredmethods="0" conditionals="0" coveredconditionals="0" statements="114" coveredstatements="14" elements="119" coveredelements="14"/>
  </project>
</coverage>

The check_skip_plugins_themes at the bottom is the interesting part, as that's the one mentioned in the linked PR comment:

      <line num="350" type="method" name="check_skip_plugins_themes" visibility="private" complexity="4" crap="5.40" count="1"/>
      <line num="351" type="stmt" count="1"/>
      <line num="352" type="stmt" count="1"/>
      <line num="353" type="stmt" count="0"/>
      <line num="355" type="stmt" count="1"/>
      <line num="356" type="stmt" count="0"/>
      <line num="358" type="stmt" count="1"/>
      <line num="359" type="stmt" count="1"/>
      <line num="361" type="stmt" count="0"/>
      <line num="362" type="stmt" count="0"/>

Lines 358 and 359 are this part:

https://github.com/wp-cli/rewrite-command/blob/c7bde759ab0bf3050e0e1171861b36dacf6067f5/src/Rewrite_Command.php#L358-L359

For reference, here is the scenario:

https://github.com/wp-cli/rewrite-command/blob/c7bde759ab0bf3050e0e1171861b36dacf6067f5/features/rewrite.feature#L104-L126

The early return on 359 matches the last ran command in the scenario.

Long story short, this actually makes me wonder whether the coverage file generated for each command overwrites the file... Because it certainly looks like it. And when looking at the /utils/generate-coverage.php it kinda makes sense. The filename is always the same and the php-code-coverage library doesn't seem to automatically merge reports. So I think we would have to do this ourselves.

This SO answer suggests writing the reports to serialized PHP files (instead of XML files) as these can be easily merged together later (maybe in afterScenario()?)

Very slow tests

Behat tests with coverage reporting take much longer to process. In the case of entity-command the job is even cancelled after 6 hours because it's still not finished.

btw looks like this resolved itself thanks to the improved filtering we added.

My assumption with the coverage files was correct.

I'm updating it now to properly create separate files instead of accidentally overriding existing ones. But that means lots of files will be created, and codecov has an upload limit of 150 files per commit. So it definitely makes sense to merge reports.

The merging could be done at the end of the test suite run in FeatureContext. But for newer Behat versions that means Xdebug has to be enabled there with the --xdebug flag. So you'll have to run WP_CLI_TEST_COVERAGE=1 composer behat -- --xdebug to collect coverage and merge coverage files.

The merging can't really happen in generate-coverage.php because it doesn't know whether it's the last run or not. And conceptually it fits better in FeatureContext in a @AfterSuite hook

@swissspidy

I believe all of the original issues here have been addressed and code coverage works as expected in most cases now. @swissspidy if you agree I think we can mark this as closed.

@mrsdizzie

@mrsdizzie Yep, I think so too. The only one that's still bugging me is wp-cli/wp-cli#6069. I wrote down details there today, in case you wanna take a look and have some ideas.

Maybe there is another thing to address: https://github.com/wp-cli/wp-cli-bundle/actions/runs/14753864867/job/41419109307#step:11:22572

Relevant line:

wp-cli-tests/utils/generate-coverage.php

Line 77 in 7ac2ff7

$filter->includeFiles( $files );

In older versions of phpunit/php-code-coverage, the method name was addFilesToWhitelist, so maybe we need to do:

if ( method_exists( $filter, 'include' ) ) {
	$filter->includeFiles( $files );
} else {
	$filter->addFilesToWhitelist( $files );
}

This is only relevant until we bump the PHP 5.6 requirement though. And only for wp-cli-bundle because it locks the config.platform to 5.6, so it installs old versions of everything.

If this is just a temporary issue until we change the PHP requirement after the next release happening soon, could we also just not run the coverage tests for the wp-cli-bundle project until then? Since it isn't going to matter and we aren't going to do anything useful with those results in the next week.

If not and the older method works the same that seems fine as a temporary solution as well.

If this is just a temporary issue until we change the PHP requirement after the next release happening soon, could we also just not run the coverage tests for the wp-cli-bundle project until then?

I don't think there's an easy way to do that at the moment.

But yeah we can just wait this one out :)

Navigation Menu

Repository navigation

Behat code coverage improvements #241

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

	if ( ! class_exists( 'SebastianBergmann\CodeCoverage\Filter' ) ) {
	require "{$root_folder}/vendor/autoload.php";
	}

Behat code coverage improvements #241

Description

Activity

swissspidy commented on Mar 9, 2025

mrsdizzie commented on Mar 10, 2025

swissspidy commented on Mar 10, 2025

swissspidy commented on Mar 10, 2025

mrsdizzie commented on Mar 10, 2025

swissspidy commented on Mar 10, 2025

swissspidy commented on Mar 14, 2025

swissspidy commented on Mar 14, 2025

swissspidy commented on Mar 15, 2025

mrsdizzie commented on Mar 15, 2025

swissspidy commented on Mar 16, 2025

swissspidy commented on Mar 18, 2025

mrsdizzie commented on Mar 21, 2025

swissspidy commented on Mar 22, 2025

swissspidy commented on Mar 23, 2025

swissspidy commented on Mar 24, 2025

mrsdizzie commented on Apr 29, 2025

swissspidy commented on Apr 29, 2025

swissspidy commented on Apr 30, 2025

mrsdizzie commented on Apr 30, 2025

swissspidy commented on May 3, 2025

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions