Merge pull request #72 from aiuto/sync1

Make the test cases much less brittle

10 files changed, 91 insertions, 129 deletions

diff --git a/examples/src/server_report.golden b/examples/src/server_report.golden
deleted file mode 100644
index 8be07a0..0000000
--- a/examples/src/server_report.golden
+++ /dev/null
@@ -1,3 +0,0 @@
-= //examples/vendor/constant_gen:license_for_emitted_code
-  kind: @//examples/my_org/licenses:unencumbered
-  conditions: []
diff --git a/examples/vendor/constant_gen/BUILD b/examples/vendor/constant_gen/BUILD
index e70f489..26e86b6 100644
--- a/examples/vendor/constant_gen/BUILD
+++ b/examples/vendor/constant_gen/BUILD
@@ -14,7 +14,6 @@
 # An example of a code generator with a distinct license for the generated code.
 
 load("@rules_license//rules:compliance.bzl", "licenses_used")
-load("@rules_license//tools:test_helpers.bzl", "golden_test")
 load("@rules_license//rules:license.bzl", "license")
 load(":defs.bzl", "constant_gen")
 
@@ -63,21 +62,21 @@ licenses_used(
     deps = [":constant_generator"],
 )
 
-golden_test(
-    name = "verify_generator_licenses",
-    golden = "generator_licenses.golden",
-    subject = ":generator_licenses.json",
-)
-
 licenses_used(
     name = "generated_code_licenses",
     # Note: using default output file name
     deps = [":libhello"],
 )
 
-golden_test(
-    name = "verify_generated_code_licenses",
-    golden = "generated_code_licenses.golden",
-    subject = ":generated_code_licenses.json",
+py_test(
+    name = "verify_licenses_test",
+    srcs = ["verify_licenses_test.py"],
+    data = [
+        ":generator_licenses.json",
+        ":generated_code_licenses.json",
+    ],
+    python_version = "PY3",
+    deps = [
+        "//tests:license_test_utils",
+    ],
 )
-
diff --git a/examples/vendor/constant_gen/generated_code_licenses.golden b/examples/vendor/constant_gen/generated_code_licenses.golden
deleted file mode 100644
index c4c8ef1..0000000
--- a/examples/vendor/constant_gen/generated_code_licenses.golden
+++ /dev/null
@@ -1,41 +0,0 @@
-[
-  {
-    "top_level_target": "//examples/vendor/constant_gen:libhello",
-    "dependencies": [
-      {
-        "target_under_license": "//examples/vendor/constant_gen:libhello",
-        "licenses": [
-          "//examples/vendor/constant_gen:license_for_emitted_code"
-        ]
-      },
-      {
-        "target_under_license": "//examples/vendor/constant_gen:libhello_src_",
-        "licenses": [
-          "//examples/vendor/constant_gen:license_for_emitted_code"
-        ]
-      }
-    ],
-    "licenses": [
-      {
-        "label": "//examples/vendor/constant_gen:license_for_emitted_code",
-        "rule": "//examples/vendor/constant_gen:license_for_emitted_code",
-        "license_kinds": [
-          {
-            "target": "@//examples/my_org/licenses:unencumbered",
-            "name": "unencumbered",
-            "conditions": []
-          }
-        ],
-        "copyright_notice": "",
-        "package_name": "Trivial Code Generator Output",
-        "package_url": "",
-        "package_version": "",
-        "license_text": "examples/vendor/constant_gen/LICENSE.on_output",
-        "used_by": [
-          "//examples/vendor/constant_gen:libhello",
-          "//examples/vendor/constant_gen:libhello_src_"
-        ]
-      }
-    ]
-  }
-]
diff --git a/examples/vendor/constant_gen/generator_licenses.golden b/examples/vendor/constant_gen/generator_licenses.golden
deleted file mode 100644
index 4b2f175..0000000
--- a/examples/vendor/constant_gen/generator_licenses.golden
+++ /dev/null
@@ -1,34 +0,0 @@
-[
-  {
-    "top_level_target": "//examples/vendor/constant_gen:constant_generator",
-    "dependencies": [
-      {
-        "target_under_license": "//examples/vendor/constant_gen:constant_generator",
-        "licenses": [
-          "//examples/vendor/constant_gen:license"
-        ]
-      }
-    ],
-    "licenses": [
-      {
-        "label": "//examples/vendor/constant_gen:license",
-        "rule": "//examples/vendor/constant_gen:license",
-        "license_kinds": [
-          {
-            "target": "@//examples/my_org/licenses:generic_restricted",
-            "name": "generic_restricted",
-            "conditions": ["restricted"]
-          }
-        ],
-        "copyright_notice": "",
-        "package_name": "Trivial Code Generator",
-        "package_url": "",
-        "package_version": "",
-        "license_text": "examples/vendor/constant_gen/LICENSE",
-        "used_by": [
-          "//examples/vendor/constant_gen:constant_generator"
-        ]
-      }
-    ]
-  }
-]
diff --git a/examples/vendor/constant_gen/verify_licenses_test.py b/examples/vendor/constant_gen/verify_licenses_test.py
new file mode 100644
index 0000000..4fddef8
--- /dev/null
+++ b/examples/vendor/constant_gen/verify_licenses_test.py
@@ -0,0 +1,40 @@
+"""Test that we see the expected licenses for some generated code."""
+
+import codecs
+import os
+
+import unittest
+from tests import license_test_utils
+
+class VerifyLicensesTest(unittest.TestCase):
+
+  def test_generater_license(self):
+    licenses_info = license_test_utils.load_licenses_info(
+        os.path.join(os.path.dirname(__file__), "generator_licenses.json"))
+
+    expected = {
+        "examples/vendor/constant_gen:constant_generator": [
+            "examples/vendor/constant_gen:license"
+        ],
+    }
+    license_test_utils.check_licenses_of_dependencies(
+        self, licenses_info, expected)
+
+  def test_generated_code_license(self):
+    licenses_info = license_test_utils.load_licenses_info(
+        os.path.join(os.path.dirname(__file__), "generated_code_licenses.json"))
+
+    expected = {
+        "examples/vendor/constant_gen:libhello": [
+            "/constant_gen:license_for_emitted_code",
+        ],
+        "examples/vendor/constant_gen:libhello_src_": [
+            "/constant_gen:license_for_emitted_code",
+        ],
+    }
+    license_test_utils.check_licenses_of_dependencies(
+        self, licenses_info, expected)
+
+if __name__ == "__main__":
+  unittest.main()
+
diff --git a/tests/BUILD b/tests/BUILD
index 10b8560..1d7961f 100644
--- a/tests/BUILD
+++ b/tests/BUILD
@@ -1,6 +1,5 @@
 # Test cases for license rules.
 
-load("@rules_license//tools:test_helpers.bzl", "golden_test")
 load("@rules_license//rules:compliance.bzl", "check_license", "licenses_used")
 load("@rules_license//rules:license.bzl", "license")
 load("@rules_license//rules:license_kind.bzl", "license_kind")
@@ -119,7 +118,10 @@ licenses_used(
 py_test(
     name = "hello_licenses_test",
     srcs = ["hello_licenses_test.py"],
-    data = [":hello_licenses.json"],
+    data = [
+        ":hello_licenses.json",
+        ":hello_cc_copyrights.txt",
+    ],
     python_version = "PY3",
     deps = [
         ":license_test_utils",
@@ -142,19 +144,3 @@ check_license(
         ":hello_java",
     ],
 )
-
-golden_test(
-    name = "verify_cc_app_test",
-    golden = "hello_cc_copyrights.golden",
-    subject = ":hello_cc_copyrights.txt",
-)
-
-golden_test(
-    name = "verify_java_app_test",
-    golden = "hello_java_copyrights.golden",
-    subject = ":hello_java_copyrights.txt",
-)
-
-exports_files([
-    "hello_licenses.golden",
-])
diff --git a/tests/hello_cc_copyrights.golden b/tests/hello_cc_copyrights.golden
deleted file mode 100755
index 38f67ad..0000000
--- a/tests/hello_cc_copyrights.golden
+++ /dev/null
@@ -1,3 +0,0 @@
-package(A test case package/0.0.4), copyright(Copyright © 2019 Uncle Toasty)
-package(A test case package), copyright()
-
diff --git a/tests/hello_java_copyrights.golden b/tests/hello_java_copyrights.golden
deleted file mode 100755
index 0ba7362..0000000
--- a/tests/hello_java_copyrights.golden
+++ /dev/null
@@ -1,2 +0,0 @@
-package(A test case package/0.0.4), copyright(Copyright © 2019 Uncle Toasty)
-package(A test case package), copyright()
diff --git a/tests/hello_licenses_test.py b/tests/hello_licenses_test.py
index 465688f..de62c7f 100644
--- a/tests/hello_licenses_test.py
+++ b/tests/hello_licenses_test.py
@@ -1,5 +1,6 @@
 """Tests for google3.tools.build_defs.license.tests.hello_licenses."""
 
+import codecs
 import os
 
 import unittest
@@ -9,13 +10,8 @@ from tests import license_test_utils
 class HelloLicensesTest(unittest.TestCase):
 
   def test_has_expected_licenses(self):
-    package_base = license_test_utils.LICENSE_PACKAGE_BASE
     licenses_info = license_test_utils.load_licenses_info(
         os.path.join(os.path.dirname(__file__), "hello_licenses.json"))
-    licenses_info = license_test_utils.filter_dependencies(
-        licenses_info,
-        target_filter=lambda targ: targ.startswith(package_base),
-        licenses_filter=lambda lic: lic.startswith(package_base))
 
     expected = {
         "/tests:hello": [
@@ -29,6 +25,18 @@ class HelloLicensesTest(unittest.TestCase):
     license_test_utils.check_licenses_of_dependencies(
         self, licenses_info, expected)
 
+  def test_has_expected_copyrights(self):
+    copyrights_file = os.path.join(os.path.dirname(__file__),
+                                   "hello_cc_copyrights.txt")
+    with codecs.open(copyrights_file, encoding="utf-8") as inp:
+      copyrights = inp.read().split('\n')
+      self.assertIn(
+          "package(A test case package/0.0.4), copyright(Copyright © 2019 Uncle Toasty)",
+          copyrights)
+      self.assertIn(
+          "package(A test case package), copyright()",
+          copyrights)
+
 
 if __name__ == "__main__":
   unittest.main()
diff --git a/tests/license_test_utils.py b/tests/license_test_utils.py
index 2c5a18a..cc29e59 100644
--- a/tests/license_test_utils.py
+++ b/tests/license_test_utils.py
@@ -3,7 +3,6 @@
 import codecs
 import json
 
-
 # This is extracted out to make it easier to keep test equivalence between
 # the OSS version and Google.
 LICENSE_PACKAGE_BASE = "/"
@@ -52,21 +51,34 @@ def check_licenses_of_dependencies(test_case, licenses_info, expected,
   Args:
     test_case: (TestCase) the test.
     licenses_info: (dict) licenses info.
-    expected: (dict) map of target names to the licenses they are under. Names
-        must be relative to the licenses package, not absolute.
+    expected: (dict) map of target name suffixes to the licenses they are under.
     path_prefix: (str) prefix to prepend to targets and licenses in expected.
        This turns the relative target names to absolute ones.
   """
 
   # Turn the list of deps into a dict by target for easier comparison.
-  print(licenses_info)
   deps_to_licenses = {
-      x["target_under_license"].lstrip('@'): set(l.strip('@') for l in x["licenses"])
+      x["target_under_license"].lstrip("@"): set(l.strip("@") for l in x["licenses"])
       for x in licenses_info[0]["dependencies"]}
-  print(deps_to_licenses)
 
-  for target, licenses in expected.items():
-    got_licenses = set(deps_to_licenses[path_prefix + target])
-    for lic in licenses:
-      test_case.assertIn(path_prefix + lic, got_licenses)
-  # future: Maybe check that deps is not larger than expected.
+  target_names = ",".join(deps_to_licenses.keys())
+  # This is n**2, but N is typically < 3 or we are doing this wrong.
+  for want_target, want_licenses in expected.items():
+    found_target = False
+    for got_target, got_licenses in deps_to_licenses.items():
+      if got_target.endswith(want_target):
+        found_target = True
+        test_case.assertEqual(len(want_licenses), len(got_licenses))
+        found_license = False
+        for want_l in want_licenses:
+          for got_l in got_licenses:
+            if got_l.endswith(want_l):
+              found_license = True
+              break
+        test_case.assertTrue(
+            found_license,
+            msg="license (%s) not a suffix in %s" % (want_l, got_licenses))
+        break
+    test_case.assertTrue(
+        found_target,
+        msg="target (%s) not a suffix in [%s]" % (want_target, target_names))