Skip to content

Commit 6a4681b

Browse files
committed
PHPLIB-1531: Replace FailPointObserver with different logic
Squashed commit of the following: commit 8921152 Author: Jeremy Mikola <[email protected]> Date: Thu Sep 19 20:41:42 2024 -0400 Ensure event listeners are still unregistered after an operation fails commit b142235 Author: Jeremy Mikola <[email protected]> Date: Thu Sep 19 20:38:26 2024 -0400 Extract fail point management to a trait used by Context commit 63d8d4f Author: Andreas Braun <[email protected]> Date: Thu Sep 19 14:27:47 2024 +0200 Replace FailPointObserver with different logic
1 parent ba3bf81 commit 6a4681b

File tree

5 files changed

+55
-81
lines changed

5 files changed

+55
-81
lines changed

tests/UnifiedSpecTests/Context.php

+2
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,8 @@
3838
*/
3939
final class Context
4040
{
41+
use ManagesFailPointsTrait;
42+
4143
private ?string $activeClient = null;
4244

4345
private EntityMap $entityMap;

tests/UnifiedSpecTests/FailPointObserver.php

-63
This file was deleted.
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,40 @@
1+
<?php
2+
3+
namespace MongoDB\Tests\UnifiedSpecTests;
4+
5+
use MongoDB\Driver\Server;
6+
use MongoDB\Operation\DatabaseCommand;
7+
use stdClass;
8+
9+
use function PHPUnit\Framework\assertIsString;
10+
use function PHPUnit\Framework\assertObjectHasAttribute;
11+
12+
trait ManagesFailPointsTrait
13+
{
14+
/** @var list<list{string, Server}> */
15+
private array $failPointsAndServers = [];
16+
17+
public function configureFailPoint(stdClass $failPoint, Server $server): void
18+
{
19+
assertObjectHasAttribute('configureFailPoint', $failPoint);
20+
assertIsString($failPoint->configureFailPoint);
21+
assertObjectHasAttribute('mode', $failPoint);
22+
23+
$operation = new DatabaseCommand('admin', $failPoint);
24+
$operation->execute($server);
25+
26+
if ($failPoint->mode !== 'off') {
27+
$this->failPointsAndServers[] = [$failPoint->configureFailPoint, $server];
28+
}
29+
}
30+
31+
public function disableFailPoints(): void
32+
{
33+
foreach ($this->failPointsAndServers as [$failPoint, $server]) {
34+
$operation = new DatabaseCommand('admin', ['configureFailPoint' => $failPoint, 'mode' => 'off']);
35+
$operation->execute($server);
36+
}
37+
38+
$this->failPointsAndServers = [];
39+
}
40+
}

tests/UnifiedSpecTests/Operation.php

+4-4
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,6 @@
1616
use MongoDB\Model\CollectionInfo;
1717
use MongoDB\Model\DatabaseInfo;
1818
use MongoDB\Model\IndexInfo;
19-
use MongoDB\Operation\DatabaseCommand;
2019
use MongoDB\Operation\FindOneAndReplace;
2120
use MongoDB\Operation\FindOneAndUpdate;
2221
use PHPUnit\Framework\Assert;
@@ -905,16 +904,17 @@ private function executeForTestRunner()
905904
assertArrayHasKey('failPoint', $args);
906905
assertInstanceOf(Client::class, $args['client']);
907906
assertInstanceOf(stdClass::class, $args['failPoint']);
908-
$args['client']->selectDatabase('admin')->command($args['failPoint']);
907+
// Configure the fail point via the Context so it can later be disabled
908+
$this->context->configureFailPoint($args['failPoint'], $args['client']->getManager()->selectServer());
909909
break;
910910
case 'targetedFailPoint':
911911
assertArrayHasKey('session', $args);
912912
assertArrayHasKey('failPoint', $args);
913913
assertInstanceOf(Session::class, $args['session']);
914914
assertInstanceOf(stdClass::class, $args['failPoint']);
915915
assertNotNull($args['session']->getServer(), 'Session is pinned');
916-
$operation = new DatabaseCommand('admin', $args['failPoint']);
917-
$operation->execute($args['session']->getServer());
916+
// Configure the fail point via the Context so it can later be disabled
917+
$this->context->configureFailPoint($args['failPoint'], $args['session']->getServer());
918918
break;
919919
case 'loop':
920920
assertArrayHasKey('operations', $args);

tests/UnifiedSpecTests/UnifiedTestRunner.php

+9-14
Original file line numberDiff line numberDiff line change
@@ -75,8 +75,6 @@ final class UnifiedTestRunner
7575
/** @var callable(EntityMap):void */
7676
private $entityMapObserver;
7777

78-
private ?FailPointObserver $failPointObserver = null;
79-
8078
private ServerParameterHelper $serverParameterHelper;
8179

8280
public function __construct(private string $internalClientUri)
@@ -147,9 +145,6 @@ private function doSetUp(): void
147145
* after transient error within a transaction" pinning test causes the
148146
* subsequent transaction test to block. */
149147
$this->killAllSessions();
150-
151-
$this->failPointObserver = new FailPointObserver();
152-
$this->failPointObserver->start();
153148
}
154149

155150
private function doTearDown(bool $hasFailed): void
@@ -160,9 +155,6 @@ private function doTearDown(bool $hasFailed): void
160155
$this->killAllSessions();
161156
}
162157

163-
$this->failPointObserver->stop();
164-
$this->failPointObserver->disableFailPoints();
165-
166158
/* Manually invoking garbage collection since each test is prone to
167159
* create cycles (perhaps due to EntityMap), which can leak and prevent
168160
* sessions from being released back into the pool. */
@@ -213,14 +205,17 @@ private function doTestCase(stdClass $test, string $schemaVersion, ?array $runOn
213205
$context->startEventObservers();
214206
$context->startEventCollectors();
215207

216-
foreach ($test->operations as $o) {
217-
$operation = new Operation($o, $context);
218-
$operation->assert();
208+
try {
209+
foreach ($test->operations as $o) {
210+
$operation = new Operation($o, $context);
211+
$operation->assert();
212+
}
213+
} finally {
214+
$context->stopEventObservers();
215+
$context->stopEventCollectors();
216+
$context->disableFailPoints();
219217
}
220218

221-
$context->stopEventObservers();
222-
$context->stopEventCollectors();
223-
224219
if (isset($test->expectEvents)) {
225220
assertIsArray($test->expectEvents);
226221
$context->assertExpectedEventsForClients($test->expectEvents);

0 commit comments

Comments
 (0)