-
Notifications
You must be signed in to change notification settings - Fork 24
DAP integration tests and ark_test crate
#1027
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
05d50cd to
fb0c28b
Compare
|
We've got flakes :/ |
df81652 to
adc1f4b
Compare
|
We're now in a good place (the failing run was due to plot flakes, see #1029)
|
And predicate infra
thread 'test_dap_error_during_debug' (11549) panicked at
crates/ark_test/src/dummy_frontend.rs:841:13:
IOPub socket has 1 unexpected non-Stream message(s) on exit:
[
CommMsg(
JupyterMessage {
zmq_identities: [],
header: JupyterHeader {
msg_id: "fb32aa47-2a8d-43d2-9c02-56e224db2ae8",
session: "20421a69-fba6-4ea2-82f4-681951d6889f",
username: "kernel",
date: "2026-02-03T19:00:56.723214957+00:00",
msg_type: "comm_msg",
version: "5.3",
},
parent_header: None,
content: CommWireMsg {
comm_id: "40b79191-82d7-4c9b-8f09-d167885dde00",
data: Object {
"method": String("execute"),
"params": Object {
"command": String("Q"),
},
},
},
},
),
]
thread 'test_dap_breakpoint_lapply_iteration' (39284) panicked at
crates/ark/tests/dap_breakpoints_stepping.rs:498:14:
assertion failed: `Stream(JupyterMessage { zmq_identities: [],
header: JupyterHeader { msg_id: "69a2afaa-1959-44a3-adca-1e395fb2c288",
session: "3d7f80b8-2a8b-40cb-abb2-388b1355f31c", username: "kernel",
date: "2026-02-03T19:29:07.985406+00:00", msg_type: "stream", version:
"5.3" }, parent_header: Some(JupyterHeader { msg_id:
"5a18b0b1-1398-4c34-bf6c-aa2f58519f68", session:
"87bfbe86-6a1a-4ac7-92a3-4a4e2a3282ec", username: "kernel", date:
"2026-02-03T19:29:07.984855+00:00", msg_type: "execute_request",
version: "5.3" }), content: StreamOutput { name: Stdout, text: "debug at
file:///private/var/folders/yz/zr09txvs5dn18vt4cn21kzl40000gn/T/.tmpsqOh5G#3:
y <- x + 1\n" } })` does not match `Message::ExecuteInput(data)`
note: run with `RUST_BACKTRACE=1` environment variable to display a
backtrace
On both macOS runners, maybe because of added debug eprintln:
thread 'test_dap_step_to_adjacent_breakpoint' (62837) panicked at
crates/ark/tests/dap_breakpoints_stepping.rs:315:14:
assertion failed: `Stream(JupyterMessage { zmq_identities: [],
header: JupyterHeader { msg_id: "bfd7baf8-e601-4f44-b1af-bc4a06b93066",
session: "1f3ee245-4529-4ae7-aacd-1fdc664f6023", username: "kernel",
date: "2026-02-03T21:29:53.363264+00:00", msg_type: "stream", version:
"5.3" }, parent_header: Some(JupyterHeader { msg_id:
"54bf8f63-5454-48b3-9428-8fe31433a06f", session:
"75f83fd3-101e-4e54-8328-88463a305493", username: "kernel", date:
"2026-02-03T21:29:53.362637+00:00", msg_type: "execute_request",
version: "5.3" }), content: StreamOutput { name: Stdout, text: "x <-
1\n" } })` does not match `Message::ExecuteInput(data)`
note: run with `RUST_BACKTRACE=1` environment variable to display a
backtrace
DAP trace: sending done signal to events thread
DAP trace: done signal sent
thread 'test_dap_breakpoint_for_loop_iteration' (19198) panicked at
crates/ark/tests/dap_breakpoints_stepping.rs:667:14:
assertion failed: `Stream(JupyterMessage { zmq_identities: [],
header: JupyterHeader { msg_id: "820fc979-58d2-48f4-915d-3c053843b0b9",
session: "c8d2b40a-1bf9-4645-83ac-db347cdeef4b", username: "kernel",
date: "2026-02-04T00:49:42.973599197+00:00", msg_type: "stream",
version: "5.3" }, parent_header: None, content: StreamOutput { name:
Stdout, text: "debug at file:///tmp/.tmprHhDBS#4: x <- i * 2\n" } })`
does not match `Message::Status(data)`
adc1f4b to
b369a12
Compare
|
@DavisVaughan I'd only review the ark-side changes in this PR. All the test related changes are better reviewed in #1033 because both the tests and the infrastructure changed significantly. |
DavisVaughan
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I skipped ark_test/ and tests/ changes for now based on your comments
|
|
||
| // Let the DAP client know that execution is now continuing | ||
| self.debug_send_dap(DapBackendEvent::Continued); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you help me understand why we dont do this here now?
| // | ||
| // fixtures/mod.rs | ||
| // | ||
| // Copyright (C) 2023-2026 Posit Software, PBC. All rights reserved. | ||
| // | ||
| // |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
needed?
| let was_debugging = self.is_debugging; | ||
| self.is_debugging = false; | ||
|
|
||
| if self.is_connected { | ||
| if was_debugging && self.is_connected { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
HA what a great bug

This PR adds comprehensive DAP (Debug Adapter Protocol) integration tests and extracts test infrastructure into a dedicated
ark_testcrate.This addresses the "Planned: DAP protocol tests" section from #1003.
Progress towards #1026.
ark_testcrateTest utilities for integration tests have been extracted from
ark::fixturesinto a newark_testcrate. This provides:DummyArkFrontend: Mock Jupyter frontend that communicates with the kernel over ZMQ sockets. Moved fromark::fixtures::dummy_frontend.DapClient: A minimal DAP client for testing. Connects to the DAP server, sends requests, and receives events/responses. Handles the initialize/attach handshake and provides typed methods for common operations (stack_trace(),set_breakpoints(),recv_stopped(), etc.).MessageAccumulator: Collects IOPub messages and coalesces stream fragments, making tests immune to batching variations. Stream messages from R can arrive as one message or split across multiple messages depending on timing. The accumulator automatically combines streams with the same parent header. This work was done to fix test flakiness with IOPub messages.SourceFile: Helper for creating temporary R source files. Paired withDummyArkFrontend::source_file()andsource_file_and_hit_breakpoint()to exercise thesource()hook in tests.Tracing infrastructure: Enable
ARK_TEST_TRACE=1(or=dap,=iopub) to see timestamped message flows during test runs. This is very helpful for agentic debugging of test failures or development of new tests.Following this reorganisation, the
ark::fixturesmodule now only contains utilities for internal unit tests (r_test_init,r_test_lock,point_from_cursor).Message ordering limitations
Comm messages vs IOPub
The
start_debugandstop_debugmessages travel through the comm manager thread before reaching IOPub, while messages likeidleare sent directly to IOPub. This means these debug lifecycle messages can arrive out of order relative to other IOPub messages. For example,idlemight arrive beforestart_debugeven though logically the debug session started first.I think the architecture should be fixed and I'm working on this in parallel. In the meantime, the test infrastructure is designed to be resilient to this non-determinism.
recv_iopub_untilandrecv_iopub_asyncFor message flows where ordering is non-deterministic, tests use
recv_iopub_until(low-level with full accumulator access) orrecv_iopub_async(convenience wrapper for predicate lists). Both useMessageAccumulatorinternally, so stream coalescing works uniformly. They accumulate messages until a condition is met, rather than asserting strict sequential order.The predicates specify what messages must arrive, not when. When ordering does matter locally,
in_order()can be used as an escape hatch:Stream message coalescing
Stream messages (stdout/stderr) from R can be batched or split unpredictably. A single
print()might produce one message or several, depending on timing. TheMessageAccumulatorautomatically coalesces stream fragments with the same parent header, so tests check the final content rather than message boundaries:This makes tests immune to batching variations while still verifying the expected output appears. The tradeoff is looser assertions: we check that expected content is present rather than asserting exact message sequences. Messages that don't match any predicate are silently drained rather than causing failures.
Bug fixes
URI normalization for symlinks
On macOS,
/var/foldersis a symlink to/private/var/folders. R'snormalizePath()resolves symlinks, so source references from R use the canonical path. However, the frontend sends URIs with the non-canonical path.ExtUrl::from_file_path()now canonicalizes paths before converting to URIs. This ensures breakpoint URIs match the source references R produces, fixing breakpoint identity mismatches on macOS.This fix was necessary to reliably work with temp files in the test.
stop_debugevent guardPreviously,
Dap::stop_debugging()would sendstop_debugeven when not in a debug session, which emitted extra unneeded messages. Now it trackswas_debuggingand only sends the event if we were actually debugging.DAP test coverage
The new tests cover the scenarios outlined in #1003. These were agent-generated. Given the volume, I focused on making sure the test infrastructure allows producing (1) clear and readable tests, and (2) reliable non-flaky tests.
Basic DAP operations (
dap.rs)Breakpoint state management (
dap_breakpoints.rs)SetBreakpointsreturns correct initial state (unverified)Breakpoint verification (
dap_breakpoints_verification.rs)Line adjustment (
dap_breakpoints_line_adjustment.rs)Integration scenarios (
dap_breakpoints_integrations.rs)source(file, echo=TRUE)(used by Positron)Stepping (
dap_breakpoints_stepping.rs,dap_step.rs){}blocks don't leave global env in debug modeReconnection (
dap_breakpoints_reconnect.rs)Variables (
dap_variables.rs)