make results_test.py work better within python unittest framework

gm/rebaseline_server/results_test.py

Index: gm/rebaseline_server/results_test.py
diff --git a/gm/rebaseline_server/results_test.py b/gm/rebaseline_server/results_test.py
index 94e9ca2de9beda31e839f04a5aeccc7f99dd37b3..421ef48125ad551bd02f5e6b1af5b97b28baf6ca 100755
--- a/gm/rebaseline_server/results_test.py
+++ b/gm/rebaseline_server/results_test.py
@@ -13,12 +13,12 @@ on the housekeeper bot, but first make sure it works properly after having been
 checked out (from both git and svn)
 
 TODO(epoger): Create a command to update the expected results (in
-OUTPUT_DIR_EXPECTATIONS) when appropriate.  For now, you should:
-1. examine the results in OUTPUT_DIR and make sure they are ok
-2. rm -rf OUTPUT_DIR_EXPECTATIONS
-3. mv OUTPUT_DIR OUTPUT_DIR_EXPECTATIONS
+OUTPUT_DIR_EXPECTED) when appropriate.  For now, you should:
+1. examine the results in OUTPUT_DIR_ACTUAL and make sure they are ok
+2. rm -rf OUTPUT_DIR_EXPECTED
+3. mv OUTPUT_DIR_ACTUAL OUTPUT_DIR_EXPECTED
 Although, if you're using an SVN checkout, this will blow away .svn directories
-within OUTPUT_DIR_EXPECTATIONS, which wouldn't be good...
+within OUTPUT_DIR_EXPECTED, which wouldn't be good...
 
 """
 
@@ -35,27 +35,43 @@ import gm_json  # must import results first, so that gm_json will be in sys.path
 
 PARENT_DIR = os.path.dirname(os.path.realpath(__file__))
 INPUT_DIR = os.path.join(PARENT_DIR, 'tests', 'inputs')
-OUTPUT_DIR = os.path.join(PARENT_DIR, 'tests', 'outputs', 'actual')
-OUTPUT_DIR_EXPECTATIONS = os.path.join(
-    PARENT_DIR, 'tests', 'outputs', 'expected')
+OUTPUT_DIR_ACTUAL   = os.path.join(PARENT_DIR, 'tests', 'outputs', 'actual')
+OUTPUT_DIR_EXPECTED = os.path.join(PARENT_DIR, 'tests', 'outputs', 'expected')
+
 
 class ResultsTest(unittest.TestCase):
 
   def setUp(self):
-    self._tempdir = tempfile.mkdtemp()
+    self._temp_dir = tempfile.mkdtemp()
+    self._output_dir_actual   = os.path.join(OUTPUT_DIR_ACTUAL, self.id())
+    self._output_dir_expected = os.path.join(OUTPUT_DIR_EXPECTED, self.id())
+    create_empty_dir(self._output_dir_actual)
 
   def tearDown(self):
-    shutil.rmtree(self._tempdir)
+    shutil.rmtree(self._temp_dir)
+    different_files = find_different_files(self._output_dir_actual,
+                                           self._output_dir_expected)
+    assert (not different_files), \

[rmistry, 2014/01/03 15:18:42]

I find it slightly strange to see an assert in tearDown, but maybe that is ok.

[epoger, 2014/01/03 15:42:07]

Good point.  Added a comment explaining the situation.  Maybe we'll come up with
a better way.

[rmistry, 2014/01/03 15:44:18]

Also another minor reason to not have it here: if somebody later comes along and
adds cleanup code below the assert then if tests fail the artifacts will not be
cleaned up.
This is not major, we can come up with a better solution later.

[epoger, 2014/01/03 15:48:09]

Good point again.  Added that to my explanatory comment.

+      ('found differing files between actual dir %s and expected dir %s: %s' %
+       (self._output_dir_actual, self._output_dir_expected, different_files))
 
   def test_gm(self):
     """Process results of a GM run with the Results object."""
     results_obj = results.Results(
         actuals_root=os.path.join(INPUT_DIR, 'gm-actuals'),
         expected_root=os.path.join(INPUT_DIR, 'gm-expectations'),
-        generated_images_root=self._tempdir)
+        generated_images_root=self._temp_dir)
     gm_json.WriteToFile(results_obj.get_results_of_type(results.RESULTS_ALL),
-                        os.path.join(OUTPUT_DIR, 'gm.json'))
+                        os.path.join(self._output_dir_actual, 'gm.json'))
+
 
+def create_empty_dir(path):
+  """Create an empty directory at the given path."""
+  if os.path.isdir(path):
+    shutil.rmtree(path)
+  elif os.path.lexists(path):
+    os.remove(path)
+  os.makedirs(path)
 

[rmistry, 2014/01/03 15:18:42]

Nit: Add one more newline here.

[epoger, 2014/01/03 15:42:07]

Done.

 def find_different_files(dir1, dir2, ignore_subtree_names=None):
   """Returns a list of any files that differ between the directory trees rooted
@@ -86,12 +102,8 @@ def find_different_files(dir1, dir2, ignore_subtree_names=None):
 
 
 def main():
-  if not os.path.isdir(OUTPUT_DIR):

[epoger, 2014/01/02 21:24:08]

Moving all the pre- and post-test steps into setUp() and tearDown() allows us to
launch this unittest automatically in the standard Python unittest way:

epoger@wpgntat-ubiq141:~/src/skia/red/skia$ python -m unittest discover
--start-directory=gm/rebaseline_server --pattern='*_test.py' --verbose
test_gm (results_test.ResultsTest)
Process results of a GM run with the Results object. ... ok

----------------------------------------------------------------------
Ran 1 test in 0.833s

OK

-    os.makedirs(OUTPUT_DIR)
   suite = unittest.TestLoader().loadTestsFromTestCase(ResultsTest)
   unittest.TextTestRunner(verbosity=2).run(suite)
-  different_files = find_different_files(OUTPUT_DIR, OUTPUT_DIR_EXPECTATIONS)
-  assert (not different_files), 'found differing files: %s' % different_files
 
 
 if __name__ == '__main__':