Clean up cc_shared_library runfiles test

Simplifies the test of `cc_shared_library`'s runfiles behavior using new rules_testing features, which also result in better failure messages. Also format `BUILD.builtin_test` with buildifier to simplify future contributions. Work towards #21833 Closes #21880. PiperOrigin-RevId: 626315579 Change-Id: Ib7b3d1367586ba7215132d6a9f3711f4ae0a9b09
bazelbuild · Apr 19, 2024 · 8ee8f79 · 8ee8f79
1 parent f2eb91a
commit 8ee8f79
Show file tree

Hide file tree

Showing 5 changed files with 85 additions and 100 deletions.
diff --git a/MODULE.bazel b/MODULE.bazel
@@ -28,7 +28,7 @@ bazel_dep(name = "rules_graalvm", version = "0.11.1")
 bazel_dep(name = "rules_proto", version = "5.3.0-21.7")
 bazel_dep(name = "rules_jvm_external", version = "6.0")
 bazel_dep(name = "rules_python", version = "0.28.0")
-bazel_dep(name = "rules_testing", version = "0.0.4")
+bazel_dep(name = "rules_testing", version = "0.6.0")
 bazel_dep(name = "googletest", version = "1.14.0", repo_name = "com_google_googletest")
 
 # TODO(pcloudy): Add remoteapis and googleapis as Bazel modules in the BCR.

diff --git a/MODULE.bazel.lock b/MODULE.bazel.lock
diff --git a/...tarlark/tests/builtins_bzl/cc/cc_shared_library/test_cc_shared_library/BUILD.builtin_test b/...tarlark/tests/builtins_bzl/cc/cc_shared_library/test_cc_shared_library/BUILD.builtin_test
@@ -1,20 +1,20 @@
+load("@rules_python//python:py_test.bzl", "py_test")
+load("@rules_testing//lib:analysis_test.bzl", "analysis_test")
 load(
     ":starlark_tests.bzl",
     "additional_inputs_test",
     "build_failure_test",
+    "check_linking_action_lib_parameters_test",
+    "exports_test",
+    "forwarding_cc_lib",
     "interface_library_output_group_test",
     "linking_order_test",
+    "nocode_cc_lib",
     "paths_test",
+    "pdb_test",
     "runfiles_test",
-    "check_linking_action_lib_parameters_test",
-    "forwarding_cc_lib",
-    "nocode_cc_lib",
     "wrapped_cc_lib",
-    "exports_test",
-    "pdb_test",
 )
-load("@rules_testing//lib:analysis_test.bzl", "analysis_test")
-load("@rules_python//python:py_test.bzl", "py_test")
 
 LINKABLE_MORE_THAN_ONCE = "LINKABLE_MORE_THAN_ONCE"
 
@@ -69,8 +69,8 @@ cc_binary(
 cc_shared_library(
     name = "python_module",
     features = ["windows_export_all_symbols"],
-    deps = [":a_suffix"],
     shared_lib_name = "python_module.pyd",
+    deps = [":a_suffix"],
 )
 
 cc_shared_library(
@@ -120,20 +120,12 @@ cc_shared_library(
         "//src/main/starlark/tests/builtins_bzl/cc/cc_shared_library/test_cc_shared_library3:diff_pkg_so",
         "private_lib_so",
     ],
-    features = [
-      "windows_export_all_symbols",
-      "generate_pdb_file",
-    ],
     exports_filter = [
         ":indirect_dep2",
     ],
-    deps = [
-        "baz",
-        "foo",
-        "cc_lib_with_no_srcs",
-        "nocode_cc_lib",
-        "should_not_be_linked_cc_lib",
-        "a_suffix",
+    features = [
+        "windows_export_all_symbols",
+        "generate_pdb_file",
     ],
     user_link_flags = select({
         "//src/conditions:linux": [
@@ -144,13 +136,22 @@ cc_shared_library(
         ],
         "//conditions:default": [],
     }),
+    deps = [
+        # do not sort
+        "baz",
+        "foo",
+        "cc_lib_with_no_srcs",
+        "nocode_cc_lib",
+        "should_not_be_linked_cc_lib",
+        "a_suffix",
+    ],
 )
 
 cc_library(
     name = "foo",
     srcs = [
-        "foo.cc",
         "direct_so_file_cc_lib2.h",
+        "foo.cc",
     ] + select({
         "//src/conditions:linux": [":renamed_so_file_copy.so"],
         "//conditions:default": [],
@@ -161,9 +162,13 @@ cc_library(
         "//conditions:default": [],
     }),
     deps = select({
-        ":is_bazel": ["qux2", "hdr_only"],
+        ":is_bazel": [
+            "hdr_only",
+            "qux2",
+        ],
         "//conditions:default": [],
     }) + [
+        # do not sort
         "bar",
         "baz",
         # Not exported.
@@ -271,19 +276,19 @@ cc_shared_library(
         "//src/main/starlark/tests/builtins_bzl/cc/cc_shared_library/test_cc_shared_library3:bar",
     ],
     features = ["windows_export_all_symbols"],
+    user_link_flags = select({
+        "//src/conditions:linux": [
+            "-Wl,--version-script=$(location :bar.lds)",
+        ],
+        "//conditions:default": [],
+    }),
     deps = [
         "bar",
         "bar2",
     ] + select({
         ":is_bazel": ["@test_repo//:bar"],
         "//conditions:default": [],
     }),
-    user_link_flags = select({
-        "//src/conditions:linux": [
-            "-Wl,--version-script=$(location :bar.lds)",
-        ],
-        "//conditions:default": [],
-    }),
 )
 
 cc_library(
@@ -303,7 +308,10 @@ cc_library(
     deps = [
         "barX",
     ] + select({
-        ":is_bazel": ["qux2", "hdr_only"],
+        ":is_bazel": [
+            "hdr_only",
+            "qux2",
+        ],
         "//conditions:default": [],
     }),
 )
@@ -414,10 +422,10 @@ filegroup(
 cc_shared_library(
     name = "renamed_so_file",
     features = ["windows_export_all_symbols"],
+    shared_lib_name = "renamed_so_file.so",
     deps = [
         ":direct_so_file_cc_lib2",
     ],
-    shared_lib_name = "renamed_so_file.so",
 )
 
 cc_library(
@@ -451,7 +459,7 @@ genrule(
 
 cc_library(
     name = "private_lib",
-    srcs = [":private_cc_library.cc"]
+    srcs = [":private_cc_library.cc"],
 )
 
 genrule(
@@ -489,14 +497,17 @@ runfiles_test(
 
 check_linking_action_lib_parameters_test(
     name = "check_binary_doesnt_take_already_linked_in_libs",
+    libs_that_shouldnt_be_present = [
+        "foo",
+        "bar",
+    ],
     target = ":binary",
-    libs_that_shouldnt_be_present = ["foo", "bar"],
 )
 
 check_linking_action_lib_parameters_test(
     name = "check_shared_lib_doesnt_take_already_linked_in_libs",
-    target = ":foo_so",
     libs_that_shouldnt_be_present = ["bar"],
+    target = ":foo_so",
 )
 
 build_failure_test(

diff --git a/...tarlark/tests/builtins_bzl/cc/cc_shared_library/test_cc_shared_library/starlark_tests.bzl b/...tarlark/tests/builtins_bzl/cc/cc_shared_library/test_cc_shared_library/starlark_tests.bzl
@@ -182,55 +182,27 @@ def _runfiles_test_impl(env, target):
     if not env.ctx.target_platform_has_constraint(env.ctx.attr._is_linux[platform_common.ConstraintValueInfo]):
         return
 
-    expected_basenames = [
-        "libfoo_so.so",
-        "libbar_so.so",
-        "libdiff_pkg_so.so",
-        "libdirect_so_file.so",
-        "libprivate_lib_so.so",
-        "renamed_so_file_copy.so",
-        "Smain_Sstarlark_Stests_Sbuiltins_Ubzl_Scc_Scc_Ushared_Ulibrary_Stest_Ucc_Ushared_Ulibrary_Slibfoo_Uso.so",
-        "Smain_Sstarlark_Stests_Sbuiltins_Ubzl_Scc_Scc_Ushared_Ulibrary_Stest_Ucc_Ushared_Ulibrary_Slibbar_Uso.so",
-        "Smain_Sstarlark_Stests_Sbuiltins_Ubzl_Scc_Scc_Ushared_Ulibrary_Stest_Ucc_Ushared_Ulibrary3_Slibdiff_Upkg_Uso.so",
+    # Ignore Python runfiles
+    runfiles = [
+        file.path
+        for file in target[DefaultInfo].default_runfiles.files.to_list()
+        if "python" not in file.path
     ]
-    runfiles = [file.path for file in target[DefaultInfo].default_runfiles.files.to_list()]
-    for runfile in runfiles:
-        # Ignore Python runfiles
-        if "python" in runfile:
-            continue
-
-        found_basename = False
-        for expected_basename in expected_basenames:
-            if runfile.endswith(expected_basename):
-                found_basename = True
-                break
-
-        env.expect.where(
-            detail = runfile + " not found in expected basenames:\n" + "\n".join(expected_basenames),
-        ).that_bool(found_basename).equals(True)
 
     # Match e.g. bazel-out/k8-fastbuild/bin/src/main/starlark/tests/builtins_bzl/cc/cc_shared_library/test_cc_shared_library/libdirect_so_file.so
     path_suffix = "/main/starlark/tests/builtins_bzl/cc/cc_shared_library/test_cc_shared_library"
-    expected_files = [
-        "libbar_so.so",
-        "libdirect_so_file.so",
-        "libfoo_so.so",
-        "libprivate_lib_so.so",
-        "python_test",
-        "renamed_so_file_copy.so",
-    ]
-    for file in expected_files:
-        path = path_suffix + "/" + file
-
-        found = False
-        for runfile in runfiles:
-            if runfile.endswith(path):
-                found = True
-                break
-
-        env.expect.where(
-            detail = file + " not found in runfiles:\n" + "\n".join(runfiles),
-        ).that_bool(found).equals(True)
+    env.expect.that_collection(runfiles).contains_exactly_predicates([
+        matching.str_endswith(path_suffix + "/libfoo_so.so"),
+        matching.str_endswith(path_suffix + "/libbar_so.so"),
+        matching.str_endswith(path_suffix + "/libdirect_so_file.so"),
+        matching.str_endswith(path_suffix + "/libprivate_lib_so.so"),
+        matching.str_endswith(path_suffix + "/renamed_so_file_copy.so"),
+        matching.str_endswith(path_suffix + "3/libdiff_pkg_so.so"),
+        matching.str_endswith("Smain_Sstarlark_Stests_Sbuiltins_Ubzl_Scc_Scc_Ushared_Ulibrary_Stest_Ucc_Ushared_Ulibrary/libbar_so.so"),
+        matching.str_endswith("Smain_Sstarlark_Stests_Sbuiltins_Ubzl_Scc_Scc_Ushared_Ulibrary_Stest_Ucc_Ushared_Ulibrary/libfoo_so.so"),
+        matching.str_endswith("Smain_Sstarlark_Stests_Sbuiltins_Ubzl_Scc_Scc_Ushared_Ulibrary_Stest_Ucc_Ushared_Ulibrary/libprivate_lib_so.so"),
+        matching.str_endswith("Smain_Sstarlark_Stests_Sbuiltins_Ubzl_Scc_Scc_Ushared_Ulibrary_Stest_Ucc_Ushared_Ulibrary3/libdiff_pkg_so.so"),
+    ])
 
 def _runfiles_test_macro(name, target):
     analysis_test(

diff --git a/workspace_deps.bzl b/workspace_deps.bzl
@@ -58,10 +58,10 @@ WORKSPACE_REPOS = {
         "urls": ["https://github.com/bazelbuild/rules_pkg/releases/download/0.9.1/rules_pkg-0.9.1.tar.gz"],
     },
     "rules_testing": {
-        "archive": "rules_testing-v0.0.4.tar.gz",
-        "sha256": "4e21f9aa7996944ce91431f27bca374bff56e680acfe497276074d56bc5d9af2",
-        "strip_prefix": "rules_testing-0.0.4",
-        "urls": ["https://github.com/bazelbuild/rules_testing/releases/download/v0.0.4/rules_testing-v0.0.4.tar.gz"],
+        "archive": "rules_testing-v0.6.0.tar.gz",
+        "sha256": "02c62574631876a4e3b02a1820cb51167bb9cdcdea2381b2fa9d9b8b11c407c4",
+        "strip_prefix": "rules_testing-0.6.0",
+        "urls": ["https://github.com/bazelbuild/rules_testing/releases/download/v0.6.0/rules_testing-v0.6.0.tar.gz"],
     },
     "remote_coverage_tools": {
         "archive": "coverage_output_generator-v2.6.zip",