[tools] Remove py_test isolation (#24748)

This is no longer necessary, so we can simplify the code.

Also remove some pre-bzlmod drake_py_unittest_main support code.

Author: jwnimmer-tri

bindings/pydrake/math/BUILD.bazel

@@ -72,7 +72,6 @@ install(
 drake_py_binary(
     name = "math_example",
     srcs = ["math_example.py"],
-    isolate = True,
     deps = [
         "//bindings/pydrake:module_py",
     ],

common/proto/BUILD.bazel

@@ -84,8 +84,6 @@ drake_py_unittest(
     ],
     # TODO(eric.cousineau): Find the source of sporadic CI failures.
     flaky = 1,
-    # We wish to access neighboring files.
-    isolate = 0,
 )
 
 drake_cc_googletest(

common/test_utilities/drake_py_unittest_main.py

@@ -79,28 +79,20 @@ def main():
     main_py = sys.argv[0]
 
     # Parse the test case name out of the runfiles directory name.
-    match = re.search("^(.*bin/(.*?/)?(py/)?([^/]*_test).runfiles/)", main_py)
+    match = re.search("^(.*bin/(.*?/)?([^/]*_test).runfiles/)", main_py)
     if not match:
         print(f"error: no test name match in {main_py}")
         sys.exit(1)
-    runfiles, test_package, _, test_name, = match.groups()
+    runfiles, test_package, test_name, = match.groups()
     test_basename = test_name + ".py"
 
     # Find the test's source file.
-    runfiles_test_filenames = [
-        # With bzlmod disabled.
-        runfiles + "drake/" + test_package + "test/" + test_basename,
-        # With bzlmod enabled.
-        runfiles + "_main/" + test_package + "test/" + test_basename,
-    ]
-    runfiles_test_filename = None
-    for x in runfiles_test_filenames:
-        if os.path.exists(x):
-            runfiles_test_filename = x
-            break
-    if runfiles_test_filename is None:
-        raise RuntimeError("Could not find {} at any of {}".format(
-            test_basename, runfiles_test_filenames))
+    runfiles_test_filename = (
+        runfiles + "_main/" + test_package + "test/" + test_basename
+    )
+    if not os.path.exists(runfiles_test_filename):
+        raise RuntimeError("Could not find {} at {}".format(
+            test_basename, runfiles_test_filename))
 
     # Check the test's source code for a (misleading) __main__.
     realpath_test_filename = os.path.realpath(runfiles_test_filename)

tools/lint/bazel_lint.bzl

@@ -1,4 +1,4 @@
-load("//tools/skylark:drake_py.bzl", "py_test_isolated")
+load("//tools/skylark:drake_py.bzl", "py_linter_test")
 
 #------------------------------------------------------------------------------
 # Internal helper; set up test given name and list of files. Will do nothing
@@ -7,7 +7,7 @@ def _bazel_lint(name, files):
     if files:
         locations = ["$(locations %s)" % f for f in files]
 
-        py_test_isolated(
+        py_linter_test(
             name = name + "_buildifier",
             size = "small",
             srcs = ["@drake//tools/lint:buildifier"],

tools/lint/cpplint.bzl

@@ -1,4 +1,4 @@
-load("//tools/skylark:drake_py.bzl", "py_test_isolated")
+load("//tools/skylark:drake_py.bzl", "py_linter_test")
 
 # From https://bazel.build/reference/be/c-cpp#cc_library.srcs.
 # Keep this list in sync with clang_format_lint.py.
@@ -74,7 +74,7 @@ def _add_linter_rules(
             cpplint_data.append(item)
 
     # Google cpplint.
-    py_test_isolated(
+    py_linter_test(
         name = name + "_cpplint",
         srcs = ["@cpplint_internal//:cpplint"],
         data = cpplint_data + source_labels,
@@ -85,7 +85,7 @@ def _add_linter_rules(
     )
 
     # Additional Drake lint.
-    py_test_isolated(
+    py_linter_test(
         name = name + "_drakelint",
         srcs = ["@drake//tools/lint:drakelint"],
         data = data + source_labels,
@@ -96,7 +96,7 @@ def _add_linter_rules(
     )
 
     # Idempotence during clang-format.
-    py_test_isolated(
+    py_linter_test(
         name = name + "_clang_format_lint",
         srcs = ["@drake//tools/lint:clang_format_lint"],
         data = data + source_labels,

tools/lint/library_lint.bzl

@@ -1,4 +1,4 @@
-load("//tools/skylark:drake_py.bzl", "py_test_isolated")
+load("//tools/skylark:drake_py.bzl", "py_linter_test")
 
 # Keep this constant in sync with library_lint_reporter.py.
 _TAG_EXCLUDE_FROM_PACKAGE = "exclude_from_package"
@@ -80,7 +80,7 @@ def library_lint(
             reporter_args += ["--extra", item]
 
     # Report all of the library_lint results.
-    py_test_isolated(
+    py_linter_test(
         name = "library_lint",
         size = "small",
         srcs = ["@drake//tools/lint:library_lint_reporter"],

tools/lint/python_lint.bzl

@@ -1,4 +1,4 @@
-load("//tools/skylark:drake_py.bzl", "py_test_isolated")
+load("//tools/skylark:drake_py.bzl", "py_linter_test")
 load("//tools/skylark:sh.bzl", "sh_test")
 
 # Internal helper.
@@ -34,7 +34,7 @@ def _python_lint(
     if disallow_executable:
         drakelint_args += ["--disallow_executable"]
     drakelint_args += locations
-    py_test_isolated(
+    py_linter_test(
         name = name_prefix + "_drakelint",
         size = "small",
         srcs = ["@drake//tools/lint:drakelint"],

tools/skylark/drake_py.bzl

@@ -20,102 +20,12 @@ def drake_py_library(
         **kwargs
     )
 
-def _redirect_test_impl(ctx):
-    info = dict(
-        bad_target = ctx.attr.bad_target,
-        good_target = ctx.attr.good_target,
-    )
-    content = """#!/bin/bash
-echo "ERROR: Please use\n    {good_target}\n  The label\n    {bad_target}\n" \
-     " does not exist." >&2
-exit 1
-""".format(**info)
-    ctx.actions.write(
-        output = ctx.outputs.executable,
-        content = content,
-    )
-    return [DefaultInfo()]
-
-# Defines a test which will fail when run via `bazel run` or `bazel test`,
-# redirecting the user to the correct binary to use. This should typically have
-# a "manual" tag.
-_redirect_test = rule(
-    attrs = {
-        "bad_target": attr.string(mandatory = True),
-        "good_target": attr.string(mandatory = True),
-    },
-    test = True,
-    implementation = _redirect_test_impl,
-)
-
-def _py_target_isolated(
-        name,
-        py_target = None,
-        srcs = None,
-        main = None,
-        isolate = True,
-        visibility = None,
-        legacy_create_init = False,
-        **kwargs):
-    # See #8041 for more details.
-    # TODO(eric.cousineau): See if we can remove these shims once we stop
-    # supporting Python 2 (#10606).
-    if py_target == None:
-        fail("Must supply macro function for defining `py_target`.")
-
-    # Targets that are already isolated (with a `py/` prefix) don't require any
-    # additional work. This can happen when linting tests (isolated by
-    # definition) are invoked for isolated Python targets. Otherwise, they get
-    # "doubly isolated" as `py/py/{name}`.
-    prefix = "py/"
-    if isolate and not name.startswith(prefix):
-        actual = prefix + name
-
-        # Preserve original functionality.
-        if not main:
-            main = name + ".py"
-        if not srcs:
-            srcs = [name + ".py"]
-        py_target(
-            name = actual,
-            srcs = srcs,
-            main = main,
-            visibility = visibility,
-            legacy_create_init = legacy_create_init,
-            **kwargs
-        )
-
-        # Disable and redirect original name.
-        package_prefix = "//" + native.package_name() + ":"
-
-        # N.B. Make sure that a test (visible to both `bazel run` and
-        # `bazel test`) with the original name redirects to the isolated
-        # instantiation so users unfamiliar with isolation that use the
-        # "obvious" spelling will be properly informed.
-        _redirect_test(
-            name = name,
-            good_target = package_prefix + actual,
-            bad_target = package_prefix + name,
-            tags = ["manual"],
-            visibility = visibility,
-        )
-    else:
-        py_target(
-            name = name,
-            srcs = srcs,
-            main = main,
-            visibility = visibility,
-            legacy_create_init = legacy_create_init,
-            **kwargs
-        )
-
 def drake_py_binary(
         name,
         srcs = None,
         main = None,
         data = [],
         deps = None,
-        isolate = False,
         tags = [],
         add_test_rule = False,
         test_rule_args = [],
@@ -127,18 +37,11 @@ def drake_py_binary(
         test_rule_rendering = False,
         **kwargs):
     """A wrapper to insert Drake-specific customizations.
-
-    @param isolate (optional, default is False)
-        If True, the binary will be placed in a folder isolated from the
-        library code. This prevents submodules from leaking in as top-level
-        submodules. For more detail, see #8041.
     """
     if main == None and len(srcs) == 1:
         main = srcs[0]
-    _py_target_isolated(
+    py_binary(
         name = name,
-        py_target = py_binary,
-        isolate = isolate,
         srcs = srcs,
         main = main,
         data = data,
@@ -160,7 +63,6 @@ def drake_py_binary(
             # files from their build actions and bazel would error out because
             # of the malformed BUILD file.
             precompile = "disabled",
-            isolate = isolate,
             args = test_rule_args,
             data = data + test_rule_data,
             size = test_rule_size,
@@ -217,7 +119,6 @@ def drake_py_test(
         size = None,
         srcs = None,
         deps = None,
-        isolate = True,
         allow_import_unittest = False,
         allow_network = None,
         display = False,
@@ -228,11 +129,6 @@ def drake_py_test(
         **kwargs):
     """A wrapper to insert Drake-specific customizations.
 
-    @param isolate (optional, default is True)
-        If True, the test binary will be placed in a folder isolated from the
-        library code. This prevents submodules from leaking in as top-level
-        submodules. For more detail, see #8041.
-
     @param allow_import_unittest (optional, default is False)
         If False, this test (and anything it imports) is prevented from doing
         `import unittest`.  This is a guard against writing `unittest`-based
@@ -288,10 +184,8 @@ def drake_py_test(
         opt_in_condition = opt_in_condition,
         opt_out_conditions = opt_out_conditions,
     )
-    _py_target_isolated(
+    py_test(
         name = name,
-        py_target = py_test,
-        isolate = isolate,
         size = size,
         srcs = srcs,
         deps = deps,
@@ -301,25 +195,16 @@ def drake_py_test(
         **kwargs
     )
 
-def py_test_isolated(
+def py_linter_test(
         name,
         **kwargs):
-    """Provides a directory-isolated Python test, robust against shadowing
-    (#8041).
-    """
-    if "lint" in (kwargs.get("tags") or []):
-        # Skip lint tests in coverage builds.
+    """Wrapper for py_test, to be used for running a linter."""
+    py_test(
+        name = name,
         target_compatible_with = select({
+            # Skip lint tests in coverage builds.
             "@drake//tools/kcov:enabled": ["@platforms//:incompatible"],
             "//conditions:default": [],
-        })
-    else:
-        target_compatible_with = None
-
-    _py_target_isolated(
-        name = name,
-        py_target = py_test,
-        isolate = True,
-        target_compatible_with = target_compatible_with,
+        }),
         **kwargs
     )