From 7615c76dddae344563b5f6ccc91f844d5fcc6d33 Mon Sep 17 00:00:00 2001 From: Jeremy Mikola Date: Thu, 19 Sep 2024 20:38:26 -0400 Subject: [PATCH 1/2] Extract fail point management to a trait used by Context --- tests/UnifiedSpecTests/Context.php | 20 +--------- .../ManagesFailPointsTrait.php | 40 +++++++++++++++++++ tests/UnifiedSpecTests/Operation.php | 15 ++----- tests/UnifiedSpecTests/UnifiedTestRunner.php | 11 +---- 4 files changed, 47 insertions(+), 39 deletions(-) create mode 100644 tests/UnifiedSpecTests/ManagesFailPointsTrait.php diff --git a/tests/UnifiedSpecTests/Context.php b/tests/UnifiedSpecTests/Context.php index f02550299..57e6e7bd2 100644 --- a/tests/UnifiedSpecTests/Context.php +++ b/tests/UnifiedSpecTests/Context.php @@ -5,7 +5,6 @@ use LogicException; use MongoDB\Client; use MongoDB\Driver\ClientEncryption; -use MongoDB\Driver\Server; use MongoDB\Driver\ServerApi; use MongoDB\Model\BSONArray; use MongoDB\Tests\FunctionalTestCase; @@ -39,6 +38,8 @@ */ final class Context { + use ManagesFailPointsTrait; + private ?string $activeClient = null; private EntityMap $entityMap; @@ -61,9 +62,6 @@ final class Context private ?object $advanceClusterTime = null; - /** @var list */ - private array $failPoints = []; - public function __construct(Client $internalClient, string $uri) { $this->entityMap = new EntityMap(); @@ -236,20 +234,6 @@ public function stopEventCollectors(): void } } - public function registerFailPoint(stdClass $failPoint, Server $server): void - { - $this->failPoints[] = [ - 'failPoint' => $failPoint, - 'server' => $server, - ]; - } - - /** @return list */ - public function getFailPoints(): array - { - return $this->failPoints; - } - /** @param string|array $readPreferenceTags */ private function convertReadPreferenceTags($readPreferenceTags): array { diff --git a/tests/UnifiedSpecTests/ManagesFailPointsTrait.php b/tests/UnifiedSpecTests/ManagesFailPointsTrait.php new file mode 100644 index 000000000..d88368fa9 --- /dev/null +++ b/tests/UnifiedSpecTests/ManagesFailPointsTrait.php @@ -0,0 +1,40 @@ + */ + private array $failPointsAndServers = []; + + public function configureFailPoint(stdClass $failPoint, Server $server): void + { + assertObjectHasAttribute('configureFailPoint', $failPoint); + assertIsString($failPoint->configureFailPoint); + assertObjectHasAttribute('mode', $failPoint); + + $operation = new DatabaseCommand('admin', $failPoint); + $operation->execute($server); + + if ($failPoint->mode !== 'off') { + $this->failPointsAndServers[] = [$failPoint->configureFailPoint, $server]; + } + } + + public function disableFailPoints(): void + { + foreach ($this->failPointsAndServers as [$failPoint, $server]) { + $operation = new DatabaseCommand('admin', ['configureFailPoint' => $failPoint, 'mode' => 'off']); + $operation->execute($server); + } + + $this->failPointsAndServers = []; + } +} diff --git a/tests/UnifiedSpecTests/Operation.php b/tests/UnifiedSpecTests/Operation.php index 58b921aab..11da19280 100644 --- a/tests/UnifiedSpecTests/Operation.php +++ b/tests/UnifiedSpecTests/Operation.php @@ -16,7 +16,6 @@ use MongoDB\Model\CollectionInfo; use MongoDB\Model\DatabaseInfo; use MongoDB\Model\IndexInfo; -use MongoDB\Operation\DatabaseCommand; use MongoDB\Operation\FindOneAndReplace; use MongoDB\Operation\FindOneAndUpdate; use PHPUnit\Framework\Assert; @@ -909,7 +908,8 @@ private function executeForTestRunner() assertArrayHasKey('failPoint', $args); assertInstanceOf(Client::class, $args['client']); assertInstanceOf(stdClass::class, $args['failPoint']); - $this->createFailPoint($args['failPoint'], $args['client']->getManager()->selectServer()); + // Configure the fail point via the Context so it can later be disabled + $this->context->configureFailPoint($args['failPoint'], $args['client']->getManager()->selectServer()); break; case 'targetedFailPoint': assertArrayHasKey('session', $args); @@ -917,7 +917,8 @@ private function executeForTestRunner() assertInstanceOf(Session::class, $args['session']); assertInstanceOf(stdClass::class, $args['failPoint']); assertNotNull($args['session']->getServer(), 'Session is pinned'); - $this->createFailPoint($args['failPoint'], $args['session']->getServer()); + // Configure the fail point via the Context so it can later be disabled + $this->context->configureFailPoint($args['failPoint'], $args['session']->getServer()); break; case 'loop': assertArrayHasKey('operations', $args); @@ -936,14 +937,6 @@ private function executeForTestRunner() } } - private function createFailPoint(stdClass $failPoint, Server $server): void - { - $operation = new DatabaseCommand('admin', $failPoint); - $operation->execute($server); - - $this->context->registerFailPoint($failPoint, $server); - } - private function getIndexNames(string $databaseName, string $collectionName): array { return array_map( diff --git a/tests/UnifiedSpecTests/UnifiedTestRunner.php b/tests/UnifiedSpecTests/UnifiedTestRunner.php index c647cd4cd..5c901078f 100644 --- a/tests/UnifiedSpecTests/UnifiedTestRunner.php +++ b/tests/UnifiedSpecTests/UnifiedTestRunner.php @@ -217,7 +217,7 @@ private function doTestCase(stdClass $test, string $schemaVersion, ?array $runOn $context->stopEventObservers(); $context->stopEventCollectors(); } finally { - $this->disableFailPoints($context->getFailPoints()); + $context->disableFailPoints(); } if (isset($test->expectEvents)) { @@ -231,15 +231,6 @@ private function doTestCase(stdClass $test, string $schemaVersion, ?array $runOn } } - /** @param list $failPoints */ - private function disableFailPoints(array $failPoints): void - { - foreach ($failPoints as ['failPoint' => $failPoint, 'server' => $server]) { - $operation = new DatabaseCommand('admin', ['configureFailPoint' => $failPoint, 'mode' => 'off']); - $operation->execute($server); - } - } - /** * Checks server version and topology requirements. * From 8d7d992bf6307e372b58a1914df26324e8278ab3 Mon Sep 17 00:00:00 2001 From: Jeremy Mikola Date: Thu, 19 Sep 2024 20:41:42 -0400 Subject: [PATCH 2/2] Ensure event listeners are still unregistered after an operation fails --- tests/UnifiedSpecTests/UnifiedTestRunner.php | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/tests/UnifiedSpecTests/UnifiedTestRunner.php b/tests/UnifiedSpecTests/UnifiedTestRunner.php index 5c901078f..f816d8456 100644 --- a/tests/UnifiedSpecTests/UnifiedTestRunner.php +++ b/tests/UnifiedSpecTests/UnifiedTestRunner.php @@ -213,10 +213,9 @@ private function doTestCase(stdClass $test, string $schemaVersion, ?array $runOn $operation = new Operation($o, $context); $operation->assert(); } - + } finally { $context->stopEventObservers(); $context->stopEventCollectors(); - } finally { $context->disableFailPoints(); }