diff --git a/crates/acp_thread/src/acp_thread.rs b/crates/acp_thread/src/acp_thread.rs index 221357418760cbb306a5f8ca5d1726e857915543..c26f4f218ce2ae656fdd2dec85eb4389bb2f7c8d 100644 --- a/crates/acp_thread/src/acp_thread.rs +++ b/crates/acp_thread/src/acp_thread.rs @@ -2661,6 +2661,128 @@ mod tests { ); } + /// Test that killing a terminal via Terminal::kill properly: + /// 1. Causes wait_for_exit to complete (doesn't hang forever) + /// 2. The underlying terminal still has the output that was written before the kill + /// + /// This test verifies that the fix to kill_active_task (which now also kills + /// the shell process in addition to the foreground process) properly allows + /// wait_for_exit to complete instead of hanging indefinitely. + #[cfg(unix)] + #[gpui::test] + async fn test_terminal_kill_allows_wait_for_exit_to_complete(cx: &mut gpui::TestAppContext) { + use std::collections::HashMap; + use task::Shell; + use util::shell_builder::ShellBuilder; + + init_test(cx); + cx.executor().allow_parking(); + + let fs = FakeFs::new(cx.executor()); + let project = Project::test(fs, [], cx).await; + let connection = Rc::new(FakeAgentConnection::new()); + let thread = cx + .update(|cx| connection.new_thread(project.clone(), Path::new(path!("/test")), cx)) + .await + .unwrap(); + + let terminal_id = acp::TerminalId::new(uuid::Uuid::new_v4().to_string()); + + // Create a real PTY terminal that runs a command which prints output then sleeps + // We use printf instead of echo and chain with && sleep to ensure proper execution + let (completion_tx, _completion_rx) = smol::channel::unbounded(); + let (program, args) = ShellBuilder::new(&Shell::System, false).build( + Some("printf 'output_before_kill\\n' && sleep 60".to_owned()), + &[], + ); + + let builder = cx + .update(|cx| { + ::terminal::TerminalBuilder::new( + None, + None, + task::Shell::WithArguments { + program, + args, + title_override: None, + }, + HashMap::default(), + ::terminal::terminal_settings::CursorShape::default(), + ::terminal::terminal_settings::AlternateScroll::On, + None, + vec![], + 0, + false, + 0, + Some(completion_tx), + cx, + vec![], + ) + }) + .await + .unwrap(); + + let lower_terminal = cx.new(|cx| builder.subscribe(cx)); + + // Create the acp_thread Terminal wrapper + thread.update(cx, |thread, cx| { + thread.on_terminal_provider_event( + TerminalProviderEvent::Created { + terminal_id: terminal_id.clone(), + label: "printf output_before_kill && sleep 60".to_string(), + cwd: None, + output_byte_limit: None, + terminal: lower_terminal.clone(), + }, + cx, + ); + }); + + // Wait for the printf command to execute and produce output + smol::Timer::after(Duration::from_millis(500)).await; + + // Get the acp_thread Terminal and kill it + let wait_for_exit = thread.update(cx, |thread, cx| { + let term = thread.terminals.get(&terminal_id).unwrap(); + let wait_for_exit = term.read(cx).wait_for_exit(); + term.update(cx, |term, cx| { + term.kill(cx); + }); + wait_for_exit + }); + + // KEY ASSERTION: wait_for_exit should complete within a reasonable time (not hang). + // Before the fix to kill_active_task, this would hang forever because + // only the foreground process was killed, not the shell, so the PTY + // child never exited and wait_for_completed_task never completed. + let exit_result = futures::select! { + result = futures::FutureExt::fuse(wait_for_exit) => Some(result), + _ = futures::FutureExt::fuse(smol::Timer::after(Duration::from_secs(5))) => None, + }; + + assert!( + exit_result.is_some(), + "wait_for_exit should complete after kill, but it timed out. \ + This indicates kill_active_task is not properly killing the shell process." + ); + + // Give the system a chance to process any pending updates + cx.run_until_parked(); + + // Verify that the underlying terminal still has the output that was + // written before the kill. This verifies that killing doesn't lose output. + let inner_content = thread.read_with(cx, |thread, cx| { + let term = thread.terminals.get(&terminal_id).unwrap(); + term.read(cx).inner().read(cx).get_content() + }); + + assert!( + inner_content.contains("output_before_kill"), + "Underlying terminal should contain output from before kill, got: {}", + inner_content + ); + } + #[gpui::test] async fn test_push_user_content_block(cx: &mut gpui::TestAppContext) { init_test(cx); diff --git a/crates/gpui/src/window.rs b/crates/gpui/src/window.rs index 94ae5f7a8ce8b2de8d44c328321283ee5f45843b..d5372c81f198c8954e6a5748d90ca769b5875f13 100644 --- a/crates/gpui/src/window.rs +++ b/crates/gpui/src/window.rs @@ -4895,6 +4895,19 @@ impl Window { pub fn set_modifiers(&mut self, modifiers: Modifiers) { self.modifiers = modifiers; } + + /// For testing: simulate a mouse move event to the given position. + /// This dispatches the event through the normal event handling path, + /// which will trigger hover states and tooltips. + #[cfg(any(test, feature = "test-support"))] + pub fn simulate_mouse_move(&mut self, position: Point, cx: &mut App) { + let event = PlatformInput::MouseMove(MouseMoveEvent { + position, + modifiers: self.modifiers, + pressed_button: None, + }); + let _ = self.dispatch_event(event, cx); + } } // #[derive(Clone, Copy, Eq, PartialEq, Hash)] diff --git a/crates/zed/src/visual_test_runner.rs b/crates/zed/src/visual_test_runner.rs index 1131e3569743a80076c7570b8fde8a2c025d83b4..8a660b20c40d6e02e636956cea4318fdc2973884 100644 --- a/crates/zed/src/visual_test_runner.rs +++ b/crates/zed/src/visual_test_runner.rs @@ -50,6 +50,8 @@ use { agent_servers::{AgentServer, AgentServerDelegate}, anyhow::{Context as _, Result}, assets::Assets, + feature_flags::FeatureFlagAppExt as _, + git_ui::project_diff::ProjectDiff, gpui::{ App, AppContext as _, Bounds, KeyBinding, Modifiers, SharedString, VisualTestAppContext, WindowBounds, WindowHandle, WindowOptions, point, px, size, @@ -435,6 +437,23 @@ fn run_visual_tests(project_path: PathBuf, update_baseline: bool) -> Result<()> } } + // Run Test 5: Diff Review Button visual tests + println!("\n--- Test 5: diff_review_button (3 variants) ---"); + match run_diff_review_visual_tests(app_state.clone(), &mut cx, update_baseline) { + Ok(TestResult::Passed) => { + println!("✓ diff_review_button: PASSED"); + passed += 1; + } + Ok(TestResult::BaselineUpdated(_)) => { + println!("✓ diff_review_button: Baselines updated"); + updated += 1; + } + Err(e) => { + eprintln!("✗ diff_review_button: FAILED - {}", e); + failed += 1; + } + } + // Clean up the main workspace's worktree to stop background scanning tasks // This prevents "root path could not be canonicalized" errors when main() drops temp_dir workspace_window @@ -1097,6 +1116,291 @@ fn run_breakpoint_hover_visual_tests( } } +/// Runs visual tests for the diff review button in git diff views. +/// +/// This test captures three states: +/// 1. Diff view with feature flag enabled (button visible) +/// 2. Diff view with feature flag disabled (no button) +/// 3. Regular editor with feature flag enabled (no button - only shows in diff views) +#[cfg(target_os = "macos")] +fn run_diff_review_visual_tests( + app_state: Arc, + cx: &mut VisualTestAppContext, + update_baseline: bool, +) -> Result { + // Create a temporary directory with test files and a real git repo + let temp_dir = tempfile::tempdir()?; + let temp_path = temp_dir.keep(); + let canonical_temp = temp_path.canonicalize()?; + let project_path = canonical_temp.join("project"); + std::fs::create_dir_all(&project_path)?; + + // Initialize a real git repository + std::process::Command::new("git") + .args(["init"]) + .current_dir(&project_path) + .output()?; + + // Configure git user for commits + std::process::Command::new("git") + .args(["config", "user.email", "test@test.com"]) + .current_dir(&project_path) + .output()?; + std::process::Command::new("git") + .args(["config", "user.name", "Test User"]) + .current_dir(&project_path) + .output()?; + + // Create a test file with original content + let original_content = "// Original content\n"; + std::fs::write(project_path.join("thread-view.tsx"), original_content)?; + + // Commit the original file + std::process::Command::new("git") + .args(["add", "thread-view.tsx"]) + .current_dir(&project_path) + .output()?; + std::process::Command::new("git") + .args(["commit", "-m", "Initial commit"]) + .current_dir(&project_path) + .output()?; + + // Modify the file to create a diff + let modified_content = r#"import { ScrollArea } from 'components'; +import { ButtonAlt, Tooltip } from 'ui'; +import { Message, FileEdit } from 'types'; +import { AiPaneTabContext } from 'context'; +"#; + std::fs::write(project_path.join("thread-view.tsx"), modified_content)?; + + // Create window for the diff view - sized to show just the editor + let window_size = size(px(600.0), px(400.0)); + let bounds = Bounds { + origin: point(px(0.0), px(0.0)), + size: window_size, + }; + + // Create project + let project = cx.update(|cx| { + project::Project::local( + app_state.client.clone(), + app_state.node_runtime.clone(), + app_state.user_store.clone(), + app_state.languages.clone(), + app_state.fs.clone(), + None, + false, + cx, + ) + }); + + // Add the test directory as a worktree + let add_worktree_task = project.update(cx, |project, cx| { + project.find_or_create_worktree(&project_path, true, cx) + }); + + cx.background_executor.allow_parking(); + let _ = cx.background_executor.block_test(add_worktree_task); + cx.background_executor.forbid_parking(); + + cx.run_until_parked(); + + // Wait for worktree to be fully scanned and git status to be detected + for _ in 0..5 { + cx.advance_clock(Duration::from_millis(100)); + cx.run_until_parked(); + } + + // Test 1: Diff view with feature flag enabled + // Enable the feature flag + cx.update(|cx| { + cx.update_flags(true, vec!["diff-review".to_string()]); + }); + + let workspace_window: WindowHandle = cx + .update(|cx| { + cx.open_window( + WindowOptions { + window_bounds: Some(WindowBounds::Windowed(bounds)), + focus: false, + show: false, + ..Default::default() + }, + |window, cx| { + cx.new(|cx| { + Workspace::new(None, project.clone(), app_state.clone(), window, cx) + }) + }, + ) + }) + .context("Failed to open diff review test window")?; + + cx.run_until_parked(); + + // Create and add the ProjectDiff using the public deploy_at method + workspace_window + .update(cx, |workspace, window, cx| { + ProjectDiff::deploy_at(workspace, None, window, cx); + }) + .ok(); + + // Wait for diff to render + for _ in 0..5 { + cx.advance_clock(Duration::from_millis(100)); + cx.run_until_parked(); + } + + // Refresh window + cx.update_window(workspace_window.into(), |_, window, _cx| { + window.refresh(); + })?; + + cx.run_until_parked(); + + // Capture Test 1: Diff with flag enabled + let test1_result = run_visual_test( + "diff_review_button_enabled", + workspace_window.into(), + cx, + update_baseline, + )?; + + // Test 2: Diff view with feature flag disabled + // Disable the feature flag + cx.update(|cx| { + cx.update_flags(false, vec![]); + }); + + // Refresh window + cx.update_window(workspace_window.into(), |_, window, _cx| { + window.refresh(); + })?; + + for _ in 0..3 { + cx.advance_clock(Duration::from_millis(100)); + cx.run_until_parked(); + } + + // Capture Test 2: Diff with flag disabled + let test2_result = run_visual_test( + "diff_review_button_disabled", + workspace_window.into(), + cx, + update_baseline, + )?; + + // Test 3: Regular editor with flag enabled (should NOT show button) + // Re-enable the feature flag + cx.update(|cx| { + cx.update_flags(true, vec!["diff-review".to_string()]); + }); + + // Create a new window with just a regular editor + let regular_window: WindowHandle = cx + .update(|cx| { + cx.open_window( + WindowOptions { + window_bounds: Some(WindowBounds::Windowed(bounds)), + focus: false, + show: false, + ..Default::default() + }, + |window, cx| { + cx.new(|cx| { + Workspace::new(None, project.clone(), app_state.clone(), window, cx) + }) + }, + ) + }) + .context("Failed to open regular editor window")?; + + cx.run_until_parked(); + + // Open a regular file (not a diff view) + let open_file_task = regular_window + .update(cx, |workspace, window, cx| { + let worktree = workspace.project().read(cx).worktrees(cx).next(); + if let Some(worktree) = worktree { + let worktree_id = worktree.read(cx).id(); + let rel_path: std::sync::Arc = + util::rel_path::rel_path("thread-view.tsx").into(); + let project_path: project::ProjectPath = (worktree_id, rel_path).into(); + Some(workspace.open_path(project_path, None, true, window, cx)) + } else { + None + } + }) + .ok() + .flatten(); + + if let Some(task) = open_file_task { + cx.background_executor.allow_parking(); + let _ = cx.background_executor.block_test(task); + cx.background_executor.forbid_parking(); + } + + // Wait for file to open + for _ in 0..3 { + cx.advance_clock(Duration::from_millis(100)); + cx.run_until_parked(); + } + + // Refresh window + cx.update_window(regular_window.into(), |_, window, _cx| { + window.refresh(); + })?; + + cx.run_until_parked(); + + // Capture Test 3: Regular editor with flag enabled (no button) + let test3_result = run_visual_test( + "diff_review_button_regular_editor", + regular_window.into(), + cx, + update_baseline, + )?; + + // Clean up: remove worktrees to stop background scanning + workspace_window + .update(cx, |workspace, _window, cx| { + let project = workspace.project().clone(); + project.update(cx, |project, cx| { + let worktree_ids: Vec<_> = + project.worktrees(cx).map(|wt| wt.read(cx).id()).collect(); + for id in worktree_ids { + project.remove_worktree(id, cx); + } + }); + }) + .ok(); + + cx.run_until_parked(); + + // Close windows + let _ = cx.update_window(workspace_window.into(), |_, window, _cx| { + window.remove_window(); + }); + let _ = cx.update_window(regular_window.into(), |_, window, _cx| { + window.remove_window(); + }); + + cx.run_until_parked(); + + // Give background tasks time to finish + for _ in 0..15 { + cx.advance_clock(Duration::from_millis(100)); + cx.run_until_parked(); + } + + // Return combined result + match (&test1_result, &test2_result, &test3_result) { + (TestResult::Passed, TestResult::Passed, TestResult::Passed) => Ok(TestResult::Passed), + (TestResult::BaselineUpdated(p), _, _) + | (_, TestResult::BaselineUpdated(p), _) + | (_, _, TestResult::BaselineUpdated(p)) => Ok(TestResult::BaselineUpdated(p.clone())), + } +} + /// A stub AgentServer for visual testing that returns a pre-programmed connection. #[derive(Clone)] #[cfg(target_os = "macos")] diff --git a/plans/visual-test-foreground-executor.md b/plans/visual-test-foreground-executor.md new file mode 100644 index 0000000000000000000000000000000000000000..5ab2f4699a1082e23d8813d0b6cc385bc29e017b --- /dev/null +++ b/plans/visual-test-foreground-executor.md @@ -0,0 +1,506 @@ +# Controllable Foreground Executor for Visual Tests + +## Status: COMPLETED ✓ + +The implementation is complete. See the summary at the end of this document. + +## Problem Statement + +The visual test framework (`VisualTestAppContext`) cannot properly test UI interactions that rely on foreground async tasks, such as tooltips, hover states with delays, and other deferred UI updates. This is because: + +1. **The visual test framework uses the real macOS platform** for actual rendering (needed for screenshot capture) +2. **But it doesn't drive the macOS run loop**, so foreground tasks spawned via `window.spawn()` never execute +3. **`run_until_parked()` only drives the background executor**, not the foreground executor + +### Specific Example: Tooltip Testing + +When trying to capture a screenshot of a tooltip: + +1. We call `window.simulate_mouse_move(position, cx)` to hover over a button +2. The button's tooltip handler in `register_tooltip_mouse_handlers` (in `div.rs`) spawns a delayed task: + ```rust + let delayed_show_task = window.spawn(cx, { + async move |cx| { + cx.background_executor().timer(TOOLTIP_SHOW_DELAY).await; // 500ms + cx.update(|window, cx| { + // Set active_tooltip state + window.refresh(); + }).ok(); + } + }); + ``` +3. We wait with `cx.background_executor().timer(700ms).await` +4. We take a screenshot - **but the tooltip never appears** + +The reason: `window.spawn()` schedules work on the **foreground executor**, but our test only drives the **background executor**. The tooltip task is sitting in the foreground queue, never being processed. + +## Architecture Overview + +### Current Executor Setup + +``` +┌─────────────────────────────────────────────────────────────┐ +│ VisualTestAppContext │ +├─────────────────────────────────────────────────────────────┤ +│ platform: Rc ←── MacPlatform (real) │ +│ background_executor: BackgroundExecutor │ +│ foreground_executor: ForegroundExecutor │ +└─────────────────────────────────────────────────────────────┘ + │ + ▼ +┌─────────────────────────────────────────────────────────────┐ +│ MacPlatform │ +├─────────────────────────────────────────────────────────────┤ +│ dispatcher: Arc │ +│ - Uses real macOS run loop (CFRunLoop) │ +│ - as_test() returns None │ +└─────────────────────────────────────────────────────────────┘ +``` + +### Test Platform (for comparison) + +``` +┌─────────────────────────────────────────────────────────────┐ +│ TestAppContext │ +├─────────────────────────────────────────────────────────────┤ +│ platform: TestPlatform │ +│ dispatcher: TestDispatcher │ +│ - Controllable task queue │ +│ - as_test() returns Some(self) │ +│ - run_until_parked() drives both bg and fg tasks │ +└─────────────────────────────────────────────────────────────┘ +``` + +## Key Files to Investigate + +### 1. Platform Dispatcher Trait +**File:** `crates/gpui/src/platform.rs` + +```rust +pub trait PlatformDispatcher: Send + Sync { + fn is_main_thread(&self) -> bool; + fn dispatch(&self, runnable: Runnable, label: Option); + fn dispatch_on_main_thread(&self, runnable: Runnable); + fn dispatch_after(&self, duration: Duration, runnable: Runnable); + fn park(&self, timeout: Option) -> bool; + fn unparker(&self) -> Unparker; + + #[cfg(any(test, feature = "test-support"))] + fn as_test(&self) -> Option<&TestDispatcher> { + None // Default implementation - real platforms return None + } +} +``` + +### 2. Test Dispatcher Implementation +**File:** `crates/gpui/src/platform/test/dispatcher.rs` + +This is the reference implementation for a controllable dispatcher: + +```rust +pub struct TestDispatcher { + state: Mutex, + // ... +} + +struct TestDispatcherState { + random: StdRng, + foreground: Vec, // ← Foreground task queue + background: Vec, // ← Background task queue + on_parking: Vec, + waiting_hint: Option, + block_on_ticks: RangeInclusive, + forbid_parking: bool, +} + +impl TestDispatcher { + pub fn run_until_parked(&self) { + // Drives BOTH foreground and background tasks + while self.poll(true) {} + } + + fn poll(&self, background_only: bool) -> bool { + // Randomly selects and runs tasks from both queues + } +} +``` + +### 3. Mac Dispatcher (Current Visual Test Dispatcher) +**File:** `crates/gpui/src/platform/mac/dispatcher.rs` + +```rust +pub(crate) struct MacDispatcher; + +impl PlatformDispatcher for MacDispatcher { + fn dispatch_on_main_thread(&self, runnable: Runnable) { + // Dispatches to actual macOS main thread via CFRunLoop + unsafe { + dispatch_async_f( + dispatch_get_main_queue(), + // ... + ); + } + } + + // as_test() returns None (default) +} +``` + +### 4. Visual Test Context Creation +**File:** `crates/gpui/src/app/visual_test_context.rs` + +```rust +impl VisualTestAppContext { + pub fn new() -> Self { + let platform = current_platform(false); // ← Gets MacPlatform + let background_executor = platform.background_executor(); + let foreground_executor = platform.foreground_executor(); + // ... + } + + pub fn run_until_parked(&self) { + self.background_executor.run_until_parked(); // ← Only bg! + } +} +``` + +### 5. BackgroundExecutor::run_until_parked +**File:** `crates/gpui/src/executor.rs` + +```rust +impl BackgroundExecutor { + pub fn run_until_parked(&self) { + self.dispatcher.as_test().unwrap().run_until_parked() + // ^^^^^^^^^^^^^^^^ + // This panics for MacDispatcher because as_test() returns None! + } +} +``` + +Wait - this means `run_until_parked()` should be panicking in visual tests. Let me check if it's actually being called... + +Actually, looking at the test output, it seems like it doesn't panic. This needs investigation - perhaps there's a different code path or the method isn't being called. + +## Proposed Solution + +### Option A: Hybrid Visual Test Platform (Recommended) + +Create a new platform type that: +- Uses **real Mac rendering** (Metal, text system, etc.) for actual pixel output +- Uses **TestDispatcher** for controllable task execution + +```rust +pub struct VisualTestPlatform { + // Real Mac components for rendering + mac_text_system: Arc, + mac_renderer: MacRenderer, // Or access via TestWindow with real backing + + // Test dispatcher for controllable execution + dispatcher: Arc, +} +``` + +**Pros:** +- Full control over task execution timing +- Deterministic test behavior +- Can still capture real rendered output + +**Cons:** +- Significant implementation effort +- Need to carefully separate "rendering" from "event loop" concerns +- May need to refactor how windows get their renderer + +### Option B: Foreground Task Pumping in Visual Tests + +Add a mechanism to manually pump foreground tasks in the visual test context: + +```rust +impl VisualTestAppContext { + pub fn pump_foreground_tasks(&mut self, duration: Duration) { + let deadline = Instant::now() + duration; + while Instant::now() < deadline { + // Manually poll the foreground task queue + if !self.platform.pump_one_foreground_task() { + std::thread::sleep(Duration::from_millis(1)); + } + } + } +} +``` + +This would require adding a new method to `PlatformDispatcher`: + +```rust +pub trait PlatformDispatcher: Send + Sync { + // Existing methods... + + /// Attempt to run one foreground task if available. + /// Returns true if a task was run, false if queue was empty. + fn pump_one_foreground_task(&self) -> bool { false } +} +``` + +**Pros:** +- Smaller change +- Doesn't require new platform type + +**Cons:** +- Less deterministic (still time-based) +- May not work well with macOS's dispatch queue semantics + +### Option C: Window-Spawned Task Interception + +Intercept tasks spawned via `window.spawn()` and redirect them to a controllable queue: + +```rust +impl Window { + pub fn spawn(&mut self, cx: &mut App, f: impl Future) -> Task { + #[cfg(any(test, feature = "test-support"))] + if cx.is_visual_test_mode() { + return self.spawn_to_test_queue(f); + } + + // Normal spawn path + self.foreground_executor.spawn(f) + } +} +``` + +**Pros:** +- Targeted fix for the specific problem +- Minimal changes to existing code + +**Cons:** +- Doesn't solve the general problem +- Test-specific code paths can diverge from production behavior + +## Implementation Plan for Option A + +### Step 1: Create TestDispatcher with Real Timing Option + +Modify `TestDispatcher` to support real-time delays instead of simulated time: + +```rust +pub struct TestDispatcher { + state: Mutex, + use_real_time: bool, // New flag +} + +impl TestDispatcher { + pub fn new_with_real_time() -> Self { + Self { + state: Mutex::new(TestDispatcherState::new()), + use_real_time: true, + } + } +} +``` + +### Step 2: Create VisualTestPlatform + +**New file:** `crates/gpui/src/platform/visual_test/mod.rs` + +```rust +pub struct VisualTestPlatform { + dispatcher: Arc, + text_system: Arc, + // Other Mac components needed for rendering +} + +impl Platform for VisualTestPlatform { + fn dispatcher(&self) -> Arc { + self.dispatcher.clone() + } + + fn text_system(&self) -> Arc { + self.text_system.clone() + } + + fn open_window(...) -> ... { + // Create window with real Metal rendering + // but using our test dispatcher + } +} +``` + +### Step 3: Create VisualTestWindow + +The window needs to use real rendering but the test dispatcher: + +```rust +pub struct VisualTestWindow { + // Real Metal/rendering components from MacWindow + renderer: Renderer, + native_view: ..., + + // But dispatches through TestDispatcher + dispatcher: Arc, +} +``` + +### Step 4: Update VisualTestAppContext + +```rust +impl VisualTestAppContext { + pub fn new() -> Self { + let dispatcher = Arc::new(TestDispatcher::new_with_real_time()); + let platform = Arc::new(VisualTestPlatform::new(dispatcher.clone())); + // ... + } + + pub fn run_until_parked(&self) { + // Now this works because we have a TestDispatcher + self.dispatcher.run_until_parked(); + } +} +``` + +### Step 5: Test the Tooltip Capture + +```rust +// In visual_test_runner.rs +cx.simulate_mouse_move(window, button_position, None, Modifiers::default()); + +// Wait real time for the tooltip delay +cx.background_executor() + .timer(Duration::from_millis(600)) + .await; + +// Drive all pending tasks including the tooltip show task +cx.run_until_parked(); + +// Now the tooltip should be visible +cx.update_window(window, |_, window, _| window.refresh())?; + +// Capture screenshot with tooltip +let screenshot = capture_screenshot(window, cx)?; +``` + +## Testing the Fix + +After implementation, these scenarios should work: + +1. **Tooltip on hover**: Mouse over button → wait → tooltip appears in screenshot +2. **Hover styles**: Mouse over element → hover style visible in screenshot +3. **Delayed animations**: Any animation triggered by foreground tasks +4. **Debounced updates**: UI updates that use debouncing/throttling + +## Files to Modify + +1. `crates/gpui/src/platform.rs` - May need new trait methods +2. `crates/gpui/src/platform/test/dispatcher.rs` - Add real-time mode +3. `crates/gpui/src/platform/visual_test/mod.rs` - New file +4. `crates/gpui/src/platform/visual_test/platform.rs` - New file +5. `crates/gpui/src/platform/visual_test/window.rs` - New file +6. `crates/gpui/src/app/visual_test_context.rs` - Use new platform +7. `crates/gpui/src/platform/mod.rs` - Export new module +8. `crates/zed/src/visual_test_runner.rs` - Update test code + +## Questions to Resolve + +1. **Can Metal rendering work without the macOS run loop?** + - Need to investigate if `CAMetalLayer` and friends require the run loop + +2. **How does `render_to_image()` work?** + - This is used for screenshot capture - need to ensure it works with test platform + +3. **What about system events (keyboard, mouse)?** + - Visual tests simulate these - should work with test dispatcher + +4. **Thread safety concerns?** + - TestDispatcher is designed for single-threaded use + - Metal rendering may have threading requirements + +## Related Code References + +- `Window::spawn` - `crates/gpui/src/window.rs` +- `register_tooltip_mouse_handlers` - `crates/gpui/src/elements/div.rs:2845` +- `handle_tooltip_mouse_move` - `crates/gpui/src/elements/div.rs:2873` +- `TOOLTIP_SHOW_DELAY` - `crates/gpui/src/elements/div.rs:48` (500ms) +- `TestWindow` - `crates/gpui/src/platform/test/window.rs` +- `MacWindow` - `crates/gpui/src/platform/mac/window.rs` + +## Success Criteria + +1. `diff_review_button_tooltip.png` shows the "Add Review" tooltip +2. Button shows hover border when mouse is over it +3. Tests remain deterministic (same output every run) +4. No reliance on wall-clock timing for correctness + +--- + +## Implementation Summary + +### Changes Made + +1. **Created `VisualTestPlatform`** (`crates/gpui/src/platform/visual_test.rs`) + - A hybrid platform that combines real Mac rendering with controllable `TestDispatcher` + - Uses `MacPlatform` for window creation, text system, rendering, and display management + - Uses `TestDispatcher` for deterministic task scheduling + - Implements the `Platform` trait, delegating rendering operations to `MacPlatform` + - Passes its own `ForegroundExecutor` (from `TestDispatcher`) to `MacWindow::open()` + +2. **Added `renderer_context()` method to `MacPlatform`** (`crates/gpui/src/platform/mac/platform.rs`) + - Allows `VisualTestPlatform` to access the renderer context for window creation + - Conditionally compiled for test-support + +3. **Updated `VisualTestAppContext`** (`crates/gpui/src/app/visual_test_context.rs`) + - Now creates and uses `VisualTestPlatform` instead of `current_platform()` + - Gets dispatcher and executors from the platform + - This ensures `App::spawn()` and `Window::spawn()` use the `TestDispatcher` + +4. **Added tests** (`crates/gpui/src/app/visual_test_context.rs`) + - `test_foreground_tasks_run_with_run_until_parked` - verifies foreground tasks execute + - `test_advance_clock_triggers_delayed_tasks` - verifies timer-based tasks work + - `test_window_spawn_uses_test_dispatcher` - verifies window.spawn uses TestDispatcher + - All tests are marked `#[ignore]` because they require macOS main thread + +### How It Works + +The key insight was that `App::new_app()` gets its executors from `platform.foreground_executor()`. Previously: + +``` +VisualTestAppContext + └── creates TestDispatcher (unused!) + └── creates App with MacPlatform + └── MacPlatform has MacDispatcher + └── App uses MacDispatcher's executors ❌ +``` + +After the fix: + +``` +VisualTestAppContext + └── creates VisualTestPlatform + └── Has TestDispatcher + └── Has MacPlatform (for rendering) + └── foreground_executor() returns TestDispatcher's executor ✓ + └── creates App with VisualTestPlatform + └── App uses TestDispatcher's executors ✓ + └── Window::spawn() uses TestDispatcher ✓ +``` + +### Running Visual Tests + +Visual tests require the macOS main thread. Run them with: + +```bash +cargo test -p gpui visual_test_context -- --ignored --test-threads=1 +cargo test -p zed visual_tests -- --ignored --test-threads=1 +``` + +### Tooltip Testing + +With this fix, tooltip testing now works: + +```rust +// Simulate hovering over a button +cx.simulate_mouse_move(window, button_position, None, Modifiers::default()); + +// Advance clock past TOOLTIP_SHOW_DELAY (500ms) +cx.advance_clock(Duration::from_millis(600)); + +// The tooltip task spawned via window.spawn() is now executed! +// Take screenshot - tooltip will be visible +let screenshot = cx.capture_screenshot(window)?; +``` \ No newline at end of file