Web UI: Refactor unpack_pak to facilitate adding more tests

Refactor the Unpack method to call a single method to unpack each
resource that has been parsed from the pak file. This allows for more
comprehensive testing, without needing to generate fake pak files.

Also add tests for 3 different common resource unpacking scenarios:
- Non-generated resource
- Generated resource
- Shared generated resource, which should not be unpacked and is
  excluded using the new "excludes" argument.

Bug: 1093405
Change-Id: I566ae9dba493931ca39756706df4239f68437c72
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2254702
Commit-Queue: Rebekah Potter <rbpotter@chromium.org>
Reviewed-by: dpapad <dpapad@chromium.org>
Cr-Commit-Position: refs/heads/master@{#780453}

chrome/browser/resources/unpack_pak.py

@@ -24,16 +24,29 @@ from grit.format import data_pack
 def ParseLine(line):
   return re.match('  {"([^"]+)", ([^},]+)', line)
 
-def GetFileAndDirName(pak_dir, resource_path):
-  dirname = os.path.dirname(resource_path)
-  # When files are generated, |dirname| becomes
-  # @out_folder@/<gen_path>/path_to_resource. To make the structure look as if
-  # this file was not generated, remove @out_folder@ and <gen_path>.
-  if ('@out_folder@' in dirname):
-    dirname = os.path.relpath(dirname, os.path.join('@out_folder@', pak_dir))
-  filename = os.path.basename(resource_path)
+def UnpackResource(root_dir, out_path, excludes, resource_path, resource_text):
+    dirname = os.path.dirname(resource_path)
+    # When files are generated, |dirname| becomes
+    # @out_folder@/<gen_path>/path_to_resource. To make the structure look as if
+    # this file was not generated, remove @out_folder@ and <gen_path>.
+    if ('@out_folder@' in dirname):
+      dirname = os.path.relpath(dirname, os.path.join('@out_folder@', root_dir))
+    filename = os.path.basename(resource_path)
+    resource_path = os.path.join(dirname, filename).replace('\\', '/')
+    if (resource_path in excludes):
+      return
+
+    out_dir = os.path.normpath(
+        os.path.join(out_path, dirname)).replace('\\', '/')
+    assert out_dir.startswith(out_path), \
+           'Cannot unpack files to locations not in %s. %s should be removed ' \
+           'from the pak file or excluded from unpack.' \
+           % (out_path, resource_path)
 
-  return (filename, dirname)
+    if not os.path.exists(out_dir):
+      os.makedirs(out_dir)
+    with open(os.path.join(out_dir, filename), 'w') as file:
+      file.write(resource_text)
 
 def Unpack(pak_path, out_path, pak_base_dir, excludes):
   pak_dir = os.path.dirname(pak_path)
@@ -63,27 +76,10 @@ def Unpack(pak_path, out_path, pak_base_dir, excludes):
   assert resource_filenames
 
   root_dir = pak_base_dir if pak_base_dir else pak_dir
-  excludes = excludes or []
   # Extract packed files, while preserving directory structure.
   for (resource_id, text) in data.resources.iteritems():
-    (filename, dirname) = GetFileAndDirName(
-        root_dir, resource_filenames[resource_ids[resource_id]])
-    resource_path = os.path.join(dirname, filename).replace('\\', '/')
-    if (resource_path in excludes):
-      continue
-
-    normalized_dir = os.path.normpath(
-        os.path.join(out_path, dirname)).replace('\\', '/')
-    assert normalized_dir.startswith(out_path), \
-           'Cannot unpack files to locations not in %s. %s should be removed ' \
-           'from the pak file or excluded from unpack.' \
-           % (out_path, resource_path)
-
-    if not os.path.exists(normalized_dir):
-      os.makedirs(normalized_dir)
-    with open(os.path.join(normalized_dir, filename), 'w') as file:
-      file.write(text)
-
+    UnpackResource(root_dir, out_path, excludes or [],
+                   resource_filenames[resource_ids[resource_id]], text)
 
 def main():
   parser = argparse.ArgumentParser()

chrome/browser/resources/unpack_pak_test.py

@@ -4,10 +4,34 @@
 # found in the LICENSE file.
 
 import unpack_pak
+import tempfile
+import os
+import shutil
 import unittest
 
+_HERE_DIR = os.path.dirname(__file__)
 
 class UnpackPakTest(unittest.TestCase):
+  def setUp(self):
+    self._out_folder = None
+    self._tmp_dirs = []
+
+  def tearDown(self):
+    for tmp_dir in self._tmp_dirs:
+      shutil.rmtree(tmp_dir)
+
+  def _setup_output_dirs(self):
+    tmp_dir = tempfile.mkdtemp(dir=_HERE_DIR)
+    self._tmp_dirs.append(tmp_dir)
+    self._root_output_dir = os.path.join(tmp_dir, 'gen', 'mywebui')
+    os.makedirs(self._root_output_dir)
+    self._unpak_dir = os.path.normpath(
+        os.path.join(self._root_output_dir, 'unpak')).replace('\\', '/')
+
+  def _read_unpacked_file(self, file_name):
+    assert self._unpak_dir
+    return open(os.path.join(self._unpak_dir, file_name), 'r').read()
+
   def testMapFileLine(self):
     self.assertTrue(unpack_pak.ParseLine('  {"path.js", IDR_PATH}'))
 
@@ -15,17 +39,35 @@ class UnpackPakTest(unittest.TestCase):
     self.assertTrue(unpack_pak.ParseLine('  {"path.js", IDR_PATH, false}'))
     self.assertTrue(unpack_pak.ParseLine('  {"path.js", IDR_PATH, true}'))
 
-  def testGetFileAndDirName(self):
-    (f, d) = unpack_pak.GetFileAndDirName('out/build/gen/foo', 'a/b.js')
-    self.assertEquals('b.js', f)
-    self.assertEquals('a', d)
-
-  def testGetFileAndDirNameForGeneratedResource(self):
-    (f, d) = unpack_pak.GetFileAndDirName(
-        'out/build/gen/foo',
-        '@out_folder@/out/build/gen/foo/a/b.js')
-    self.assertEquals('b.js', f)
-    self.assertEquals('a', d)
+  def testUnpackResource(self):
+    self._setup_output_dirs()
+    unpack_pak.UnpackResource(
+        os.path.join('gen', 'mywebui'), self._unpak_dir, [],
+        'sub_dir/some_element.js', 'alert(\'hello from element in sub_dir\');')
+    unpacked_contents = self._read_unpacked_file('sub_dir/some_element.js')
+    self.assertIn('hello from element in sub_dir', unpacked_contents)
+
+  def testUnpackGeneratedResource(self):
+    self._setup_output_dirs()
+    generated_resource_path = os.path.join(
+        '@out_folder@', 'gen', 'mywebui', 'sub_dir', 'some_element.js')
+    unpack_pak.UnpackResource(
+        os.path.join('gen', 'mywebui'), self._unpak_dir,
+        [], generated_resource_path,
+        'alert(\'hello from element in sub_dir\');')
+    unpacked_contents = self._read_unpacked_file('sub_dir/some_element.js')
+    self.assertIn('hello from element in sub_dir', unpacked_contents)
+
+  def testUnpackExcludedResource(self):
+    self._setup_output_dirs()
+    generated_shared_resource_path = os.path.join(
+        '@out_folder@', 'gen', 'shared', 'shared_element.js')
+    self.assertEqual(0, len(os.listdir(self._root_output_dir)))
+    unpack_pak.UnpackResource(
+        os.path.join('gen', 'mywebui'), self._unpak_dir,
+        ['../shared/shared_element.js'], generated_shared_resource_path,
+        'alert(\'hello from shared element\');')
+    self.assertEqual(0, len(os.listdir(self._root_output_dir)))
 
 if __name__ == '__main__':
   unittest.main()