Provide isolated, read/write storage fixtures for unit tests

Summary:
  - Unit tests can request storage fixtures.
  - We build one fixture across all tests in the process, which can quickstart (takes roughly 1s to build, 200ms to destroy for me). This is a one-time cost for running an arbitrary number of fixture-based tests.
  - We isolate all the connections inside transactions for each test, so individual tests don't affect one another.

Test Plan: Ran unit tests, which cover the important properties of fixtures.

Reviewers: btrahan, vrana, jungejason, edward

Reviewed By: btrahan

CC: aran, davidreuss

Maniphest Tasks: T140

Differential Revision: https://secure.phabricator.com/D2345

src/__phutil_library_map__.php

@@ -453,6 +453,7 @@ phutil_register_library_map(array(
     'JavelinViewExampleServerView' => 'applications/uiexample/examples/client',
     'LiskDAO' => 'storage/lisk/dao',
     'LiskEphemeralObjectException' => 'storage/lisk/dao',
+    'LiskFixtureTestCase' => 'storage/lisk/dao/__tests__',
     'LiskIsolationTestCase' => 'storage/lisk/dao/__tests__',
     'LiskIsolationTestDAO' => 'storage/lisk/dao/__tests__',
     'LiskIsolationTestDAOException' => 'storage/lisk/dao/__tests__',
@@ -896,6 +897,7 @@ phutil_register_library_map(array(
     'PhabricatorSortTableExample' => 'applications/uiexample/examples/sorttable',
     'PhabricatorStandardPageView' => 'view/page/standard',
     'PhabricatorStatusController' => 'applications/status/base',
+    'PhabricatorStorageFixtureScopeGuard' => 'infrastructure/testing/fixture/storage',
     'PhabricatorStorageManagementAPI' => 'infrastructure/setup/storage/management',
     'PhabricatorStorageManagementDatabasesWorkflow' => 'infrastructure/setup/storage/workflow/databases',
     'PhabricatorStorageManagementDestroyWorkflow' => 'infrastructure/setup/storage/workflow/destroy',
@@ -1395,6 +1397,7 @@ phutil_register_library_map(array(
     'JavelinReactorExample' => 'PhabricatorUIExample',
     'JavelinViewExample' => 'PhabricatorUIExample',
     'JavelinViewExampleServerView' => 'AphrontView',
+    'LiskFixtureTestCase' => 'PhabricatorTestCase',
     'LiskIsolationTestCase' => 'PhabricatorTestCase',
     'LiskIsolationTestDAO' => 'LiskDAO',
     'ManiphestAction' => 'PhrictionConstants',

src/applications/base/storage/lisk/PhabricatorLiskDAO.php

@@ -72,7 +72,7 @@ abstract class PhabricatorLiskDAO extends LiskDAO {
   /**
    * @task config
    */
-  public static function popStorageNamespace($namespace) {
+  public static function popStorageNamespace() {
     array_pop(self::$namespaceStack);
   }
 

src/infrastructure/testing/fixture/storage/PhabricatorStorageFixtureScopeGuard.php

@@ -0,0 +1,51 @@
+<?php
+
+/*
+ * Copyright 2012 Facebook, Inc.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+/**
+ * Used by unit tests to build storage fixtures.
+ */
+final class PhabricatorStorageFixtureScopeGuard {
+
+  private $name;
+
+  public function __construct($name) {
+    $this->name = $name;
+
+    execx(
+      '%s upgrade --force --namespace %s',
+      $this->getStorageBinPath(),
+      $this->name);
+
+    PhabricatorLiskDAO::pushStorageNamespace($name);
+  }
+
+  public function __destruct() {
+    PhabricatorLiskDAO::popStorageNamespace();
+
+    execx(
+      '%s destroy --force --namespace %s',
+      $this->getStorageBinPath(),
+      $this->name);
+  }
+
+  private function getStorageBinPath() {
+    $root = dirname(phutil_get_library_root('phabricator'));
+    return $root.'/bin/storage';
+  }
+
+}

src/infrastructure/testing/fixture/storage/__init__.php

@@ -0,0 +1,15 @@
+<?php
+/**
+ * This file is automatically generated. Lint this module to rebuild it.
+ * @generated
+ */
+
+
+
+phutil_require_module('phabricator', 'applications/base/storage/lisk');
+
+phutil_require_module('phutil', 'future/exec');
+phutil_require_module('phutil', 'moduleutils');
+
+
+phutil_require_source('PhabricatorStorageFixtureScopeGuard.php');

src/infrastructure/testing/testcase/PhabricatorTestCase.php

@@ -28,19 +28,46 @@ abstract class PhabricatorTestCase extends ArcanistPhutilTestCase {
    * not rely on external resources like databases, and should not produce
    * side effects.
    */
-  const PHABRICATOR_TESTCONFIG_ISOLATE_LISK     = 'isolate-lisk';
+  const PHABRICATOR_TESTCONFIG_ISOLATE_LISK           = 'isolate-lisk';
+
+  /**
+   * If true, build storage fixtures before running tests, and connect to them
+   * during test execution. This will impose a performance penalty on test
+   * execution (currently, it takes roughly one second to build the fixture)
+   * but allows you to perform tests which require data to be read from storage
+   * after writes. The fixture is shared across all test cases in this process.
+   * Defaults to false.
+   *
+   * NOTE: All connections to fixture storage open transactions when established
+   * and roll them back when tests complete. Each test must independently
+   * write data it relies on; data will not persist across tests.
+   *
+   * NOTE: Enabling this implies disabling process isolation.
+   */
+  const PHABRICATOR_TESTCONFIG_BUILD_STORAGE_FIXTURES = 'storage-fixtures';
 
   private $configuration;
   private $env;
 
+  private static $storageFixtureReferences = 0;
+  private static $storageFixture;
+
   protected function getPhabricatorTestCaseConfiguration() {
     return array();
   }
 
   private function getComputedConfiguration() {
-    return $this->getPhabricatorTestCaseConfiguration() + array(
+    $config = $this->getPhabricatorTestCaseConfiguration() + array(
-      self::PHABRICATOR_TESTCONFIG_ISOLATE_LISK     => true,
+      self::PHABRICATOR_TESTCONFIG_ISOLATE_LISK             => true,
+      self::PHABRICATOR_TESTCONFIG_BUILD_STORAGE_FIXTURES   => false,
     );
+
+    if ($config[self::PHABRICATOR_TESTCONFIG_BUILD_STORAGE_FIXTURES]) {
+      // Fixtures don't make sense with process isolation.
+      $config[self::PHABRICATOR_TESTCONFIG_ISOLATE_LISK] = false;
+    }
+
+    return $config;
   }
 
   protected function willRunTests() {
@@ -53,6 +80,13 @@ abstract class PhabricatorTestCase extends ArcanistPhutilTestCase {
       LiskDAO::beginIsolateAllLiskEffectsToCurrentProcess();
     }
 
+    if ($config[self::PHABRICATOR_TESTCONFIG_BUILD_STORAGE_FIXTURES]) {
+      ++self::$storageFixtureReferences;
+      if (!self::$storageFixture) {
+        self::$storageFixture = $this->newStorageFixture();
+      }
+    }
+
     $this->env = PhabricatorEnv::beginScopedEnv();
   }
 
@@ -63,6 +97,13 @@ abstract class PhabricatorTestCase extends ArcanistPhutilTestCase {
       LiskDAO::endIsolateAllLiskEffectsToCurrentProcess();
     }
 
+    if (self::$storageFixture) {
+      self::$storageFixtureReferences--;
+      if (!self::$storageFixtureReferences) {
+        self::$storageFixture = null;
+      }
+    }
+
     try {
       unset($this->env);
     } catch (Exception $ex) {
@@ -72,4 +113,27 @@ abstract class PhabricatorTestCase extends ArcanistPhutilTestCase {
     }
   }
 
+  protected function willRunOneTest($test) {
+    $config = $this->getComputedConfiguration();
+
+    if ($config[self::PHABRICATOR_TESTCONFIG_BUILD_STORAGE_FIXTURES]) {
+      LiskDAO::beginIsolateAllLiskEffectsToTransactions();
+    }
+  }
+
+  protected function didRunOneTest($test) {
+    $config = $this->getComputedConfiguration();
+
+    if ($config[self::PHABRICATOR_TESTCONFIG_BUILD_STORAGE_FIXTURES]) {
+      LiskDAO::endIsolateAllLiskEffectsToTransactions();
+    }
+  }
+
+  protected function newStorageFixture() {
+    $bytes = Filesystem::readRandomCharacters(24);
+    $name = 'phabricator_unittest_'.$bytes;
+
+    return new PhabricatorStorageFixtureScopeGuard($name);
+  }
+
 }

src/infrastructure/testing/testcase/__init__.php

@@ -9,8 +9,10 @@
 phutil_require_module('arcanist', 'unit/engine/phutil/testcase');
 
 phutil_require_module('phabricator', 'infrastructure/env');
+phutil_require_module('phabricator', 'infrastructure/testing/fixture/storage');
 phutil_require_module('phabricator', 'storage/lisk/dao');
 
+phutil_require_module('phutil', 'filesystem');
 phutil_require_module('phutil', 'moduleutils');
 
 

src/storage/lisk/dao/LiskDAO.php

@@ -190,7 +190,8 @@ abstract class LiskDAO {
 
   private $__dirtyFields            = array();
   private $__missingFields          = array();
-  private static $processIsolationLevel = 0;
+  private static $processIsolationLevel       = 0;
+  private static $transactionIsolationLevel   = 0;
   private static $__checkedClasses = array();
 
   private $__ephemeral = false;
@@ -809,12 +810,22 @@ abstract class LiskDAO {
       return $connection;
     }
 
+    if (self::shouldIsolateAllLiskEffectsToTransactions()) {
+      // If we're doing fixture transaction isolation, force the mode to 'w'
+      // so we always get the same connection for reads and writes, and thus
+      // can see the writes inside the transaction.
+      $mode = 'w';
+    }
+
     // TODO There is currently no protection on 'r' queries against writing
     // or on 'w' queries against reading
 
     $connection = $this->getEstablishedConnection($mode);
     if (!$connection) {
       $connection = $this->establishLiveConnection($mode);
+      if (self::shouldIsolateAllLiskEffectsToTransactions()) {
+        $connection->openTransaction();
+      }
       $this->setEstablishedConnection($mode, $connection);
     }
 
@@ -1356,6 +1367,44 @@ abstract class LiskDAO {
     return new AphrontIsolatedDatabaseConnection($config);
   }
 
+  /**
+   * @task isolate
+   */
+  public static function beginIsolateAllLiskEffectsToTransactions() {
+    if (self::$transactionIsolationLevel === 0) {
+      self::closeAllConnections();
+    }
+    self::$transactionIsolationLevel++;
+  }
+
+  /**
+   * @task isolate
+   */
+  public static function endIsolateAllLiskEffectsToTransactions() {
+    self::$transactionIsolationLevel--;
+    if (self::$transactionIsolationLevel < 0) {
+      throw new Exception(
+        "Lisk transaction isolation level was reduced below 0.");
+    } else if (self::$transactionIsolationLevel == 0) {
+      foreach (self::$connections as $key => $conn) {
+        if ($conn) {
+          $conn->killTransaction();
+        }
+      }
+      self::closeAllConnections();
+    }
+  }
+
+  /**
+   * @task isolate
+   */
+  public static function shouldIsolateAllLiskEffectsToTransactions() {
+    return (bool)self::$transactionIsolationLevel;
+  }
+
+  public static function closeAllConnections() {
+    self::$connections = array();
+  }
 
 /* -(  Utilities  )---------------------------------------------------------- */
 

src/storage/lisk/dao/__tests__/LiskFixtureTestCase.php

@@ -0,0 +1,69 @@
+<?php
+
+/*
+ * Copyright 2012 Facebook, Inc.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+final class LiskFixtureTestCase extends PhabricatorTestCase {
+
+  public function getPhabricatorTestCaseConfiguration() {
+    return array(
+      self::PHABRICATOR_TESTCONFIG_BUILD_STORAGE_FIXTURES => true,
+    );
+  }
+
+  public function testTransactionalIsolation1of2() {
+    // NOTE: These tests are verifying that data is destroyed between tests.
+    // If the user from either test persists, the other test will fail.
+    $this->assertEqual(
+      0,
+      count(id(new PhabricatorUser())->loadAll()));
+
+    id(new PhabricatorUser())
+      ->setUserName('alincoln')
+      ->setRealName('Abraham Lincoln')
+      ->setEmail('alincoln@example.com')
+      ->save();
+  }
+
+  public function testTransactionalIsolation2of2() {
+    $this->assertEqual(
+      0,
+      count(id(new PhabricatorUser())->loadAll()));
+
+    id(new PhabricatorUser())
+      ->setUserName('ugrant')
+      ->setRealName('Ulysses S. Grant')
+      ->setEmail('ugrant@example.com')
+      ->save();
+  }
+
+  public function testFixturesBasicallyWork() {
+    $this->assertEqual(
+      0,
+      count(id(new PhabricatorUser())->loadAll()));
+
+    id(new PhabricatorUser())
+      ->setUserName('gwashington')
+      ->setRealName('George Washington')
+      ->setEmail('gwashington@example.com')
+      ->save();
+
+    $this->assertEqual(
+      1,
+      count(id(new PhabricatorUser())->loadAll()));
+  }
+
+}

src/storage/lisk/dao/__tests__/__init__.php

@@ -6,11 +6,15 @@
 
 
 
+phutil_require_module('phabricator', 'applications/people/storage/user');
 phutil_require_module('phabricator', 'applications/phid/storage/phid');
 phutil_require_module('phabricator', 'infrastructure/testing/testcase');
 phutil_require_module('phabricator', 'storage/lisk/dao');
 
+phutil_require_module('phutil', 'utils');
 
+
+phutil_require_source('LiskFixtureTestCase.php');
 phutil_require_source('LiskIsolationTestCase.php');
 phutil_require_source('LiskIsolationTestDAO.php');
 phutil_require_source('LiskIsolationTestDAOException.php');