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_folders=None):
+    """Returns all relative filepaths in a directory and children.
+
+    Args:
+      directory: Path to directory.
+      ignore_folders: List of folder names to ignore.

[Matt Giuca, 2016/02/04 06:36:34]

directory / folder are interchangeable, so don't mix them. (Consistently use
"directory".)

Also make it a set.

[Matthew Alger, 2016/02/04 22:47:06]

Done.

+
+    Returns:
+      set of relative filepaths.
+    """
+    if not ignore_folders:
+      ignore_folders = []
+
+    filepaths = set()
+    for dirname, _, filenames in os.walk(directory):
+      skip = False
+      for folder in ignore_folders:
+        if folder in dirname:

[Matt Giuca, 2016/02/04 06:36:34]

This is not precise enough. If ignore_folders = ['bower_components'], this will
match any path with "bower_components" anywhere in the string, including if it
is part of a directory name.

Is |ignore_folders| supposed to be any folder called that, or only folders whose
full path (relative to |directory|) matches?

Have a look at os.walk with topdown=True. The documentation there explicitly
mentions pruning the search. If |ignore_folders| is supposed to be any directory
name, you can just delete any item in |dirnames| (in the result from walk) that
is in ignore_folders, and you're done.

[Matthew Alger, 2016/02/04 22:47:06]

Oh, that's much cleaner. Done.

+          skip = True
+          break
+
+      if skip:
+        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'])

[Matt Giuca, 2016/02/04 06:36:34]

{'bower_components'}

[Matthew Alger, 2016/02/04 22:47:06]

Done.

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