From a0ff9e26897a8413351f121e8bb80d810fdfb3b9 Mon Sep 17 00:00:00 2001 From: Huzaifa Al Mesbah Date: Wed, 21 Oct 2026 02:38:42 +0600 Subject: [PATCH 1/2] Tests: Add unit test coverage for the Plugin_Upgrader upgrade callbacks. --- .../admin/pluginUpgraderUpgradeHooks.php | 349 ++++++++++++++++++ 1 file changed, 349 insertions(+) create mode 100644 tests/phpunit/tests/admin/pluginUpgraderUpgradeHooks.php diff --git a/tests/phpunit/tests/admin/pluginUpgraderUpgradeHooks.php b/tests/phpunit/tests/admin/pluginUpgraderUpgradeHooks.php new file mode 100644 index 0000000000000..18cfe016b5fa1 --- /dev/null +++ b/tests/phpunit/tests/admin/pluginUpgraderUpgradeHooks.php @@ -0,0 +1,349 @@ +had_filesystem = array_key_exists( 'wp_filesystem', $GLOBALS ); + $this->original_filesystem = $this->had_filesystem ? $GLOBALS['wp_filesystem'] : null; + + $this->filesystem = $this->createMock( 'WP_Filesystem_Base' ); + $this->filesystem->method( 'wp_plugins_dir' )->willReturn( self::PLUGINS_DIR ); + $this->filesystem->method( 'abspath' )->willReturn( '/remote/' ); + + $GLOBALS['wp_filesystem'] = $this->filesystem; + + $this->upgrader = new Plugin_Upgrader( $this->createMock( 'WP_Upgrader_Skin' ) ); + $this->upgrader->generic_strings(); + $this->upgrader->upgrade_strings(); + + update_option( 'active_plugins', array() ); + update_site_option( 'active_sitewide_plugins', array() ); + } + + /** + * Restores the filesystem global. + */ + public function tear_down() { + if ( $this->had_filesystem ) { + $GLOBALS['wp_filesystem'] = $this->original_filesystem; + } else { + unset( $GLOBALS['wp_filesystem'] ); + } + + parent::tear_down(); + } + + /** + * Tests that an error response is passed through by each callback. + * + * @ticket 65819 + * + * @dataProvider data_callbacks + * + * @param string $callback The callback name. + */ + public function test_should_pass_error_through( $callback ) { + // Background upgrades only toggle maintenance mode, while manual ones deactivate the plugin. + add_filter( 'wp_doing_cron', 'deactivate_plugin_before_upgrade' === $callback ? '__return_false' : '__return_true' ); + update_option( 'active_plugins', array( self::FILE ) ); + + $this->filesystem->method( 'exists' )->willReturn( true ); + $this->filesystem->expects( $this->never() )->method( 'delete' ); + $this->filesystem->expects( $this->never() )->method( 'put_contents' ); + + $error = new WP_Error( 'previous_error', 'Previous error.' ); + $args = array( 'plugin' => self::FILE ); + + if ( 'delete_old_plugin' === $callback ) { + $result = $this->upgrader->delete_old_plugin( $error, '', '', $args ); + } else { + $result = $this->upgrader->$callback( $error, $args ); + } + + $this->assertSame( $error, $result, 'The error should be returned.' ); + $this->assertSame( array( self::FILE ), get_option( 'active_plugins' ), 'The plugin should stay active.' ); + } + + /** + * Data provider. + * + * @return array[] + */ + public static function data_callbacks() { + return self::text_array_to_dataprovider( array( 'deactivate_plugin_before_upgrade', 'active_before', 'active_after', 'delete_old_plugin' ) ); + } + + /** + * Tests that an active plugin is deactivated silently before a manual upgrade. + * + * @ticket 65819 + */ + public function test_deactivate_should_deactivate_active_plugin_silently() { + update_option( 'active_plugins', array( 'other-plugin/other-plugin.php', self::FILE ) ); + + $deactivate = new MockAction(); + add_action( 'deactivate_plugin', array( $deactivate, 'action' ) ); + + $this->assertTrue( $this->upgrader->deactivate_plugin_before_upgrade( true, array( 'plugin' => self::FILE ) ), 'The response should be returned.' ); + $this->assertSame( array( 'other-plugin/other-plugin.php' ), array_values( get_option( 'active_plugins' ) ), 'Only the upgraded plugin should be deactivated.' ); + $this->assertSame( 0, $deactivate->get_call_count(), 'No deactivation hooks should run.' ); + } + + /** + * Tests that an inactive plugin is left alone before a manual upgrade. + * + * @ticket 65819 + */ + public function test_deactivate_should_leave_inactive_plugin() { + update_option( 'active_plugins', array( 'other-plugin/other-plugin.php' ) ); + + $this->assertTrue( $this->upgrader->deactivate_plugin_before_upgrade( true, array( 'plugin' => self::FILE ) ), 'The response should be returned.' ); + $this->assertSame( array( 'other-plugin/other-plugin.php' ), get_option( 'active_plugins' ), 'The active plugins should not change.' ); + } + + /** + * Tests that a plugin is not deactivated during a background upgrade. + * + * @ticket 65819 + */ + public function test_deactivate_should_not_deactivate_plugin_in_cron() { + add_filter( 'wp_doing_cron', '__return_true' ); + update_option( 'active_plugins', array( self::FILE ) ); + + $this->assertTrue( $this->upgrader->deactivate_plugin_before_upgrade( true, array( 'plugin' => self::FILE ) ), 'The response should be returned.' ); + $this->assertSame( array( self::FILE ), get_option( 'active_plugins' ), 'The plugin should stay active.' ); + } + + /** + * Tests that a missing plugin is a bad request. + * + * @ticket 65819 + * + * @dataProvider data_missing_plugins + * + * @param string $callback The callback name. + * @param array $args The hook extra arguments. + */ + public function test_should_return_bad_request_without_plugin( $callback, $args ) { + if ( 'delete_old_plugin' === $callback ) { + $result = $this->upgrader->delete_old_plugin( true, '', '', $args ); + } else { + $result = $this->upgrader->$callback( true, $args ); + } + + $this->assertWPError( $result, 'An error should be returned.' ); + $this->assertSame( 'bad_request', $result->get_error_code(), 'The error code should match.' ); + $this->assertSame( 'Invalid data provided.', $result->get_error_message(), 'The error message should match.' ); + } + + /** + * Data provider. + * + * @return array[] + */ + public static function data_missing_plugins() { + return array( + 'deactivate, no plugin key' => array( 'deactivate_plugin_before_upgrade', array() ), + 'deactivate, empty plugin' => array( 'deactivate_plugin_before_upgrade', array( 'plugin' => '' ) ), + 'delete, no plugin key' => array( 'delete_old_plugin', array() ), + 'delete, empty plugin' => array( 'delete_old_plugin', array( 'plugin' => '' ) ), + ); + } + + /** + * Tests that maintenance mode is enabled and disabled for an active plugin during a background upgrade. + * + * Runs in a separate process because the maintenance file is written with FS_CHMOD_FILE, + * which WP_Filesystem() defines. + * + * @ticket 65819 + * + * @runInSeparateProcess + * @preserveGlobalState disabled + */ + public function test_active_before_and_after_should_toggle_maintenance_mode_in_cron() { + define( 'FS_CHMOD_FILE', 0644 ); + + add_filter( 'wp_doing_cron', '__return_true' ); + update_option( 'active_plugins', array( self::FILE ) ); + + $this->filesystem->expects( $this->once() ) + ->method( 'put_contents' ) + ->with( '/remote/.maintenance', $this->stringStartsWith( 'filesystem->method( 'exists' )->willReturn( true ); + $this->filesystem->expects( $this->exactly( 2 ) )->method( 'delete' )->with( '/remote/.maintenance' ); + + $this->assertTrue( $this->upgrader->active_before( true, array( 'plugin' => self::FILE ) ), 'active_before() should return the response.' ); + $this->assertTrue( $this->upgrader->active_after( true, array( 'plugin' => self::FILE ) ), 'active_after() should return the response.' ); + } + + /** + * Tests that maintenance mode is left alone unless an active plugin is upgraded alone in the background. + * + * @ticket 65819 + * + * @dataProvider data_no_maintenance_mode + * + * @param bool $cron Whether the upgrade runs in cron. + * @param bool $active Whether the plugin is active. + * @param bool $bulk Whether it is a bulk upgrade. + */ + public function test_active_before_and_after_should_not_toggle_maintenance_mode( $cron, $active, $bulk ) { + add_filter( 'wp_doing_cron', $cron ? '__return_true' : '__return_false' ); + update_option( 'active_plugins', $active ? array( self::FILE ) : array() ); + $this->upgrader->bulk = $bulk; + + $this->filesystem->method( 'exists' )->willReturn( true ); + $this->filesystem->expects( $this->never() )->method( 'put_contents' ); + $this->filesystem->expects( $this->never() )->method( 'delete' ); + + $this->assertTrue( $this->upgrader->active_before( true, array( 'plugin' => self::FILE ) ), 'active_before() should return the response.' ); + $this->assertTrue( $this->upgrader->active_after( true, array( 'plugin' => self::FILE ) ), 'active_after() should return the response.' ); + } + + /** + * Data provider. + * + * @return array[] + */ + public static function data_no_maintenance_mode() { + return array( + 'manual upgrade' => array( false, true, false ), + 'inactive plugin' => array( true, false, false ), + 'bulk upgrade' => array( true, true, true ), + ); + } + + /** + * Tests how the old plugin files are deleted. + * + * @ticket 65819 + * + * @dataProvider data_old_plugin_files + * + * @param string $plugin The plugin file. + * @param string $path The expected deleted path. + * @param bool $recursive Whether the deletion is expected to be recursive. + */ + public function test_delete_old_plugin_should_delete_plugin_files( $plugin, $path, $recursive ) { + $this->filesystem->method( 'exists' )->willReturn( true ); + $this->filesystem->expects( $this->once() ) + ->method( 'delete' ) + ->with( $path, $recursive ) + ->willReturn( true ); + + $this->assertTrue( $this->upgrader->delete_old_plugin( null, '', '', array( 'plugin' => $plugin ) ) ); + } + + /** + * Data provider. + * + * @return array[] + */ + public static function data_old_plugin_files() { + return array( + 'plugin in a directory' => array( self::FILE, self::PLUGINS_DIR . 'my-plugin/', true ), + 'single-file plugin' => array( 'my-plugin.php', self::PLUGINS_DIR . 'my-plugin.php', false ), + ); + } + + /** + * Tests that a plugin directory that is already gone is not deleted. + * + * @ticket 65819 + */ + public function test_delete_old_plugin_should_skip_missing_directory() { + $this->filesystem->expects( $this->once() ) + ->method( 'exists' ) + ->with( self::PLUGINS_DIR . 'my-plugin/' ) + ->willReturn( false ); + $this->filesystem->expects( $this->never() )->method( 'delete' ); + + $this->assertNull( $this->upgrader->delete_old_plugin( null, '', '', array( 'plugin' => self::FILE ) ) ); + } + + /** + * Tests that a failed deletion is reported. + * + * @ticket 65819 + */ + public function test_delete_old_plugin_should_report_failed_deletion() { + $this->filesystem->method( 'exists' )->willReturn( true ); + $this->filesystem->method( 'delete' )->willReturn( false ); + + $result = $this->upgrader->delete_old_plugin( null, '', '', array( 'plugin' => self::FILE ) ); + + $this->assertWPError( $result, 'An error should be returned.' ); + $this->assertSame( 'remove_old_failed', $result->get_error_code(), 'The error code should match.' ); + $this->assertSame( 'Could not remove the old plugin.', $result->get_error_message(), 'The error message should match.' ); + } +} From 9f4985a0629c351efc1764b6e72b70be2be6e5a1 Mon Sep 17 00:00:00 2001 From: Huzaifa Al Mesbah Date: Wed, 21 Oct 2026 12:32:17 +0600 Subject: [PATCH 2/2] Tests: Run the Plugin_Upgrader maintenance mode test in the main process. --- .../tests/admin/pluginUpgraderUpgradeHooks.php | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/tests/phpunit/tests/admin/pluginUpgraderUpgradeHooks.php b/tests/phpunit/tests/admin/pluginUpgraderUpgradeHooks.php index 18cfe016b5fa1..0729681e1a15c 100644 --- a/tests/phpunit/tests/admin/pluginUpgraderUpgradeHooks.php +++ b/tests/phpunit/tests/admin/pluginUpgraderUpgradeHooks.php @@ -222,23 +222,21 @@ public static function data_missing_plugins() { /** * Tests that maintenance mode is enabled and disabled for an active plugin during a background upgrade. * - * Runs in a separate process because the maintenance file is written with FS_CHMOD_FILE, - * which WP_Filesystem() defines. + * The maintenance file is written with FS_CHMOD_FILE, which WP_Filesystem() defines. * * @ticket 65819 - * - * @runInSeparateProcess - * @preserveGlobalState disabled */ public function test_active_before_and_after_should_toggle_maintenance_mode_in_cron() { - define( 'FS_CHMOD_FILE', 0644 ); + if ( ! defined( 'FS_CHMOD_FILE' ) ) { + define( 'FS_CHMOD_FILE', 0644 ); + } add_filter( 'wp_doing_cron', '__return_true' ); update_option( 'active_plugins', array( self::FILE ) ); $this->filesystem->expects( $this->once() ) ->method( 'put_contents' ) - ->with( '/remote/.maintenance', $this->stringStartsWith( 'with( '/remote/.maintenance', $this->stringStartsWith( 'filesystem->method( 'exists' )->willReturn( true ); $this->filesystem->expects( $this->exactly( 2 ) )->method( 'delete' )->with( '/remote/.maintenance' );