Skip to content
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

Add an --exclude=<plugin> argument to wp plugin verify-checksums #104

Conversation

stoyan-g
Copy link
Contributor

@stoyan-g stoyan-g commented Apr 7, 2023

I've added an exclude option flag to the wp plugin verify-checksums cli command. This will skip the verification and increase the skipped count and also will not show the "Could not retrieve the version for" warning inside the console.

Copy link
Member

@danielbachhuber danielbachhuber left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the pull request!

Can you include some functional tests for this change? Here is some guidance on our pull request best practices, if it's helpful.

@stoyan-g
Copy link
Contributor Author

stoyan-g commented Apr 7, 2023

@danielbachhuber, sure thing! Will do them!

@danielbachhuber
Copy link
Member

@stoyan-g Still planning to add some tests here?

@stoyan-g
Copy link
Contributor Author

Hey there @danielbachhuber,
Sorry for the delay. Will add them tomorrow

Fix some formatting

Add feature test

Fix issues after tests
@stoyan-g stoyan-g force-pushed the Issue#101-Could-not-retrieve-the-checksums-still-visible-with-skip-plugins-flag branch from fe97228 to cb9fe73 Compare June 6, 2023 09:51
@stoyan-g
Copy link
Contributor Author

stoyan-g commented Jun 6, 2023

Hey there @danielbachhuber , did some changes and rebased the branch since it was a bit behind main and there were some issues with the behat tests. Hope everything runs smooth now.

@danielbachhuber danielbachhuber self-requested a review June 6, 2023 10:45
Then STDOUT should contain:
"""
Verified 0 of 1 plugins (1 skipped).
"""
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Given the test failures, I think I'd recommend testing against akismet instead of hello-dolly.

Can we also include an assertion that the plugin is verified when the --exclude= argument isn't included?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've added the assertion you've mentioned and also improved the previous Scenario a bit

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@stoyan-g Looks better!

It looks like the tests aren't yet passing, and there's a PHPCS failure. Can you make sure everything passes as expected?

I think I'd recommend deleting hello-dolly at the beginning of each Scenario, so your test is running directly against akismet.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@danielbachhuber, yup I will fix the PHPCSS... I cannot believe I missed that... I've noticed that in the previous Scenarios, the plugins installed with a command for example:

 Scenario: Verify plugin checksums
    Given a WP install

    When I run `wp plugin install duplicate-post --version=3.2.1`
    Then STDOUT should not be empty
    And STDERR should be empty

and from the test result I see that mine fail with the following

Scenario: Plugin is verified when the --exclude argument isn't included # features/checksum-plugin.feature:189
      And these installed and active plugins:                               # features/checksum-plugin.feature:191
        $ wp plugin install akismet --activate
        Installing Akismet Anti-Spam: Spam Protection (5.1)
        Downloading install package from https://downloads.wordpress.org/plugin/akismet.5.1.zip...
        Using cached file '/tmp/wp-cli-home/.wp-cli/cache/plugin/akismet-5.1.zip'...
        Unpacking the package...
        Installing the plugin...
        Plugin install failed.

Is it better to do it with the command or rely on the environment to install it for me?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@stoyan-g akismet is bundled with WordPress, and it's safe to assume it's available.

@danielbachhuber danielbachhuber changed the title Issue#101 could not retrieve the checksums still visible with skip plugins flag Add an --exclude=<plugin> argument to wp plugin verify-checksums Jun 6, 2023
@danielbachhuber danielbachhuber added this to the 2.2.3 milestone Jun 6, 2023
@danielbachhuber
Copy link
Member

Nice work on this, @stoyan-g !

@danielbachhuber danielbachhuber merged commit 6d2be27 into wp-cli:main Jun 8, 2023
30 checks passed
@kanlukasz
Copy link

Hello,
In which version of WP-CLI i can use this?
I have WP-CLI 2.8.1 but there is error:

Error: Parameter errors:
 unknown --exclude parameter

@danielbachhuber
Copy link
Member

@kanlukasz It's unreleased, but you can use it in the nightly:

wp cli update --nightly

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants