Test DebugAdapter requests, and fix issues (pantsbuild#17678)

Fixes pantsbuild#17672 and fixes pantsbuild#17692

Added new test-code which tests the `DebugAdapterRequest` by running it without `--wait-for-client`. This wouldn't catch issues like pantsbuild#17540 which require a client, but would catches issues like pantsbuild#17672 and pantsbuild#17692.

Fixes:
- Bad debugpy ICs
- Untracked issue where the process would always return 0
- The config wouldn't be used if provided
- Removes the reliance on `importlib_metadata` to load the entrypoint, instead re-enters the already executing pex in-process

src/python/pants/backend/python/goals/BUILD

@@ -3,6 +3,8 @@
 
 python_sources()
 
+python_test_utils(name="test_utils")
+
 python_tests(
     name="tests",
     overrides={
@@ -19,7 +21,7 @@ python_tests(
         },
         "pytest_runner_integration_test.py": {
             "tags": ["platform_specific_behavior"],
-            "timeout": 480,
+            "timeout": 600,
         },
         "repl_integration_test.py": {
             "tags": ["platform_specific_behavior"],

src/python/pants/backend/python/goals/conftest.py

@@ -0,0 +1,18 @@
+# Copyright 2022 Pants project contributors (see CONTRIBUTORS.md).
+# Licensed under the Apache License, Version 2.0 (see LICENSE).
+
+import pytest
+
+from pants.backend.python.subsystems.debugpy import DebugPy
+
+
+@pytest.fixture(autouse=True)
+def debugpy_dont_wait_for_client(monkeypatch):
+    old_debugpy_get_args = DebugPy.get_args
+
+    def get_debugpy_args_but_dont_wait_for_client(*args, **kwargs):
+        result = list(old_debugpy_get_args(*args, **kwargs))
+        result.remove("--wait-for-client")
+        return tuple(result)
+
+    monkeypatch.setattr(DebugPy, "get_args", get_debugpy_args_but_dont_wait_for_client)

src/python/pants/backend/python/goals/pytest_runner.py

@@ -19,7 +19,6 @@
 from pants.backend.python.subsystems.debugpy import DebugPy
 from pants.backend.python.subsystems.pytest import PyTest, PythonTestFieldSet
 from pants.backend.python.subsystems.setup import PythonSetup
-from pants.backend.python.target_types import MainSpecification
 from pants.backend.python.util_rules.interpreter_constraints import InterpreterConstraints
 from pants.backend.python.util_rules.local_dists import LocalDistsPex, LocalDistsPexRequest
 from pants.backend.python.util_rules.pex import Pex, PexRequest, VenvPex, VenvPexProcess
@@ -76,6 +75,7 @@
 )
 from pants.engine.unions import UnionMembership, UnionRule, union
 from pants.option.global_options import GlobalOptions
+from pants.util.frozendict import FrozenDict
 from pants.util.logging import LogLevel
 
 logger = logging.getLogger()
@@ -184,7 +184,7 @@ class TestSetupRequest:
     field_sets: Tuple[PythonTestFieldSet, ...]
     metadata: TestMetadata
     is_debug: bool
-    main: Optional[MainSpecification] = None  # Defaults to pytest.main
+    extra_env: FrozenDict[str, str] = FrozenDict()
     prepend_argv: Tuple[str, ...] = ()
     additional_pexes: Tuple[Pex, ...] = ()
 
@@ -284,7 +284,7 @@ async def setup_pytest_for_target(
         PexRequest(
             output_filename="pytest_runner.pex",
             interpreter_constraints=interpreter_constraints,
-            main=request.main or pytest.main,
+            main=pytest.main,
             internal_only=True,
             pex_path=[pytest_pex, requirements_pex, local_dists.pex, *request.additional_pexes],
         ),
@@ -365,6 +365,7 @@ async def setup_pytest_for_target(
 
     extra_env = {
         "PEX_EXTRA_SYS_PATH": ":".join(prepared_sources.source_roots),
+        **request.extra_env,
         **test_extra_env.env,
         # NOTE: field_set_extra_env intentionally after `test_extra_env` to allow overriding within
         # `python_tests`.
@@ -401,10 +402,10 @@ async def setup_pytest_for_target(
         VenvPexProcess(
             pytest_runner_pex,
             argv=(
-                *(("-c", pytest.config) if pytest.config else ()),
-                *(("-n", "{pants_concurrency}") if xdist_concurrency else ()),
                 *request.prepend_argv,
                 *pytest.args,
+                *(("-c", pytest.config) if pytest.config else ()),
+                *(("-n", "{pants_concurrency}") if xdist_concurrency else ()),
                 # N.B.: Now that we're using command-line options instead of the PYTEST_ADDOPTS
                 # environment variable, it's critical that `pytest_args` comes after `pytest.args`.
                 *pytest_args,
@@ -538,18 +539,26 @@ async def debugpy_python_test(
     batch: PyTestRequest.Batch[PythonTestFieldSet, TestMetadata],
     debugpy: DebugPy,
     debug_adapter: DebugAdapterSubsystem,
-    pytest: PyTest,
+    python_setup: PythonSetup,
 ) -> TestDebugAdapterRequest:
-    debugpy_pex = await Get(Pex, PexRequest, debugpy.to_pex_request())
+    debugpy_pex = await Get(
+        Pex,
+        PexRequest,
+        debugpy.to_pex_request(
+            interpreter_constraints=InterpreterConstraints.create_from_compatibility_fields(
+                [field_set.interpreter_constraints for field_set in batch.elements], python_setup
+            )
+        ),
+    )
 
     setup = await Get(
         TestSetup,
         TestSetupRequest(
             batch.elements,
             batch.partition_metadata,
             is_debug=True,
-            main=debugpy.main,
-            prepend_argv=debugpy.get_args(debug_adapter, pytest.main),
+            prepend_argv=debugpy.get_args(debug_adapter),
+            extra_env=FrozenDict(PEX_MODULE="debugpy"),
             additional_pexes=(debugpy_pex,),
         ),
     )

src/python/pants/backend/python/goals/pytest_runner_integration_test.py

@@ -128,6 +128,7 @@ def run_pytest(
     *,
     extra_args: list[str] | None = None,
     env: dict[str, str] | None = None,
+    test_debug_adapter: bool = True,
 ) -> TestResult:
     _configure_pytest_runner(rule_runner, extra_args=extra_args, env=env)
     batch = _get_pytest_batch(rule_runner, test_targets)
@@ -137,6 +138,19 @@ def run_pytest(
         with mock_console(rule_runner.options_bootstrapper):
             debug_result = rule_runner.run_interactive_process(debug_request.process)
             assert test_result.exit_code == debug_result.exit_code
+
+    if test_debug_adapter:
+        debug_adapter_request = rule_runner.request(TestDebugAdapterRequest, [batch])
+        if debug_adapter_request.process is not None:
+            with mock_console(rule_runner.options_bootstrapper) as mocked_console:
+                _, stdioreader = mocked_console
+                debug_adapter_result = rule_runner.run_interactive_process(
+                    debug_adapter_request.process
+                )
+                assert (
+                    test_result.exit_code == debug_adapter_result.exit_code
+                ), f"{stdioreader.get_stdout()}\n{stdioreader.get_stderr()}"
+
     return test_result
 
 
@@ -289,14 +303,14 @@ def test() -> None:
     py2_tgt = rule_runner.get_target(
         Address(PACKAGE, target_name="py2", relative_file_path="tests.py")
     )
-    result = run_pytest(rule_runner, [py2_tgt], extra_args=extra_args)
+    result = run_pytest(rule_runner, [py2_tgt], extra_args=extra_args, test_debug_adapter=False)
     assert result.exit_code == 2
     assert "SyntaxError: invalid syntax" in result.stdout
 
     py3_tgt = rule_runner.get_target(
         Address(PACKAGE, target_name="py3", relative_file_path="tests.py")
     )
-    result = run_pytest(rule_runner, [py3_tgt], extra_args=extra_args)
+    result = run_pytest(rule_runner, [py3_tgt], extra_args=extra_args, test_debug_adapter=False)
     assert result.exit_code == 0
     assert f"{PACKAGE}/tests.py ." in result.stdout
 
@@ -802,7 +816,6 @@ def test_debug_adaptor_request_argv(rule_runner: RuleRunner) -> None:
         "./pytest_runner.pex_pex_shim.sh",
         "--listen",
         "127.0.0.1:5678",
-        "--wait-for-client",
         "-c",
         unittest.mock.ANY,
         "--color=no",

src/python/pants/backend/python/goals/run_helper.py

@@ -2,9 +2,10 @@
 # Licensed under the Apache License, Version 2.0 (see LICENSE).
 from __future__ import annotations
 
+import dataclasses
 import os
 import textwrap
-from typing import Optional
+from typing import Iterable, Optional
 
 from pants.backend.python.subsystems.debugpy import DebugPy
 from pants.backend.python.target_types import (
@@ -39,6 +40,7 @@ async def _create_python_source_run_request(
     entry_point_field: PexEntryPointField,
     pex_env: PexEnvironment,
     run_in_sandbox: bool,
+    pex_path: Iterable[Pex] = (),
     console_script: Optional[ConsoleScript] = None,
 ) -> RunRequest:
     addresses = [address]
@@ -79,6 +81,8 @@ async def _create_python_source_run_request(
         ),
     )
 
+    pex_request = dataclasses.replace(pex_request, pex_path=(*pex_request.pex_path, *pex_path))
+
     complete_pex_environment = pex_env.in_workspace()
     venv_pex = await Get(VenvPex, VenvPexRequest(pex_request, complete_pex_environment))
     input_digests = [
@@ -116,25 +120,17 @@ async def _create_python_source_run_request(
 async def _create_python_source_run_dap_request(
     regular_run_request: RunRequest,
     *,
-    entry_point_field: PexEntryPointField,
     debugpy: DebugPy,
     debug_adapter: DebugAdapterSubsystem,
-    console_script: Optional[ConsoleScript] = None,
 ) -> RunDebugAdapterRequest:
-    entry_point, debugpy_pex, launcher_digest = await MultiGet(
-        Get(
-            ResolvedPexEntryPoint,
-            ResolvePexEntryPointRequest(entry_point_field),
-        ),
-        Get(Pex, PexRequest, debugpy.to_pex_request()),
-        Get(
-            Digest,
-            CreateDigest(
-                [
-                    FileContent(
-                        "__debugpy_launcher.py",
-                        textwrap.dedent(
-                            """
+    launcher_digest = await Get(
+        Digest,
+        CreateDigest(
+            [
+                FileContent(
+                    "__debugpy_launcher.py",
+                    textwrap.dedent(
+                        """
                             import os
                             CHROOT = os.environ["PANTS_CHROOT"]
 
@@ -162,10 +158,9 @@ def patched_resolve_remote_root(self, local_root, remote_root):
                             from debugpy.server import cli
                             cli.main()
                             """
-                        ).encode("utf-8"),
-                    ),
-                ]
-            ),
+                    ).encode("utf-8"),
+                ),
+            ]
         ),
     )
 
@@ -174,28 +169,17 @@ def patched_resolve_remote_root(self, local_root, remote_root):
         MergeDigests(
             [
                 regular_run_request.digest,
-                debugpy_pex.digest,
                 launcher_digest,
             ]
         ),
     )
     extra_env = dict(regular_run_request.extra_env)
-    extra_env["PEX_PATH"] = os.pathsep.join(
-        [
-            extra_env["PEX_PATH"],
-            # For debugpy to work properly, we need to have just one "environment" for our
-            # command to run in. Therefore, we cobble one together with PEX_PATH.
-            _in_chroot(debugpy_pex.name),
-        ]
-    )
     extra_env["PEX_INTERPRETER"] = "1"
     extra_env["PANTS_CHROOT"] = _in_chroot("").rstrip("/")
-    main = console_script or entry_point.val
-    assert main is not None
     args = [
         *regular_run_request.args,
         _in_chroot("__debugpy_launcher.py"),
-        *debugpy.get_args(debug_adapter, main),
+        *debugpy.get_args(debug_adapter),
     ]
 
     return RunDebugAdapterRequest(digest=merged_digest, args=args, extra_env=extra_env)

src/python/pants/backend/python/goals/run_python_source.py

@@ -10,10 +10,13 @@
 from pants.backend.python.subsystems.debugpy import DebugPy
 from pants.backend.python.subsystems.setup import PythonSetup
 from pants.backend.python.target_types import (
+    InterpreterConstraintsField,
     PexEntryPointField,
     PythonRunGoalUseSandboxField,
     PythonSourceField,
 )
+from pants.backend.python.util_rules.interpreter_constraints import InterpreterConstraints
+from pants.backend.python.util_rules.pex import Pex, PexRequest
 from pants.backend.python.util_rules.pex_environment import PexEnvironment
 from pants.core.goals.run import RunDebugAdapterRequest, RunFieldSet, RunRequest
 from pants.core.subsystems.debug_adapter import DebugAdapterSubsystem
@@ -28,22 +31,24 @@ class PythonSourceFieldSet(RunFieldSet):
     required_fields = (PythonSourceField, PythonRunGoalUseSandboxField)
 
     source: PythonSourceField
-    run_goal_use_sandbox: PythonRunGoalUseSandboxField
+    interpreter_constraints: InterpreterConstraintsField
+    _run_goal_use_sandbox: PythonRunGoalUseSandboxField
+
+    def should_use_sandbox(self, python_setup: PythonSetup) -> bool:
+        if self._run_goal_use_sandbox.value is None:
+            return python_setup.default_run_goal_use_sandbox
+        return self._run_goal_use_sandbox.value
 
 
 @rule(level=LogLevel.DEBUG)
 async def create_python_source_run_request(
-    field_set: PythonSourceFieldSet, pex_env: PexEnvironment, python: PythonSetup
+    field_set: PythonSourceFieldSet, pex_env: PexEnvironment, python_setup: PythonSetup
 ) -> RunRequest:
-    run_goal_use_sandbox = field_set.run_goal_use_sandbox.value
-    if run_goal_use_sandbox is None:
-        run_goal_use_sandbox = python.default_run_goal_use_sandbox
-
     return await _create_python_source_run_request(
         field_set.address,
         entry_point_field=PexEntryPointField(field_set.source.value, field_set.address),
         pex_env=pex_env,
-        run_in_sandbox=run_goal_use_sandbox,
+        run_in_sandbox=field_set.should_use_sandbox(python_setup),
     )
 
 
@@ -52,11 +57,32 @@ async def create_python_source_debug_adapter_request(
     field_set: PythonSourceFieldSet,
     debugpy: DebugPy,
     debug_adapter: DebugAdapterSubsystem,
+    pex_env: PexEnvironment,
+    python_setup: PythonSetup,
 ) -> RunDebugAdapterRequest:
-    run_request = await Get(RunRequest, PythonSourceFieldSet, field_set)
+    debugpy_pex = await Get(
+        # NB: We fold the debugpy PEX into the normally constructed VenvPex so that debugpy is in the
+        # venv, but isn't the main entrypoint. Then we use PEX_* env vars to dynamically have debugpy
+        # be invoked in that VenvPex. Hence, a vanilla Pex.
+        Pex,
+        PexRequest,
+        debugpy.to_pex_request(
+            interpreter_constraints=InterpreterConstraints.create_from_compatibility_fields(
+                [field_set.interpreter_constraints], python_setup
+            )
+        ),
+    )
+
+    run_request = await _create_python_source_run_request(
+        field_set.address,
+        entry_point_field=PexEntryPointField(field_set.source.value, field_set.address),
+        pex_env=pex_env,
+        pex_path=[debugpy_pex],
+        run_in_sandbox=field_set.should_use_sandbox(python_setup),
+    )
+
     return await _create_python_source_run_dap_request(
         run_request,
-        entry_point_field=PexEntryPointField(field_set.source.value, field_set.address),
         debugpy=debugpy,
         debug_adapter=debug_adapter,
     )