Add Categories and Checks CLI commands #339

ernilambar · 2023-12-14T10:18:19Z

Fixes #303

Command:
wp plugin list-check-categories

Output:

Command:
wp plugin list-checks

Output:

swissspidy · 2023-12-21T09:57:37Z

includes/CLI/Plugin_Check_Checks_Command.php

+		// Create the runner if not already initialized.
+		if ( is_null( $runner ) ) {
+			$runner = new CLI_Checks_Runner();
+		}
+
+		// Make sure we are using the correct runner instance.
+		if ( ! ( $runner instanceof CLI_Checks_Runner ) ) {
+			WP_CLI::error(
+				__( 'CLI Checks Runner was not initialized correctly.', 'plugin-check' )
+			);
+		}


Hmm I really don't like that this simple command requires a new *_Runner just because the check registration happens in Abstract_Check_Runner::register_checks()

Especially since the current CLI_Runner class is mostly a hack anyway

I think we need some refactoring to move the registration out of the runner class, maybe to a factory method on Default_Check_Repository or so.

Listing all registered checks should be as simple as getting a Default_Check_Repository and listing what's in there.

cc @felixarntz for thoughts

swissspidy · 2023-12-21T09:58:38Z

includes/CLI/Plugin_Check_Categories_Command.php

+		/*
+		 * Bail early if the Plugin Checker is not activated.
+		 *
+		 * If the Plugin Checker is not activated, it will throw an error and
+		 * CLI commands won't be usable.
+		 */
+		if ( is_plugin_inactive( $this->plugin_context->basename() ) ) {
+			WP_CLI::error(
+				__( 'Plugin Checker is not active.', 'plugin-check' )
+			);
+		}


This code can be removed as it's dead code.

This class can only ever be loaded when the plugin is active, so it does not make sense to check whether the plugin is active.

swissspidy · 2023-12-21T10:00:56Z

includes/CLI/Plugin_Check_Categories_Command.php

+/**
+ * Plugin check categories command.
+ */
+final class Plugin_Check_Categories_Command {


Would it be possible to add some functional tests for this command in tests/behat/features/list-categories.feature?

swissspidy · 2023-12-21T10:01:05Z

includes/CLI/Plugin_Check_Checks_Command.php

+/**
+ * Plugin check checks command.
+ */
+final class Plugin_Check_Checks_Command {


Would it be possible to add some functional tests for this command in tests/behat/features/list-checks.feature?

cli.php

felixarntz

Thank you for the PR @ernilambar! Looks great so far. I've added a few points of feedback below. Let me know if you have any questions.

includes/CLI/Plugin_Check_Categories_Command.php

includes/CLI/Plugin_Check_Checks_Command.php

ernilambar · 2023-12-22T12:16:33Z

Code is updated as per the comments. New commands will be:

wp plugin list-checks
wp plugin list-check-categories

Behat tests implementation for new commands is remaining.

@swissspidy Is it possible to run behat tests in local environment? Or is it implemented now in GitHub actions only?

swissspidy · 2023-12-22T12:18:28Z

Use composer behat to run the Behat tests.

You may need to run composer prepare-behat-tests once first to create the test database

ernilambar · 2023-12-25T07:58:13Z

PR ready for review.

CLI commands updated as per the comments.
Behat tests added
Updated PHP Unit tests to reflect new changes.

felixarntz

@ernilambar The logic itself looks great now. I think there are a couple places where we can drastically simplify the code and avoid duplicate code, most importantly by using the existing Plugin_Check_Command class for the 2 new CLI commands.

cli.php

includes/CLI/Plugin_List_Check_Categories_Command.php

includes/CLI/Plugin_List_Checks_Command.php

tests/phpunit/tests/Checker/Default_Check_Collection_Tests.php

tests/phpunit/tests/Checker/Default_Check_Repository_Tests.php

includes/CLI/Plugin_List_Checks_Command.php

includes/CLI/Plugin_List_Check_Categories_Command.php

ernilambar · 2024-01-03T11:34:57Z

PR has been update as per the review comments.

Note:

Since PHPMD was generating error, I have added ignore @SuppressWarnings(PHPMD.ExcessiveClassComplexity) in Plugin_Check_Command.
get_list_check_categories_options() and get_list_checks_options() methods are similar now. But there is some room for enhancements for those listing commands. When we implement those, methods would be different.

felixarntz

@ernilambar I think this is almost good to go now. A few follow up feedback points.

includes/CLI/Plugin_Check_Command.php

includes/Checker/Default_Check_Repository.php

includes/Checker/Empty_Check_Repository.php

tests/phpunit/tests/Checker/Check_Categories_Tests.php

includes/CLI/Plugin_Check_Command.php

felixarntz

This looks great @ernilambar!

Just a few last nit-picks, but no real blockers. Thank you for all your work on this! 🙌

includes/CLI/Plugin_Check_Command.php

mukeshpanchal27 · 2024-01-05T05:29:04Z

In the follow-up PR, it's essential to update the documentation to reflect the changes made by this pull request. Specifically, we removed the Abstract_Check_Runner::register_checks() method. cc. @felixarntz

The affected documentation files that need updating are:

ernilambar · 2024-01-05T05:55:51Z

PR updated as per the comments. Fixed spacing issues and improved function docs.

ernilambar force-pushed the 303-cli-commands branch from 7fbb19b to 33d5e39 Compare December 14, 2023 10:23

swissspidy reviewed Dec 21, 2023

View reviewed changes

felixarntz requested changes Dec 21, 2023

View reviewed changes

includes/CLI/Plugin_Check_Categories_Command.php Outdated Show resolved Hide resolved

includes/CLI/Plugin_Check_Checks_Command.php Outdated Show resolved Hide resolved

includes/CLI/Plugin_Check_Checks_Command.php Outdated Show resolved Hide resolved

ernilambar force-pushed the 303-cli-commands branch 3 times, most recently from 361e9f3 to 0074b28 Compare December 22, 2023 12:04

ernilambar force-pushed the 303-cli-commands branch 11 times, most recently from e6d16e9 to 29e177f Compare December 25, 2023 07:54

felixarntz reviewed Jan 2, 2024

View reviewed changes

ernilambar force-pushed the 303-cli-commands branch 8 times, most recently from aa2f059 to e6ea8b2 Compare January 3, 2024 11:21

Add List Checks and List Check Categories commands

6e79e77

felixarntz reviewed Jan 3, 2024

View reviewed changes

includes/Checker/Default_Check_Repository.php Outdated Show resolved Hide resolved

includes/Checker/Empty_Check_Repository.php Outdated Show resolved Hide resolved

tests/phpunit/tests/Checker/Check_Categories_Tests.php Outdated Show resolved Hide resolved

ernilambar force-pushed the 303-cli-commands branch 7 times, most recently from c07f66f to 772805c Compare January 4, 2024 06:23

ernilambar commented Jan 4, 2024

View reviewed changes

includes/CLI/Plugin_Check_Command.php Show resolved Hide resolved

ernilambar commented Jan 4, 2024

View reviewed changes

includes/CLI/Plugin_Check_Command.php Show resolved Hide resolved

ernilambar force-pushed the 303-cli-commands branch from 772805c to e965985 Compare January 4, 2024 06:50

Simplify methods in check CLI class

1934de3

ernilambar force-pushed the 303-cli-commands branch from e965985 to 1934de3 Compare January 4, 2024 06:56

felixarntz approved these changes Jan 4, 2024

View reviewed changes

felixarntz requested a review from swissspidy January 4, 2024 16:48

felixarntz mentioned this pull request Jan 4, 2024

Introduce proper user-facing names for check categories #373

Closed

mukeshpanchal27 reviewed Jan 5, 2024

View reviewed changes

includes/CLI/Plugin_Check_Command.php Outdated Show resolved Hide resolved

ernilambar force-pushed the 303-cli-commands branch from c6680b1 to b1eb98b Compare January 5, 2024 05:44

Fix spacing issues and improve function docs

4b879dc

ernilambar force-pushed the 303-cli-commands branch from b1eb98b to 4b879dc Compare January 5, 2024 05:50

This was referenced Jan 5, 2024

Update documentation after moving default checks registration #374

Merged

Validate fields names #361

Closed

swissspidy approved these changes Jan 5, 2024

View reviewed changes

swissspidy merged commit 0e2a2ed into WordPress:trunk Jan 5, 2024
25 checks passed

swissspidy mentioned this pull request Jan 5, 2024

Introduce check category labels #375

Merged

ernilambar deleted the 303-cli-commands branch January 5, 2024 14:40

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add Categories and Checks CLI commands #339

Add Categories and Checks CLI commands #339

ernilambar commented Dec 14, 2023 •

edited

Loading

swissspidy Dec 21, 2023

swissspidy Dec 21, 2023

swissspidy Dec 21, 2023

swissspidy Dec 21, 2023

felixarntz left a comment

ernilambar commented Dec 22, 2023

swissspidy commented Dec 22, 2023

ernilambar commented Dec 25, 2023

felixarntz left a comment

ernilambar commented Jan 3, 2024

felixarntz left a comment

felixarntz left a comment

mukeshpanchal27 commented Jan 5, 2024

ernilambar commented Jan 5, 2024

Add Categories and Checks CLI commands #339

Add Categories and Checks CLI commands #339

Conversation

ernilambar commented Dec 14, 2023 • edited Loading

swissspidy Dec 21, 2023

Choose a reason for hiding this comment

swissspidy Dec 21, 2023

Choose a reason for hiding this comment

swissspidy Dec 21, 2023

Choose a reason for hiding this comment

swissspidy Dec 21, 2023

Choose a reason for hiding this comment

felixarntz left a comment

Choose a reason for hiding this comment

ernilambar commented Dec 22, 2023

swissspidy commented Dec 22, 2023

ernilambar commented Dec 25, 2023

felixarntz left a comment

Choose a reason for hiding this comment

ernilambar commented Jan 3, 2024

felixarntz left a comment

Choose a reason for hiding this comment

felixarntz left a comment

Choose a reason for hiding this comment

mukeshpanchal27 commented Jan 5, 2024

ernilambar commented Jan 5, 2024

ernilambar commented Dec 14, 2023 •

edited

Loading