Behat code coverage improvements

[swissspidy]

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

See also this related Slack discussion.

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)

[swissspidy]

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)

[mrsdizzie]

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?

[swissspidy]

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?

[swissspidy]

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.

[mrsdizzie]

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

[swissspidy]

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.