Add --all flag for plugin uninstall command by BhargavBhandari90 · Pull Request #84 · wp-cli/extension-command

BhargavBhandari90 · 2018-02-12T11:09:32Z

BhargavBhandari90 · 2018-02-12T19:30:05Z

I think the test is not failing because of my changes.

It's showing error from features/plugin-install.feature and features/upgradables.feature which is not changed.

gitlost · 2018-02-13T19:02:49Z

src/Plugin_Command.php

 	 * will be run.
 	 *
+	 * [--all]
+	 * : If set, all deactive plugins will be uninstalled.


Don't need deactive, as the --deactivate option covers that.

gitlost · 2018-02-13T19:03:35Z

features/plugin-uninstall.feature

+    Then STDOUT should be:
+      """
+      Uninstalled and deleted 'akismet' plugin.
+      Uninstalled and deleted 'hello.php' plugin.


This will change to 'hello' - see below re using check_optional_args_and_all().

It's giving me error in testing if I make it hello.

gitlost · 2018-02-13T19:04:57Z

features/plugin-uninstall.feature

      """
    And the return code should be 0
+
+  Scenario: Uninstall all installed plugin


Typo 'plugin' -> 'plugins'.

gitlost · 2018-02-13T19:06:42Z

features/plugin-uninstall.feature

+      Uninstalled and deleted 'hello.php' plugin.
+      Success: Uninstalled 2 of 2 plugins.
+      """
+    And the return code should be 0


Also should maybe do When I run the previous command again step here with STDOUT Success: No plugins installed..

Also add scenario Uninstall all installed plugins when one or more activated with one plugin activated and run without and with --deactivate flag.

@gitlost I am having some problem with this test. Can you please check.

gitlost · 2018-02-13T19:08:47Z

src/Plugin_Command.php

 		$plugins = $this->fetcher->get_many( $args );
+
+		// If get all parameter from command, then uninstall all deactive plugins.
+		if ( $all ) {


So this block goes.

gitlost · 2018-02-13T19:08:59Z

src/Plugin_Command.php

 		}
 		if ( ! $this->chained_command ) {
-			Utils\report_batch_operation_results( 'plugin', 'uninstall', count( $args ), $successes, $errors );
+			Utils\report_batch_operation_results( 'plugin', 'uninstall', count( $plugins ), $successes, $errors );


And this change isn't needed (if the if ( count( $plugins ) < count( $args ) ) { logic above is put in).

gitlost · 2018-02-13T19:11:15Z

src/Plugin_Command.php

 		$successes = $errors = 0;
+
+		$all = Utils\get_flag_value( $assoc_args, 'all', false );
+


Could you do

if ( ! ( $args = $this->check_optional_args_and_all( $args, $all ) ) ) { return; }

here. This will mean the if ( $all ) { block isn't needed.

gitlost · 2018-02-13T19:20:39Z

src/Plugin_Command.php

+		$all = Utils\get_flag_value( $assoc_args, 'all', false );
+
 		$plugins = $this->fetcher->get_many( $args );
+


Hmm, the other --all commands do this logic after fetching:

if ( count( $plugins ) < count( $args ) ) { $errors = count( $args ) - count( $plugins ); }

which looks like it should be done here too but causes for instance features/plugin-uninstall.feature:28 to fail with Error: No plugins uninstalled. instead of succeeding with Success: Plugin already uninstalled.. And also the final step of features/plugin.feature:3. So this would be a BC break. But maybe the right thing to do anyway...

gitlost · 2018-02-13T19:41:39Z

Ah, so you have to do your review comments in code order, not very second-thoughts friendly...

BhargavBhandari90 · 2018-02-15T12:05:35Z

@gitlost See this.

schlessera · 2018-02-15T12:46:20Z

@BhargavBhandari90 When you run a test that expects an error to be thrown, like it is the case here, you need to use And I try ..., not And I run ....
run => expect successful result
try => expect error

gitlost · 2018-02-15T16:06:18Z

src/Plugin_Command.php

+		}
+
+		// If get all parameter from command, then uninstall all deactive plugins.
+		if ( $all ) {


This block should be removed as it isn't needed after setting the args with check_optional_args_and_all().

This will then change the name of hello.php to hello.

gitlost · 2018-02-15T16:10:08Z

features/plugin-uninstall.feature

+      """
+      Success: Activated 2 of 2 plugins.
+      """
+    And I run `wp plugin uninstall --all`


As @schlessera mentioned need this to be When I try as it will fail without the --deactivate flag. Then have another successful step with the --deactivate flag.

gitlost · 2018-02-15T16:13:15Z

The other failures happening are due to the BC break and are mentioned here #84 (comment). So those tests will have to be adjusted (assuming everyone's ok with the BC break).

schlessera · 2018-02-19T19:14:50Z

features/plugin-uninstall.feature


  Scenario: Attempting to uninstall a plugin that doesn't exist
    When I try `wp plugin uninstall edit-flow`
+    And STDERR should be:


And => Then

schlessera · 2018-02-19T19:15:47Z

features/plugin-uninstall.feature

+    When I run the previous command again
+    Then STDOUT should be:
      """
+      Success: No plugins installed.


No plugins installed => No plugins uninstalled

This will require a change to the check_args_and_all() method, as the installed is currently hardcoded in there.

@schlessera I did little different for this. Let me know if this solution is good or not.

schlessera · 2018-02-19T19:17:27Z

features/plugin-uninstall.feature

+      """
+
+  Scenario:  Uninstall all installed plugins when one or more activated
+    And I run `wp plugin activate --all`


And => When

schlessera · 2018-02-19T19:18:08Z

features/plugin-uninstall.feature

+      """
+      Success: Activated 2 of 2 plugins.
+      """
+    And I try `wp plugin uninstall --all`


And => new line + When

schlessera · 2018-02-19T19:18:22Z

features/plugin-uninstall.feature

+      Success: Activated 2 of 2 plugins.
+      """
+    And I try `wp plugin uninstall --all`
    And STDERR should be:


And => Then

schlessera · 2018-02-19T19:18:43Z

features/plugin-uninstall.feature

+      Error: No plugins uninstalled.
+      """
+    And the return code should be 1
+    And I run `wp plugin uninstall --deactivate --all`


And => new line + When

schlessera · 2018-02-19T19:23:05Z

@BhargavBhandari90 I've added a few change requests.

For the basic tests setup, it should always read like normal sentences:

When ___
Then ___
And ___
[ And ___ ]
[ And ___ ]
...

When ___
Then ___
And ___
[ And ___ ]
[ And ___ ]
...

BhargavBhandari90 · 2018-02-25T05:20:50Z

@schlessera, Done with the changes. Please review.

schlessera · 2018-02-25T09:33:30Z

Thanks for the PR, @BhargavBhandari90 !

Add --all flag for plugin uninstall command

44138cb

BhargavBhandari90 changed the title ~~WIP: #75Add --all flag for plugin uninstall command~~ WIP: #75 Add --all flag for plugin uninstall command Feb 12, 2018

BhargavBhandari90 added 2 commits February 12, 2018 16:47

Add featured test for uninstalling all plugins

2b505b0

Improve feature testing

5fd28a1

BhargavBhandari90 changed the title ~~WIP: #75 Add --all flag for plugin uninstall command~~ Add --all flag for plugin uninstall command Feb 12, 2018

gitlost mentioned this pull request Feb 12, 2018

Allow for modern-wordpress redirect; explicit list not search in upgr… #85

Merged

gitlost suggested changes Feb 13, 2018

View reviewed changes

gitlost added the command:plugin-uninstall Related to 'plugin uninstall' command label Feb 13, 2018

gitlost added this to the 1.1.10 milestone Feb 13, 2018

BhargavBhandari90 added 3 commits February 15, 2018 13:02

Update feature test

f4c9f1f

Update feature test

a5a525a

Update feature test

6a4a37d

gitlost reviewed Feb 15, 2018

View reviewed changes

Improvise logic and feature test

a65e6c2

schlessera requested changes Feb 19, 2018

View reviewed changes

BhargavBhandari90 and others added 5 commits February 22, 2018 22:42

Use past tense verbe according to command

c7760ad

Improve feature testing

c6634f2

Improve feature testing

8db94fc

Merge remote-tracking branch 'upstream/master' into wp-cliGH-75

7edee61

Make plugin uninstall test compat with BC break.

d197330

gitlost approved these changes Feb 24, 2018

View reviewed changes

gitlost added the breaking-change label Feb 24, 2018

schlessera approved these changes Feb 25, 2018

View reviewed changes

schlessera merged commit a4a2553 into wp-cli:master Feb 25, 2018

schlessera mentioned this pull request Feb 25, 2018

Add --all flag to plugin uninstall and delete #75

Closed

gitlost mentioned this pull request Mar 1, 2018

Add --all flag to plugin delete #88

Closed

		$successes = $errors = 0;

		$all = Utils\get_flag_value( $assoc_args, 'all', false );

		$all = Utils\get_flag_value( $assoc_args, 'all', false );

		$plugins = $this->fetcher->get_many( $args );

Conversation

BhargavBhandari90 commented Feb 12, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

BhargavBhandari90 commented Feb 12, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gitlost commented Feb 13, 2018

Uh oh!

BhargavBhandari90 commented Feb 15, 2018

Uh oh!

schlessera commented Feb 15, 2018

Uh oh!

gitlost Feb 15, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gitlost commented Feb 15, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

schlessera commented Feb 19, 2018

Uh oh!

BhargavBhandari90 commented Feb 25, 2018

Uh oh!

schlessera commented Feb 25, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

BhargavBhandari90 commented Feb 12, 2018 •

edited

Loading

gitlost Feb 15, 2018 •

edited

Loading