From e9f1866c53a61329d6c07a533a32f497145797c1 Mon Sep 17 00:00:00 2001 From: Rich Chiodo false Date: Mon, 8 Dec 2025 17:08:04 -0800 Subject: [PATCH 1/8] Fix arq quoting to work in runInTerminal --- CONTRIBUTING.md | 2 ++ src/debugpy/adapter/launchers.py | 17 +++++++++++++++ tests/debug/session.py | 15 ++++++++++--- tests/debugpy/test_args.py | 37 +++++++++++++++++++++++++++++++- tests/debugpy/test_django.py | 1 - tests/debugpy/test_flask.py | 1 - 6 files changed, 67 insertions(+), 6 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 89197fa08..4906cdcf7 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -93,6 +93,8 @@ The tests are run concurrently, and the default number of workers is 8. You can While tox is the recommended way to run the test suite, pytest can also be invoked directly from the root of the repository. This requires packages in tests/requirements.txt to be installed first. +Using a venv created by tox in the '.tox' folder can make it easier to get the pytest configuration correct. Debugpy needs to be installed into the venv for the tests to run, so using the tox generated .venv makes that easier. + #### Keeping logs on test success There's an internal setting `debugpy_log_passed` that if set to true will not erase the logs after a successful test run. Just search for this in the code and remove the code that deletes the logs on success. diff --git a/src/debugpy/adapter/launchers.py b/src/debugpy/adapter/launchers.py index 62f948817..60e201d04 100644 --- a/src/debugpy/adapter/launchers.py +++ b/src/debugpy/adapter/launchers.py @@ -153,6 +153,23 @@ def on_launcher_connected(sock): request_args["cwd"] = cwd if shell_expand_args: request_args["argsCanBeInterpretedByShell"] = True + + # VS Code debugger extension may pass us an argument indicating the + # quoting character to use in the terminal. Otherwise default based on platform. + default_quote = "'" if os.name != "nt" else '"' + quote_char = arguments["terminalQuoteCharacter"] if "terminalQuoteCharacter" in arguments else default_quote + + # VS code doesn't quote arguments if `argsCanBeInterpretedByShell` is true, + # so we need to do it ourselves for the arguments up to the call to the adapter. + args = request_args["args"] + for i in range(len(args)): + if args[i] == "--": + break + s = args[i] + if " " in s and not (s.startswith('"') and s.endswith('"')) or (s.startswith("'") and s.endswith("'")): + s = f"{quote_char}{s}{quote_char}" + args[i] = s + try: # It is unspecified whether this request receives a response immediately, or only # after the spawned command has completed running, so do not block waiting for it. diff --git a/tests/debug/session.py b/tests/debug/session.py index 05abea62a..728489881 100644 --- a/tests/debug/session.py +++ b/tests/debug/session.py @@ -366,9 +366,18 @@ def _make_env(self, base_env, codecov=True): return env def _make_python_cmdline(self, exe, *args): - return [ - str(s.strpath if isinstance(s, py.path.local) else s) for s in [exe, *args] - ] + def normalize(s): + # Convert py.path.local to string + if isinstance(s, py.path.local): + s = s.strpath + else: + s = str(s) + # Strip surrounding quotes if present + if len(s) >= 2 and (s[0] == s[-1] == '"' or s[0] == s[-1] == "'"): + s = s[1:-1] + return s + + return [normalize(x) for x in (exe, *args)] def spawn_debuggee(self, args, cwd=None, exe=sys.executable, setup=None): assert self.debuggee is None diff --git a/tests/debugpy/test_args.py b/tests/debugpy/test_args.py index 263325528..639615c64 100644 --- a/tests/debugpy/test_args.py +++ b/tests/debugpy/test_args.py @@ -2,6 +2,8 @@ # Licensed under the MIT License. See LICENSE in the project root # for license information. +import os +import sys import pytest from debugpy.common import log @@ -35,7 +37,8 @@ def code_to_debug(): @pytest.mark.parametrize("target", targets.all) @pytest.mark.parametrize("run", runners.all_launch) @pytest.mark.parametrize("expansion", ["preserve", "expand"]) -def test_shell_expansion(pyfile, target, run, expansion): +@pytest.mark.parametrize("python_with_space", [False, True]) +def test_shell_expansion(pyfile, tmpdir, target, run, expansion, python_with_space): if expansion == "expand" and run.console == "internalConsole": pytest.skip('Shell expansion is not supported for "internalConsole"') @@ -57,14 +60,34 @@ def expand(args): args[i] = arg[1:] log.info("After expansion: {0}", args) + captured_run_in_terminal_args = [] + class Session(debug.Session): def run_in_terminal(self, args, cwd, env): + captured_run_in_terminal_args.append(args[:]) # Capture a copy of the args expand(args) return super().run_in_terminal(args, cwd, env) argslist = ["0", "$1", "2"] args = argslist if expansion == "preserve" else " ".join(argslist) + with Session() as session: + # Create a Python wrapper with a space in the path if requested + if python_with_space: + # Create a directory with a space in the name + python_dir = tmpdir / "python with space" + python_dir.mkdir() + + if sys.platform == "win32": + wrapper = python_dir / "python.cmd" + wrapper.write(f'@echo off\n"{sys.executable}" %*') + else: + wrapper = python_dir / "python.sh" + wrapper.write(f'#!/bin/sh\nexec "{sys.executable}" "$@"') + os.chmod(wrapper.strpath, 0o777) + + session.config["python"] = wrapper.strpath + backchannel = session.open_backchannel() with run(session, target(code_to_debug, args=args)): pass @@ -73,3 +96,15 @@ def run_in_terminal(self, args, cwd, env): expand(argslist) assert argv == [some.str] + argslist + + # Verify that the python executable path is correctly quoted if it contains spaces + if python_with_space and captured_run_in_terminal_args: + terminal_args = captured_run_in_terminal_args[0] + log.info("Captured runInTerminal args: {0}", terminal_args) + + # Check if the python executable (first arg) contains a space + python_arg = terminal_args[0] + assert "python with space" in python_arg, \ + f"Expected 'python with space' in python path: {python_arg}" + if expansion == "expand": + assert (python_arg.startswith('"') or python_arg.startswith("'")), f"Python_arg is not quoted: {python_arg}" diff --git a/tests/debugpy/test_django.py b/tests/debugpy/test_django.py index 31571f3e4..a5d776327 100644 --- a/tests/debugpy/test_django.py +++ b/tests/debugpy/test_django.py @@ -25,7 +25,6 @@ class lines: @pytest.fixture -@pytest.mark.parametrize("run", [runners.launch, runners.attach_connect["cli"]]) def start_django(run): def start(session, multiprocess=False): # No clean way to kill Django server, expect non-zero exit code diff --git a/tests/debugpy/test_flask.py b/tests/debugpy/test_flask.py index d4a68fc2b..a14e9fc3d 100644 --- a/tests/debugpy/test_flask.py +++ b/tests/debugpy/test_flask.py @@ -27,7 +27,6 @@ class lines: @pytest.fixture -@pytest.mark.parametrize("run", [runners.launch, runners.attach_connect["cli"]]) def start_flask(run): def start(session, multiprocess=False): # No clean way to kill Flask server, expect non-zero exit code From db23ab4881605d0100b97cd3fe1116e091e6e8e9 Mon Sep 17 00:00:00 2001 From: Rich Chiodo false Date: Tue, 9 Dec 2025 09:38:51 -0800 Subject: [PATCH 2/8] Default was backwards --- src/debugpy/adapter/launchers.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/debugpy/adapter/launchers.py b/src/debugpy/adapter/launchers.py index 60e201d04..04a40fbcb 100644 --- a/src/debugpy/adapter/launchers.py +++ b/src/debugpy/adapter/launchers.py @@ -156,7 +156,7 @@ def on_launcher_connected(sock): # VS Code debugger extension may pass us an argument indicating the # quoting character to use in the terminal. Otherwise default based on platform. - default_quote = "'" if os.name != "nt" else '"' + default_quote = '"' if os.name != "nt" else "'" quote_char = arguments["terminalQuoteCharacter"] if "terminalQuoteCharacter" in arguments else default_quote # VS code doesn't quote arguments if `argsCanBeInterpretedByShell` is true, From b064943d3936a93624078461db921a0ea44d4674 Mon Sep 17 00:00:00 2001 From: Rich Chiodo false Date: Tue, 9 Dec 2025 10:09:35 -0800 Subject: [PATCH 3/8] Fix ruff errors --- tests/debugpy/test_django.py | 2 +- tests/debugpy/test_flask.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/debugpy/test_django.py b/tests/debugpy/test_django.py index a5d776327..b0a0862ce 100644 --- a/tests/debugpy/test_django.py +++ b/tests/debugpy/test_django.py @@ -5,7 +5,7 @@ import pytest from tests import code, debug, log, net, test_data -from tests.debug import runners, targets +from tests.debug import targets from tests.patterns import some pytestmark = pytest.mark.timeout(60) diff --git a/tests/debugpy/test_flask.py b/tests/debugpy/test_flask.py index a14e9fc3d..1d458268e 100644 --- a/tests/debugpy/test_flask.py +++ b/tests/debugpy/test_flask.py @@ -6,7 +6,7 @@ import sys from tests import code, debug, log, net, test_data -from tests.debug import runners, targets +from tests.debug import targets from tests.patterns import some pytestmark = pytest.mark.timeout(60) From d694a4263e4a3c2ac1a8f9b8539b8c223ba076eb Mon Sep 17 00:00:00 2001 From: Rich Chiodo false Date: Tue, 9 Dec 2025 13:22:48 -0800 Subject: [PATCH 4/8] Fix failing tests --- src/debugpy/adapter/launchers.py | 2 +- tests/debug/session.py | 19 +++++++++++++++---- 2 files changed, 16 insertions(+), 5 deletions(-) diff --git a/src/debugpy/adapter/launchers.py b/src/debugpy/adapter/launchers.py index 04a40fbcb..086178515 100644 --- a/src/debugpy/adapter/launchers.py +++ b/src/debugpy/adapter/launchers.py @@ -166,7 +166,7 @@ def on_launcher_connected(sock): if args[i] == "--": break s = args[i] - if " " in s and not (s.startswith('"') and s.endswith('"')) or (s.startswith("'") and s.endswith("'")): + if " " in s and not ((s.startswith('"') and s.endswith('"')) or (s.startswith("'") and s.endswith("'"))): s = f"{quote_char}{s}{quote_char}" args[i] = s diff --git a/tests/debug/session.py b/tests/debug/session.py index 728489881..9a3271336 100644 --- a/tests/debug/session.py +++ b/tests/debug/session.py @@ -366,18 +366,29 @@ def _make_env(self, base_env, codecov=True): return env def _make_python_cmdline(self, exe, *args): - def normalize(s): + def normalize(s, strip_quotes=False): # Convert py.path.local to string if isinstance(s, py.path.local): s = s.strpath else: s = str(s) - # Strip surrounding quotes if present - if len(s) >= 2 and (s[0] == s[-1] == '"' or s[0] == s[-1] == "'"): + # Strip surrounding quotes if requested (for launcher args only) + if strip_quotes and len(s) >= 2 and (s[0] == s[-1] == '"' or s[0] == s[-1] == "'"): s = s[1:-1] return s - return [normalize(x) for x in (exe, *args)] + # Strip quotes from exe and args before '--', but not from debuggee args after '--' + # (exe and launcher paths may be quoted when argsCanBeInterpretedByShell is set) + result = [normalize(exe, strip_quotes=True)] + found_separator = False + for arg in args: + if arg == "--": + found_separator = True + result.append(arg) + else: + # Strip quotes before '--', but not after (debuggee args) + result.append(normalize(arg, strip_quotes=not found_separator)) + return result def spawn_debuggee(self, args, cwd=None, exe=sys.executable, setup=None): assert self.debuggee is None From 0385fe9ab90e7d57902e7b151266874f7324064e Mon Sep 17 00:00:00 2001 From: Rich Chiodo false Date: Tue, 9 Dec 2025 14:25:32 -0800 Subject: [PATCH 5/8] Only strip quotes on the exe --- tests/debug/session.py | 16 +++++----------- 1 file changed, 5 insertions(+), 11 deletions(-) diff --git a/tests/debug/session.py b/tests/debug/session.py index 9a3271336..d43ee0c7e 100644 --- a/tests/debug/session.py +++ b/tests/debug/session.py @@ -372,22 +372,16 @@ def normalize(s, strip_quotes=False): s = s.strpath else: s = str(s) - # Strip surrounding quotes if requested (for launcher args only) - if strip_quotes and len(s) >= 2 and (s[0] == s[-1] == '"' or s[0] == s[-1] == "'"): + # Strip surrounding quotes if requested + if strip_quotes and len(s) >= 2 and " " in s and (s[0] == s[-1] == '"' or s[0] == s[-1] == "'"): s = s[1:-1] return s - # Strip quotes from exe and args before '--', but not from debuggee args after '--' - # (exe and launcher paths may be quoted when argsCanBeInterpretedByShell is set) + # Strip quotes from exe result = [normalize(exe, strip_quotes=True)] - found_separator = False for arg in args: - if arg == "--": - found_separator = True - result.append(arg) - else: - # Strip quotes before '--', but not after (debuggee args) - result.append(normalize(arg, strip_quotes=not found_separator)) + # Don't strip quotes on anything except the exe + result.append(normalize(arg, strip_quotes=False)) return result def spawn_debuggee(self, args, cwd=None, exe=sys.executable, setup=None): From 991fede562aaef383ffe2f1fe0514aa93e63c74b Mon Sep 17 00:00:00 2001 From: Rich Chiodo false Date: Tue, 9 Dec 2025 14:36:15 -0800 Subject: [PATCH 6/8] Try fixing gw worker failures --- pytest.ini | 2 +- tests/debug/session.py | 20 ++++++++++++++++++-- tests/net.py | 28 ++++++++++++++++++++++++++-- tests/pytest_fixtures.py | 14 +++++++++++--- 4 files changed, 56 insertions(+), 8 deletions(-) diff --git a/pytest.ini b/pytest.ini index 7f3d7d6b8..3439ac33d 100644 --- a/pytest.ini +++ b/pytest.ini @@ -1,5 +1,5 @@ [pytest] testpaths=tests -timeout=60 +timeout=120 timeout_method=thread addopts=-n8 diff --git a/tests/debug/session.py b/tests/debug/session.py index d43ee0c7e..0b2bea26c 100644 --- a/tests/debug/session.py +++ b/tests/debug/session.py @@ -281,7 +281,11 @@ def __exit__(self, exc_type, exc_val, exc_tb): if self.adapter_endpoints is not None and self.expected_exit_code is not None: log.info("Waiting for {0} to close listener ports ...", self.adapter_id) + timeout_start = time.time() while self.adapter_endpoints.check(): + if time.time() - timeout_start > 10: + log.warning("{0} listener ports did not close within 10 seconds", self.adapter_id) + break time.sleep(0.1) if self.adapter is not None: @@ -290,8 +294,20 @@ def __exit__(self, exc_type, exc_val, exc_tb): self.adapter_id, self.adapter.pid, ) - self.adapter.wait() - watchdog.unregister_spawn(self.adapter.pid, self.adapter_id) + try: + self.adapter.wait(timeout=10) + except Exception: + log.warning("{0} did not exit gracefully within 10 seconds, force-killing", self.adapter_id) + try: + self.adapter.kill() + self.adapter.wait(timeout=5) + except Exception as e: + log.error("Failed to force-kill {0}: {1}", self.adapter_id, e) + + try: + watchdog.unregister_spawn(self.adapter.pid, self.adapter_id) + except Exception as e: + log.warning("Failed to unregister adapter spawn: {0}", e) self.adapter = None if self.backchannel is not None: diff --git a/tests/net.py b/tests/net.py index f248c1a4e..2f468050d 100644 --- a/tests/net.py +++ b/tests/net.py @@ -17,7 +17,7 @@ used_ports = set() -def get_test_server_port(): +def get_test_server_port(max_retries=10): """Returns a server port number that can be safely used for listening without clashing with another test worker process, when running with pytest-xdist. @@ -27,6 +27,9 @@ def get_test_server_port(): Note that if multiple test workers invoke this function with different ranges that overlap, conflicts are possible! + + Args: + max_retries: Number of times to retry finding an available port """ try: @@ -39,11 +42,32 @@ def get_test_server_port(): ), "Unrecognized PYTEST_XDIST_WORKER format" n = int(worker_id[2:]) + # Try multiple times to find an available port, with retry logic + for attempt in range(max_retries): + port = 5678 + (n * 300) + attempt + while port in used_ports: + port += 1 + + # Verify the port is actually available by trying to bind to it + sock = socket.socket(socket.AF_INET, socket.SOCK_STREAM) + sock.setsockopt(socket.SOL_SOCKET, socket.SO_REUSEADDR, 1) + try: + sock.bind(("127.0.0.1", port)) + sock.close() + used_ports.add(port) + log.info("Allocated port {0} for worker {1}", port, n) + return port + except OSError as e: + log.warning("Port {0} unavailable (attempt {1}/{2}): {3}", port, attempt + 1, max_retries, e) + sock.close() + time.sleep(0.1 * (attempt + 1)) # Exponential backoff + + # Fall back to original behavior if all retries fail port = 5678 + (n * 300) while port in used_ports: port += 1 used_ports.add(port) - + log.warning("Using fallback port {0} after {1} retries", port, max_retries) return port diff --git a/tests/pytest_fixtures.py b/tests/pytest_fixtures.py index 27d21f1a7..6c99b760d 100644 --- a/tests/pytest_fixtures.py +++ b/tests/pytest_fixtures.py @@ -46,19 +46,27 @@ def write_log(filename, data): session.Session.reset_counter() - session.Session.tmpdir = long_tmpdir + # Add worker-specific isolation for tmpdir and log directory + try: + worker_id = os.environ.get("PYTEST_XDIST_WORKER", "gw0") + worker_suffix = f"_{worker_id}" + except Exception: + worker_suffix = "" + + session.Session.tmpdir = long_tmpdir / f"session{worker_suffix}" + session.Session.tmpdir.ensure(dir=True) original_log_dir = log.log_dir failed = True try: if log.log_dir is None: - log.log_dir = (long_tmpdir / "debugpy_logs").strpath + log.log_dir = (long_tmpdir / f"debugpy_logs{worker_suffix}").strpath else: log_subdir = request.node.nodeid log_subdir = log_subdir.replace("::", "/") for ch in r":?*|<>": log_subdir = log_subdir.replace(ch, f"&#{ord(ch)};") - log.log_dir += "/" + log_subdir + log.log_dir += "/" + log_subdir + worker_suffix try: py.path.local(log.log_dir).remove() From 5a78750119dd2877bcdc9accc267cf197b442c51 Mon Sep 17 00:00:00 2001 From: Rich Chiodo false Date: Tue, 9 Dec 2025 16:37:46 -0800 Subject: [PATCH 7/8] Skip certain test because of cmd limitations --- tests/debug/comms.py | 11 +++++++++-- tests/debugpy/test_args.py | 7 +++++++ 2 files changed, 16 insertions(+), 2 deletions(-) diff --git a/tests/debug/comms.py b/tests/debug/comms.py index a46ad5ca5..f83f90fad 100644 --- a/tests/debug/comms.py +++ b/tests/debug/comms.py @@ -27,6 +27,7 @@ def listen(self): self._server_socket = sockets.create_server("127.0.0.1", 0, self.TIMEOUT) _, self.port = sockets.get_address(self._server_socket) self._server_socket.listen(0) + log.info("{0} created server socket on port {1}", self, self.port) def accept_worker(): log.info( @@ -67,8 +68,14 @@ def _setup_stream(self): self._established.set() def receive(self): - self._established.wait() - return self._stream.read_json() + log.info("{0} waiting for connection to be established...", self) + if not self._established.wait(timeout=self.TIMEOUT): + log.error("{0} timed out waiting for connection after {1} seconds", self, self.TIMEOUT) + raise TimeoutError(f"{self} timed out waiting for debuggee to connect") + log.info("{0} connection established, reading JSON...", self) + result = self._stream.read_json() + log.info("{0} received: {1}", self, result) + return result def send(self, value): self.session.timeline.unfreeze() diff --git a/tests/debugpy/test_args.py b/tests/debugpy/test_args.py index 639615c64..b17db78f3 100644 --- a/tests/debugpy/test_args.py +++ b/tests/debugpy/test_args.py @@ -41,6 +41,13 @@ def code_to_debug(): def test_shell_expansion(pyfile, tmpdir, target, run, expansion, python_with_space): if expansion == "expand" and run.console == "internalConsole": pytest.skip('Shell expansion is not supported for "internalConsole"') + + # Skip tests with python_with_space=True and target="code" on Windows with runInTerminal + # because cmd.exe cannot properly handle multiline string arguments when invoking a .cmd wrapper + if (python_with_space and target == targets.Code and + run.console in ("integratedTerminal", "externalTerminal") and + sys.platform == "win32"): + pytest.skip('Windows cmd.exe cannot handle multiline code arguments with .cmd wrapper') @pyfile def code_to_debug(): From 87d1e9571d5e6cb3db8bdd9b92b057d1728c32c7 Mon Sep 17 00:00:00 2001 From: Rich Chiodo false Date: Wed, 10 Dec 2025 09:23:05 -0800 Subject: [PATCH 8/8] Need to skip all 'code' based tests on windows --- tests/debugpy/test_args.py | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/tests/debugpy/test_args.py b/tests/debugpy/test_args.py index b17db78f3..da83437cd 100644 --- a/tests/debugpy/test_args.py +++ b/tests/debugpy/test_args.py @@ -42,12 +42,10 @@ def test_shell_expansion(pyfile, tmpdir, target, run, expansion, python_with_spa if expansion == "expand" and run.console == "internalConsole": pytest.skip('Shell expansion is not supported for "internalConsole"') - # Skip tests with python_with_space=True and target="code" on Windows with runInTerminal - # because cmd.exe cannot properly handle multiline string arguments when invoking a .cmd wrapper - if (python_with_space and target == targets.Code and - run.console in ("integratedTerminal", "externalTerminal") and - sys.platform == "win32"): - pytest.skip('Windows cmd.exe cannot handle multiline code arguments with .cmd wrapper') + # Skip tests with python_with_space=True and target="code" on Windows + # because .cmd wrappers cannot properly handle multiline string arguments + if (python_with_space and target == targets.Code and sys.platform == "win32"): + pytest.skip('Windows .cmd wrapper cannot handle multiline code arguments') @pyfile def code_to_debug():