Refactor zip tests for more granularity. (#569)

* Refactor zip tests for more granularity.

The single zip_test was getting too big. I want to start creating
tests for more cases, and want to reuse the comparator tooling.

* order methods the right way

4 files changed, 185 insertions, 110 deletions

diff --git a/tests/zip/BUILD b/tests/zip/BUILD
index 332827b..fc689f5 100644
--- a/tests/zip/BUILD
+++ b/tests/zip/BUILD
@@ -16,13 +16,24 @@
 load("//pkg:mappings.bzl", "pkg_attributes", "pkg_mkdirs", "pkg_mklink")
 load("//pkg:zip.bzl", "pkg_zip")
 load("//tests/util:defs.bzl", "directory", "fake_artifact")
-load("@rules_python//python:defs.bzl", "py_test")
+load("@rules_python//python:defs.bzl", "py_library", "py_test")
 load("@bazel_skylib//rules:copy_file.bzl", "copy_file")
 
 package(default_applicable_licenses = ["//:license"])
 
 licenses(["notice"])
 
+py_library(
+    name = "zip_test_lib",
+    srcs = [
+        "zip_test_lib.py",
+    ],
+    srcs_version = "PY3",
+    deps = [
+        "@bazel_tools//tools/python/runfiles",
+    ],
+)
+
 genrule(
     name = "generate_files",
     outs = [
@@ -204,28 +215,43 @@ py_test(
         "zip_test.py",
     ],
     data = [
-        "test_zip_empty.zip",
-        "test_zip_basic.zip",
-        "test_zip_basic_renamed",
+        ":test-zip-strip_prefix-dot.zip",
+        ":test-zip-strip_prefix-empty.zip",
+        ":test-zip-strip_prefix-none.zip",
+        ":test-zip-strip_prefix-zipcontent.zip",
+        ":test_zip_basic.zip",
+        ":test_zip_empty.zip",
+        ":test_zip_package_dir0.zip",
+        ":test_zip_permissions.zip",
+        ":test_zip_timestamp.zip",
+        ":test_zip_tree.zip",
+    ],
+    python_version = "PY3",
+    deps = [
+        ":zip_test_lib",
+        "@bazel_tools//tools/python/runfiles",
+    ],
+)
+
+py_test(
+    name = "zip_byte_for_byte_test",
+    srcs = [
+        "zip_byte_for_byte_test.py",
+    ],
+    data = [
+        ":test_zip_basic.zip",
+        ":test_zip_basic_renamed",
         # We do not actually use this file, we just want to make sure we build
         # it with this name.
-        "test_zip_out.foo",
-        "test_zip_package_dir0.zip",
-        "test_zip_permissions.zip",
-        "test_zip_timestamp.zip",
-        "test_zip_tree.zip",
-        "test-zip-strip_prefix-empty.zip",
-        "test-zip-strip_prefix-none.zip",
-        "test-zip-strip_prefix-zipcontent.zip",
-        "test-zip-strip_prefix-dot.zip",
-
+        ":test_zip_out.foo",
+        ":test_zip_package_dir0.zip",
         # these could be replaced with diff_test() rules (from skylib)
-        "test_zip_basic_timestamp_before_epoch.zip",
-        "test_zip_package_dir1.zip",
-        "test_zip_package_dir2.zip",
+        ":test_zip_basic_timestamp_before_epoch.zip",
+        ":test_zip_package_dir1.zip",
+        ":test_zip_package_dir2.zip",
     ],
     python_version = "PY3",
     deps = [
-        "@bazel_tools//tools/python/runfiles",
+        ":zip_test_lib",
     ],
 )
diff --git a/tests/zip/zip_byte_for_byte_test.py b/tests/zip/zip_byte_for_byte_test.py
new file mode 100644
index 0000000..05744d4
--- /dev/null
+++ b/tests/zip/zip_byte_for_byte_test.py
@@ -0,0 +1,58 @@
+# Copyright 2019 The Bazel Authors. 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.
+
+import filecmp
+import unittest
+
+from tests.zip import zip_test_lib
+
+
+class ZipEquivalency(zip_test_lib.ZipTest):
+  """Check that some generated zip files are equivalent to each-other."""
+
+  def assertFilesEqual(self, actual, expected):
+    """Assert that two zip files contain the same bytes."""
+
+    zips_are_equal = filecmp.cmp(
+        self.get_test_zip(actual),
+        self.get_test_zip(expected),
+    )
+    self.assertTrue(zips_are_equal)
+
+  def test_small_timestamp(self):
+    self.assertFilesEqual(
+        "test_zip_basic_timestamp_before_epoch.zip",
+        "test_zip_basic.zip",
+    )
+
+  def test_extension(self):
+    self.assertFilesEqual(
+        "test_zip_basic_renamed.foo",
+        "test_zip_basic.zip",
+    )
+
+  def test_package_dir1(self):
+    self.assertFilesEqual(
+        "test_zip_package_dir1.zip",
+        "test_zip_package_dir0.zip",
+    )
+
+  def test_package_dir2(self):
+    self.assertFilesEqual(
+        "test_zip_package_dir2.zip",
+        "test_zip_package_dir0.zip",
+    )
+
+if __name__ == "__main__":
+  unittest.main()
diff --git a/tests/zip/zip_test.py b/tests/zip/zip_test.py
index 725f946..b82a442 100644
--- a/tests/zip/zip_test.py
+++ b/tests/zip/zip_test.py
@@ -18,68 +18,14 @@ import unittest
 import zipfile
 
 from bazel_tools.tools.python.runfiles import runfiles
+from tests.zip import zip_test_lib
 
 HELLO_CRC = 2069210904
 LOREM_CRC = 2178844372
 EXECUTABLE_CRC = 342626072
 
-# Unix dir bit and Windows dir bit. Magic from zip spec
-UNIX_DIR_BIT = 0o40000
-MSDOS_DIR_BIT = 0x10
-UNIX_RWX_BITS = 0o777
 
-# The ZIP epoch date: (1980, 1, 1, 0, 0, 0)
-_ZIP_EPOCH_DT = datetime.datetime(1980, 1, 1, 0, 0, 0, tzinfo=datetime.timezone.utc)
-_ZIP_EPOCH_S = int(_ZIP_EPOCH_DT.timestamp())
-
-def seconds_to_ziptime(s):
-  dt = datetime.datetime.utcfromtimestamp(s)
-  return (dt.year, dt.month, dt.day, dt.hour, dt.minute, dt.second)
-
-
-class ZipTest(unittest.TestCase):
-
-  def get_test_zip(self, zip_file):
-    """Get the file path to a generated zip in the runfiles."""
-
-    return self.data_files.Rlocation(
-        "rules_pkg/tests/zip/" + zip_file
-    )
-
-  def setUp(self):
-    super(ZipTest, self).setUp()
-    self.data_files = runfiles.Create()
-
-
-class ZipContentsCase(ZipTest):
-  """Use zipfile to check the contents of some generated zip files."""
-
-  def assertZipFileContent(self, zip_file, content):
-    """Assert that zip_file contains the entries described by content.
-
-    Args:
-        zip_file: the test-package-relative path to a zip file to test.
-        content: an array of dictionaries containing a filename and crc key,
-                 and optionally a timestamp key.
-    """
-    with zipfile.ZipFile(self.get_test_zip(zip_file)) as f:
-      infos = f.infolist()
-      self.assertEqual(len(infos), len(content))
-
-      for info, expected in zip(infos, content):
-        self.assertEqual(info.filename, expected["filename"])
-        if "crc" in expected:
-          self.assertEqual(info.CRC, expected["crc"])
-
-        ts = seconds_to_ziptime(expected.get("timestamp", _ZIP_EPOCH_S))
-        self.assertEqual(info.date_time, ts)
-        if "isdir" in expected:
-          expect_dir_bits = UNIX_DIR_BIT << 16 | MSDOS_DIR_BIT
-          self.assertEqual(info.external_attr & expect_dir_bits,
-                           expect_dir_bits)
-        got = (info.external_attr >> 16) & UNIX_RWX_BITS
-        exp = expected.get("attr", 0o555)
-        self.assertEqual(got, exp, 'got %o, expected %o' % (got, exp))
+class ZipContentsTests(zip_test_lib.ZipContentsTestBase):
 
   def test_empty(self):
     self.assertZipFileContent("test_zip_empty.zip", [])
@@ -143,41 +89,5 @@ class ZipContentsCase(ZipTest):
     ])
 
 
-class ZipEquivalency(ZipTest):
-  """Check that some generated zip files are equivalent to each-other."""
-
-  def assertFilesEqual(self, actual, expected):
-    """Assert that two zip files contain the same bytes."""
-
-    zips_are_equal = filecmp.cmp(
-        self.get_test_zip(actual),
-        self.get_test_zip(expected),
-    )
-    self.assertTrue(zips_are_equal)
-
-  def test_small_timestamp(self):
-    self.assertFilesEqual(
-        "test_zip_basic_timestamp_before_epoch.zip",
-        "test_zip_basic.zip",
-    )
-
-  def test_extension(self):
-    self.assertFilesEqual(
-        "test_zip_basic_renamed.foo",
-        "test_zip_basic.zip",
-    )
-
-  def test_package_dir1(self):
-    self.assertFilesEqual(
-        "test_zip_package_dir1.zip",
-        "test_zip_package_dir0.zip",
-    )
-
-  def test_package_dir2(self):
-    self.assertFilesEqual(
-        "test_zip_package_dir2.zip",
-        "test_zip_package_dir0.zip",
-    )
-
 if __name__ == "__main__":
   unittest.main()
diff --git a/tests/zip/zip_test_lib.py b/tests/zip/zip_test_lib.py
new file mode 100644
index 0000000..99ca997
--- /dev/null
+++ b/tests/zip/zip_test_lib.py
@@ -0,0 +1,81 @@
+# Copyright 2019 The Bazel Authors. 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.
+
+import datetime
+import filecmp
+import unittest
+import zipfile
+
+from bazel_tools.tools.python.runfiles import runfiles
+
+
+# Unix dir bit and Windows dir bit. Magic from zip spec
+UNIX_DIR_BIT = 0o40000
+MSDOS_DIR_BIT = 0x10
+UNIX_RWX_BITS = 0o777
+
+# The ZIP epoch date: (1980, 1, 1, 0, 0, 0)
+_ZIP_EPOCH_DT = datetime.datetime(1980, 1, 1, 0, 0, 0, tzinfo=datetime.timezone.utc)
+_ZIP_EPOCH_S = int(_ZIP_EPOCH_DT.timestamp())
+
+def seconds_to_ziptime(s):
+  dt = datetime.datetime.utcfromtimestamp(s)
+  return (dt.year, dt.month, dt.day, dt.hour, dt.minute, dt.second)
+
+
+class ZipTest(unittest.TestCase):
+
+  def setUp(self):
+    super(ZipTest, self).setUp()
+    self.data_files = runfiles.Create()
+
+  def get_test_zip(self, zip_file):
+    """Get the file path to a generated zip in the runfiles."""
+
+    return self.data_files.Rlocation(
+        "rules_pkg/tests/zip/" + zip_file
+    )
+
+
+class ZipContentsTestBase(ZipTest):
+  """Use zipfile to check the contents of some generated zip files."""
+
+  def assertZipFileContent(self, zip_file, content):
+    """Assert that zip_file contains the entries described by content.
+
+    Args:
+        zip_file: the test-package-relative path to a zip file to test.
+        content: an array of dictionaries containing a filename and crc key,
+                 and optionally a timestamp key.
+    """
+    with zipfile.ZipFile(self.get_test_zip(zip_file)) as f:
+      infos = f.infolist()
+      self.assertEqual(len(infos), len(content))
+
+      for info, expected in zip(infos, content):
+        self.assertEqual(info.filename, expected["filename"])
+        if "crc" in expected:
+          self.assertEqual(info.CRC, expected["crc"])
+
+        ts = seconds_to_ziptime(expected.get("timestamp", _ZIP_EPOCH_S))
+        self.assertEqual(info.date_time, ts)
+        if "isdir" in expected:
+          expect_dir_bits = UNIX_DIR_BIT << 16 | MSDOS_DIR_BIT
+          self.assertEqual(info.external_attr & expect_dir_bits,
+                           expect_dir_bits)
+          self.assertEqual((info.external_attr >> 16) & UNIX_RWX_BITS,
+                           expected.get("attr", 0o755))
+        else:
+          self.assertEqual((info.external_attr >> 16) & UNIX_RWX_BITS,
+                           expected.get("attr", 0o555))