Visual test infrastructure improvements (#46324)

Richard Feldman created

This PR improves the visual test infrastructure:

- Adds controllable foreground executor for deterministic task
scheduling
- Adds tooltip hover testing capability
- Improves error handling in visual test runner
- Includes planning documentation for the approach

Release Notes:

- N/A

Change summary

crates/acp_thread/src/acp_thread.rs      | 122 ++++++
crates/gpui/src/window.rs                |  13 
crates/zed/src/visual_test_runner.rs     | 304 +++++++++++++++
plans/visual-test-foreground-executor.md | 506 ++++++++++++++++++++++++++
4 files changed, 945 insertions(+)

Detailed changes

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);

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<Pixels>, 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)]

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<AppState>,
+    cx: &mut VisualTestAppContext,
+    update_baseline: bool,
+) -> Result<TestResult> {
+    // 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<Workspace> = 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<Workspace> = 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::RelPath> =
+                    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")]

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<dyn Platform>  ←── MacPlatform (real)         │
+│  background_executor: BackgroundExecutor                     │
+│  foreground_executor: ForegroundExecutor                     │
+└─────────────────────────────────────────────────────────────┘
+                              │
+                              ▼
+┌─────────────────────────────────────────────────────────────┐
+│                       MacPlatform                            │
+├─────────────────────────────────────────────────────────────┤
+│  dispatcher: Arc<MacDispatcher>                              │
+│    - 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<TaskLabel>);
+    fn dispatch_on_main_thread(&self, runnable: Runnable);
+    fn dispatch_after(&self, duration: Duration, runnable: Runnable);
+    fn park(&self, timeout: Option<Duration>) -> 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<TestDispatcherState>,
+    // ...
+}
+
+struct TestDispatcherState {
+    random: StdRng,
+    foreground: Vec<TestTask>,     // ← Foreground task queue
+    background: Vec<TestTask>,     // ← Background task queue
+    on_parking: Vec<UnparkCallback>,
+    waiting_hint: Option<String>,
+    block_on_ticks: RangeInclusive<usize>,
+    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<MacTextSystem>,
+    mac_renderer: MacRenderer,  // Or access via TestWindow with real backing
+    
+    // Test dispatcher for controllable execution
+    dispatcher: Arc<TestDispatcher>,
+}
+```
+
+**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<R>(&mut self, cx: &mut App, f: impl Future<Output = R>) -> Task<R> {
+        #[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<TestDispatcherState>,
+    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<TestDispatcher>,
+    text_system: Arc<MacTextSystem>,
+    // Other Mac components needed for rendering
+}
+
+impl Platform for VisualTestPlatform {
+    fn dispatcher(&self) -> Arc<dyn PlatformDispatcher> {
+        self.dispatcher.clone()
+    }
+    
+    fn text_system(&self) -> Arc<dyn PlatformTextSystem> {
+        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<TestDispatcher>,
+}
+```
+
+### 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)?;
+```