Merge verify_archive_test_lib with verify_archive_test_main.py.tpl (#705)

* Use Label constructor for verify_archive_test macro

One of the `deps` within the `verify_archive_test` macro
depends on a label, which would be incorrectly evaluated
in a caller's BUILD file to be a non-external package.

Add a Label() call to the label to ensure it is evaluated in the
context of rules_pkg. Also add a test to ensure this can be
correctly used from another workspace.

* Merge verify_archive_test_lib into test template

Instead of exporting verify_archive_test_lib as a
public library embed it into the template for the
test itself.

6 files changed, 108 insertions, 123 deletions

diff --git a/pkg/BUILD b/pkg/BUILD
index e24fc6b..b07253e 100644
--- a/pkg/BUILD
+++ b/pkg/BUILD
@@ -101,12 +101,4 @@ py_binary(
     visibility = ["//visibility:public"],
 )
 
-# This might be public, but use at your own risk
-py_library(
-    name = "verify_archive_test_lib",
-    srcs = ["verify_archive_test_lib.py"],
-    srcs_version = "PY3",
-    visibility = ["//visibility:public"],
-)
-
 exports_files(["verify_archive_test_main.py.tpl"])
diff --git a/pkg/verify_archive.bzl b/pkg/verify_archive.bzl
index 35d78de..39ac059 100644
--- a/pkg/verify_archive.bzl
+++ b/pkg/verify_archive.bzl
@@ -115,14 +115,8 @@ def verify_archive_test(name, target,
     )
     py_test(
         name = name,
-        # Hey reviewer!!! What if we just added the source to the test lib
-        # here, so we would not have to make the library for that public?
         srcs = [":" + test_src],
         main = test_src,
         data = [target],
         python_version = "PY3",
-        deps = [
-            "//pkg:verify_archive_test_lib",
-            "@bazel_tools//tools/python/runfiles",
-        ],
     )
diff --git a/pkg/verify_archive_test_lib.py b/pkg/verify_archive_test_lib.py
deleted file mode 100644
index b3288d3..0000000
--- a/pkg/verify_archive_test_lib.py
+++ /dev/null
@@ -1,107 +0,0 @@
-# Copyright 2023 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.
-"""Compare to content manifest files."""
-
-import json
-import re
-import tarfile
-import unittest
-
-from bazel_tools.tools.python.runfiles import runfiles
-
-
-class VerifyArchiveTest(unittest.TestCase):
-  """Test harness to see if we wrote the content manifest correctly."""
-  
-  #run_files = runfiles.Create()
-  #target_path = VerifyArchiveTest.run_files.Rlocation('rules_pkg/' + target)
-
-  def setUp(self):
-    super(VerifyArchiveTest, self).setUp()
-
-  def scan_target(self, target):
-    parts = target.split('.')
-    ext = parts[-1]
-    if ext[0] == 't' or parts[-2] == 'tar':
-      self.load_tar(target)
-    elif ext[0] == 'z':
-      self.fail('Can not process zip yet')
-    else:
-      self.fail('Can not figure out the archive type for (%s)' % target)
-
-  def load_tar(self, path):
-    self.paths = []
-    with tarfile.open(path, 'r:*') as f:
-      i = 0
-      for info in f:
-        self.paths.append(info.name)
-
-  def assertMinSize(self, min_size):
-    """Check that the archive contains at least min_size entries.
-
-    Args:
-        min_size: The minimum number of targets we expect.
-    """
-    actual_size = len(self.paths)
-    self.assertGreaterEqual(
-        len(self.paths),
-        min_size,
-        msg = "Expected at least %d files, but found only %d" % (
-            min_size, actual_size))
-
-  def assertMaxSize(self, max_size):
-    """Check that the archive contains at most max_size entries.
-
-    Args:
-        max_size: The maximum number of targets we expect.
-    """
-    actual_size = len(self.paths)
-    self.assertLessEqual(
-        len(self.paths),
-        max_size,
-        msg = "Expected at most %d files, but found %d" % (
-            max_size, actual_size))
-
-  def check_must_contain(self, must_contain):
-    plain_patterns = set(must_contain)
-    for path in self.paths:
-      if path in plain_patterns:
-        plain_patterns.remove(path)
-    if len(plain_patterns) > 0:
-      self.fail('These required paths were not found: %s' % ','.join(plain_patterns))
-
-  def check_must_not_contain(self, must_not_contain):
-    plain_patterns = set(must_not_contain)
-    for path in self.paths:
-      if path in plain_patterns:
-        self.fail('Found disallowed path (%s) in the archive' % path)
-
-  def check_must_contain_regex(self, must_contain_regex):
-    for pattern in must_contain_regex:
-      r_comp = re.compile(pattern)
-      matched = False
-      for path in self.paths:
-        if r_comp.match(path):
-          matched = True
-          break
-      if not match:
-        self.fail('Did not find pattern (%s) in the archive' % pattern)
-
-  def check_must_not_contain_regex(self, must_not_contain_regex):
-    for pattern in must_not_contain_regex:
-      r_comp = re.compile(pattern)
-      matched = False
-      for path in self.paths:
-        if r_comp.match(path):
-          self.fail('Found disallowed pattern (%s) in the archive' % pattern)
diff --git a/pkg/verify_archive_test_main.py.tpl b/pkg/verify_archive_test_main.py.tpl
index d9b17cc..2d6a192 100644
--- a/pkg/verify_archive_test_main.py.tpl
+++ b/pkg/verify_archive_test_main.py.tpl
@@ -13,11 +13,95 @@
 # limitations under the License.
 """Tests for generated content manifest."""
 
+import re
+import tarfile
 import unittest
 
-from pkg import verify_archive_test_lib
+class VerifyArchiveTest(unittest.TestCase):
+  """Test harness to see if we wrote the content manifest correctly."""
 
-class ${TEST_NAME}(verify_archive_test_lib.VerifyArchiveTest):
+
+  def setUp(self):
+    super(VerifyArchiveTest, self).setUp()
+
+  def scan_target(self, target):
+    parts = target.split('.')
+    ext = parts[-1]
+    if ext[0] == 't' or parts[-2] == 'tar':
+      self.load_tar(target)
+    elif ext[0] == 'z':
+      self.fail('Can not process zip yet')
+    else:
+      self.fail('Can not figure out the archive type for (%s)' % target)
+
+  def load_tar(self, path):
+    self.paths = []
+    with tarfile.open(path, 'r:*') as f:
+      i = 0
+      for info in f:
+        self.paths.append(info.name)
+
+  def assertMinSize(self, min_size):
+    """Check that the archive contains at least min_size entries.
+
+    Args:
+        min_size: The minimum number of targets we expect.
+    """
+    actual_size = len(self.paths)
+    self.assertGreaterEqual(
+        len(self.paths),
+        min_size,
+        msg = "Expected at least %d files, but found only %d" % (
+            min_size, actual_size))
+
+  def assertMaxSize(self, max_size):
+    """Check that the archive contains at most max_size entries.
+
+    Args:
+        max_size: The maximum number of targets we expect.
+    """
+    actual_size = len(self.paths)
+    self.assertLessEqual(
+        len(self.paths),
+        max_size,
+        msg = "Expected at most %d files, but found %d" % (
+            max_size, actual_size))
+
+  def check_must_contain(self, must_contain):
+    plain_patterns = set(must_contain)
+    for path in self.paths:
+      if path in plain_patterns:
+        plain_patterns.remove(path)
+    if len(plain_patterns) > 0:
+      self.fail('These required paths were not found: %s' % ','.join(plain_patterns))
+
+  def check_must_not_contain(self, must_not_contain):
+    plain_patterns = set(must_not_contain)
+    for path in self.paths:
+      if path in plain_patterns:
+        self.fail('Found disallowed path (%s) in the archive' % path)
+
+  def check_must_contain_regex(self, must_contain_regex):
+    for pattern in must_contain_regex:
+      r_comp = re.compile(pattern)
+      matched = False
+      for path in self.paths:
+        if r_comp.match(path):
+          matched = True
+          break
+      if not match:
+        self.fail('Did not find pattern (%s) in the archive' % pattern)
+
+  def check_must_not_contain_regex(self, must_not_contain_regex):
+    for pattern in must_not_contain_regex:
+      r_comp = re.compile(pattern)
+      matched = False
+      for path in self.paths:
+        if r_comp.match(path):
+          self.fail('Found disallowed pattern (%s) in the archive' % pattern)
+
+
+class ${TEST_NAME}(VerifyArchiveTest):
 
   def setUp(self):
     super(${TEST_NAME}, self).setUp()
diff --git a/tests/mappings/external_repo/pkg/BUILD b/tests/mappings/external_repo/pkg/BUILD
index e5157fb..33db6a8 100644
--- a/tests/mappings/external_repo/pkg/BUILD
+++ b/tests/mappings/external_repo/pkg/BUILD
@@ -13,6 +13,8 @@
 # limitations under the License.
 
 load("@//pkg:mappings.bzl", "pkg_files", "strip_prefix")
+load("@//pkg:tar.bzl", "pkg_tar")
+load("@//pkg:verify_archive.bzl", "verify_archive_test")
 load("@//tests/util:defs.bzl", "fake_artifact")
 load("test.bzl", "test_referencing_remote_file")
 
@@ -39,3 +41,18 @@ pkg_files(
 test_referencing_remote_file(
     name = "pf_local_file_in_extrepo",
 )
+
+# Add a target to ensure verify_archive_test can be
+# called from another workspace
+pkg_tar(
+    name = "external_archive",
+    srcs = ["dir/extproj.sh"],
+)
+
+verify_archive_test(
+    name = "external_archive_test",
+    max_size = 1,
+    min_size = 1,
+    must_contain = ["extproj.sh"],
+    target = ":external_archive",
+)
diff --git a/tests/mappings/mappings_external_repo_test.bzl b/tests/mappings/mappings_external_repo_test.bzl
index aeb3223..09d0e67 100644
--- a/tests/mappings/mappings_external_repo_test.bzl
+++ b/tests/mappings/mappings_external_repo_test.bzl
@@ -133,5 +133,10 @@ def mappings_external_repo_analysis_tests():
             # The main purpose behind it is to verify cases wherein we build a
             # file, but then have it consumed by some remote package.
             "@mappings_test_external_repo//pkg:pf_local_file_in_extrepo",
+            # This test is more focused around the verify_archive_test repo but is still
+            # based on using someting from another repo. In particular, ensuring the
+            # verify_archive_test macro can be called properly from a workspace outside
+            # of the main one.
+            "@mappings_test_external_repo//pkg:external_archive_test",
         ],
     )