Add a little more unit test documentation, fail loudly when isolation prevents a

query

Summary:
  - Provide an example unit test, and document it.
  - Document database isolation better.
  - When we issue an unsimulated query to the isolated connection, throw a
helpful message.
  - Pygments is complaining about my madeup "lang=demo", change it to
"lang=text".

Test Plan:
  - Ran the unit test (sanity check).
  - Ran all other unit tests (verify I didn't break isolation).
  - Added a queryfx(..., 'SELECT 1') to a test and verified it throws.
  - Read the documentation.

Reviewed By: edward
Reviewers: edward, jungejason, tuomaspelkonen, aran
CC: aran, edward
Differential Revision: 773

src/__phutil_library_map__.php

@@ -568,6 +568,7 @@ phutil_register_library_map(array(
     'PhabricatorTimelineIterator' => 'infrastructure/daemon/timeline/cursor/iterator',
     'PhabricatorTimer' => 'applications/countdown/storage/timer',
     'PhabricatorTransformedFile' => 'applications/files/storage/transformed',
+    'PhabricatorTrivialTestCase' => 'infrastructure/testing/testcase/__tests__',
     'PhabricatorTypeaheadCommonDatasourceController' => 'applications/typeahead/controller/common',
     'PhabricatorTypeaheadDatasourceController' => 'applications/typeahead/controller/base',
     'PhabricatorUIExample' => 'applications/uiexample/examples/base',
@@ -1096,6 +1097,7 @@ phutil_register_library_map(array(
     'PhabricatorTimelineEventData' => 'PhabricatorTimelineDAO',
     'PhabricatorTimer' => 'PhabricatorCountdownDAO',
     'PhabricatorTransformedFile' => 'PhabricatorFileDAO',
+    'PhabricatorTrivialTestCase' => 'PhabricatorTestCase',
     'PhabricatorTypeaheadCommonDatasourceController' => 'PhabricatorTypeaheadDatasourceController',
     'PhabricatorTypeaheadDatasourceController' => 'PhabricatorController',
     'PhabricatorUIExampleController' => 'PhabricatorController',

src/docs/developer/unit_tests.diviner

@@ -32,3 +32,56 @@ Once you've added test classes, you can run them with:
   - ##arc unit##, to run tests for all modules affected by changes in the
     working copy.
   - ##arc diff## will also run ##arc unit## for you.
+
+= Example Test Case =
+
+Here's a simple example test:
+
+  lang=php
+  class PhabricatorTrivialTestCase extends PhabricatorTestCase {
+
+    private $two;
+
+    public function willRunOneTest($test_name) {
+      // You can execute setup steps which will run before each test in this
+      // method.
+      $this->two = 2;
+    }
+
+    public function testAllIsRightWithTheWorld() {
+      $this->assertEqual(4, 2 + 2, '2 + 2 = 4');
+    }
+
+  }
+
+You can see this class at @{class:PhabricatorTrivialTestCase} and run it with:
+
+  phabricator/ $ arc unit src/infrastructure/testing/testcase/
+   PASS   <1ms*  testAllIsRightWithTheWorld
+
+For more information on writing tests, see
+@{class@arcnaist:ArcanistPhutilTestCase} and @{class:PhabricatorTestCase}.
+
+= Database Isolation =
+
+By default, Phabricator isolates unit tests from the database. It makes a crude
+effort to simulate some side effects (principally, ID assignment on insert), but
+any queries which read data will fail to select any rows and throw an exception
+about isolation. In general, isolation is good, but this can make certain types
+of tests difficult to write. When you encounter issues, you can deal with them
+in a number of ways. From best to worst:
+
+  - Encounter no issues; your tests are fast and isolated.
+  - Add more simulated side effects if you encounter minor issues and simulation
+    is reasonable.
+  - Build a real database simulation layer (fairly complex).
+  - Disable isolation for a single test by using
+    ##LiskDAO::endIsolateAllLiskEffectsToCurrentProcess();## before your test
+    and ##LiskDAO::beginIsolateAllLiskEffectsToCurrentProcess();## after your
+    test. This will disable isolation for one test. NOT RECOMMENDED.
+  - Disable isolation for your entire test case by overriding
+    ##getPhabricatorTestCaseConfiguration()## and providing
+    ##self::PHABRICATOR_TESTCONFIG_ISOLATE_LISK => false## in the configuration
+    dictionary you return. This will disable isolation entirely. STRONGLY NOT
+    RECOMMENDED.
+

src/docs/userguide/remarkup.diviner

@@ -34,7 +34,7 @@ This produces headers like the ones in this document.
 
 Make **lists** by indenting two spaces and beginning each item with a "-":
 
-  lang=demo
+  lang=text
   - milk
   - eggs
   - bread
@@ -51,7 +51,7 @@ Make **code blocks** by indenting two spaces:
 
 You can specify a language for syntax highlighting with "lang=xxx":
 
-  lang=demo
+  lang=text
   lang=html
   <a href="#">...</a>
 
@@ -64,7 +64,7 @@ available (in most cases, this means you need to configure Pygments):
 You can also use a "COUNTEREXAMPLE" header to show that a block of code is
 bad and shouldn't be copied:
 
-  lang=demo
+  lang=text
   COUNTEREXAMPLE
   function f() {
     global $$variable_variable;

src/infrastructure/testing/testcase/__tests__/PhabricatorTrivialTestCase.php

@@ -0,0 +1,38 @@
+<?php
+
+/*
+ * Copyright 2011 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.
+ */
+
+/**
+ * Trivial example test case.
+ */
+class PhabricatorTrivialTestCase extends PhabricatorTestCase {
+
+  // NOTE: Update developer/unit_tests.diviner when updating this class!
+
+  private $two;
+
+  public function willRunOneTest($test_name) {
+    // You can execute setup steps which will run before each test in this
+    // method.
+    $this->two = 2;
+  }
+
+  public function testAllIsRightWithTheWorld() {
+    $this->assertEqual(4, $this->two + $this->two, '2 + 2 = 4');
+  }
+
+}

src/infrastructure/testing/testcase/__tests__/__init__.php

@@ -0,0 +1,12 @@
+<?php
+/**
+ * This file is automatically generated. Lint this module to rebuild it.
+ * @generated
+ */
+
+
+
+phutil_require_module('phabricator', 'infrastructure/testing/testcase');
+
+
+phutil_require_source('PhabricatorTrivialTestCase.php');

src/storage/connection/isolated/AphrontIsolatedDatabaseConnection.php

@@ -73,6 +73,18 @@ class AphrontIsolatedDatabaseConnection extends AphrontDatabaseConnection {
 
   public function executeRawQuery($raw_query) {
 
+    // NOTE: "[\s<>K]*" allows any number of (properly escaped) comments to
+    // appear prior to the INSERT/UPDATE, since this connection escapes them
+    // as "<K>" (above).
+    if (!preg_match('/^[\s<>K]*(INSERT|UPDATE)\s*/i', $raw_query)) {
+      $doc_uri = PhabricatorEnv::getDoclink('article/Writing_Unit_Tests.html');
+      throw new Exception(
+        "Database isolation currently only supports INSERT and UPDATE ".
+        "queries. For more information, see <{$doc_uri}>. You are trying to ".
+        "issue a query which does not begin with INSERT or UPDATE: ".
+        "'".$raw_query."'");
+    }
+
     // NOTE: This method is intentionally simplified for now, since we're only
     // using it to stub out inserts/updates. In the future it will probably need
     // to grow more powerful.

src/storage/connection/isolated/__init__.php

@@ -6,6 +6,7 @@
 
 
 
+phutil_require_module('phabricator', 'infrastructure/env');
 phutil_require_module('phabricator', 'storage/connection/base');
 
 phutil_require_module('phutil', 'utils');