Remove boilerplate for run and test plugins (#17774)

This change provides default rules for `run` and `test` plugins for the `--debug` and `--debug-adapter` flags so that clients don't have to define them unless they opt-in. It also provides us a single exception raised if a feature isn't supported (some of the plugins raised a string "this is a stub", which isn't very user-friendly). While I was in the neighborhood, I fixed some holdover code from when test plugins registered using `TestFieldSet` as the union. I don't love adding a `rules()` to a `FieldSet` subclass because some plugin subclasses inherit from multiple field sets. That can be solved with some `super()` magic, but the alternative is to break the `run` API and add a new request type solely for this. Right now this 100% backwards compatible (a feat) so I'm open to comments.
pantsbuild · Dec 11, 2022 · 4dc2d02 · 4dc2d02
1 parent e621bca
commit 4dc2d02
Show file tree

Hide file tree

Showing 19 changed files with 157 additions and 206 deletions.
diff --git a/docs/markdown/Writing Plugins/common-plugin-tasks/plugin-upgrade-guide.md b/docs/markdown/Writing Plugins/common-plugin-tasks/plugin-upgrade-guide.md
@@ -6,6 +6,25 @@ hidden: false
 createdAt: "2020-10-12T16:19:01.543Z"
 updatedAt: "2022-07-25T20:02:17.695Z"
 ---
+
+2.16
+----
+
+### `RunFieldSet` and `TestRequest` now have a `.rules()` method
+
+These methods should be used to register your run/test plugins:
+
+```python
+
+def rules():
+    return [
+        *MyRunFieldSetSubclass.rules(),
+        *MyTestRequestSubclass.rules(),
+    ]
+```
+
+Additionally, these types now by-default register the implementations for the rules used for `--debug`/`--debug-adapter`. If your plugin doesn't support these flags, simply remove the rules you've declared and let the default ones handle erroring. If your plugin does support these, set the class property(s) `supports_debug = True`/`supports_debug_adapter = True`, respectively.
+
 2.15
 ----
 

diff --git a/docs/markdown/Writing Plugins/common-plugin-tasks/plugins-run-goal.md b/docs/markdown/Writing Plugins/common-plugin-tasks/plugins-run-goal.md
@@ -72,7 +72,7 @@ from pants.engine.unions import UnionRule
 def rules():
     return [
       	*collect_rules(),
-        UnionRule(RunFieldSet, BashRunFieldSet),
+        *BashRunFieldSet.rules(),
     ]
 ```
 
@@ -156,7 +156,32 @@ def rules():
 
 Now, when you run `./pants run path/to/binary.sh`, Pants should run the program.
 
-4. Add tests (optional)
+4. Define `@rule`s for debugging
+------------------------------------
+
+`./pants run` exposes `--debug-adapter` options for debugging code. To hook into this behavior, opt-in in your `RunRequest` subclass and define an additional rule:
+
+```python
+from pants.core.goals.run import RunDebugAdapterRequest
+from pants.core.subsystems.debug_adapter import DebugAdapterSubsystem
+
+@dataclass(frozen=True)
+class BashRunFieldSet(RunFieldSet):
+    ...  # Fields from earlier
+    supports_debug_adapter = True  # Supports --debug-adapter
+
+
+@rule
+async def run_bash_binary_debug_adapter(
+    field_set: BashRunFieldSet,
+    debug_adapter: DebugAdapterSubsystem,
+) -> RunDebugAdapterRequest:
+    ...
+```
+
+Your rule should be configured to wait for client connection before continuing.
+
+5. Add tests (optional)
 -----------------------
 
 Refer to [Testing rules](doc:rules-api-testing). TODO
diff --git a/docs/markdown/Writing Plugins/common-plugin-tasks/plugins-test-goal.md b/docs/markdown/Writing Plugins/common-plugin-tasks/plugins-test-goal.md
@@ -76,18 +76,6 @@ class ExampleTestFieldSet(TestFieldSet):
         return tgt.get(SkipExampleTestsField).value
 ```
 
-Register your new subclass as a valid `TestFieldSet` using a `UnionRule`:
-
-```python
-from pants.engine.unions import UnionRule
-
-def rules():
-    return [
-        # Add to any other existing rules here:
-        UnionRule(TestFieldSet, ExampleTestFieldSet),
-    ]
-```
-
 3. Set up a `Subsystem` for your test runner
 --------------------------------------------
 
@@ -231,10 +219,17 @@ If you didn't override the `partitioner_type` in your `TestRequest` subclass, `e
 7. Define `@rule`s for debug testing
 ------------------------------------
 
-`./pants test` exposes `--debug` and `--debug-adapter` options for interactive execution of tests. To hook into these execution modes, define two additional rules:
+`./pants test` exposes `--debug` and `--debug-adapter` options for interactive execution of tests. To hook into these execution modes, opt-in in your `TestRequest` subclass and define one/both additional rules:
 
 ```python
 from pants.core.goals.test import TestDebugAdapterRequest, TestDebugRequest
+from pants.core.subsystems.debug_adapter import DebugAdapterSubsystem
+
+@dataclass(frozen=True)
+class ExampleTestRequest(TestRequest):
+    ...  # Fields from earlier
+    supports_debug = True  # Supports --debug
+    supports_debug_adapter = True  # Supports --debug-adapter
 
 @rule
 async def setup_example_debug_test(
@@ -245,8 +240,7 @@ async def setup_example_debug_test(
 @rule
 async def setup_example_debug_adapter_test(
     batch: ExampleTestRequest.Batch[ExampleTestFieldSet, ExampleTestMetadata],
+    debug_adapter: DebugAdapterSubsystem,
 ) -> TestDebugAdapterRequest:
     ...
 ```
-
-You _must_ define these rules to avoid rule-graph errors. If your test runner is not compatible with the Debug Adapter, or if it doesn't benefit from running in `--debug` mode, you can simply `raise` a `NotImplementedError` saying so in one/both of these rules.
diff --git a/src/python/pants/backend/docker/goals/run_image.py b/src/python/pants/backend/docker/goals/run_image.py
@@ -11,11 +11,10 @@
 from pants.backend.docker.target_types import DockerImageRegistriesField, DockerImageSourceField
 from pants.backend.docker.util_rules.docker_binary import DockerBinary
 from pants.core.goals.package import BuiltPackage, PackageFieldSet
-from pants.core.goals.run import RunDebugAdapterRequest, RunFieldSet, RunRequest
+from pants.core.goals.run import RunFieldSet, RunRequest
 from pants.engine.env_vars import EnvironmentVars, EnvironmentVarsRequest
 from pants.engine.rules import Get, MultiGet, collect_rules, rule
 from pants.engine.target import WrappedTarget, WrappedTargetRequest
-from pants.engine.unions import UnionRule
 
 
 @dataclass(frozen=True)
@@ -60,17 +59,8 @@ async def docker_image_run_request(
     )
 
 
-@rule
-async def docker_image_run_debug_adapter_request(
-    field_set: DockerRunFieldSet,
-) -> RunDebugAdapterRequest:
-    raise NotImplementedError(
-        "Debugging a Docker image using a debug adapter has not yet been implemented."
-    )
-
-
 def rules():
     return [
         *collect_rules(),
-        UnionRule(RunFieldSet, DockerRunFieldSet),
+        *DockerRunFieldSet.rules(),
     ]
diff --git a/src/python/pants/backend/go/goals/run_binary.py b/src/python/pants/backend/go/goals/run_binary.py
@@ -5,10 +5,9 @@
 
 from pants.backend.go.goals.package_binary import GoBinaryFieldSet
 from pants.core.goals.package import BuiltPackage, PackageFieldSet
-from pants.core.goals.run import RunDebugAdapterRequest, RunFieldSet, RunRequest
+from pants.core.goals.run import RunRequest
 from pants.engine.internals.selectors import Get
 from pants.engine.rules import collect_rules, rule
-from pants.engine.unions import UnionRule
 
 
 @rule
@@ -19,14 +18,8 @@ async def create_go_binary_run_request(field_set: GoBinaryFieldSet) -> RunReques
     return RunRequest(digest=binary.digest, args=(os.path.join("{chroot}", artifact_relpath),))
 
 
-@rule
-async def go_binary_run_debug_adapter_request(
-    field_set: GoBinaryFieldSet,
-) -> RunDebugAdapterRequest:
-    raise NotImplementedError(
-        "Debugging a Go binary using a debug adapter has not yet been implemented."
-    )
-
-
 def rules():
-    return [*collect_rules(), UnionRule(RunFieldSet, GoBinaryFieldSet)]
+    return [
+        *collect_rules(),
+        *GoBinaryFieldSet.rules(),
+    ]
diff --git a/src/python/pants/backend/go/goals/test.py b/src/python/pants/backend/go/goals/test.py
@@ -40,15 +40,7 @@
 from pants.backend.go.util_rules.import_analysis import ImportConfig, ImportConfigRequest
 from pants.backend.go.util_rules.link import LinkedGoBinary, LinkGoBinaryRequest
 from pants.backend.go.util_rules.tests_analysis import GeneratedTestMain, GenerateTestMainRequest
-from pants.core.goals.test import (
-    TestDebugAdapterRequest,
-    TestDebugRequest,
-    TestExtraEnv,
-    TestFieldSet,
-    TestRequest,
-    TestResult,
-    TestSubsystem,
-)
+from pants.core.goals.test import TestExtraEnv, TestFieldSet, TestRequest, TestResult, TestSubsystem
 from pants.core.target_types import FileSourceField
 from pants.core.util_rules.source_files import SourceFiles, SourceFilesRequest
 from pants.engine.env_vars import EnvironmentVars, EnvironmentVarsRequest
@@ -57,7 +49,6 @@
 from pants.engine.process import FallibleProcessResult, Process, ProcessCacheScope
 from pants.engine.rules import Get, MultiGet, collect_rules, rule
 from pants.engine.target import Dependencies, DependenciesRequest, SourcesField, Target, Targets
-from pants.engine.unions import UnionRule
 from pants.util.logging import LogLevel
 from pants.util.ordered_set import FrozenOrderedSet
 
@@ -476,21 +467,8 @@ def compilation_failure(exit_code: int, stdout: str | None, stderr: str | None)
     )
 
 
-@rule
-async def generate_go_tests_debug_request(_: GoTestRequest.Batch) -> TestDebugRequest:
-    raise NotImplementedError("This is a stub.")
-
-
-@rule
-async def generate_go_tests_debug_adapter_request(
-    _: GoTestRequest.Batch,
-) -> TestDebugAdapterRequest:
-    raise NotImplementedError("This is a stub.")
-
-
 def rules():
     return [
         *collect_rules(),
-        UnionRule(TestFieldSet, GoTestFieldSet),
         *GoTestRequest.rules(),
     ]
diff --git a/src/python/pants/backend/helm/test/unittest.py b/src/python/pants/backend/helm/test/unittest.py
@@ -24,14 +24,7 @@
 from pants.backend.helm.util_rules.chart import HelmChart, HelmChartRequest
 from pants.backend.helm.util_rules.sources import HelmChartRoot, HelmChartRootRequest
 from pants.backend.helm.util_rules.tool import HelmProcess
-from pants.core.goals.test import (
-    TestDebugAdapterRequest,
-    TestDebugRequest,
-    TestFieldSet,
-    TestRequest,
-    TestResult,
-    TestSubsystem,
-)
+from pants.core.goals.test import TestFieldSet, TestRequest, TestResult, TestSubsystem
 from pants.core.target_types import ResourceSourceField
 from pants.core.util_rules.source_files import SourceFiles, SourceFilesRequest
 from pants.engine.addresses import Address
@@ -45,7 +38,6 @@
     TransitiveTargets,
     TransitiveTargetsRequest,
 )
-from pants.engine.unions import UnionRule
 from pants.util.logging import LogLevel
 
 logger = logging.getLogger(__name__)
@@ -166,24 +158,11 @@ async def run_helm_unittest(
     )
 
 
-@rule
-async def generate_helm_unittest_debug_request(_: HelmUnitTestRequest.Batch) -> TestDebugRequest:
-    raise NotImplementedError("Can not debug Helm unit tests")
-
-
-@rule
-async def generate_helm_unittest_debug_adapter_request(
-    _: HelmUnitTestRequest.Batch,
-) -> TestDebugAdapterRequest:
-    raise NotImplementedError("Can not debug Helm unit tests")
-
-
 def rules():
     return [
         *collect_rules(),
         *subsystem_rules(),
         *dependency_rules(),
         *tool.rules(),
-        UnionRule(TestFieldSet, HelmUnitTestFieldSet),
         *HelmUnitTestRequest.rules(),
     ]
diff --git a/src/python/pants/backend/python/goals/pytest_runner.py b/src/python/pants/backend/python/goals/pytest_runner.py
@@ -34,7 +34,6 @@
     TestDebugAdapterRequest,
     TestDebugRequest,
     TestExtraEnv,
-    TestFieldSet,
     TestRequest,
     TestResult,
     TestSubsystem,
@@ -430,6 +429,8 @@ class PyTestRequest(TestRequest):
     tool_subsystem = PyTest
     field_set_type = PythonTestFieldSet
     partitioner_type = PartitionerType.CUSTOM
+    supports_debug = True
+    supports_debug_adapter = True
 
 
 @rule(desc="Partition Pytest", level=LogLevel.DEBUG)
@@ -595,7 +596,6 @@ def rules():
     return [
         *collect_rules(),
         *pytest.rules(),
-        UnionRule(TestFieldSet, PythonTestFieldSet),
         UnionRule(PytestPluginSetupRequest, RuntimePackagesPluginRequest),
         *PyTestRequest.rules(),
     ]
diff --git a/src/python/pants/backend/python/goals/run_pex_binary.py b/src/python/pants/backend/python/goals/run_pex_binary.py
@@ -4,23 +4,15 @@
 import os
 
 from pants.backend.python.goals.package_pex_binary import PexBinaryFieldSet
-from pants.backend.python.subsystems.debugpy import DebugPy
-from pants.backend.python.target_types import PexBinaryDefaults, PexLayout
-from pants.backend.python.util_rules.pex_environment import PexEnvironment
+from pants.backend.python.target_types import PexLayout
 from pants.core.goals.package import BuiltPackage
-from pants.core.goals.run import RunDebugAdapterRequest, RunFieldSet, RunRequest
-from pants.core.subsystems.debug_adapter import DebugAdapterSubsystem
+from pants.core.goals.run import RunRequest
 from pants.engine.rules import Get, collect_rules, rule
-from pants.engine.unions import UnionRule
 from pants.util.logging import LogLevel
 
 
 @rule(level=LogLevel.DEBUG)
-async def create_pex_binary_run_request(
-    field_set: PexBinaryFieldSet,
-    pex_binary_defaults: PexBinaryDefaults,
-    pex_env: PexEnvironment,
-) -> RunRequest:
+async def create_pex_binary_run_request(field_set: PexBinaryFieldSet) -> RunRequest:
     built_pex = await Get(BuiltPackage, PexBinaryFieldSet, field_set)
     relpath = built_pex.artifacts[0].relpath
     assert relpath is not None
@@ -33,18 +25,13 @@ async def create_pex_binary_run_request(
     )
 
 
-@rule
-async def run_pex_debug_adapter_binary(
-    field_set: PexBinaryFieldSet,
-    debugpy: DebugPy,
-    debug_adapter: DebugAdapterSubsystem,
-) -> RunDebugAdapterRequest:
-    # NB: Technically we could run this using `debugpy`, however it is unclear how the user
-    # would be able to debug the code, as the client and server will disagree on the code's path.
-    raise NotImplementedError(
-        "Debugging a `pex_binary` using a debug adapter has not yet been implemented."
-    )
+# NB: Technically we could implement RunDebugAdapterRequest by using `debugpy`.
+# However it is unclear how the user would be able to debug the code,
+# as the client and server will disagree on the code's path.
 
 
 def rules():
-    return [*collect_rules(), UnionRule(RunFieldSet, PexBinaryFieldSet)]
+    return [
+        *collect_rules(),
+        *PexBinaryFieldSet.rules(),
+    ]
diff --git a/src/python/pants/backend/python/goals/run_python_source.py b/src/python/pants/backend/python/goals/run_python_source.py
@@ -22,12 +22,12 @@
 from pants.core.subsystems.debug_adapter import DebugAdapterSubsystem
 from pants.engine.internals.selectors import Get
 from pants.engine.rules import collect_rules, rule
-from pants.engine.unions import UnionRule
 from pants.util.logging import LogLevel
 
 
 @dataclass(frozen=True)
 class PythonSourceFieldSet(RunFieldSet):
+    supports_debug_adapter = True
     required_fields = (PythonSourceField, PythonRunGoalUseSandboxField)
 
     source: PythonSourceField
@@ -91,5 +91,5 @@ async def create_python_source_debug_adapter_request(
 def rules():
     return [
         *collect_rules(),
-        UnionRule(RunFieldSet, PythonSourceFieldSet),
+        *PythonSourceFieldSet.rules(),
     ]