Fixes Caterpillar crashing while generating report.

src/end_to_end_test.py

Index: src/end_to_end_test.py
diff --git a/src/end_to_end_test.py b/src/end_to_end_test.py
index 8c3582154c9e9bb3cc253fe54c07e95a1a37b6bd..92076d8f2b41219fa237ffb6044b4c3f6341ec7a 100644
--- a/src/end_to_end_test.py
+++ b/src/end_to_end_test.py
@@ -22,12 +22,13 @@ from __future__ import print_function, division, unicode_literals
 import difflib
 import os
 import re
+import shutil
 import subprocess
 import sys
+import tempfile
 import unittest
 
 import caterpillar
-import caterpillar_test
 
 TEST_DIR = os.path.dirname(os.path.realpath(__file__))
 CATERPILLAR_PATH = os.path.join(TEST_DIR, 'caterpillar.py')
@@ -39,71 +40,85 @@ TTS_REFERENCE_PATH = os.path.join(
     os.path.dirname(TEST_DIR), 'tests', TTS_REFERENCE_NAME)
 
 
-class TestEndToEndConvert(caterpillar_test.TestCaseWithTempDir):
+class TestEndToEndConvert(unittest.TestCase):

[Matt Giuca, 2016/02/05 00:23:49]

Why no longer using TestCaseWithTempDir?

   """Converts an entire Chrome App and checks the output is correct."""
 
-  def setUp(self):
+  @classmethod
+  def setUpClass(cls):
     """Converts a test Chrome App."""
-    super(TestEndToEndConvert, self).setUp()
-    self.input_dir = TTS_PATH
-    self.output_dir = os.path.join(self.temp_path, 'tts test output')
-    self.config_path = os.path.join(self.temp_path, 'config.json')
+    cls.temp_path = tempfile.mkdtemp()
+    cls.input_dir = TTS_PATH
+    cls.output_dir = os.path.join(cls.temp_path, 'tts test output')
+    cls.config_path = os.path.join(cls.temp_path, 'config.json')
 
     encoding = sys.getfilesystemencoding()
 
-    self.boilerplate_dir = 'caterpillar-📂'
-    self.report_dir = 'report ✓✓✓'
-    self.start_url = 'ttstest.html'
+    cls.boilerplate_dir = 'caterpillar-📂'
+    cls.report_dir = 'report ✓✓✓'
+    cls.start_url = 'ttstest.html'
     config_input = '{}\n{}\n{}\n'.format(
-        self.boilerplate_dir, self.report_dir, self.start_url).encode(encoding)
+        cls.boilerplate_dir, cls.report_dir, cls.start_url).encode(encoding)
 
     # Generate a config file using Caterpillar.
     process = subprocess.Popen(
-        [CATERPILLAR_PATH, 'config', '-i', self.config_path],
+        [CATERPILLAR_PATH, 'config', '-i', cls.config_path],
         stdin=subprocess.PIPE,
         stdout=subprocess.PIPE,
         stderr=subprocess.STDOUT)
     process.communicate(input=config_input)
     process.wait()
 
-    if not os.path.exists(self.config_path):
+    if not os.path.exists(cls.config_path):
       raise RuntimeError('Configuration file generation failed.')
+    if process.wait():
+      raise subprocess.CalledProcessError()
 
-    process = subprocess.Popen(
-        [CATERPILLAR_PATH, 'convert', '-c', self.config_path, self.input_dir,
-         self.output_dir],
-        stdin=subprocess.PIPE,
-        stdout=subprocess.PIPE,
-        stderr=subprocess.STDOUT)
-    process.wait()
+    if not os.path.exists(cls.config_path):
+      raise RuntimeError('Configuration file generation failed.')
 
-  def test_output_matches_reference(self):
-    """Tests that the output matches the reference output."""
-    expected_files = set()
-    for dirname, _, filenames in os.walk(TTS_REFERENCE_PATH):
-      for filename in filenames:
-        relpath = os.path.relpath(
-            os.path.join(dirname, filename), TTS_REFERENCE_PATH)
-        expected_files.add(relpath)
+    output = subprocess.check_output(

[Matt Giuca, 2016/02/05 00:23:49]

It's really quite strange that you run the entire test in setUp (now
setUpClass), and just do checking in the individual test methods.

But for performance reasons, I suppose this makes sense to do it.

However, this shouldn't be done in this CL. This CL is fixing a major bug ---
Caterpillar is broken and the tests aren't testing it. So you shouldn't be
refactoring or fixing test performance here. Please do this later.

Come to think of it, you should probably save the get_relative_filepaths
refactor for that cleanup CL as well.

+        [CATERPILLAR_PATH, 'convert', '-c', cls.config_path, cls.input_dir,
+         cls.output_dir])
 
-    for dirname, _, filenames in os.walk(self.output_dir):
-      for filename in filenames:
-        relpath = os.path.relpath(
-            os.path.join(dirname, filename), self.output_dir)
-        self.assertIn(relpath, expected_files)
+  @classmethod
+  def tearDownClass(cls):
+    """Removes the converted test Chrome App."""
+    shutil.rmtree(cls.temp_path)
 
-    expected_files = set()
-    for dirname, _, filenames in os.walk(self.output_dir):
-      for filename in filenames:
-        relpath = os.path.relpath(
-            os.path.join(dirname, filename), self.output_dir)
-        expected_files.add(relpath)
+  def get_relative_filepaths(self, directory, ignore_directories=None):
+    """Returns all relative filepaths in a directory and children.
+
+    Args:
+      directory: Path to directory.
+      ignore_directories: Set of directory names to ignore. Note that all
+        children will be ignored too.
+
+    Returns:
+      set of relative filepaths.
+    """
+    if not ignore_directories:
+      ignore_directories = set()
+
+    filepaths = set()
+    for dirname, dirnames, filenames in os.walk(directory, topdown=True):
+      basename = os.path.basename(os.path.normpath(dirname))
+      if basename in ignore_directories:
+        dirnames[:] = []
+        continue
 
-    for dirname, _, filenames in os.walk(TTS_REFERENCE_PATH):
       for filename in filenames:
-        relpath = os.path.relpath(
-            os.path.join(dirname, filename), TTS_REFERENCE_PATH)
-        self.assertIn(relpath, expected_files)
+        relpath = os.path.relpath(os.path.join(dirname, filename), directory)
+        filepaths.add(relpath)
+
+    return filepaths
+
+  def test_output_matches_reference(self):
+    """Tests that the output matches the reference output."""
+    expected_files = self.get_relative_filepaths(TTS_REFERENCE_PATH,
+                                                 {'bower_components'})
+    actual_files = self.get_relative_filepaths(self.output_dir,
+                                               {'bower_components'})
+    self.assertEqual(expected_files, actual_files)
 
   def test_all_correct_contents(self):
     """Tests that the content of all non-static output files is expected."""