From 6c7b352e41eaef76be9c26c654f92198be106944 Mon Sep 17 00:00:00 2001 From: Fini Date: Wed, 27 May 2026 23:19:48 +0800 Subject: [PATCH] refactor(host): split tests + trim comments for 800-line cap MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit chat_subprocess.rs (850) and design_session.rs (807) blew past the ceiling after the Codex provider + design-viewport work landed in 63f787c3. Move their inline `mod tests` bodies out to sibling `*_tests.rs` files via `#[cfg(test)] #[path = …] mod tests;`. app_handler.rs (805) is one bare `impl ApplicationHandler` with no natural split point; trim the file-level doc and two inline comments in `exiting` to land at exactly 800. After: chat_subprocess.rs 650, design_session.rs 508, app_handler.rs 800. 114 host-desktop tests pass. --- crates/op-host-desktop/src/app_handler.rs | 21 +- crates/op-host-desktop/src/chat_subprocess.rs | 204 +----------- .../src/chat_subprocess_tests.rs | 195 +++++++++++ crates/op-host-desktop/src/design_session.rs | 303 +----------------- .../src/design_session_tests.rs | 299 +++++++++++++++++ 5 files changed, 506 insertions(+), 516 deletions(-) create mode 100644 crates/op-host-desktop/src/chat_subprocess_tests.rs create mode 100644 crates/op-host-desktop/src/design_session_tests.rs diff --git a/crates/op-host-desktop/src/app_handler.rs b/crates/op-host-desktop/src/app_handler.rs index 695ed3ef8..6de905a15 100644 --- a/crates/op-host-desktop/src/app_handler.rs +++ b/crates/op-host-desktop/src/app_handler.rs @@ -1,7 +1,5 @@ -//! `impl ApplicationHandler for DesktopApp` — the winit event-loop -//! body (new_events / resumed / window_event). Split out of `main.rs` -//! to keep that file under the 800-line cap; `main.rs` keeps the -//! `DesktopApp` struct, its helper `impl`, and `fn main`. +//! winit `ApplicationHandler` impl for `DesktopApp`. Split out of +//! `main.rs` to keep that file under the 800-line cap. use crate::{ chat_attachment, chat_session, cursor_icon, design_session, figma_import_session, frame, @@ -832,17 +830,14 @@ impl ApplicationHandler for DesktopApp { } fn exiting(&mut self, _event_loop: &ActiveEventLoop) { - // Belt-and-suspenders: macOS Cmd+Q / Alt+F4 / window-manager - // close can deliver `exiting` without `CloseRequested`. Flush - // any in-progress MCP port draft before snapshotting so a - // focused-but-uncommitted edit isn't silently dropped. + // macOS Cmd+Q / Alt+F4 / WM-close can deliver `exiting` without + // `CloseRequested`; flush MCP port draft before snapshotting so + // a focused-but-uncommitted edit isn't silently dropped. self.host.flush_settings_input(); settings_io::save(self.host.editor_state()); - // Persist the window geometry so the next launch restores - // where the user left the window. Guarded on a window having - // existed: a failed startup (create_window / Skia init error) - // reaches `exiting` with unseeded geometry, and saving that - // would clobber the previous session's good geometry. + // Save window geometry for next launch. Guarded on a window + // having existed — a failed startup reaches `exiting` with + // unseeded geometry and would clobber the previous good save. if self.window.is_some() { window_state::save(&window_state::WindowState::from_window( self.win_pos, diff --git a/crates/op-host-desktop/src/chat_subprocess.rs b/crates/op-host-desktop/src/chat_subprocess.rs index 1aa11218c..b99a4b35b 100644 --- a/crates/op-host-desktop/src/chat_subprocess.rs +++ b/crates/op-host-desktop/src/chat_subprocess.rs @@ -646,205 +646,5 @@ fn map_stop_reason(s: Option<&str>) -> StopReason { } #[cfg(test)] -mod tests { - use super::*; - - #[test] - fn parse_line_text_delta() { - match parse_line(r#"{"type":"text","delta":"Hello"}"#) { - ChatDelta::TextDelta(s) => assert_eq!(s, "Hello"), - other => panic!("expected TextDelta, got {other:?}"), - } - } - - #[test] - fn parse_line_thinking_delta() { - match parse_line(r#"{"type":"thinking","delta":"reasoning..."}"#) { - ChatDelta::Thinking(s) => assert_eq!(s, "reasoning..."), - other => panic!("expected Thinking, got {other:?}"), - } - } - - #[test] - fn parse_line_tool_use() { - match parse_line(r#"{"type":"tool_use","name":"bash","args":{"cmd":"ls"}}"#) { - ChatDelta::ToolUse { name, args } => { - assert_eq!(name, "bash"); - assert!(args.contains("ls")); - } - other => panic!("expected ToolUse, got {other:?}"), - } - } - - #[test] - fn parse_line_done_with_stop_reason() { - match parse_line(r#"{"type":"done","stop_reason":"max_tokens"}"#) { - ChatDelta::Done { stop_reason } => { - assert!(matches!(stop_reason, StopReason::MaxTokens)); - } - other => panic!("expected Done, got {other:?}"), - } - } - - #[test] - fn parse_line_codex_agent_message_completed() { - match parse_line( - r#"{"type":"item.completed","item":{"id":"item_0","type":"agent_message","text":"hi"}}"#, - ) { - ChatDelta::TextDelta(s) => assert_eq!(s, "hi"), - other => panic!("expected TextDelta, got {other:?}"), - } - } - - #[test] - fn parse_line_codex_turn_completed() { - match parse_line( - r#"{"type":"turn.completed","usage":{"input_tokens":1,"output_tokens":2}}"#, - ) { - ChatDelta::Done { stop_reason } => { - assert!(matches!(stop_reason, StopReason::EndTurn)); - } - other => panic!("expected Done, got {other:?}"), - } - } - - #[test] - fn for_cli_constructs_codex_exec_provider() { - let provider = SubprocessProvider::for_cli(CliName::Codex); - assert!( - provider.is_some(), - "Codex CLI should be wired through exec --json" - ); - } - - #[test] - fn parse_line_error() { - match parse_line(r#"{"type":"error","message":"rate limited"}"#) { - ChatDelta::Error(s) => assert_eq!(s, "rate limited"), - other => panic!("expected Error, got {other:?}"), - } - } - - #[test] - fn parse_line_plain_text_falls_through_to_text_delta() { - match parse_line("Just some plain text") { - ChatDelta::TextDelta(s) => assert_eq!(s, "Just some plain text\n"), - other => panic!("expected TextDelta, got {other:?}"), - } - } - - #[test] - fn parse_line_malformed_json_falls_through() { - // Looks like JSON ({ ... }) but isn't valid → raw text. - match parse_line("{not json") { - ChatDelta::TextDelta(s) => assert_eq!(s, "{not json\n"), - other => panic!("expected TextDelta, got {other:?}"), - } - } - - #[test] - fn parse_line_unknown_type_falls_through() { - match parse_line(r#"{"type":"frobnicate","payload":42}"#) { - ChatDelta::TextDelta(s) => assert!(s.contains("frobnicate")), - other => panic!("expected TextDelta fallback, got {other:?}"), - } - } - - #[test] - fn for_cli_claude_code_seeds_print_stream_json_flags() { - let p = SubprocessProvider::for_cli(CliName::ClaudeCode).unwrap(); - // `binary` is the resolved absolute path when claude is on - // PATH (or one of the npm-global / nvm fallback locations); - // otherwise it falls back to the bare name. Either way the - // file name component must match. - assert!(p.binary.ends_with("claude"), "binary={}", p.binary); - assert!(p.args.iter().any(|a| a == "--print")); - assert!(p.args.iter().any(|a| a == "--verbose")); - assert!(p.args.iter().any(|a| a == "stream-json")); - assert_eq!(p.label, "Claude Code"); - assert_eq!(p.prompt_mode, PromptMode::PositionalArg); - } - - #[test] - fn for_cli_uses_default_binary_per_cli_name() { - // Resolved path may be bare or absolute depending on what's - // installed on the test host; check the basename only. - let gemini = SubprocessProvider::for_cli(CliName::Gemini).unwrap(); - assert!( - gemini.binary.ends_with("gemini"), - "binary={}", - gemini.binary - ); - assert_eq!(gemini.prompt_mode, PromptMode::Stdin); - let copilot = SubprocessProvider::for_cli(CliName::Copilot).unwrap(); - assert!( - copilot.binary.ends_with("gh-copilot"), - "binary={}", - copilot.binary - ); - assert_eq!(copilot.prompt_mode, PromptMode::Stdin); - } - - #[test] - fn for_cli_rejects_opencode() { - assert!(SubprocessProvider::for_cli(CliName::OpenCode).is_none()); - } - - #[test] - fn parse_line_malformed_structured_event_is_error() { - // "text" with no "delta" → Error - match parse_line(r#"{"type":"text"}"#) { - ChatDelta::Error(s) => assert!(s.contains("malformed text")), - other => panic!("expected Error, got {other:?}"), - } - // "tool_use" missing "name" → Error - match parse_line(r#"{"type":"tool_use","args":{}}"#) { - ChatDelta::Error(s) => assert!(s.contains("malformed tool_use")), - other => panic!("expected Error, got {other:?}"), - } - // "error" missing "message" → Error - match parse_line(r#"{"type":"error"}"#) { - ChatDelta::Error(s) => assert!(s.contains("malformed error")), - other => panic!("expected Error, got {other:?}"), - } - } - - #[test] - fn spawn_failure_surfaces_error_and_done() { - // Use a binary path that's guaranteed not to exist on PATH. - // Cross-platform expectation: - // - macOS / Linux: `Command::new` returns spawn-time Err - // because execvp fails → `ChatDelta::Error("spawn ...")` - // emitted before EOF. - // - Windows: `build_command` routes through `cmd /c`, which - // successfully spawns. The inner CLI then exits non-zero - // ("not recognized as an internal or external command") - // → `ChatDelta::Error("CLI exited with status N")` from - // the exit-status path. - // Either way the bridge must emit at least one Error delta - // plus a terminal Done. - let p = SubprocessProvider::with_binary( - "definitely-not-a-binary-3kf9j2-xyz", - Vec::new(), - "bogus", - ); - let deltas: Vec = p - .send(ChatRequest { - system_prompt: String::new(), - user_message: "hi".into(), - max_output_tokens: 64, - ..Default::default() - }) - .collect(); - assert!( - deltas.iter().any(|d| matches!(d, ChatDelta::Error(_))), - "expected at least one Error delta, got {:?}", - deltas - ); - assert!( - deltas.iter().any(|d| matches!(d, ChatDelta::Done { .. })), - "expected terminal Done in deltas, got {:?}", - deltas - ); - } -} +#[path = "chat_subprocess_tests.rs"] +mod tests; diff --git a/crates/op-host-desktop/src/chat_subprocess_tests.rs b/crates/op-host-desktop/src/chat_subprocess_tests.rs new file mode 100644 index 000000000..1a0f360d9 --- /dev/null +++ b/crates/op-host-desktop/src/chat_subprocess_tests.rs @@ -0,0 +1,195 @@ +use super::*; + +#[test] +fn parse_line_text_delta() { + match parse_line(r#"{"type":"text","delta":"Hello"}"#) { + ChatDelta::TextDelta(s) => assert_eq!(s, "Hello"), + other => panic!("expected TextDelta, got {other:?}"), + } +} + +#[test] +fn parse_line_thinking_delta() { + match parse_line(r#"{"type":"thinking","delta":"reasoning..."}"#) { + ChatDelta::Thinking(s) => assert_eq!(s, "reasoning..."), + other => panic!("expected Thinking, got {other:?}"), + } +} + +#[test] +fn parse_line_tool_use() { + match parse_line(r#"{"type":"tool_use","name":"bash","args":{"cmd":"ls"}}"#) { + ChatDelta::ToolUse { name, args } => { + assert_eq!(name, "bash"); + assert!(args.contains("ls")); + } + other => panic!("expected ToolUse, got {other:?}"), + } +} + +#[test] +fn parse_line_done_with_stop_reason() { + match parse_line(r#"{"type":"done","stop_reason":"max_tokens"}"#) { + ChatDelta::Done { stop_reason } => { + assert!(matches!(stop_reason, StopReason::MaxTokens)); + } + other => panic!("expected Done, got {other:?}"), + } +} + +#[test] +fn parse_line_codex_agent_message_completed() { + match parse_line( + r#"{"type":"item.completed","item":{"id":"item_0","type":"agent_message","text":"hi"}}"#, + ) { + ChatDelta::TextDelta(s) => assert_eq!(s, "hi"), + other => panic!("expected TextDelta, got {other:?}"), + } +} + +#[test] +fn parse_line_codex_turn_completed() { + match parse_line(r#"{"type":"turn.completed","usage":{"input_tokens":1,"output_tokens":2}}"#) { + ChatDelta::Done { stop_reason } => { + assert!(matches!(stop_reason, StopReason::EndTurn)); + } + other => panic!("expected Done, got {other:?}"), + } +} + +#[test] +fn for_cli_constructs_codex_exec_provider() { + let provider = SubprocessProvider::for_cli(CliName::Codex); + assert!( + provider.is_some(), + "Codex CLI should be wired through exec --json" + ); +} + +#[test] +fn parse_line_error() { + match parse_line(r#"{"type":"error","message":"rate limited"}"#) { + ChatDelta::Error(s) => assert_eq!(s, "rate limited"), + other => panic!("expected Error, got {other:?}"), + } +} + +#[test] +fn parse_line_plain_text_falls_through_to_text_delta() { + match parse_line("Just some plain text") { + ChatDelta::TextDelta(s) => assert_eq!(s, "Just some plain text\n"), + other => panic!("expected TextDelta, got {other:?}"), + } +} + +#[test] +fn parse_line_malformed_json_falls_through() { + // Looks like JSON ({ ... }) but isn't valid → raw text. + match parse_line("{not json") { + ChatDelta::TextDelta(s) => assert_eq!(s, "{not json\n"), + other => panic!("expected TextDelta, got {other:?}"), + } +} + +#[test] +fn parse_line_unknown_type_falls_through() { + match parse_line(r#"{"type":"frobnicate","payload":42}"#) { + ChatDelta::TextDelta(s) => assert!(s.contains("frobnicate")), + other => panic!("expected TextDelta fallback, got {other:?}"), + } +} + +#[test] +fn for_cli_claude_code_seeds_print_stream_json_flags() { + let p = SubprocessProvider::for_cli(CliName::ClaudeCode).unwrap(); + // `binary` is the resolved absolute path when claude is on + // PATH (or one of the npm-global / nvm fallback locations); + // otherwise it falls back to the bare name. Either way the + // file name component must match. + assert!(p.binary.ends_with("claude"), "binary={}", p.binary); + assert!(p.args.iter().any(|a| a == "--print")); + assert!(p.args.iter().any(|a| a == "--verbose")); + assert!(p.args.iter().any(|a| a == "stream-json")); + assert_eq!(p.label, "Claude Code"); + assert_eq!(p.prompt_mode, PromptMode::PositionalArg); +} + +#[test] +fn for_cli_uses_default_binary_per_cli_name() { + // Resolved path may be bare or absolute depending on what's + // installed on the test host; check the basename only. + let gemini = SubprocessProvider::for_cli(CliName::Gemini).unwrap(); + assert!( + gemini.binary.ends_with("gemini"), + "binary={}", + gemini.binary + ); + assert_eq!(gemini.prompt_mode, PromptMode::Stdin); + let copilot = SubprocessProvider::for_cli(CliName::Copilot).unwrap(); + assert!( + copilot.binary.ends_with("gh-copilot"), + "binary={}", + copilot.binary + ); + assert_eq!(copilot.prompt_mode, PromptMode::Stdin); +} + +#[test] +fn for_cli_rejects_opencode() { + assert!(SubprocessProvider::for_cli(CliName::OpenCode).is_none()); +} + +#[test] +fn parse_line_malformed_structured_event_is_error() { + // "text" with no "delta" → Error + match parse_line(r#"{"type":"text"}"#) { + ChatDelta::Error(s) => assert!(s.contains("malformed text")), + other => panic!("expected Error, got {other:?}"), + } + // "tool_use" missing "name" → Error + match parse_line(r#"{"type":"tool_use","args":{}}"#) { + ChatDelta::Error(s) => assert!(s.contains("malformed tool_use")), + other => panic!("expected Error, got {other:?}"), + } + // "error" missing "message" → Error + match parse_line(r#"{"type":"error"}"#) { + ChatDelta::Error(s) => assert!(s.contains("malformed error")), + other => panic!("expected Error, got {other:?}"), + } +} + +#[test] +fn spawn_failure_surfaces_error_and_done() { + // Use a binary path that's guaranteed not to exist on PATH. + // Cross-platform expectation: + // - macOS / Linux: `Command::new` returns spawn-time Err + // because execvp fails → `ChatDelta::Error("spawn ...")` + // emitted before EOF. + // - Windows: `build_command` routes through `cmd /c`, which + // successfully spawns. The inner CLI then exits non-zero + // ("not recognized as an internal or external command") + // → `ChatDelta::Error("CLI exited with status N")` from + // the exit-status path. + // Either way the bridge must emit at least one Error delta + // plus a terminal Done. + let p = + SubprocessProvider::with_binary("definitely-not-a-binary-3kf9j2-xyz", Vec::new(), "bogus"); + let deltas: Vec = p + .send(ChatRequest { + system_prompt: String::new(), + user_message: "hi".into(), + max_output_tokens: 64, + ..Default::default() + }) + .collect(); + assert!( + deltas.iter().any(|d| matches!(d, ChatDelta::Error(_))), + "expected at least one Error delta, got {:?}", + deltas + ); + assert!( + deltas.iter().any(|d| matches!(d, ChatDelta::Done { .. })), + "expected terminal Done in deltas, got {:?}", + deltas + ); +} diff --git a/crates/op-host-desktop/src/design_session.rs b/crates/op-host-desktop/src/design_session.rs index 986a85efa..47e58c68a 100644 --- a/crates/op-host-desktop/src/design_session.rs +++ b/crates/op-host-desktop/src/design_session.rs @@ -504,304 +504,5 @@ fn progress_label(p: &Progress) -> String { } #[cfg(test)] -mod tests { - use super::*; - use op_editor_core::EditorCommand; - use op_orchestrator::{RunSummary, SubtaskOutcome}; - use std::time::{Duration, Instant}; - - /// `RemoteDocSink::apply` blocks until UI acks. When the UI side - /// drops the receiver, `apply` returns false instead of hanging. - #[test] - fn remote_doc_sink_returns_false_when_ui_channel_closed() { - let (tx, rx) = mpsc::channel::(); - let mut sink = RemoteDocSink::new(tx, EditorState::new()); - drop(rx); // simulate UI session dropped before the worker called apply - let applied = sink.apply(EditorCommand::ClearSelection); - assert!(!applied, "apply on closed channel must return false"); - } - - /// Happy-path round-trip: worker sends an apply request; UI thread - /// acks with an updated state snapshot; worker's mirror reflects it. - #[test] - fn remote_doc_sink_updates_mirror_on_ack() { - let (tx, rx) = mpsc::channel::(); - let initial = EditorState::new(); - let mut sink = RemoteDocSink::new(tx, initial.clone()); - - // Spawn UI-side faker that acks one request with a modified state. - let ui_thread = thread::spawn(move || { - let req = rx.recv().expect("worker should send one request"); - let mut new_state = initial.clone(); - // Mutate something the test can observe — viewport zoom. - new_state.viewport.zoom = 2.0; - let ack = DesignCmdAck { - applied: true, - new_state, - }; - req.ack.send(ack).expect("ack must reach worker"); - }); - - let applied = sink.apply(EditorCommand::ClearSelection); - ui_thread.join().expect("ui thread must finish"); - assert!(applied, "ack reported applied=true"); - assert_eq!( - sink.state().viewport.zoom, - 2.0, - "mirror should reflect ack snapshot" - ); - } - - /// `BeginUndoBatch` and `EndUndoBatch` are forwarded as their own - /// `DesignCmdOp` variants so the UI can route them through the - /// real `History::begin_batch` / `end_batch` once wired. - #[test] - fn undo_batch_signals_are_distinguishable_on_the_wire() { - let (tx, rx) = mpsc::channel::(); - let mut sink = RemoteDocSink::new(tx, EditorState::new()); - let ui = thread::spawn(move || { - let mut kinds = Vec::new(); - while let Ok(req) = rx.recv() { - let label = match req.op { - DesignCmdOp::Apply(_) => "apply", - DesignCmdOp::BeginUndoBatch => "begin", - DesignCmdOp::EndUndoBatch => "end", - }; - kinds.push(label.to_string()); - let _ = req.ack.send(DesignCmdAck { - applied: true, - new_state: EditorState::new(), - }); - } - kinds - }); - sink.begin_undo_batch(); - sink.apply(EditorCommand::ClearSelection); - sink.end_undo_batch(); - drop(sink); // close the channel so the ui-side recv loop exits - let kinds = ui.join().expect("ui thread finishes"); - assert_eq!(kinds, vec!["begin", "apply", "end"]); - } - - /// End-to-end smoke through `pump_commands` + `pump_progress`: - /// a fake worker thread drives a `RemoteDocSink` against - /// real-looking channels, the UI loop drains both pumps, and we - /// assert that the chat bubble carries the rendered progress + - /// terminal summary line, and that the session clears itself - /// after `Done`. - /// - /// This is the host-side complement to the orchestrator's own - /// end-to-end tests — it exercises the actor seam without - /// requiring an `agent::Provider` / `ANTHROPIC_API_KEY`. Task #28 - /// covers the live LLM smoke separately. - #[test] - fn end_to_end_pump_round_trips_apply_and_progress_via_actor_channels() { - let (delta_tx, delta_rx) = mpsc::channel::(); - let (cmd_tx, cmd_rx) = mpsc::channel::(); - let mut current = Some(DesignSession::from_channels(delta_rx, cmd_rx)); - let mut host = WidgetHostNative::new(); - // Seed a streaming assistant bubble — `chat.begin_send` - // creates one in production; the pumps fold the worker's - // progress + summary into it. - host.editor_state_mut() - .chat - .messages - .push(op_editor_core::ChatMessage::assistant_streaming()); - - // Fake worker — emits one progress event, asks UI to apply - // ClearSelection, then a successful `Done`. - let fake_worker = thread::spawn(move || { - // Progress first so the bubble starts streaming text - // before the doc mutation. - let _ = delta_tx.send(DesignDelta::Progress(Progress::Planning)); - let mut sink = RemoteDocSink::new(cmd_tx, EditorState::new()); - sink.apply(EditorCommand::ClearSelection); - let _ = delta_tx.send(DesignDelta::Done(Ok(RunSummary { - root_frame_id: "root".into(), - subtasks: vec![SubtaskOutcome { - id: "s1".into(), - node_count: 3, - error: None, - }], - total_nodes: 3, - }))); - // Hold the sink so its channel survives until the UI has - // had a chance to drain (the test polls until `Done`). - sink - }); - - // UI drives the pumps until the session clears (mirrors the - // event-loop `RedrawRequested` block). Bound the loop with a - // timeout so a hung worker fails the test instead of hanging. - let deadline = Instant::now() + Duration::from_secs(5); - while current.is_some() && Instant::now() < deadline { - let _ = pump_commands(&mut host, &mut current, 1440.0, 900.0); - let _ = pump_progress(&mut host, &mut current); - if current.is_none() { - break; - } - thread::sleep(Duration::from_millis(2)); - } - // Worker can join now — the sink it returned (and thus its - // cmd_tx) drops at this scope end. - let _ = fake_worker.join().expect("fake worker exits cleanly"); - - assert!( - current.is_none(), - "session must clear after Done — leaving it set would keep the\ - event loop ticking and pump_progress retrying" - ); - let bubble = host - .editor_state() - .chat - .messages - .last() - .expect("seeded bubble survives"); - assert!( - bubble.content.contains("Planning"), - "progress line should render Planning, got: {:?}", - bubble.content - ); - assert!( - bubble.content.contains("1 subtask"), - "summary should report 1 subtask succeeded, got: {:?}", - bubble.content - ); - assert!( - !bubble.streaming, - "summary path must clear streaming so the chat panel stops the animation" - ); - } - - #[test] - fn fit_design_viewport_centers_and_fits_mobile_root() { - let mut state = EditorState::new(); - state.doc.children = vec![mobile_root()]; - - assert!(fit_design_viewport_to_content(&mut state, 1440.0, 900.0)); - - let bounds = active_content_bounds(&state).expect("root bounds"); - let (canvas_w, canvas_h) = design_canvas_size(&state, 1440.0, 900.0); - let left = state.viewport.pan_x + bounds.x as f32 * state.viewport.zoom; - let top = state.viewport.pan_y + bounds.y as f32 * state.viewport.zoom; - let right = left + bounds.w as f32 * state.viewport.zoom; - let bottom = top + bounds.h as f32 * state.viewport.zoom; - let center_x = (left + right) / 2.0; - let center_y = (top + bottom) / 2.0; - - assert!(left >= 0.0, "left edge should be visible, got {left}"); - assert!(top >= 0.0, "top edge should be visible, got {top}"); - assert!( - right <= canvas_w, - "right edge should be visible: {right} > {canvas_w}" - ); - assert!( - bottom <= canvas_h, - "bottom edge should be visible: {bottom} > {canvas_h}" - ); - assert!((center_x - canvas_w / 2.0).abs() < 0.5); - assert!((center_y - canvas_h / 2.0).abs() < 0.5); - } - - #[test] - fn pump_commands_refits_viewport_after_design_insert() { - let (_delta_tx, delta_rx) = mpsc::channel::(); - let (cmd_tx, cmd_rx) = mpsc::channel::(); - let mut current = Some(DesignSession::from_channels(delta_rx, cmd_rx)); - let mut host = WidgetHostNative::new(); - host.editor_state_mut().doc.children.clear(); - let before = host.editor_state().viewport; - - let (ack_tx, ack_rx) = mpsc::sync_channel::(1); - cmd_tx - .send(DesignCmdReq { - op: DesignCmdOp::Apply(EditorCommand::InsertSubtree { - nodes: vec![mobile_root()], - parent_id: op_editor_core::NodeId::NONE, - }), - ack: ack_tx, - }) - .expect("request should queue"); - - assert!(pump_commands(&mut host, &mut current, 1440.0, 900.0)); - let ack = ack_rx - .recv_timeout(Duration::from_secs(1)) - .expect("pump should ack apply request"); - assert!(ack.applied); - assert!( - !ack.new_state.doc.children.is_empty(), - "ack snapshot should include inserted root" - ); - assert_eq!( - host.editor_state().doc.children.len(), - 1, - "host state should receive inserted root" - ); - - let after = host.editor_state().viewport; - assert_ne!(before, after, "design insert should refit viewport"); - assert!( - (after.zoom - 0.905).abs() < 0.01, - "mobile root should fit viewport height, got zoom {}", - after.zoom - ); - } - - #[test] - fn fit_design_viewport_uses_resolved_layout_for_fit_content_root() { - let mut state = EditorState::new(); - state.doc.children = vec![mobile_fit_content_root()]; - - assert!(fit_design_viewport_to_content(&mut state, 1440.0, 900.0)); - - let bounds = active_content_bounds(&state).expect("resolved root bounds"); - assert!( - (bounds.h - 844.0).abs() < 1.0, - "fit_content root should resolve to full mobile height, got {}", - bounds.h - ); - assert!( - (state.viewport.zoom - 0.905).abs() < 0.01, - "full mobile root should remain fully visible, got zoom {}", - state.viewport.zoom - ); - } - - fn mobile_root() -> jian_ops_schema::node::PenNode { - serde_json::from_value(serde_json::json!({ - "type": "frame", - "id": "root", - "name": "Mobile Root", - "x": 80, - "y": 40, - "width": 390, - "height": 844, - "children": [] - })) - .expect("mobile root fixture parses") - } - - fn mobile_fit_content_root() -> jian_ops_schema::node::PenNode { - serde_json::from_value(serde_json::json!({ - "type": "frame", - "id": "root", - "name": "Mobile Root", - "x": 80, - "y": 40, - "width": 390, - "height": "fit_content", - "layout": "vertical", - "gap": 0, - "children": [ - {"type": "frame", "id": "status", "name": "Status Bar", "width": "fill_container", "height": 32}, - {"type": "frame", "id": "header", "name": "Header", "width": "fill_container", "height": 92}, - {"type": "frame", "id": "search", "name": "Search", "width": "fill_container", "height": 104}, - {"type": "frame", "id": "promo", "name": "Promo", "width": "fill_container", "height": 132}, - {"type": "frame", "id": "categories", "name": "Categories", "width": "fill_container", "height": 86}, - {"type": "frame", "id": "restaurants", "name": "Restaurants", "width": "fill_container", "height": 314}, - {"type": "frame", "id": "bottom-nav", "name": "Bottom Nav", "width": "fill_container", "height": 84} - ] - })) - .expect("fit_content mobile root fixture parses") - } -} +#[path = "design_session_tests.rs"] +mod tests; diff --git a/crates/op-host-desktop/src/design_session_tests.rs b/crates/op-host-desktop/src/design_session_tests.rs new file mode 100644 index 000000000..485203d5a --- /dev/null +++ b/crates/op-host-desktop/src/design_session_tests.rs @@ -0,0 +1,299 @@ +use super::*; +use op_editor_core::EditorCommand; +use op_orchestrator::{RunSummary, SubtaskOutcome}; +use std::time::{Duration, Instant}; + +/// `RemoteDocSink::apply` blocks until UI acks. When the UI side +/// drops the receiver, `apply` returns false instead of hanging. +#[test] +fn remote_doc_sink_returns_false_when_ui_channel_closed() { + let (tx, rx) = mpsc::channel::(); + let mut sink = RemoteDocSink::new(tx, EditorState::new()); + drop(rx); // simulate UI session dropped before the worker called apply + let applied = sink.apply(EditorCommand::ClearSelection); + assert!(!applied, "apply on closed channel must return false"); +} + +/// Happy-path round-trip: worker sends an apply request; UI thread +/// acks with an updated state snapshot; worker's mirror reflects it. +#[test] +fn remote_doc_sink_updates_mirror_on_ack() { + let (tx, rx) = mpsc::channel::(); + let initial = EditorState::new(); + let mut sink = RemoteDocSink::new(tx, initial.clone()); + + // Spawn UI-side faker that acks one request with a modified state. + let ui_thread = thread::spawn(move || { + let req = rx.recv().expect("worker should send one request"); + let mut new_state = initial.clone(); + // Mutate something the test can observe — viewport zoom. + new_state.viewport.zoom = 2.0; + let ack = DesignCmdAck { + applied: true, + new_state, + }; + req.ack.send(ack).expect("ack must reach worker"); + }); + + let applied = sink.apply(EditorCommand::ClearSelection); + ui_thread.join().expect("ui thread must finish"); + assert!(applied, "ack reported applied=true"); + assert_eq!( + sink.state().viewport.zoom, + 2.0, + "mirror should reflect ack snapshot" + ); +} + +/// `BeginUndoBatch` and `EndUndoBatch` are forwarded as their own +/// `DesignCmdOp` variants so the UI can route them through the +/// real `History::begin_batch` / `end_batch` once wired. +#[test] +fn undo_batch_signals_are_distinguishable_on_the_wire() { + let (tx, rx) = mpsc::channel::(); + let mut sink = RemoteDocSink::new(tx, EditorState::new()); + let ui = thread::spawn(move || { + let mut kinds = Vec::new(); + while let Ok(req) = rx.recv() { + let label = match req.op { + DesignCmdOp::Apply(_) => "apply", + DesignCmdOp::BeginUndoBatch => "begin", + DesignCmdOp::EndUndoBatch => "end", + }; + kinds.push(label.to_string()); + let _ = req.ack.send(DesignCmdAck { + applied: true, + new_state: EditorState::new(), + }); + } + kinds + }); + sink.begin_undo_batch(); + sink.apply(EditorCommand::ClearSelection); + sink.end_undo_batch(); + drop(sink); // close the channel so the ui-side recv loop exits + let kinds = ui.join().expect("ui thread finishes"); + assert_eq!(kinds, vec!["begin", "apply", "end"]); +} + +/// End-to-end smoke through `pump_commands` + `pump_progress`: +/// a fake worker thread drives a `RemoteDocSink` against +/// real-looking channels, the UI loop drains both pumps, and we +/// assert that the chat bubble carries the rendered progress + +/// terminal summary line, and that the session clears itself +/// after `Done`. +/// +/// This is the host-side complement to the orchestrator's own +/// end-to-end tests — it exercises the actor seam without +/// requiring an `agent::Provider` / `ANTHROPIC_API_KEY`. Task #28 +/// covers the live LLM smoke separately. +#[test] +fn end_to_end_pump_round_trips_apply_and_progress_via_actor_channels() { + let (delta_tx, delta_rx) = mpsc::channel::(); + let (cmd_tx, cmd_rx) = mpsc::channel::(); + let mut current = Some(DesignSession::from_channels(delta_rx, cmd_rx)); + let mut host = WidgetHostNative::new(); + // Seed a streaming assistant bubble — `chat.begin_send` + // creates one in production; the pumps fold the worker's + // progress + summary into it. + host.editor_state_mut() + .chat + .messages + .push(op_editor_core::ChatMessage::assistant_streaming()); + + // Fake worker — emits one progress event, asks UI to apply + // ClearSelection, then a successful `Done`. + let fake_worker = thread::spawn(move || { + // Progress first so the bubble starts streaming text + // before the doc mutation. + let _ = delta_tx.send(DesignDelta::Progress(Progress::Planning)); + let mut sink = RemoteDocSink::new(cmd_tx, EditorState::new()); + sink.apply(EditorCommand::ClearSelection); + let _ = delta_tx.send(DesignDelta::Done(Ok(RunSummary { + root_frame_id: "root".into(), + subtasks: vec![SubtaskOutcome { + id: "s1".into(), + node_count: 3, + error: None, + }], + total_nodes: 3, + }))); + // Hold the sink so its channel survives until the UI has + // had a chance to drain (the test polls until `Done`). + sink + }); + + // UI drives the pumps until the session clears (mirrors the + // event-loop `RedrawRequested` block). Bound the loop with a + // timeout so a hung worker fails the test instead of hanging. + let deadline = Instant::now() + Duration::from_secs(5); + while current.is_some() && Instant::now() < deadline { + let _ = pump_commands(&mut host, &mut current, 1440.0, 900.0); + let _ = pump_progress(&mut host, &mut current); + if current.is_none() { + break; + } + thread::sleep(Duration::from_millis(2)); + } + // Worker can join now — the sink it returned (and thus its + // cmd_tx) drops at this scope end. + let _ = fake_worker.join().expect("fake worker exits cleanly"); + + assert!( + current.is_none(), + "session must clear after Done — leaving it set would keep the\ + event loop ticking and pump_progress retrying" + ); + let bubble = host + .editor_state() + .chat + .messages + .last() + .expect("seeded bubble survives"); + assert!( + bubble.content.contains("Planning"), + "progress line should render Planning, got: {:?}", + bubble.content + ); + assert!( + bubble.content.contains("1 subtask"), + "summary should report 1 subtask succeeded, got: {:?}", + bubble.content + ); + assert!( + !bubble.streaming, + "summary path must clear streaming so the chat panel stops the animation" + ); +} + +#[test] +fn fit_design_viewport_centers_and_fits_mobile_root() { + let mut state = EditorState::new(); + state.doc.children = vec![mobile_root()]; + + assert!(fit_design_viewport_to_content(&mut state, 1440.0, 900.0)); + + let bounds = active_content_bounds(&state).expect("root bounds"); + let (canvas_w, canvas_h) = design_canvas_size(&state, 1440.0, 900.0); + let left = state.viewport.pan_x + bounds.x as f32 * state.viewport.zoom; + let top = state.viewport.pan_y + bounds.y as f32 * state.viewport.zoom; + let right = left + bounds.w as f32 * state.viewport.zoom; + let bottom = top + bounds.h as f32 * state.viewport.zoom; + let center_x = (left + right) / 2.0; + let center_y = (top + bottom) / 2.0; + + assert!(left >= 0.0, "left edge should be visible, got {left}"); + assert!(top >= 0.0, "top edge should be visible, got {top}"); + assert!( + right <= canvas_w, + "right edge should be visible: {right} > {canvas_w}" + ); + assert!( + bottom <= canvas_h, + "bottom edge should be visible: {bottom} > {canvas_h}" + ); + assert!((center_x - canvas_w / 2.0).abs() < 0.5); + assert!((center_y - canvas_h / 2.0).abs() < 0.5); +} + +#[test] +fn pump_commands_refits_viewport_after_design_insert() { + let (_delta_tx, delta_rx) = mpsc::channel::(); + let (cmd_tx, cmd_rx) = mpsc::channel::(); + let mut current = Some(DesignSession::from_channels(delta_rx, cmd_rx)); + let mut host = WidgetHostNative::new(); + host.editor_state_mut().doc.children.clear(); + let before = host.editor_state().viewport; + + let (ack_tx, ack_rx) = mpsc::sync_channel::(1); + cmd_tx + .send(DesignCmdReq { + op: DesignCmdOp::Apply(EditorCommand::InsertSubtree { + nodes: vec![mobile_root()], + parent_id: op_editor_core::NodeId::NONE, + }), + ack: ack_tx, + }) + .expect("request should queue"); + + assert!(pump_commands(&mut host, &mut current, 1440.0, 900.0)); + let ack = ack_rx + .recv_timeout(Duration::from_secs(1)) + .expect("pump should ack apply request"); + assert!(ack.applied); + assert!( + !ack.new_state.doc.children.is_empty(), + "ack snapshot should include inserted root" + ); + assert_eq!( + host.editor_state().doc.children.len(), + 1, + "host state should receive inserted root" + ); + + let after = host.editor_state().viewport; + assert_ne!(before, after, "design insert should refit viewport"); + assert!( + (after.zoom - 0.905).abs() < 0.01, + "mobile root should fit viewport height, got zoom {}", + after.zoom + ); +} + +#[test] +fn fit_design_viewport_uses_resolved_layout_for_fit_content_root() { + let mut state = EditorState::new(); + state.doc.children = vec![mobile_fit_content_root()]; + + assert!(fit_design_viewport_to_content(&mut state, 1440.0, 900.0)); + + let bounds = active_content_bounds(&state).expect("resolved root bounds"); + assert!( + (bounds.h - 844.0).abs() < 1.0, + "fit_content root should resolve to full mobile height, got {}", + bounds.h + ); + assert!( + (state.viewport.zoom - 0.905).abs() < 0.01, + "full mobile root should remain fully visible, got zoom {}", + state.viewport.zoom + ); +} + +fn mobile_root() -> jian_ops_schema::node::PenNode { + serde_json::from_value(serde_json::json!({ + "type": "frame", + "id": "root", + "name": "Mobile Root", + "x": 80, + "y": 40, + "width": 390, + "height": 844, + "children": [] + })) + .expect("mobile root fixture parses") +} + +fn mobile_fit_content_root() -> jian_ops_schema::node::PenNode { + serde_json::from_value(serde_json::json!({ + "type": "frame", + "id": "root", + "name": "Mobile Root", + "x": 80, + "y": 40, + "width": 390, + "height": "fit_content", + "layout": "vertical", + "gap": 0, + "children": [ + {"type": "frame", "id": "status", "name": "Status Bar", "width": "fill_container", "height": 32}, + {"type": "frame", "id": "header", "name": "Header", "width": "fill_container", "height": 92}, + {"type": "frame", "id": "search", "name": "Search", "width": "fill_container", "height": 104}, + {"type": "frame", "id": "promo", "name": "Promo", "width": "fill_container", "height": 132}, + {"type": "frame", "id": "categories", "name": "Categories", "width": "fill_container", "height": 86}, + {"type": "frame", "id": "restaurants", "name": "Restaurants", "width": "fill_container", "height": 314}, + {"type": "frame", "id": "bottom-nav", "name": "Bottom Nav", "width": "fill_container", "height": 84} + ] + })) + .expect("fit_content mobile root fixture parses") +}