Fixes Caterpillar crashing while generating report.

src/end_to_end_test.py

 #!/usr/bin/env python2
 # -*- coding: utf-8 -*-
 
 # Copyright 2016 Google Inc. All Rights Reserved.
 #
 # 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.
 
 """End-to-end tests for Caterpillar."""
 
 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')
 TTS_APP_NAME = 'test_app_tts'
 TTS_REFERENCE_NAME = 'test_app_tts_output'
 TTS_PATH = os.path.join(
     os.path.dirname(TEST_DIR), 'tests', TTS_APP_NAME)
 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()
+
+    if not os.path.exists(cls.config_path):
       raise RuntimeError('Configuration file generation failed.')
 
-    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()
+    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])
+
+  @classmethod
+  def tearDownClass(cls):
+    """Removes the converted test Chrome App."""
+    shutil.rmtree(cls.temp_path)
+
+  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 filename in filenames:
+        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 = 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)
-
-    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)
-
-    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)
-
-    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)
+    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."""
     for dirname, _, filenames in os.walk(TTS_REFERENCE_PATH):
       for filename in filenames:
         if filename == caterpillar.SW_SCRIPT_NAME:
           # Service worker is partly random, so test it elsewhere.
           continue
 
         reference_path = os.path.join(dirname, filename)
@@ skipping 38 matching lines @@
                 '\n'.join(difflib.unified_diff(
                     output_data.split('\n'),
                     reference_data.split('\n'),
                     fromfile=reference_sw_path,
                     tofile=output_sw_path,
                     n=0))))
 
 
 if __name__ == '__main__':
   unittest.main()