Skip to content

Commit e65e886

Browse files
committed
Ignore UI comm busy events by default in tests
1 parent bc636c5 commit e65e886

3 files changed

Lines changed: 78 additions & 37 deletions

File tree

crates/ark/tests/plots.rs

Lines changed: 18 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -770,11 +770,11 @@ fn test_dev_hold_across_execute_requests() {
770770
fn test_positron_simple_plot() {
771771
let frontend = DummyArkFrontend::lock();
772772
frontend.open_ui_comm();
773-
frontend.set_buffer_ui_events(true);
774773

775774
frontend.send_execute_request("plot(1:10)", ExecuteRequestOptions::default());
776775
frontend.recv_iopub_busy();
777776
frontend.recv_iopub_execute_input();
777+
frontend.recv_iopub_ui_prompt_state();
778778
frontend.recv_iopub_idle();
779779
frontend.recv_shell_execute_reply();
780780

@@ -787,11 +787,11 @@ fn test_positron_simple_plot() {
787787
fn test_positron_multiple_plots() {
788788
let frontend = DummyArkFrontend::lock();
789789
frontend.open_ui_comm();
790-
frontend.set_buffer_ui_events(true);
791790

792791
frontend.send_execute_request("plot(1:10)\nplot(2:20)", ExecuteRequestOptions::default());
793792
frontend.recv_iopub_busy();
794793
frontend.recv_iopub_execute_input();
794+
frontend.recv_iopub_ui_prompt_state();
795795
frontend.recv_iopub_idle();
796796
frontend.recv_shell_execute_reply();
797797

@@ -810,16 +810,16 @@ fn test_positron_multiple_plots() {
810810
fn test_positron_par_multi_panel() {
811811
let frontend = DummyArkFrontend::lock();
812812
frontend.open_ui_comm();
813-
frontend.set_buffer_ui_events(true);
814813

815814
let code = "par(mfrow = c(2, 1))\nplot(1:10)\nplot(2:20)";
816815
frontend.send_execute_request(code, ExecuteRequestOptions::default());
817816
frontend.recv_iopub_busy();
818817
frontend.recv_iopub_execute_input();
819818

820-
// Panel update arrives directly on IOPub (before idle)
819+
// Panel update arrives on IOPub (from on_did_execute_request)
821820
let update = frontend.recv_iopub_comm_msg();
822821

822+
frontend.recv_iopub_ui_prompt_state();
823823
frontend.recv_iopub_idle();
824824
frontend.recv_shell_execute_reply();
825825

@@ -837,7 +837,6 @@ fn test_positron_par_multi_panel() {
837837
fn test_positron_layout_multi_plot() {
838838
let frontend = DummyArkFrontend::lock();
839839
frontend.open_ui_comm();
840-
frontend.set_buffer_ui_events(true);
841840

842841
let code = r#"
843842
plt2 = function() {
@@ -851,9 +850,10 @@ plt2()
851850
frontend.recv_iopub_busy();
852851
frontend.recv_iopub_execute_input();
853852

854-
// Second panel update (direct IOPub)
853+
// Second panel update (from on_did_execute_request)
855854
let update = frontend.recv_iopub_comm_msg();
856855

856+
frontend.recv_iopub_ui_prompt_state();
857857
frontend.recv_iopub_idle();
858858
frontend.recv_shell_execute_reply();
859859

@@ -870,7 +870,6 @@ plt2()
870870
fn test_positron_dev_hold_suppresses() {
871871
let frontend = DummyArkFrontend::lock();
872872
frontend.open_ui_comm();
873-
frontend.set_buffer_ui_events(true);
874873

875874
let code = r#"
876875
invisible(dev.hold())
@@ -881,6 +880,7 @@ invisible(dev.flush())
881880
frontend.send_execute_request(code, ExecuteRequestOptions::default());
882881
frontend.recv_iopub_busy();
883882
frontend.recv_iopub_execute_input();
883+
frontend.recv_iopub_ui_prompt_state();
884884
frontend.recv_iopub_idle();
885885
frontend.recv_shell_execute_reply();
886886

@@ -897,7 +897,6 @@ invisible(dev.flush())
897897
fn test_positron_dev_hold_across_requests() {
898898
let frontend = DummyArkFrontend::lock();
899899
frontend.open_ui_comm();
900-
frontend.set_buffer_ui_events(true);
901900

902901
// Hold and plot without flushing. No plot comm should open.
903902
frontend.send_execute_request(
@@ -906,13 +905,15 @@ fn test_positron_dev_hold_across_requests() {
906905
);
907906
frontend.recv_iopub_busy();
908907
frontend.recv_iopub_execute_input();
908+
frontend.recv_iopub_ui_prompt_state();
909909
frontend.recv_iopub_idle();
910910
frontend.recv_shell_execute_reply();
911911

912912
// Flush in a separate request. The held plot should now appear.
913913
frontend.send_execute_request("invisible(dev.flush())", ExecuteRequestOptions::default());
914914
frontend.recv_iopub_busy();
915915
frontend.recv_iopub_execute_input();
916+
frontend.recv_iopub_ui_prompt_state();
916917
frontend.recv_iopub_idle();
917918
frontend.recv_shell_execute_reply();
918919

@@ -930,13 +931,13 @@ fn test_positron_dev_hold_across_requests() {
930931
fn test_positron_sequential_plots() {
931932
let frontend = DummyArkFrontend::lock();
932933
frontend.open_ui_comm();
933-
frontend.set_buffer_ui_events(true);
934934

935935
for i in 1..=3 {
936936
let code = format!("plot({i}:10)");
937937
frontend.send_execute_request(&code, ExecuteRequestOptions::default());
938938
frontend.recv_iopub_busy();
939939
frontend.recv_iopub_execute_input();
940+
frontend.recv_iopub_ui_prompt_state();
940941
frontend.recv_iopub_idle();
941942
frontend.recv_shell_execute_reply();
942943

@@ -953,7 +954,6 @@ fn test_positron_sequential_plots() {
953954
fn test_positron_graphics_device_swap() {
954955
let frontend = DummyArkFrontend::lock();
955956
frontend.open_ui_comm();
956-
frontend.set_buffer_ui_events(true);
957957

958958
let code = r#"
959959
plot(1:10)
@@ -964,6 +964,7 @@ invisible(dev.off())
964964
frontend.send_execute_request(code, ExecuteRequestOptions::default());
965965
frontend.recv_iopub_busy();
966966
frontend.recv_iopub_execute_input();
967+
frontend.recv_iopub_ui_prompt_state();
967968
frontend.recv_iopub_idle();
968969
frontend.recv_shell_execute_reply();
969970

@@ -977,7 +978,6 @@ invisible(dev.off())
977978
fn test_positron_loop_plots() {
978979
let frontend = DummyArkFrontend::lock();
979980
frontend.open_ui_comm();
980-
frontend.set_buffer_ui_events(true);
981981

982982
let code = r#"
983983
for (i in 1:3) {
@@ -987,6 +987,7 @@ for (i in 1:3) {
987987
frontend.send_execute_request(code, ExecuteRequestOptions::default());
988988
frontend.recv_iopub_busy();
989989
frontend.recv_iopub_execute_input();
990+
frontend.recv_iopub_ui_prompt_state();
990991
frontend.recv_iopub_idle();
991992
frontend.recv_shell_execute_reply();
992993

@@ -1006,7 +1007,6 @@ for (i in 1:3) {
10061007
fn test_positron_par_overflow_to_new_page() {
10071008
let frontend = DummyArkFrontend::lock();
10081009
frontend.open_ui_comm();
1009-
frontend.set_buffer_ui_events(true);
10101010

10111011
let code = r#"
10121012
par(mfrow = c(3, 1))
@@ -1020,10 +1020,11 @@ par(mfrow = c(1, 1))
10201020
frontend.recv_iopub_busy();
10211021
frontend.recv_iopub_execute_input();
10221022

1023-
// Panels 2 and 3 update the first page (direct IOPub, before idle)
1023+
// Panels 3 and 4 update the first page (from before.plot.new during R eval)
10241024
let update1 = frontend.recv_iopub_comm_msg();
10251025
let update2 = frontend.recv_iopub_comm_msg();
10261026

1027+
frontend.recv_iopub_ui_prompt_state();
10271028
frontend.recv_iopub_idle();
10281029
frontend.recv_shell_execute_reply();
10291030

@@ -1048,33 +1049,36 @@ par(mfrow = c(1, 1))
10481049
fn test_positron_dev_hold_flush_interactive() {
10491050
let frontend = DummyArkFrontend::lock();
10501051
frontend.open_ui_comm();
1051-
frontend.set_buffer_ui_events(true);
10521052

10531053
// Hold
10541054
frontend.send_execute_request("invisible(dev.hold())", ExecuteRequestOptions::default());
10551055
frontend.recv_iopub_busy();
10561056
frontend.recv_iopub_execute_input();
1057+
frontend.recv_iopub_ui_prompt_state();
10571058
frontend.recv_iopub_idle();
10581059
frontend.recv_shell_execute_reply();
10591060

10601061
// Draw first plot (held, no comm should open)
10611062
frontend.send_execute_request("plot(1:10)", ExecuteRequestOptions::default());
10621063
frontend.recv_iopub_busy();
10631064
frontend.recv_iopub_execute_input();
1065+
frontend.recv_iopub_ui_prompt_state();
10641066
frontend.recv_iopub_idle();
10651067
frontend.recv_shell_execute_reply();
10661068

10671069
// Draw over it (still held)
10681070
frontend.send_execute_request("abline(1, 2)", ExecuteRequestOptions::default());
10691071
frontend.recv_iopub_busy();
10701072
frontend.recv_iopub_execute_input();
1073+
frontend.recv_iopub_ui_prompt_state();
10711074
frontend.recv_iopub_idle();
10721075
frontend.recv_shell_execute_reply();
10731076

10741077
// Flush - the combined plot should now appear
10751078
frontend.send_execute_request("invisible(dev.flush())", ExecuteRequestOptions::default());
10761079
frontend.recv_iopub_busy();
10771080
frontend.recv_iopub_execute_input();
1081+
frontend.recv_iopub_ui_prompt_state();
10781082
frontend.recv_iopub_idle();
10791083
frontend.recv_shell_execute_reply();
10801084

crates/ark/tests/ui-prompt-state.rs

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ use ark_test::DummyArkFrontend;
1414
fn test_prompt_state_after_execution() {
1515
let frontend = DummyArkFrontend::lock();
1616
let comm_id = frontend.open_ui_comm();
17+
frontend.set_ignore_ui_busy(false);
1718

1819
frontend.send_execute_request("1 + 1", ExecuteRequestOptions::default());
1920
frontend.recv_iopub_busy();
@@ -34,6 +35,7 @@ fn test_prompt_state_after_execution() {
3435
fn test_prompt_state_custom_prompt() {
3536
let frontend = DummyArkFrontend::lock();
3637
let comm_id = frontend.open_ui_comm();
38+
frontend.set_ignore_ui_busy(false);
3739

3840
// Change the prompt
3941
frontend.send_execute_request(
@@ -67,6 +69,7 @@ fn test_prompt_state_custom_prompt() {
6769
fn test_prompt_state_browser() {
6870
let frontend = DummyArkFrontend::lock();
6971
let comm_id = frontend.open_ui_comm();
72+
frontend.set_ignore_ui_busy(false);
7073

7174
// Enter the browser. The busy sequence differs from normal execution:
7275
// R briefly goes idle entering the browser's ReadConsole, then busy
@@ -102,6 +105,7 @@ fn test_prompt_state_browser() {
102105
fn test_prompt_state_custom_continuation() {
103106
let frontend = DummyArkFrontend::lock();
104107
let comm_id = frontend.open_ui_comm();
108+
frontend.set_ignore_ui_busy(false);
105109

106110
frontend.send_execute_request(
107111
"options(continue = '... ')",

crates/ark_test/src/dummy_frontend.rs

Lines changed: 56 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -71,13 +71,15 @@ pub struct DummyArkFrontend {
7171
streams_handled: Cell<bool>,
7272
/// Whether we're currently in a debug context (between start_debug and stop_debug)
7373
in_debug: Cell<bool>,
74-
/// Comm ID of the open UI comm, if any.
74+
/// Comm ID of the open UI comm, if any. Set by `open_ui_comm()` so
75+
/// that `recv_iopub_ui_busy()` and `recv_iopub_prompt_state()` can
76+
/// identify UI comm messages.
7577
ui_comm_id: RefCell<Option<String>>,
76-
/// When true, UI comm event messages (prompt_state, working_directory,
77-
/// busy) are auto-buffered by `recv_iopub_next()` so they don't
78-
/// interfere with plot and other assertions. Off by default so that
79-
/// tests like `ui-prompt-state` can assert on these events explicitly.
80-
buffer_ui_events: Cell<bool>,
78+
/// When true (the default), UI comm `busy` events are silently
79+
/// skipped by `recv_iopub_next()`. R fires busy(true)/busy(false)
80+
/// per expression in multi-line requests, producing a variable
81+
/// number of events that most tests don't care about.
82+
ignore_ui_busy: Cell<bool>,
8183
/// Comm ID of the open variables comm, if any.
8284
variables_comm_id: RefCell<Option<String>>,
8385
/// Buffered variables comm events, auto-collected by `recv_iopub_next()`.
@@ -163,7 +165,7 @@ impl DummyArkFrontend {
163165
streams_handled: Cell::new(false),
164166
in_debug: Cell::new(false),
165167
ui_comm_id: RefCell::new(None),
166-
buffer_ui_events: Cell::new(false),
168+
ignore_ui_busy: Cell::new(true),
167169
variables_comm_id: RefCell::new(None),
168170
variables_events: RefCell::new(VecDeque::new()),
169171
}
@@ -256,14 +258,10 @@ impl DummyArkFrontend {
256258
self.buffer_stream(&data.content);
257259
true
258260
},
259-
// When `buffer_ui_events` is enabled, silently consume UI comm
260-
// event messages (prompt_state, working_directory, busy) so they
261-
// don't interfere with plot and other assertions. RPC replies
262-
// (no "method" field) always pass through.
263261
Message::CommMsg(ref data)
264-
if self.buffer_ui_events.get() &&
262+
if self.ignore_ui_busy.get() &&
265263
self.is_ui_comm(&data.content.comm_id) &&
266-
data.content.data.get("method").is_some() =>
264+
data.content.data.get("method").and_then(|v| v.as_str()) == Some("busy") =>
267265
{
268266
trace_iopub_msg(msg);
269267
true
@@ -284,15 +282,47 @@ impl DummyArkFrontend {
284282
.is_some_and(|id| id == comm_id)
285283
}
286284

287-
/// Enable or disable automatic buffering of UI comm event messages.
285+
/// Ignore UI comm `busy` events in `recv_iopub_next()`.
288286
///
289-
/// When enabled, prompt_state, working_directory, and busy events from
290-
/// the UI comm are silently consumed by `recv_iopub_next()`. This is
291-
/// useful for plot tests that don't care about these events. Tests that
292-
/// need to assert on UI comm events (e.g. `ui-prompt-state`) should
293-
/// leave this disabled (the default).
294-
pub fn set_buffer_ui_events(&self, enabled: bool) {
295-
self.buffer_ui_events.set(enabled);
287+
/// R fires `busy(true)`/`busy(false)` for each top-level expression
288+
/// in a multi-line request, so the number of events depends on how
289+
/// many expressions the code contains. Enable this in tests that
290+
/// don't care about busy transitions (e.g. plot tests).
291+
pub fn set_ignore_ui_busy(&self, ignore: bool) {
292+
self.ignore_ui_busy.set(ignore);
293+
}
294+
295+
/// Receive from IOPub and assert a UI comm `busy` event.
296+
/// Automatically skips any Stream messages.
297+
#[track_caller]
298+
pub fn recv_iopub_ui_busy(&self, expected: bool) {
299+
let msg = self.recv_iopub_next();
300+
match msg {
301+
Message::CommMsg(data) if self.is_ui_comm(&data.content.comm_id) => {
302+
assert_eq!(
303+
data.content.data.get("method").and_then(|v| v.as_str()),
304+
Some("busy")
305+
);
306+
assert_eq!(data.content.data["params"]["busy"], expected);
307+
},
308+
other => panic!("Expected UI busy={expected} CommMsg, got {other:?}"),
309+
}
310+
}
311+
312+
/// Receive from IOPub and assert a UI comm `prompt_state` event.
313+
/// Automatically skips any Stream messages.
314+
#[track_caller]
315+
pub fn recv_iopub_ui_prompt_state(&self) {
316+
let msg = self.recv_iopub_next();
317+
match msg {
318+
Message::CommMsg(data) if self.is_ui_comm(&data.content.comm_id) => {
319+
assert_eq!(
320+
data.content.data.get("method").and_then(|v| v.as_str()),
321+
Some("prompt_state")
322+
);
323+
},
324+
other => panic!("Expected prompt_state CommMsg on UI comm, got {other:?}"),
325+
}
296326
}
297327

298328
fn is_variables_comm(&self, comm_id: &str) -> bool {
@@ -1155,8 +1185,11 @@ impl DummyArkFrontend {
11551185

11561186
self.recv_iopub_idle();
11571187

1158-
// Enable auto-buffering for subsequent UI comm events (prompt_state
1159-
// and working_directory refreshes sent after every execute request).
1188+
// Store the UI comm ID so `recv_iopub_ui_busy()` and
1189+
// `recv_iopub_prompt_state()` can identify UI comm messages.
1190+
// `ignore_ui_busy` is on by default, so UI busy events are
1191+
// auto-skipped. Tests that care (e.g. `ui-prompt-state`) can
1192+
// call `set_ignore_ui_busy(false)` to receive them inline.
11601193
*self.ui_comm_id.borrow_mut() = Some(comm_id.clone());
11611194

11621195
comm_id

0 commit comments

Comments
 (0)