-
Notifications
You must be signed in to change notification settings - Fork 53
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
CLI: fully structured output for easier parsing, send to a file #748
Comments
Hi there, If you run PS. If your testing is done on GitHub Actions, I'd recommend using https://github.com/WordPress/plugin-check-action directly. |
This is exactly what I'm doing right now. The output looks like:
While yes, this is parse-able (and I'm doing so), it's not as easy as just full JSON output.
Again, I'm doing this already. It includes the status messages, as mentioned, which I would like to leave on the console and not have interleaved with the results.
This is nice (and should be linked from this repo and the wordpress.org page more prominently), but not quite what I need for my use case. It's a good integration though! |
Apologies, I misread then. Reopening. |
Similar issue was raised when PCP was being integrated in the WP.org. Implementing there also output is parsed like you are currently doing. May be we could add FILE value directly in the output as key so that we wont need separate line for Eg: |
That would be super repetitive though. Honestly parsing the current format is not that bad, see https://github.com/WordPress/plugin-check-action/blob/0e9c1a736a7e3c8a32eaafe4db4a694f064be80b/src/main.ts#L24-L45 |
I am using following for validating the check result. It could be reference for future readers. /**
* Checks if given string is valid and expected output from plugin check command.
*
* @since 1.0.0
*
* @param string $input String to check for validity.
* @return bool true if valid, otherwise false.
*/
protected function is_valid_check_result( $input ) {
// Bail if input is empty.
if ( empty( $input ) ) {
return false;
}
// Split the input into blocks based on the pattern of "FILE:".
$blocks = preg_split( '/(?=FILE:)/', $input, -1, PREG_SPLIT_NO_EMPTY );
// Bail if splitted array is empty.
if ( empty( $blocks ) || ! is_array( $blocks ) ) {
return false;
}
foreach ( $blocks as $block ) {
$block = trim( $block );
// Split each block into a filename and a JSON string.
$lines = explode( "\n", $block, 2 );
if ( count( $lines ) < 2 ) {
return false;
}
// Check if the first line is "FILE:" line.
if ( ! str_starts_with( $lines[0], 'FILE:' ) ) {
return false;
}
// Check if the second part is a valid JSON.
if ( ! $this->is_valid_json_string( $lines[1] ) ) {
return false;
}
}
return true;
} |
May be we could introduce format like |
FYI for anyone thinking about picking this up, fully fixing it will AFAIK require wp-cli changes, not just plugin-check changes, based on my initial investigation/attempt to fix it myself. On the plugin-check side, I believe a new flag like To add a new format type instead would require wp-cli changes, since those formats are all defined in Because that formatter is also what's writing the output, getting an output file argument down to it would also require wp-cli changes. Alternatively, plugin-check could take over the formatting itself in this case. Something like a TLDR: it's possible, but either slightly hacky (take things away from wp-cli and into plugin-check), or more involved (make wp-cli changes first, and then use them in plugin-check once released). |
I'm integrating plugin-check into a testing pipeline via wp-cli, and consuming the output programmatically is a little annoying (though totally doable) at the moment. Two things that would make it easier:
FILE: path.php\n<json blob>
there is now. This is relatively easy to parse, but still requires custom code to do so (and associate the path with each entry). Either a nested structure like[{path: "path.php", items: <same json blob as now>}, ...]
or just a straight list like[<single item as now but with path added>,...]
would work fine for this. If memory is a concern, then the second option would also work in a line-by-line format, rather than one big JSON array.The text was updated successfully, but these errors were encountered: