Fix path in CLI output

[ernilambar]

In PHPCS runner file name is like this:

/private/var/tmp/plugin-check/1725720105/foo-bar-wp/foo-bar-wp.php

Plugin path is:

/var/tmp/plugin-check/1725720105/foo-bar-wp/

So, with str_replace( $result->plugin()->path(), '', $file ),, filename becomes /privatefoo-bar-wp.php

That is why, Behat test is failing locally. Strangely not in GH Action.

[ernilambar]

[ernilambar]

@swissspidy Could this be issue of Docker or something? Any idea?

[github-actions]

The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the props-bot label.

If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.

Co-authored-by: ernilambar <[email protected]>
Co-authored-by: swissspidy <[email protected]>
Co-authored-by: davidperezgar <[email protected]>

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

[ernilambar]

When using PCP in Local WP setup, I get full path like this.

[ernilambar]

Just noticed that this happens only for those coming from Abstract_PHP_CodeSniffer_Check.

[swissspidy]

Could this be issue of Docker or something? Any idea?

Note that these tests don't run in Docker.

None of the Behat tests are failing for me locally.

When using PCP in Local WP setup, I get full path like this.

OK so what I notice from this screenshot is that you are inside wp-content/plugins, not the root folder.

Strangely, I can reproduce that.

When I am in the root folder of the WP install, I get:

$ wp plugin check performance-lab
FILE: includes/admin/load.php

In this scenario, $result->plugin()->path() is the full absolute path to the plugin being tested, i.e. /Users/pascalb/Local Sites/plugin-check/app/public/wp-content/plugins/performance-lab/

But when I cd to wp-content/plugins, I get:

$wp plugin check performance-lab
FILE: /Users/pascalb/Local Sites/plugin-check/app/public/wp-content/plugins/includes/admin/load.php

Now, $result->plugin()->path() is a relative path, i.e. performance-lab/.

From what I can see, the problem is here:

plugin-check/includes/Checker/Abstract_Check_Runner.php

Lines 549 to 557 in 9d72214

	if ( filter_var( $plugin, FILTER_VALIDATE_URL ) ) {
	$this->plugin_basename = Plugin_Request_Utility::download_plugin( $plugin );
	$this->delete_plugin_folder = true;
	} elseif ( Plugin_Request_Utility::is_directory_valid_plugin( $plugin ) ) {
	$this->plugin_basename = $plugin;
	} else {
	$this->plugin_basename = Plugin_Request_Utility::get_plugin_basename_from_input( $plugin );
	}

When you are inside wp-content/plugins, Plugin_Request_Utility::is_directory_valid_plugin( $plugin ) now of course is true.

Hence you get the full paths not the relative ones.

To fix this, we could change the logic in Abstract_Check_Runner::get_plugin_basename() to something like this:

if ( filter_var( $plugin, FILTER_VALIDATE_URL ) ) {
	$this->plugin_basename = Plugin_Request_Utility::download_plugin( $plugin );

	$this->delete_plugin_folder = true;
} else {
	try {
		$this->plugin_basename = Plugin_Request_Utility::get_plugin_basename_from_input( $plugin );
	} catch( Exception $exception ) {
		if ( Plugin_Request_Utility::is_directory_valid_plugin( $plugin ) ) {
			$this->plugin_basename = $plugin;
		} else {
			throw $exception;
		}
	}
}

[ernilambar]

@swissspidy I have updated the PR with your suggestion. This PR does not particularly fix the issue I am having but this will surely make conditional check more robust. So it would be good to merge I believe.

[swissspidy]

Could you maybe elaborate a bit more on the problem you're seeing and how to reproduce it?

[ernilambar]

Like I mentioned earlier in the first comment, FILE: /privatefoo-bar-wp.php when I run composer run behat -- tests/behat/features/plugin-check-remote.feature
Since this is passing in the GH Action, may be this is defect in my setup or something.

For an addon, I am parsing the STDOUT of the plugin check command and output of FILE: xxx was unpredictable. Sometime correct file is given, some time full path and sometime filename with extra path string like above example. So, I have not handled three cases separately in my addon.
I am currently checking in Local WP. I will ping again if I face same issue with reproducible steps.

[swissspidy]

OK I see it now.

For checking a remote ZIP file we are using a location within get_temp_dir()

On Mac, $TMPDIR is somewhere in /var/folders/..., but that's actually in /private/var/folders. In Plugin_Context we were only storing the former though. However, the file names returned by PHPCS had the full path including /private/

So in add_result_message_for_file() when we do the str_replace(), we were stripping everything off the file name except the /private part. Hence you got /privatefoo-bar-wp.php as a result.

As you can see in my attempts above, realpath() would be a way to solve this but I am probably not applying it in the right place :-)

Maybe you wanna take it from here?

[ernilambar]

@swissspidy Are we supposed to have /private in plugin context or removed?

[ernilambar]

realpath() could fix some issue we would have to rewrite several tests. This will give false if file or folder does not exist. But in our PHPUnit test there are several testcase where we are testing such.

[swissspidy]

Are we supposed to have /private in plugin context or removed?

/private/var/folders/ is the full absolute path to the test plugin directory, so I'd say it needs to be included, hence the realpath try.

[ernilambar]

Behat tests are passing and Unit tests are failing due to above mentioned issue.

But this is good sign as it shows it will work. We just need to find alternate way for writing Unit tests.

[swissspidy]

@ernilambar Seems like this one is super close. Maybe something for 1.2.0 even?

[swissspidy]

Debugging leftover?

[ernilambar]

@ernilambar Seems like this one is super close. Maybe something for 1.2.0 even?

I could not make all tests pass to finalize the PR.