Skip to content

Commit 57fbf69

Browse files
pdepetrometa-codesync[bot]
authored andcommitted
Make the DAP server's pytb_file CLI argument optional
Summary: The `tintype_dap_server` CLI's positional `pytb_file` argument was a redundant pre-flight check duplicating logic that already lives in the DAP `launch` handler. The path the server actually opens is the one in the `launch` request body (`arguments.pytbPath`), which `SnapshotDebugSession.handle_launch` validates (`os.path.isfile`) and hands to `SnapshotReader` — the CLI's copy is discarded after the `os.path.isfile` fast-fail. This diff relaxes the positional to `nargs="?"` and gates the existence check on a non-`None` value. Callers that still pass the path get the same fast-fail UX as before; callers that don't (the VS Code extension is about to stop doing so — see the next diff in the stack) let `handle_launch` validate. Everything downstream is unchanged: `serve()` / `run_session_on_stdio()` never received the path, `handle_launch` continues to require `arguments.pytbPath`, the outer Meta wrapper (`fbcode/tintype/fb/tintype_dap_server.py`) and the `--show-runtime-path(s)` short-circuit don't inspect `pytb_file`. Reviewed By: aperez Differential Revision: D106546434 fbshipit-source-id: c2c0d59aef1d097c79ffda0f2eb3291c761f2411
1 parent f4a98c4 commit 57fbf69

2 files changed

Lines changed: 52 additions & 8 deletions

File tree

dap/cli.py

Lines changed: 18 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -91,7 +91,14 @@ def _parse_args(argv: list[str] | None = None) -> argparse.Namespace:
9191
)
9292
parser.add_argument(
9393
"pytb_file",
94-
help="Path to the .pytb snapshot file.",
94+
nargs="?",
95+
default=None,
96+
help=(
97+
"Optional path to the .pytb snapshot file. When supplied, the CLI "
98+
"fast-fails with INVALID_PYTB if the path is missing or not a "
99+
"regular file. When omitted, all path validation is deferred to "
100+
"the DAP ``launch`` request's ``pytbPath`` argument."
101+
),
95102
)
96103
parser.add_argument(
97104
"--listen",
@@ -114,23 +121,26 @@ def _parse_args(argv: list[str] | None = None) -> argparse.Namespace:
114121
def main(argv: list[str] | None = None) -> LauncherExitCode:
115122
"""Run the DAP server over stdio (default) or TCP (``--listen``).
116123
117-
Fast-fails on a missing ``.pytb`` path with
118-
:attr:`LauncherExitCode.INVALID_PYTB`; full snapshot validity is
119-
left to the DAP ``launch`` handler, which surfaces failures as a
120-
DAP error the client can render cleanly.
124+
When ``pytb_file`` is supplied as a positional argument, fast-fails
125+
on a missing path with :attr:`LauncherExitCode.INVALID_PYTB` before
126+
the transport starts. When omitted, all path validation is
127+
deferred to the DAP ``launch`` handler — which surfaces failures
128+
as DAP errors the client can render cleanly via
129+
``arguments.pytbPath`` — so the path can live entirely in the
130+
launch request body.
121131
122132
Returns:
123133
:attr:`LauncherExitCode.OK` on clean shutdown.
124134
:attr:`LauncherExitCode.ERROR` when the session loop returns a
125135
non-zero status.
126-
:attr:`LauncherExitCode.INVALID_PYTB` when ``pytb_file`` does
127-
not exist or is not a regular file.
136+
:attr:`LauncherExitCode.INVALID_PYTB` when a ``pytb_file`` was
137+
supplied but does not exist or is not a regular file.
128138
:attr:`LauncherExitCode.KEYBOARD_INTERRUPT` on SIGINT during the
129139
serve loop.
130140
"""
131141
args = _parse_args(argv)
132142

133-
if not os.path.isfile(args.pytb_file):
143+
if args.pytb_file is not None and not os.path.isfile(args.pytb_file):
134144
print(
135145
f"tintype_dap_server: pytb path does not exist or is not a "
136146
f"file: {args.pytb_file}",

dap/tests/test_cli.py

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,13 @@ def test_rejects_malformed_listen(self) -> None:
4646
with self.assertRaises(SystemExit):
4747
module._parse_args(["/tmp/snap.pytb", "--listen", "not-a-port"])
4848

49+
def test_pytb_file_optional(self) -> None:
50+
"""``pytb_file`` is optional — when omitted, validation defers to
51+
the DAP ``launch`` handler. Argparse must not raise."""
52+
args = module._parse_args([])
53+
self.assertIsNone(args.pytb_file)
54+
self.assertIsNone(args.listen)
55+
4956

5057
class MainFileValidationTest(unittest.TestCase):
5158
"""The CLI fast-fails on obvious path mistakes before touching transports."""
@@ -114,6 +121,33 @@ def test_keyboard_interrupt_maps_to_exit_code(self) -> None:
114121
):
115122
self.assertEqual(module.main([path]), LauncherExitCode.KEYBOARD_INTERRUPT)
116123

124+
def test_no_pytb_dispatches_to_stdio(self) -> None:
125+
"""``pytb_file`` may be omitted entirely; the CLI's existence
126+
check is skipped and the transport launches as usual. Path
127+
validation is left to ``handle_launch``."""
128+
with (
129+
unittest.mock.patch(
130+
"tintype.dap.cli.run_session_on_stdio", return_value=0
131+
) as stdio_mock,
132+
unittest.mock.patch("tintype.dap.cli.serve") as serve_mock,
133+
):
134+
rc = module.main([])
135+
self.assertEqual(rc, LauncherExitCode.OK)
136+
stdio_mock.assert_called_once_with()
137+
serve_mock.assert_not_called()
138+
139+
def test_no_pytb_with_listen_dispatches_to_serve(self) -> None:
140+
"""Same as ``test_no_pytb_dispatches_to_stdio`` but for the
141+
TCP transport branch — ``--listen`` works without a positional."""
142+
with (
143+
unittest.mock.patch("tintype.dap.cli.serve", return_value=0) as serve_mock,
144+
unittest.mock.patch("tintype.dap.cli.run_session_on_stdio") as stdio_mock,
145+
):
146+
rc = module.main(["--listen", "0"])
147+
self.assertEqual(rc, LauncherExitCode.OK)
148+
serve_mock.assert_called_once_with(host="127.0.0.1", port=0)
149+
stdio_mock.assert_not_called()
150+
117151

118152
class LauncherExitCodeTest(unittest.TestCase):
119153
"""Sanity checks on the enum itself so callers relying on the

0 commit comments

Comments
 (0)