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
@@ skipping 50 matching lines @@
     process = subprocess.Popen(
         [CATERPILLAR_PATH, 'config', '-i', self.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):
       raise RuntimeError('Configuration file generation failed.')
+    if process.wait():
+      raise subprocess.CalledProcessError()
+
+    if not os.path.exists(self.config_path):
+      raise RuntimeError('Configuration file generation failed.')
 
-    process = subprocess.Popen(
+    output = subprocess.check_output(
         [CATERPILLAR_PATH, 'convert', '-c', self.config_path, self.input_dir,
-         self.output_dir],
-        stdin=subprocess.PIPE,
-        stdout=subprocess.PIPE,
-        stderr=subprocess.STDOUT)
-    process.wait()
+         self.output_dir])
+
+  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))

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

This is descending into the ignored directory, and then deleting all of its
subdirectories.

Instead, you should just delete the ignored directory when it shows up in its
parent (then you can just directory compare directory names without having to
mess around with normpath and basename).

# Do not descend into ignored directories.
for i in range(len(dirnames) - 1, -1, -1):
  if dirnames[i] in ignore_directories:
    del dirnames[i]

I think this is the intended usage of os.walk(topdown=true).

+      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()