Provide setup/teardown hooks for ArcanistPhutilTestCase

Summary:
I'm making a move on Lisk stubbability and need these so I can set up
Lisk isolation unconditionally in Phabricator tests. Also document and organize
this class.

Test Plan:
Changed the expected counts in the meta-tests and got failures. Ran
all unit tests.

Reviewed By: aran
Reviewers: aran, jungejason, tuomaspelkonen
CC: aran
Differential Revision: 192

src/unit/engine/phutil/__tests__/PhutilUnitTestEngineTestCase.php

@@ -23,12 +23,67 @@
  */
 class PhutilUnitTestEngineTestCase extends ArcanistPhutilTestCase {
 
+  static $allTestsCounter = 0;
+  static $oneTestCounter = 0;
+  static $distinctWillRunTests = array();
+  static $distinctDidRunTests = array();
+
+  protected function willRunTests() {
+    self::$allTestsCounter++;
+  }
+
+  protected function didRunTests() {
+    $this->assertEqual(
+      1,
+      self::$allTestsCounter,
+      'Expect willRunTests() has been called once.');
+
+    self::$allTestsCounter--;
+
+    $actual_test_count = 2;
+
+    $this->assertEqual(
+      $actual_test_count,
+      count(self::$distinctWillRunTests),
+      'Expect willRunOneTest() was called once for each test.');
+    $this->assertEqual(
+      $actual_test_count,
+      count(self::$distinctDidRunTests),
+      'Expect didRunOneTest() was called once for each test.');
+    $this->assertEqual(
+      self::$distinctWillRunTests,
+      self::$distinctDidRunTests,
+      'Expect same tests had pre- and post-run callbacks invoked.');
+  }
+
+  public function __destruct() {
+    if (self::$allTestsCounter !== 0) {
+      throw new Exception(
+        "didRunTests() was not called correctly after tests completed!");
+    }
+  }
+
+  protected function willRunOneTest($test) {
+    self::$distinctWillRunTests[$test] = true;
+    self::$oneTestCounter++;
+  }
+
+  protected function didRunOneTest($test) {
+    $this->assertEqual(
+      1,
+      self::$oneTestCounter,
+      'Expect willRunOneTest depth to be one.');
+
+    self::$distinctDidRunTests[$test] = true;
+    self::$oneTestCounter--;
+  }
+
   public function testPass() {
     $this->assertEqual(1, 1, 'This test is expected to pass.');
   }
 
   public function testFail() {
-    $this->assertEqual(1, 2, 'This test is expected to fail.');
+    $this->assertFailure('This test is expected to fail.');
   }
 
 }

src/unit/engine/phutil/testcase/ArcanistPhutilTestCase.php

@@ -19,6 +19,10 @@
 /**
  * Base test case for the very simple libphutil test framework.
  *
+ * @task assert   Making Test Assertions
+ * @task hook     Hooks for Setup and Teardown
+ * @task internal Internals
+ *
  * @group unitrun
  */
 abstract class ArcanistPhutilTestCase {
@@ -26,10 +30,27 @@ abstract class ArcanistPhutilTestCase {
   private $runningTest;
   private $results = array();
 
-  final public function __construct() {
 
-  }
+/* -(  Making Test Assertions  )--------------------------------------------- */
 
+
+  /**
+   * Assert that two values are equal. The test fails if they are not.
+   *
+   * NOTE: This method uses PHP's strict equality test operator ("===") to
+   * compare values. This means values and types must be equal, key order must
+   * be identical in arrays, and objects must be referentially identical.
+   *
+   * @param wild    The theoretically expected value, generated by careful
+   *                reasoning about the properties of the system.
+   * @param wild    The empirically derived value, generated by executing the
+   *                test.
+   * @param string  A human-readable description of what these values represent,
+   *                and particularly of what a discrepancy means.
+   *
+   * @return void
+   * @task assert
+   */
   final protected function assertEqual($expect, $result, $message = null) {
     if ($expect === $result) {
       return;
@@ -45,14 +66,98 @@ abstract class ArcanistPhutilTestCase {
 
     $message = "Values {$expect} and {$result} differ: {$message}";
     $this->failTest($message);
-    throw new ArcanistPhutilTestTerminatedException();
+    throw new ArcanistPhutilTestTerminatedException($message);
   }
 
+
+  /**
+   * Assert an unconditional failure. This is just a convenience method that
+   * better indicates intent than using dummy values with assertEqual(). This
+   * causes test failure.
+   *
+   * @param   string  Human-readable description of the reason for test failure.
+   * @return  void
+   * @task    assert
+   */
   final protected function assertFailure($message) {
     $this->failTest($message);
-    throw new ArcanistPhutilTestTerminatedException();
+    throw new ArcanistPhutilTestTerminatedException($message);
   }
 
+
+/* -(  Hooks for Setup and Teardown  )--------------------------------------- */
+
+
+  /**
+   * This hook is invoked once, before any tests in this class are run. It
+   * gives you an opportunity to perform setup steps for the entire class.
+   *
+   * @return void
+   * @task hook
+   */
+  protected function willRunTests() {
+    return;
+  }
+
+
+  /**
+   * This hook is invoked once, after any tests in this class are run. It gives
+   * you an opportunity to perform teardown steps for the entire class.
+   *
+   * @return void
+   * @task hook
+   */
+  protected function didRunTests() {
+    return;
+  }
+
+
+  /**
+   * This hook is invoked once per test, before the test method is invoked.
+   *
+   * @param string Method name of the test which will be invoked.
+   * @return void
+   * @task hook
+   */
+  protected function willRunOneTest($test_method_name) {
+    return;
+  }
+
+
+  /**
+   * This hook is invoked once per test, after the test method is invoked.
+   *
+   * @param string Method name of the test which was invoked.
+   * @return void
+   * @task hook
+   */
+  protected function didRunOneTest($test_method_name) {
+    return;
+  }
+
+
+/* -(  Internals  )---------------------------------------------------------- */
+
+
+  /**
+   * Construct a new test case. This method is ##final##, use willRunTests() to
+   * provide test-wide setup logic.
+   *
+   * @task internal
+   */
+  final public function __construct() {
+
+  }
+
+
+  /**
+   * Mark the currently-running test as a failure.
+   *
+   * @param string  Human-readable description of problems.
+   * @return void
+   *
+   * @task internal
+   */
   final private function failTest($reason) {
     $result = new ArcanistUnitTestResult();
     $result->setName($this->runningTest);
@@ -61,6 +166,15 @@ abstract class ArcanistPhutilTestCase {
     $this->results[] = $result;
   }
 
+
+  /**
+   * This was a triumph. I'm making a note here: HUGE SUCCESS.
+   *
+   * @param string  Human-readable overstatement of satisfaction.
+   * @return void
+   *
+   * @task internal
+   */
   final private function passTest($reason) {
     $result = new ArcanistUnitTestResult();
     $result->setName($this->runningTest);
@@ -69,19 +183,47 @@ abstract class ArcanistPhutilTestCase {
     $this->results[] = $result;
   }
 
+
+  /**
+   * Execute the tests in this test case. You should not call this directly;
+   * use @{class:PhutilUnitTestEngine} to orchestrate test execution.
+   *
+   * @return void
+   * @task internal
+   */
   final public function run() {
     $this->results = array();
 
     $reflection = new ReflectionClass($this);
-    foreach ($reflection->getMethods() as $method) {
+    $methods = $reflection->getMethods();
+
+    // Try to ensure that poorly-written tests which depend on execution order
+    // (and are thus not properly isolated) will fail.
+    shuffle($methods);
+
+    $this->willRunTests();
+    foreach ($methods as $method) {
       $name = $method->getName();
       if (preg_match('/^test/', $name)) {
         $this->runningTest = $name;
+
         try {
-          call_user_func_array(
-            array($this, $name),
-            array());
-          $this->passTest("All assertions passed.");
+          $this->willRunOneTest($name);
+
+          $test_exception = null;
+          try {
+            call_user_func_array(
+              array($this, $name),
+              array());
+            $this->passTest("All assertions passed.");
+          } catch (Exception $ex) {
+            $test_exception = $ex;
+          }
+
+          $this->didRunOneTest($name);
+          if ($test_exception) {
+            throw $test_exception;
+          }
         } catch (ArcanistPhutilTestTerminatedException $ex) {
           // Continue with the next test.
         } catch (Exception $ex) {
@@ -89,6 +231,7 @@ abstract class ArcanistPhutilTestCase {
         }
       }
     }
+    $this->didRunTests();
 
     return $this->results;
   }