-
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
Open
lionel-
wants to merge
34
commits into
main
Choose a base branch
from
task/dap-protocol-tests
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
And predicate infra
05d50cd to
fb0c28b
Compare
Contributor
Author
617b31c to
f48a415
Compare
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
c9cf78b to
bf64438
Compare
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
0e0f479 to
9b3828b
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
Closes #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)