From 24b6c9f35e92200e2a39cc2df6abe186a7cab910 Mon Sep 17 00:00:00 2001 From: Luca Tumedei Date: Sat, 25 May 2024 12:43:29 +0200 Subject: [PATCH] feat(WPLoader) add wpdb connection id strict check --- .../includes/abstract-testcase.php.patch | 28 ++- docs/modules/WPLoader.md | 2 + .../includes/abstract-testcase.php | 6 +- src/Module/WPLoader.php | 19 +- src/TestCase/WPTestCase.php | 23 +++ .../Module/WPLoaderDbConnectionClosedTest.php | 88 --------- .../WPBrowser/Module/WPTestCaseStrictTest.php | 171 ++++++++++++++++++ 7 files changed, 237 insertions(+), 100 deletions(-) delete mode 100644 tests/unit/lucatume/WPBrowser/Module/WPLoaderDbConnectionClosedTest.php create mode 100644 tests/unit/lucatume/WPBrowser/Module/WPTestCaseStrictTest.php diff --git a/config/patches/core-phpunit/includes/abstract-testcase.php.patch b/config/patches/core-phpunit/includes/abstract-testcase.php.patch index 2c593caca..8cdb5ca38 100644 --- a/config/patches/core-phpunit/includes/abstract-testcase.php.patch +++ b/config/patches/core-phpunit/includes/abstract-testcase.php.patch @@ -1,8 +1,16 @@ diff --git a/includes/core-phpunit/includes/abstract-testcase.php b/includes/core-phpunit/includes/abstract-testcase.php -index f2978644..e092beca 100644 +index f2978644..22427643 100644 --- a/includes/core-phpunit/includes/abstract-testcase.php +++ b/includes/core-phpunit/includes/abstract-testcase.php -@@ -20,6 +20,8 @@ abstract class WP_UnitTestCase_Base extends PHPUnit_Adapter_TestCase { +@@ -1,5 +1,7 @@ + suppress_errors = false; $wpdb->show_errors = true; - $wpdb->db_connect(); - ini_set( 'display_errors', 1 ); -+ if ( empty( lucatume\WPBrowser\Utils\Property::readPrivate( $wpdb, 'dbh' ) ) ) { ++ if ( WPTestCase::isStrictAboutWpdbConnectionId() && $wpdb->get_var( 'SELECT CONNECTION_ID()' ) !== WPTestCase::getWpdbConnectionId() ) { + self::fail( 'The database connection went away. A `setUpBeforeClassMethod` likely closed the connection.' ); ++ } else { ++ $wpdb->db_connect(); + } + ini_set( 'display_errors', 1 ); @@ -45,7 +55,7 @@ index f2978644..e092beca 100644 if ( method_exists( $class, 'wpSetUpBeforeClass' ) ) { call_user_func( array( $class, 'wpSetUpBeforeClass' ), static::factory() ); -@@ -82,7 +86,7 @@ public static function set_up_before_class() { +@@ -82,7 +90,7 @@ public static function set_up_before_class() { * Runs the routine after all tests have been run. */ public static function tear_down_after_class() { @@ -54,7 +64,7 @@ index f2978644..e092beca 100644 if ( method_exists( $class, 'wpTearDownAfterClass' ) ) { call_user_func( array( $class, 'wpTearDownAfterClass' ) ); -@@ -651,7 +655,7 @@ public function expectedDeprecated() { +@@ -651,7 +659,7 @@ public function expectedDeprecated() { * * @since 4.2.0 */ @@ -63,7 +73,7 @@ index f2978644..e092beca 100644 $this->expectedDeprecated(); } -@@ -1660,4 +1664,9 @@ public static function touch( $file ) { +@@ -1660,4 +1668,9 @@ public static function touch( $file ) { touch( $file ); } diff --git a/docs/modules/WPLoader.md b/docs/modules/WPLoader.md index 021ceb56b..be9d825fa 100644 --- a/docs/modules/WPLoader.md +++ b/docs/modules/WPLoader.md @@ -112,6 +112,8 @@ When used in this mode, the module supports the following configuration paramete the following runs. To force the installation to run again, rerun the suite using the WPLoader module using the `--debug` flag or delete the `_wploader-state.sql` file in the suite directory. This configuration parameter is ignored when the `loadOnly` parameter is set to `true`. +* `beStrictAboutWpdbConnectionId` - a boolean value to indicate if the `WPTestCase` class should throw an exception if + the database connection is closed during any `setUpBeforeClass` method; default is `true`. This is an example of an integration suite configured to use the module: diff --git a/includes/core-phpunit/includes/abstract-testcase.php b/includes/core-phpunit/includes/abstract-testcase.php index 4c233b408..224276433 100644 --- a/includes/core-phpunit/includes/abstract-testcase.php +++ b/includes/core-phpunit/includes/abstract-testcase.php @@ -1,5 +1,7 @@ suppress_errors = false; $wpdb->show_errors = true; - if ( empty( lucatume\WPBrowser\Utils\Property::readPrivate( $wpdb, 'dbh' ) ) ) { + if ( WPTestCase::isStrictAboutWpdbConnectionId() && $wpdb->get_var( 'SELECT CONNECTION_ID()' ) !== WPTestCase::getWpdbConnectionId() ) { self::fail( 'The database connection went away. A `setUpBeforeClassMethod` likely closed the connection.' ); + } else { + $wpdb->db_connect(); } ini_set( 'display_errors', 1 ); diff --git a/src/Module/WPLoader.php b/src/Module/WPLoader.php index 66c16f3be..902f0a7b6 100644 --- a/src/Module/WPLoader.php +++ b/src/Module/WPLoader.php @@ -19,6 +19,7 @@ use lucatume\WPBrowser\Process\Loop; use lucatume\WPBrowser\Process\ProcessException; use lucatume\WPBrowser\Process\WorkerException; +use lucatume\WPBrowser\TestCase\WPTestCase; use lucatume\WPBrowser\Utils\Arr; use lucatume\WPBrowser\Utils\CorePHPUnit; use lucatume\WPBrowser\Utils\Db as DbUtils; @@ -127,6 +128,7 @@ class WPLoader extends Module * backupStaticAttributes?: bool, * backupStaticAttributesExcludeList?: array, * skipInstall?: bool, + * beStrictAboutWpdbConnectionId?: bool * } */ protected array $config = [ @@ -164,7 +166,8 @@ class WPLoader extends Module 'backupGlobalsExcludeList' => [], 'backupStaticAttributes' => false, 'backupStaticAttributesExcludeList' => [], - 'skipInstall' => false + 'skipInstall' => false, + 'beStrictAboutWpdbConnectionId' => true ]; private string $wpBootstrapFile; @@ -318,6 +321,14 @@ protected function validateConfig(): void ); } + if (isset($this->config['beStrictAboutWpdbConnectionId']) + && !is_bool($this->config['beStrictAboutWpdbConnectionId'])) { + throw new ModuleConfigException( + __CLASS__, + 'The `beStrictAboutWpdbConnectionId` configuration parameter must be a boolean.' + ); + } + parent::validateConfig(); } @@ -378,7 +389,8 @@ public function _initialize(): void * backupGlobalsExcludeList: string[], * backupStaticAttributes: bool, * backupStaticAttributesExcludeList: array, - * skipInstall: bool + * skipInstall: bool, + * beStrictAboutWpdbConnectionId: bool * } $config */ $config = $this->config; @@ -489,6 +501,8 @@ public function _initialize(): void // If the database does not already exist, then create it now. $db->create(); + WPTestCase::beStrictAboutWpdbConnectionId($config['beStrictAboutWpdbConnectionId']); + $this->loadWordPress(); } @@ -1002,6 +1016,7 @@ private function includeCorePHPUniteSuiteBootstrapFile(): void try { require $this->wpBootstrapFile; + WPTestCase::setWpdbConnectionId((string)$GLOBALS['wpdb']->get_var('SELECT CONNECTION_ID()')); } catch (Throwable $t) { // Not an early exit: Codeception will handle the Exception and print it. $this->earlyExit = false; diff --git a/src/TestCase/WPTestCase.php b/src/TestCase/WPTestCase.php index 4cb9594fa..9444c952a 100644 --- a/src/TestCase/WPTestCase.php +++ b/src/TestCase/WPTestCase.php @@ -80,6 +80,9 @@ class WPTestCase extends Unit { use WPTestCasePHPUnitMethodsTrait; + public static bool $beStrictAboutWpdbConnectionId = true; + private static ?string $wpdbConnectionId = null; + /** * @var string[]|null */ @@ -247,6 +250,26 @@ private static function getCoreTestCase(): WP_UnitTestCase return $coreTestCase; } + public static function isStrictAboutWpdbConnectionId(): bool + { + return self::$beStrictAboutWpdbConnectionId; + } + + public static function beStrictAboutWpdbConnectionId(bool $beStrictAboutWpdbConnectionId): void + { + self::$beStrictAboutWpdbConnectionId = $beStrictAboutWpdbConnectionId; + } + + public static function getWpdbConnectionId(): ?string + { + return self::$wpdbConnectionId; + } + + public static function setWpdbConnectionId(string $wpdbConnectionId): void + { + self::$wpdbConnectionId = $wpdbConnectionId; + } + protected function backupAdditionalGlobals(): void { if (isset($GLOBALS['_wp_registered_theme_features'])) { diff --git a/tests/unit/lucatume/WPBrowser/Module/WPLoaderDbConnectionClosedTest.php b/tests/unit/lucatume/WPBrowser/Module/WPLoaderDbConnectionClosedTest.php deleted file mode 100644 index cadd35ecb..000000000 --- a/tests/unit/lucatume/WPBrowser/Module/WPLoaderDbConnectionClosedTest.php +++ /dev/null @@ -1,88 +0,0 @@ -mockModuleContainer = new ModuleContainer(new Di(), $moduleContainerConfig); - return new WPLoader($this->mockModuleContainer, ($moduleConfig ?? $this->config)); - } - - public function test_will_fail_if_db_connection_closed_during_setup_before_class(): void - { - $wpRootDir = FS::tmpDir('wploader_'); - $dbName = Random::dbName(); - $dbHost = Env::get('WORDPRESS_DB_HOST'); - $dbUser = Env::get('WORDPRESS_DB_USER'); - $dbPassword = Env::get('WORDPRESS_DB_PASSWORD'); - $db = new MysqlDatabase($dbName, $dbUser, $dbPassword, $dbHost); - Installation::scaffold($wpRootDir); - $db->create(); - $this->config = [ - 'wpRootFolder' => $wpRootDir, - 'dbUrl' => $db->getDbUrl() - ]; - $testcaseFile = $wpRootDir . '/BreakingTest.php'; - $testCaseFileContents = <<< PHP - close(); - - parent::set_up_before_class(); - } - - public function test_something():void{ - \$this->assertTrue(true); - } - } - PHP; - if(!file_put_contents($testcaseFile, $testCaseFileContents, LOCK_EX)) { - throw new \RuntimeException('Could not write BreakingTest.php.'); - } - - $wpLoader = $this->module(); - - $this->assertInIsolation(static function () use ($wpLoader, $testcaseFile) { - $wpLoader->_initialize(); - - require_once $testcaseFile; - - try { - \BreakingTest::setUpBeforeClass(); - } catch (\Throwable $e) { - Assert::assertStringContainsString( - 'The database connection went away. A `setUpBeforeClassMethod` likely closed the connection', - $e->getMessage() - ); - return; - } - - Assert::fail('The test should have failed.'); - }); - } -} diff --git a/tests/unit/lucatume/WPBrowser/Module/WPTestCaseStrictTest.php b/tests/unit/lucatume/WPBrowser/Module/WPTestCaseStrictTest.php new file mode 100644 index 000000000..9183492e5 --- /dev/null +++ b/tests/unit/lucatume/WPBrowser/Module/WPTestCaseStrictTest.php @@ -0,0 +1,171 @@ +mockModuleContainer = new ModuleContainer(new Di(), $moduleContainerConfig); + return new WPLoader($this->mockModuleContainer, ($moduleConfig ?? $this->config)); + } + + public function nonBooleanVAluesProvider(): array + { + return [ + 'int' => [1], + 'float' => [1.1], + 'array' => [[]], + 'object' => [new \stdClass()], + 'true string' => ['true'], + 'false string' => ['false'], + ]; + } + + /** + * @dataProvider nonBooleanVAluesProvider + */ + public function test_will_throw_if_beStrictAboutWpdbConnectionId_is_not_boolean($value): void + { + $this->expectException(ModuleConfigException::class); + $this->expectExceptionMessage('The `beStrictAboutWpdbConnectionId` configuration parameter must be a boolean.'); + + $this->config = [ + 'wpRootFolder' => __DIR__, + 'dbUrl' => 'mysql://root:root@mysql:3306/wordpress', + 'beStrictAboutWpdbConnectionId' => $value + ]; + $this->module(); + } + + public function test_will_fail_if_db_connection_closed_during_setup_before_class(): void + { + $wpRootDir = FS::tmpDir('wploader_'); + $dbName = Random::dbName(); + $dbHost = Env::get('WORDPRESS_DB_HOST'); + $dbUser = Env::get('WORDPRESS_DB_USER'); + $dbPassword = Env::get('WORDPRESS_DB_PASSWORD'); + $db = new MysqlDatabase($dbName, $dbUser, $dbPassword, $dbHost); + Installation::scaffold($wpRootDir); + $db->create(); + $testcaseFile = $wpRootDir . '/BreakingTest.php'; + $testCaseFileContents = <<< PHP + close(); + + parent::set_up_before_class(); + } + + public function test_something():void{ + \$this->assertTrue(true); + } + } + PHP; + if(!file_put_contents($testcaseFile, $testCaseFileContents, LOCK_EX)) { + throw new \RuntimeException('Could not write BreakingTest.php.'); + } + + // Run a test using the default value, strict. + $this->config = [ + 'wpRootFolder' => $wpRootDir, + 'dbUrl' => $db->getDbUrl() + ]; + $wpLoader = $this->module(); + + $this->assertInIsolation(static function () use ($wpLoader, $testcaseFile) { + $wpLoader->_initialize(); + $connectionId = WPTestCase::getWpdbConnectionId(); + Assert::assertnotEmpty($connectionId); + Assert::assertTrue(WPTestCase::isStrictAboutWpdbConnectionId()); + + require_once $testcaseFile; + + try { + \BreakingTest::setUpBeforeClass(); + } catch (\Throwable $e) { + Assert::assertNotSame($connectionId, $GLOBALS['wpdb']->get_var('SELECT CONNECTION_ID()')); + Assert::assertStringContainsString( + 'The database connection went away. A `setUpBeforeClassMethod` likely closed the connection', + $e->getMessage() + ); + return; + } + + Assert::fail('The test should have failed.'); + }); + + // Run a test in strict mode. + $this->config = [ + 'wpRootFolder' => $wpRootDir, + 'dbUrl' => $db->getDbUrl(), + 'beStrictAboutWpdbConnectionId' => true + ]; + $wpLoader = $this->module(); + + $this->assertInIsolation(static function () use ($wpLoader, $testcaseFile) { + $wpLoader->_initialize(); + $connectionId = WPTestCase::getWpdbConnectionId(); + Assert::assertnotEmpty($connectionId); + Assert::assertTrue(WPTestCase::isStrictAboutWpdbConnectionId()); + + require_once $testcaseFile; + + try { + \BreakingTest::setUpBeforeClass(); + } catch (\Throwable $e) { + Assert::assertNotSame($connectionId, $GLOBALS['wpdb']->get_var('SELECT CONNECTION_ID()')); + Assert::assertStringContainsString( + 'The database connection went away. A `setUpBeforeClassMethod` likely closed the connection', + $e->getMessage() + ); + return; + } + + Assert::fail('The test should have failed.'); + }); + + // Run a test in non-strict mode. + $this->config = [ + 'wpRootFolder' => $wpRootDir, + 'dbUrl' => $db->getDbUrl(), + 'beStrictAboutWpdbConnectionId' => false + ]; + $wpLoader = $this->module(); + + $this->assertInIsolation(static function () use ($wpLoader, $testcaseFile) { + $wpLoader->_initialize(); + $connectionId = WPTestCase::getWpdbConnectionId(); + Assert::assertFalse(WPTestCase::isStrictAboutWpdbConnectionId()); + + require_once $testcaseFile; + + \BreakingTest::setUpBeforeClass(); + Assert::assertNotSame($connectionId, $GLOBALS['wpdb']->get_var('SELECT CONNECTION_ID()')); + }); + } +}