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 from task/dap-protocol-tests

.github/workflows/test-windows.yml

-Original file line number
+Diff line change
@@ Expand Up / @@ -60,5 +60,7 @@ jobs: @@
           - name: Run Tests
             env:
               NEXTEST_PROFILE: "ci"
+              # Enable tracing to debug Windows ARM timeout issues with debug mode tests
+              ARK_TEST_TRACE: "all"
             run: |
               cargo nextest run

AGENTS.md

-Original file line number
+Diff line change
@@ Expand Up / @@ -57,6 +57,55 @@ just test <test_name> @@
     just test -p ark
     ```
+    ### Kernel and DAP Test Infrastructure
+    Integration tests for the kernel and DAP server live in `crates/ark/tests/` and use the test utilities from `crates/ark_test/`.
+    **Key components:**
+    - **`DummyArkFrontend`**: A mock Jupyter frontend that communicates with the kernel over ZMQ sockets. Use `DummyArkFrontend::lock()` to acquire it (only one per process).
+    - **`DapClient`**: A DAP client for testing the debugger. Obtained via `frontend.dap_client()` after starting the kernel.
+    - **`MessageAccumulator`**: Collects IOPub messages and coalesces stream fragments, making tests immune to batching variations.
+    **Common patterns:**
+    ```rust
+    // Lock the frontend and send an execute request
+    let frontend = DummyArkFrontend::lock();
+    frontend.send_execute_request("1 + 1", ExecuteRequestOptions::default());
+    frontend.recv_iopub_busy();
+    frontend.recv_iopub_execute_input();
+    frontend.recv_iopub_execute_result();
+    frontend.recv_iopub_idle();
+    frontend.recv_shell_execute_reply();
+    // For complex async message flows, use recv_iopub_until with MessageAccumulator
+    frontend.recv_iopub_until(|acc| {
+        acc.has_comm_method("start_debug") &&
+        acc.streams_contain("debug at") &&
+        acc.saw_idle()
+    });
+    // Use in_order() to verify message ordering
+    frontend.recv_iopub_until(|acc| {
+        acc.in_order(&[is_execute_result(), is_idle()])
+    });
+    ```
+    **Debugging tests:**
+    Log messages (from the `log` crate) are not shown in test output. Use `eprintln!` for printf-style debugging.
+    Enable message tracing to see timestamped DAP and IOPub message flows:
+    ```bash
+    ARK_TEST_TRACE=1 just test test_name      # All messages
+    ARK_TEST_TRACE=dap just test test_name    # DAP events only
+    ARK_TEST_TRACE=iopub just test test_name  # IOPub messages only
+    ```
     ### Required R Packages for Testing
     The following R packages are required for tests:
@@ Expand Down @@

Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

crates/ark/Cargo.toml

-Original file line number
+Diff line change
@@ Expand Up / @@ -70,6 +70,7 @@ rustc-hash = "2.1.1" @@
     tracing-error = "0.2.0"
     [dev-dependencies]
+    ark_test = { path = "../ark_test" }
     insta = { version = "1.39.0" }
     stdext = { path = "../stdext", features = ["testing"] }
     tempfile = "3.13.0"
@@ Expand Down @@

crates/ark/src/console.rs

-Original file line number
+Diff line change
@@ Expand Up / @@ -1569,9 +1569,6 @@ impl Console { @@
                 // For continue-like commands, we do not preserve focus,
                 // i.e. we let the cursor jump to the stopped position.
                 self.debug_preserve_focus = false;
-                // Let the DAP client know that execution is now continuing
-                self.debug_send_dap(DapBackendEvent::Continued);
             }
             // Forward the command to R's base REPL.
@@ Expand Down @@

crates/ark/src/dap/dap.rs

-Original file line number
+Diff line change
@@ Expand Up / @@ -220,6 +220,11 @@ impl Dap { @@
             self.stack = Some(stack);
             log::trace!("DAP: Sending `start_debug` events");
+            eprintln!(
+                "DAP trace: start_debug: comm_tx={}, backend_events_tx={}",
+                self.comm_tx.is_some(),
+                self.backend_events_tx.is_some()
+            );
             if let Some(comm_tx) = &self.comm_tx {
                 // Ask frontend to connect to the DAP
@@ Expand All / @@ -228,9 +233,17 @@ impl Dap { @@
                     .log_err();
                 if let Some(dap_tx) = &self.backend_events_tx {
-                    dap_tx
-                        .send(DapBackendEvent::Stopped(DapStoppedEvent { preserve_focus }))
-                        .log_err();
+                    eprintln!("DAP trace: start_debug: sending Stopped event");
+                    match dap_tx.send(DapBackendEvent::Stopped(DapStoppedEvent { preserve_focus })) {
+                        Ok(()) => eprintln!("DAP trace: start_debug: Stopped event sent"),
+                        Err(err) => {
+                            eprintln!("DAP trace: start_debug: failed to send Stopped event: {err:?}")
+                        },
+                    }
+                } else {
+                    eprintln!(
+                        "DAP trace: start_debug: backend_events_tx is None, cannot send Stopped event"
+                    );
                 }
             }
         }
@@ Expand All / @@ -247,9 +260,11 @@ impl Dap { @@
             self.fallback_sources.clear();
             self.clear_variables_reference_maps();
             self.reset_variables_reference_count();
+            let was_debugging = self.is_debugging;
             self.is_debugging = false;
-            if self.is_connected {
+            if was_debugging && self.is_connected {
                 log::trace!("DAP: Sending `stop_debug` events");
                 if let Some(comm_tx) = &self.comm_tx {
@@ Expand Down Expand Up / @@ -507,3 +522,158 @@ impl ServerHandler for Dap { @@
             return Ok(());
         }
     }
+    #[cfg(test)]
+    mod tests {
+        use crossbeam::channel::unbounded;
+        use super::*;
+        fn create_test_dap() -> (Dap, crossbeam::channel::Receiver<DapBackendEvent>) {
+            let (backend_events_tx, backend_events_rx) = unbounded();
+            let (r_request_tx, _r_request_rx) = unbounded();
+            let dap = Dap {
+                is_debugging: false,
+                is_connected: true,
+                backend_events_tx: Some(backend_events_tx),
+                stack: None,
+                breakpoints: HashMap::new(),
+                fallback_sources: HashMap::new(),
+                frame_id_to_variables_reference: HashMap::new(),
+                variables_reference_to_r_object: HashMap::new(),
+                current_variables_reference: 1,
+                current_breakpoint_id: 1,
+                comm_tx: None,
+                r_request_tx,
+                shared_self: None,
+            };
+            (dap, backend_events_rx)
+        }
+        #[test]
+        fn test_did_change_document_removes_breakpoints() {
+            let (mut dap, rx) = create_test_dap();
+            let uri = Url::parse("file:///test.R").unwrap();
+            let hash = blake3::hash(b"test content");
+            dap.breakpoints.insert(
+                uri.clone(),
+                (hash, vec![
+                    Breakpoint::new(1, 10, BreakpointState::Verified),
+                    Breakpoint::new(2, 20, BreakpointState::Verified),
+                ]),
+            );
+            dap.did_change_document(&uri);
+            assert!(dap.breakpoints.get(&uri).is_none());
+            let event1 = rx.try_recv().unwrap();
+            let event2 = rx.try_recv().unwrap();
+            assert!(matches!(event1, DapBackendEvent::BreakpointState {
+                id: 1,
+                verified: false,
+                ..
+            }));
+            assert!(matches!(event2, DapBackendEvent::BreakpointState {
+                id: 2,
+                verified: false,
+                ..
+            }));
+            assert!(rx.try_recv().is_err());
+        }
+        #[test]
+        fn test_did_change_document_no_breakpoints_is_noop() {
+            let (mut dap, rx) = create_test_dap();
+            let uri = Url::parse("file:///test.R").unwrap();
+            dap.did_change_document(&uri);
+            assert!(rx.try_recv().is_err());
+        }
+        #[test]
+        fn test_did_change_document_only_affects_target_uri() {
+            let (mut dap, rx) = create_test_dap();
+            let uri1 = Url::parse("file:///test1.R").unwrap();
+            let uri2 = Url::parse("file:///test2.R").unwrap();
+            let hash1 = blake3::hash(b"content 1");
+            let hash2 = blake3::hash(b"content 2");
+            dap.breakpoints.insert(
+                uri1.clone(),
+                (hash1, vec![Breakpoint::new(
+,
+,
+                    BreakpointState::Verified,
+                )]),
+            );
+            dap.breakpoints.insert(
+                uri2.clone(),
+                (hash2, vec![Breakpoint::new(
+,
+,
+                    BreakpointState::Verified,
+                )]),
+            );
+            dap.did_change_document(&uri1);
+            assert!(dap.breakpoints.get(&uri1).is_none());
+            assert!(dap.breakpoints.get(&uri2).is_some());
+            let event = rx.try_recv().unwrap();
+            assert!(matches!(event, DapBackendEvent::BreakpointState {
+                id: 1,
+                ..
+            }));
+            assert!(rx.try_recv().is_err());
+        }
+        #[test]
+        fn test_did_change_document_without_backend_tx_is_noop() {
+            let (r_request_tx, _r_request_rx) = unbounded();
+            let mut dap = Dap {
+                is_debugging: false,
+                is_connected: false,
+                backend_events_tx: None,
+                stack: None,
+                breakpoints: HashMap::new(),
+                fallback_sources: HashMap::new(),
+                frame_id_to_variables_reference: HashMap::new(),
+                variables_reference_to_r_object: HashMap::new(),
+                current_variables_reference: 1,
+                current_breakpoint_id: 1,
+                comm_tx: None,
+                r_request_tx,
+                shared_self: None,
+            };
+            let uri = Url::parse("file:///test.R").unwrap();
+            let hash = blake3::hash(b"test content");
+            dap.breakpoints.insert(
+                uri.clone(),
+                (hash, vec![Breakpoint::new(
+,
+,
+                    BreakpointState::Verified,
+                )]),
+            );
+            // Should not panic even without `backend_events_tx`
+            dap.did_change_document(&uri);
+            // Breakpoints should still be removed
+            assert!(dap.breakpoints.get(&uri).is_none());
+        }
+    }

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

DAP integration tests and `ark_test` crate #1027

Uh oh!

Diff view

Diff view

There are no files selected for viewing

Uh oh!

Uh oh!

Uh oh!

DAP integration tests and ark_test crate #1027

Are you sure you want to change the base?

Uh oh!

DAP integration tests and ark_test crate #1027

Uh oh!

Uh oh!

Diff view

Diff view

There are no files selected for viewing

Uh oh!

Uh oh!

Uh oh!

DAP integration tests and `ark_test` crate #1027

DAP integration tests and `ark_test` crate #1027