Note cycling references and killAllSessions before each test

Author: jmikola

tests/UnifiedSpecTests/Constraint/Matches.php

@@ -16,6 +16,7 @@
 use Symfony\Bridge\PhpUnit\ConstraintTrait;
 use function array_keys;
 use function assertInternalType;
+use function assertNotNull;
 use function assertRegExp;
 use function assertThat;
 use function containsOnly;

tests/UnifiedSpecTests/Constraint/MatchesTest.php

@@ -7,10 +7,7 @@
 use MongoDB\Tests\UnifiedSpecTests\EntityMap;
 use PHPUnit\Framework\ExpectationFailedException;
 use stdClass;
-use function fopen;
-use function fwrite;
 use function hex2bin;
-use function rewind;
 use function version_compare;
 
 class MatchesTest extends FunctionalTestCase

tests/UnifiedSpecTests/EntityMap.php

@@ -41,7 +41,10 @@ class EntityMap implements ArrayAccess
     public function __destruct()
     {
         /* TODO: Determine if this is actually necessary. References to session
-         * entities should not persist between tests. */
+         * entities should not persist between tests.
+         *
+         * Note: This does not appear to trigger after a test due to cyclic
+         * references (see comment in UnifiedSpecTest.php). */
         foreach ($this->map as $entity) {
             if ($entity->value instanceof Session) {
                 $entity->value->endSession();

tests/UnifiedSpecTests/UnifiedSpecTest.php

@@ -3,10 +3,10 @@
 namespace MongoDB\Tests\UnifiedSpecTests;
 
 use MongoDB\Client;
-use MongoDB\Driver\Command;
 use MongoDB\Driver\Exception\ServerException;
 use MongoDB\Driver\ReadPreference;
 use MongoDB\Driver\Server;
+use MongoDB\Operation\DatabaseCommand;
 use MongoDB\Tests\FunctionalTestCase;
 use stdClass;
 use Symfony\Bridge\PhpUnit\SetUpTearDownTrait;
@@ -45,16 +45,23 @@ private static function doSetUpBeforeClass()
     {
         parent::setUpBeforeClass();
 
-        /* TODO: FunctionalTestCase::getUri() restricts to a single mongos by
-         * default. Determine if there's a need to override that. */
-        self::$internalClient = new Client(static::getUri());
+        /* Provide internal client unmodified URI, since it may need to execute
+         * commands on multiple mongoses (e.g. killAllSessions) */
+        self::$internalClient = new Client(static::getUri(true));
         self::killAllSessions();
     }
 
     private function doSetUp()
     {
         parent::setUp();
 
+        /* TODO: The transactions spec advises calling killAllSessions only at
+         * the start of the test suite and after failed tests; however, the
+         * "unpin after transient error within a transaction" pinning test
+         * causes the subsequent transaction test to block. This can be removed
+         * once that is addressed. */
+        self::killAllSessions();
+
         $this->failPointObserver = new FailPointObserver();
         $this->failPointObserver->start();
     }
@@ -68,6 +75,10 @@ private function doTearDown()
         $this->failPointObserver->stop();
         $this->failPointObserver->disableFailPoints();
 
+        /* TODO: Consider manually invoking gc_collect_cycles since each test is
+         * prone to create cycles (perhaps due to EntityMap), which can leak and
+         * prevent sessions from being released back into the pool. */
+
         parent::tearDown();
     }
 
@@ -299,7 +310,9 @@ private static function killAllSessions()
                 if (! isset($server->getInfo()['logicalSessionTimeoutMinutes'])) {
                     continue;
                 }
-                $server->executeCommand('admin', new Command(['killAllSessions' => []]));
+
+                $command = new DatabaseCommand('admin', ['killAllSessions' => []]);
+                $command->execute($server);
             } catch (ServerException $e) {
                 // Interrupted error is safe to ignore (see: SERVER-38335)
                 if ($e->getCode() != self::SERVER_ERROR_INTERRUPTED) {