diff --git a/crates/zed/src/visual_test_runner.rs b/crates/zed/src/visual_test_runner.rs index 1651b46d33..0c65c59c36 100644 --- a/crates/zed/src/visual_test_runner.rs +++ b/crates/zed/src/visual_test_runner.rs @@ -26,8 +26,8 @@ use anyhow::{Context, Result}; use gpui::{ - App, AppContext as _, Application, Bounds, Window, WindowBounds, WindowHandle, WindowOptions, - point, px, size, + App, AppContext as _, Application, Bounds, Pixels, Size, Window, WindowBounds, WindowHandle, + WindowOptions, point, px, }; use image::RgbaImage; use project_panel::ProjectPanel; @@ -53,6 +53,144 @@ const EMBEDDED_TEST_IMAGE: &[u8] = include_bytes!("../resources/app-icon.png"); /// Images must match at least this percentage to pass const MATCH_THRESHOLD: f64 = 0.99; +/// Window size for workspace tests (project panel, editor) +fn workspace_window_size() -> Size { + Size { + width: px(1280.0), + height: px(800.0), + } +} + +/// Window size for agent panel tests +fn agent_panel_window_size() -> Size { + Size { + width: px(500.0), + height: px(900.0), + } +} + +/// Helper struct for setting up test workspaces +struct TestWorkspace { + window: WindowHandle, +} + +impl TestWorkspace { + async fn new( + app_state: Arc, + window_size: Size, + project_path: &Path, + cx: &mut gpui::AsyncApp, + ) -> Result { + 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, + ) + })?; + + let add_worktree_task = project.update(cx, |project, cx| { + project.find_or_create_worktree(project_path, true, cx) + })?; + add_worktree_task.await?; + + let bounds = Bounds { + origin: point(px(0.0), px(0.0)), + size: window_size, + }; + + let 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) + }) + }, + ) + })??; + + cx.background_executor() + .timer(std::time::Duration::from_millis(100)) + .await; + + Ok(Self { window }) + } +} + +async fn setup_project_panel( + workspace: &TestWorkspace, + cx: &mut gpui::AsyncApp, +) -> Result> { + let panel_task = workspace.window.update(cx, |_workspace, window, cx| { + let weak_workspace = cx.weak_entity(); + let async_window_cx = window.to_async(cx); + window.spawn(cx, async move |_cx| { + ProjectPanel::load(weak_workspace, async_window_cx).await + }) + })?; + + let panel = panel_task.await?; + + workspace.window.update(cx, |ws, window, cx| { + ws.add_panel(panel.clone(), window, cx); + ws.open_panel::(window, cx); + })?; + + cx.background_executor() + .timer(std::time::Duration::from_millis(100)) + .await; + + Ok(panel) +} + +async fn open_file( + workspace: &TestWorkspace, + relative_path: &str, + cx: &mut gpui::AsyncApp, +) -> Result<()> { + let open_file_task = workspace.window.update(cx, |ws, window, cx| { + let worktree = ws.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(relative_path).into(); + let project_path: project::ProjectPath = (worktree_id, rel_path).into(); + Some(ws.open_path(project_path, None, true, window, cx)) + } else { + None + } + })?; + + if let Some(task) = open_file_task { + let item = task.await?; + workspace.window.update(cx, |ws, window, cx| { + let pane = ws.active_pane().clone(); + pane.update(cx, |pane, cx| { + if let Some(index) = pane.index_for_item(item.as_ref()) { + pane.activate_item(index, true, true, window, cx); + } + }); + })?; + } + + cx.background_executor() + .timer(std::time::Duration::from_millis(100)) + .await; + + Ok(()) +} + fn main() { env_logger::builder() .filter_level(log::LevelFilter::Info) @@ -60,13 +198,6 @@ fn main() { let update_baseline = std::env::var("UPDATE_BASELINE").is_ok(); - if update_baseline { - println!("=== Visual Test Runner (UPDATE MODE) ===\n"); - println!("Baseline images will be updated.\n"); - } else { - println!("=== Visual Test Runner ===\n"); - } - // Create a temporary directory for test files let temp_dir = tempfile::tempdir().expect("Failed to create temp directory"); let project_path = temp_dir.path().join("project"); @@ -107,200 +238,72 @@ fn main() { language_model::init(app_state.client.clone(), cx); language_models::init(app_state.user_store.clone(), app_state.client.clone(), cx); - // Open a real Zed workspace window - let window_size = size(px(1280.0), px(800.0)); - // Window can be hidden since we use direct texture capture (reading pixels from - // Metal texture) instead of ScreenCaptureKit which requires visible windows. - let bounds = Bounds { - origin: point(px(0.0), px(0.0)), - size: window_size, - }; - - // Create a project for the workspace - let project = 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, - ); - - let workspace_window: WindowHandle = 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) - }) - }, - ) - .expect("Failed to open workspace window"); - - // Add the test project as a worktree directly to the project - let add_worktree_task = workspace_window - .update(cx, |workspace, _window, cx| { - workspace.project().update(cx, |project, cx| { - project.find_or_create_worktree(&project_path, true, cx) - }) - }) - .expect("Failed to update workspace"); - // Clone app_state for the async block let app_state_for_tests = app_state.clone(); // Spawn async task to set up the UI and capture screenshot cx.spawn(async move |mut cx| { - // Wait for the worktree to be added - if let Err(e) = add_worktree_task.await { - eprintln!("Failed to add worktree: {:?}", e); - } + let project_path_clone = project_path.clone(); - // Wait for UI to settle - cx.background_executor() - .timer(std::time::Duration::from_millis(500)) - .await; - - // Create and add the project panel to the workspace - let panel_task = cx.update(|cx| { - workspace_window - .update(cx, |_workspace, window, cx| { - let weak_workspace = cx.weak_entity(); - window.spawn(cx, async move |cx| { - ProjectPanel::load(weak_workspace, cx.clone()).await - }) - }) - .ok() - }); - - if let Ok(Some(task)) = panel_task { - if let Ok(panel) = task.await { - cx.update(|cx| { - workspace_window - .update(cx, |workspace, window, cx| { - workspace.add_panel(panel, window, cx); - }) - .ok(); - }) - .ok(); + // Create the test workspace + let workspace = match TestWorkspace::new( + app_state_for_tests.clone(), + workspace_window_size(), + &project_path_clone, + &mut cx, + ) + .await + { + Ok(ws) => ws, + Err(e) => { + log::error!("Failed to create workspace: {}", e); + cx.update(|cx| cx.quit()).ok(); + std::process::exit(1); } + }; + + // Set up project panel + if let Err(e) = setup_project_panel(&workspace, &mut cx).await { + log::error!("Failed to setup project panel: {}", e); + cx.update(|cx| cx.quit()).ok(); + std::process::exit(1); } - // Wait for panel to be added - cx.background_executor() - .timer(std::time::Duration::from_millis(500)) - .await; - - // Open the project panel - cx.update(|cx| { - workspace_window - .update(cx, |workspace, window, cx| { - workspace.open_panel::(window, cx); - }) - .ok(); - }) - .ok(); - - // Wait for project panel to render - cx.background_executor() - .timer(std::time::Duration::from_millis(500)) - .await; - // Open main.rs in the editor - let open_file_task = cx.update(|cx| { - workspace_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("src/main.rs").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 Ok(Some(task)) = open_file_task { - if let Ok(item) = task.await { - // Focus the opened item to dismiss the welcome screen - cx.update(|cx| { - workspace_window - .update(cx, |workspace, window, cx| { - let pane = workspace.active_pane().clone(); - pane.update(cx, |pane, cx| { - if let Some(index) = pane.index_for_item(item.as_ref()) - { - pane.activate_item(index, true, true, window, cx); - } - }); - }) - .ok(); - }) - .ok(); - - // Wait for item activation to render - cx.background_executor() - .timer(std::time::Duration::from_millis(500)) - .await; - } + if let Err(e) = open_file(&workspace, "src/main.rs", &mut cx).await { + log::error!("Failed to open file: {}", e); + cx.update(|cx| cx.quit()).ok(); + std::process::exit(1); } // Request a window refresh to ensure all pending effects are processed cx.refresh().ok(); - - // Wait for UI to fully stabilize cx.background_executor() - .timer(std::time::Duration::from_secs(2)) + .timer(std::time::Duration::from_millis(500)) .await; - // Track test results - let mut passed = 0; - let mut failed = 0; - let mut updated = 0; + // Track if any test failed + let mut any_failed = false; // Run Test 1: Project Panel (with project panel visible) - println!("\n--- Test 1: project_panel ---"); - let test_result = run_visual_test( + if run_visual_test( "project_panel", - workspace_window.into(), + workspace.window.into(), &mut cx, update_baseline, ) - .await; - - match test_result { - Ok(TestResult::Passed) => { - println!("✓ project_panel: PASSED"); - passed += 1; - } - Ok(TestResult::BaselineUpdated(path)) => { - println!("✓ project_panel: Baseline updated at {}", path.display()); - updated += 1; - } - Err(e) => { - eprintln!("✗ project_panel: FAILED - {}", e); - failed += 1; - } + .await + .is_err() + { + any_failed = true; } // Close the project panel for the second test cx.update(|cx| { - workspace_window - .update(cx, |workspace, window, cx| { - workspace.close_panel::(window, cx); + workspace + .window + .update(cx, |ws, window, cx| { + ws.close_panel::(window, cx); }) .ok(); }) @@ -309,77 +312,37 @@ fn main() { // Refresh and wait for panel to close cx.refresh().ok(); cx.background_executor() - .timer(std::time::Duration::from_millis(500)) + .timer(std::time::Duration::from_millis(100)) .await; // Run Test 2: Workspace with Editor (without project panel) - println!("\n--- Test 2: workspace_with_editor ---"); - let test_result = run_visual_test( + if run_visual_test( "workspace_with_editor", - workspace_window.into(), + workspace.window.into(), &mut cx, update_baseline, ) - .await; - - match test_result { - Ok(TestResult::Passed) => { - println!("✓ workspace_with_editor: PASSED"); - passed += 1; - } - Ok(TestResult::BaselineUpdated(path)) => { - println!( - "✓ workspace_with_editor: Baseline updated at {}", - path.display() - ); - updated += 1; - } - Err(e) => { - eprintln!("✗ workspace_with_editor: FAILED - {}", e); - failed += 1; - } + .await + .is_err() + { + any_failed = true; } // Run Test 3: Agent Thread View with Image (collapsed and expanded) - println!("\n--- Test 3: agent_thread_with_image (collapsed + expanded) ---"); - let test_result = run_agent_thread_view_test( + if run_agent_thread_view_test( app_state_for_tests.clone(), &mut cx, update_baseline, ) - .await; - - match test_result { - Ok(TestResult::Passed) => { - println!("✓ agent_thread_with_image (collapsed + expanded): PASSED"); - passed += 1; - } - Ok(TestResult::BaselineUpdated(_)) => { - println!( - "✓ agent_thread_with_image: Baselines updated (collapsed + expanded)" - ); - updated += 1; - } - Err(e) => { - eprintln!("✗ agent_thread_with_image: FAILED - {}", e); - failed += 1; - } + .await + .is_err() + { + any_failed = true; } - // Print summary - println!("\n=== Test Summary ==="); - println!("Passed: {}", passed); - println!("Failed: {}", failed); - if updated > 0 { - println!("Baselines Updated: {}", updated); - } - - if failed > 0 { - eprintln!("\n=== Visual Tests FAILED ==="); + if any_failed { cx.update(|cx| cx.quit()).ok(); std::process::exit(1); - } else { - println!("\n=== All Visual Tests PASSED ==="); } cx.update(|cx| cx.quit()).ok(); @@ -423,7 +386,6 @@ async fn run_visual_test( // Save the actual screenshot screenshot.save(&actual_path)?; - println!("Screenshot saved to: {}", actual_path.display()); if update_baseline { // Update the baseline @@ -449,13 +411,6 @@ async fn run_visual_test( let comparison = compare_images(&baseline, &screenshot); - println!( - "Image comparison: {:.2}% match ({} different pixels out of {})", - comparison.match_percentage * 100.0, - comparison.diff_pixel_count, - comparison.total_pixels - ); - if comparison.match_percentage >= MATCH_THRESHOLD { Ok(TestResult::Passed) } else { @@ -463,7 +418,6 @@ async fn run_visual_test( if let Some(diff_image) = comparison.diff_image { let diff_path = Path::new(&output_dir).join(format!("{}_diff.png", test_name)); diff_image.save(&diff_path)?; - println!("Diff image saved to: {}", diff_path.display()); } Err(anyhow::anyhow!( @@ -495,8 +449,6 @@ fn get_baseline_path(test_name: &str) -> PathBuf { struct ImageComparison { match_percentage: f64, diff_image: Option, - diff_pixel_count: u64, - total_pixels: u64, } fn compare_images(baseline: &RgbaImage, actual: &RgbaImage) -> ImageComparison { @@ -505,8 +457,6 @@ fn compare_images(baseline: &RgbaImage, actual: &RgbaImage) -> ImageComparison { return ImageComparison { match_percentage: 0.0, diff_image: None, - diff_pixel_count: baseline.width() as u64 * baseline.height() as u64, - total_pixels: baseline.width() as u64 * baseline.height() as u64, }; } @@ -520,7 +470,7 @@ fn compare_images(baseline: &RgbaImage, actual: &RgbaImage) -> ImageComparison { let baseline_pixel = baseline.get_pixel(x, y); let actual_pixel = actual.get_pixel(x, y); - if pixels_are_similar(baseline_pixel, actual_pixel) { + if pixels_match(baseline_pixel, actual_pixel) { // Matching pixel - show as dimmed version of actual diff_image.put_pixel( x, @@ -549,19 +499,11 @@ fn compare_images(baseline: &RgbaImage, actual: &RgbaImage) -> ImageComparison { ImageComparison { match_percentage, diff_image: Some(diff_image), - diff_pixel_count: diff_count, - total_pixels, } } -fn pixels_are_similar(a: &image::Rgba, b: &image::Rgba) -> bool { - // Allow small differences due to anti-aliasing, font rendering, etc. - const TOLERANCE: i16 = 2; - - (a[0] as i16 - b[0] as i16).abs() <= TOLERANCE - && (a[1] as i16 - b[1] as i16).abs() <= TOLERANCE - && (a[2] as i16 - b[2] as i16).abs() <= TOLERANCE - && (a[3] as i16 - b[3] as i16).abs() <= TOLERANCE +fn pixels_match(a: &image::Rgba, b: &image::Rgba) -> bool { + a == b } fn capture_screenshot(window: gpui::AnyWindowHandle, cx: &mut gpui::App) -> Result { @@ -571,12 +513,6 @@ fn capture_screenshot(window: gpui::AnyWindowHandle, cx: &mut gpui::App) -> Resu window.render_to_image() })??; - println!( - "Screenshot captured: {}x{} pixels", - screenshot.width(), - screenshot.height() - ); - Ok(screenshot) } @@ -585,9 +521,20 @@ fn create_test_files(project_path: &Path) { let src_dir = project_path.join("src"); std::fs::create_dir_all(&src_dir).expect("Failed to create src directory"); - std::fs::write( - src_dir.join("main.rs"), - r#"fn main() { + std::fs::write(src_dir.join("main.rs"), MAIN_RS_CONTENT).expect("Failed to write main.rs"); + + std::fs::write(src_dir.join("lib.rs"), LIB_RS_CONTENT).expect("Failed to write lib.rs"); + + std::fs::write(src_dir.join("utils.rs"), UTILS_RS_CONTENT).expect("Failed to write utils.rs"); + + std::fs::write(project_path.join("Cargo.toml"), CARGO_TOML_CONTENT) + .expect("Failed to write Cargo.toml"); + + std::fs::write(project_path.join("README.md"), README_MD_CONTENT) + .expect("Failed to write README.md"); +} + +const MAIN_RS_CONTENT: &str = r#"fn main() { println!("Hello, world!"); let message = greet("Zed"); @@ -607,13 +554,9 @@ mod tests { assert_eq!(greet("World"), "Welcome to World, the editor of the future!"); } } -"#, - ) - .expect("Failed to write main.rs"); +"#; - std::fs::write( - src_dir.join("lib.rs"), - r#"//! A sample library for visual testing. +const LIB_RS_CONTENT: &str = r#"//! A sample library for visual testing. pub mod utils; @@ -641,13 +584,9 @@ mod tests { assert_eq!(subtract(5, 3), 2); } } -"#, - ) - .expect("Failed to write lib.rs"); +"#; - std::fs::write( - src_dir.join("utils.rs"), - r#"//! Utility functions for the sample project. +const UTILS_RS_CONTENT: &str = r#"//! Utility functions for the sample project. /// Formats a greeting message. pub fn format_greeting(name: &str) -> String { @@ -658,13 +597,9 @@ pub fn format_greeting(name: &str) -> String { pub fn format_farewell(name: &str) -> String { format!("Goodbye, {}!", name) } -"#, - ) - .expect("Failed to write utils.rs"); +"#; - std::fs::write( - project_path.join("Cargo.toml"), - r#"[package] +const CARGO_TOML_CONTENT: &str = r#"[package] name = "test-project" version = "0.1.0" edition = "2021" @@ -672,13 +607,9 @@ edition = "2021" [dependencies] [dev-dependencies] -"#, - ) - .expect("Failed to write Cargo.toml"); +"#; - std::fs::write( - project_path.join("README.md"), - r#"# Test Project +const README_MD_CONTENT: &str = r#"# Test Project This is a test project for visual testing of Zed. @@ -704,10 +635,7 @@ cargo build ```bash cargo test ``` -"#, - ) - .expect("Failed to write README.md"); -} +"#; /// Initialize AppState with real filesystem for visual testing. fn init_app_state(cx: &mut gpui::App) -> Arc { @@ -814,7 +742,6 @@ async fn run_agent_thread_view_test( // Wait for worktree to scan and find the image file let worktree_name = worktree.read_with(cx, |wt, _| wt.root_name_str().to_string())?; - // Wait for worktree to be fully scanned cx.background_executor() .timer(std::time::Duration::from_millis(100)) .await; @@ -900,14 +827,12 @@ async fn run_agent_thread_view_test( let stub_agent: Rc = Rc::new(StubAgentServer::new(connection.clone())); - // Create a window sized for the agent panel (500x900) - let window_size = size(px(500.0), px(900.0)); + // Create a workspace window let bounds = Bounds { origin: point(px(0.0), px(0.0)), - size: window_size, + size: agent_panel_window_size(), }; - // Create a workspace window let workspace_window: WindowHandle = cx.update(|cx| { cx.open_window( WindowOptions { @@ -922,7 +847,6 @@ async fn run_agent_thread_view_test( ) })??; - // Wait for workspace to initialize cx.background_executor() .timer(std::time::Duration::from_millis(100)) .await; @@ -943,9 +867,8 @@ async fn run_agent_thread_view_test( workspace.open_panel::(window, cx); })?; - // Wait for panel to be ready cx.background_executor() - .timer(std::time::Duration::from_millis(200)) + .timer(std::time::Duration::from_millis(100)) .await; // Inject the stub server and open the stub thread @@ -955,9 +878,8 @@ async fn run_agent_thread_view_test( }); })?; - // Wait for thread view to initialize cx.background_executor() - .timer(std::time::Duration::from_millis(200)) + .timer(std::time::Duration::from_millis(100)) .await; // Get the thread view and send a message @@ -974,9 +896,8 @@ async fn run_agent_thread_view_test( .update(cx, |thread, cx| thread.send_raw("Show me the Zed logo", cx))? .await?; - // Wait for response to be processed cx.background_executor() - .timer(std::time::Duration::from_millis(500)) + .timer(std::time::Duration::from_millis(200)) .await; // Get the tool call ID for expanding later @@ -1001,7 +922,7 @@ async fn run_agent_thread_view_test( )?; cx.background_executor() - .timer(std::time::Duration::from_millis(300)) + .timer(std::time::Duration::from_millis(100)) .await; // First, capture the COLLAPSED state (image tool call not expanded) @@ -1018,9 +939,8 @@ async fn run_agent_thread_view_test( view.expand_tool_call(tool_call_id, cx); })?; - // Wait for UI to update cx.background_executor() - .timer(std::time::Duration::from_millis(300)) + .timer(std::time::Duration::from_millis(100)) .await; // Refresh window for expanded state @@ -1032,7 +952,7 @@ async fn run_agent_thread_view_test( )?; cx.background_executor() - .timer(std::time::Duration::from_millis(300)) + .timer(std::time::Duration::from_millis(100)) .await; // Capture the EXPANDED state (image visible) diff --git a/plans/visual-test-improvements.md b/plans/visual-test-improvements.md new file mode 100644 index 0000000000..40f88392d9 --- /dev/null +++ b/plans/visual-test-improvements.md @@ -0,0 +1,661 @@ +# Visual Test Runner Improvements + +This document describes improvements to make the visual test runner in `crates/zed/src/visual_test_runner.rs` more deterministic, faster, and less hacky. + +## Background + +The visual test runner captures screenshots of Zed's UI and compares them against baseline images. It currently works but has several issues: + +1. **Non-deterministic timing**: Uses `timer().await` calls scattered throughout +2. **Real filesystem I/O**: Uses `tempfile` and `RealFs` instead of `FakeFs` +3. **Dead code**: Unused variables and counters from removed print statements +4. **Code duplication**: Similar setup code repeated across tests +5. **Limited production code coverage**: Some areas use stubs where real code could run + +## How to Run the Visual Tests + +```bash +# Run the visual tests (compare against baselines) +cargo run -p zed --bin zed_visual_test_runner --features visual-tests + +# Update baseline images (when UI intentionally changes) +UPDATE_BASELINE=1 cargo run -p zed --bin zed_visual_test_runner --features visual-tests +``` + +The test runner is a separate binary, not a `#[test]` function. It uses `Application::new().run()` to create a real GPUI application context. + +--- + +## Improvement 1: Replace Timer-Based Waits with `run_until_parked()` + +### Problem + +The code is littered with timing-based waits like: + +```rust +cx.background_executor() + .timer(std::time::Duration::from_millis(500)) + .await; +``` + +These appear ~15 times in the file. They are: +- **Slow**: Adds up to several seconds of unnecessary waiting +- **Non-deterministic**: Could flake on slow CI machines +- **Arbitrary**: The durations (100ms, 200ms, 300ms, 500ms, 2s) were chosen by trial and error + +### Solution + +Use `run_until_parked()` which runs all pending async tasks until there's nothing left to do. This is: +- **Instant**: Returns immediately when work is complete +- **Deterministic**: Waits exactly as long as needed +- **Standard**: Used throughout Zed's test suite + +### How to Implement + +The challenge is that the visual test runner uses `AsyncApp` (from `cx.spawn()`), not `TestAppContext`. The `run_until_parked()` method is on `BackgroundExecutor` which is accessible via `cx.background_executor()`. + +However, `run_until_parked()` is a **blocking** call that runs the executor synchronously, while the visual tests are currently written as async code. You'll need to restructure the code. + +**Option A: Keep async structure, call run_until_parked between awaits** + +```rust +// Before: +cx.background_executor() + .timer(std::time::Duration::from_millis(500)) + .await; + +// After - run all pending work synchronously: +cx.background_executor().run_until_parked(); +``` + +But this won't work directly in async context because `run_until_parked()` blocks. + +**Option B: Restructure to use synchronous test pattern** + +The standard Zed test pattern uses `#[gpui::test]` with `TestAppContext`: + +```rust +#[gpui::test] +async fn test_something(cx: &mut TestAppContext) { + cx.run_until_parked(); // This works! +} +``` + +For the visual test runner, you'd need to convert from `Application::new().run()` to the test harness. This is a bigger change but would be more idiomatic. + +**Option C: Use cx.refresh() + small delay for rendering** + +For purely rendering-related waits, `cx.refresh()` forces a repaint. A single small delay after refresh may be acceptable for GPU readback timing: + +```rust +cx.refresh().ok(); +// Minimal delay just for GPU work, not async task completion +cx.background_executor() + .timer(std::time::Duration::from_millis(16)) // ~1 frame + .await; +``` + +### Locations to Change + +Search for `timer(std::time::Duration` in the file. Each occurrence should be evaluated: + +| Line | Current Duration | Purpose | Replacement | +|------|-----------------|---------|-------------| +| 160-162 | 500ms | Wait for worktree | `run_until_parked()` or await the task | +| 190-192 | 500ms | Wait for panel add | `run_until_parked()` | +| 205-207 | 500ms | Wait for panel render | `cx.refresh()` | +| 248-250 | 500ms | Wait for item activation | `run_until_parked()` | +| 258-260 | 2000ms | Wait for UI to stabilize | `cx.refresh()` + minimal delay | +| 294-296 | 500ms | Wait for panel close | `run_until_parked()` | +| 752-754 | 100ms | Wait for worktree scan | `run_until_parked()` | +| 860-862 | 100ms | Wait for workspace init | `run_until_parked()` | +| 881-883 | 200ms | Wait for panel ready | `run_until_parked()` | +| 893-895 | 200ms | Wait for thread view | `run_until_parked()` | +| 912-914 | 500ms | Wait for response | `run_until_parked()` | +| 937-939 | 300ms | Wait for refresh | `cx.refresh()` | +| 956-958 | 300ms | Wait for UI update | `run_until_parked()` | +| 968-970 | 300ms | Wait for refresh | `cx.refresh()` | + +### How to Verify + +After making changes: +1. Run the tests: `cargo run -p zed --bin zed_visual_test_runner --features visual-tests` +2. They should pass with the same baseline images +3. They should run faster (measure before/after) + +--- + +## Improvement 2: Use `FakeFs` Instead of Real Filesystem + +### Problem + +The code currently uses: +```rust +let fs = Arc::new(RealFs::new(None, cx.background_executor().clone())); +let temp_dir = tempfile::tempdir().expect("Failed to create temp directory"); +``` + +This is: +- **Slow**: Real I/O is slower than in-memory operations +- **Non-deterministic**: Filesystem timing varies +- **Messy**: Leaves temp directories on failure + +### Solution + +Use `FakeFs` which is an in-memory filesystem used throughout Zed's tests: + +```rust +use fs::FakeFs; + +let fs = FakeFs::new(cx.background_executor().clone()); +fs.insert_tree( + "/project", + json!({ + "src": { + "main.rs": "fn main() { println!(\"Hello\"); }" + }, + "Cargo.toml": "[package]\nname = \"test\"" + }), +).await; +``` + +### How to Implement + +1. **Add the dependency** in `crates/zed/Cargo.toml` if not present: + ```toml + fs = { workspace = true, features = ["test-support"] } + ``` + +2. **Replace RealFs creation** in `init_app_state()` (around line 655): + + ```rust + // Before: + let fs = Arc::new(RealFs::new(None, cx.background_executor().clone())); + + // After: + let fs = FakeFs::new(cx.background_executor().clone()); + ``` + +3. **Replace tempdir + file creation** (around lines 66-71 and 518-643): + + ```rust + // Before: + let temp_dir = tempfile::tempdir().expect("..."); + let project_path = temp_dir.path().join("project"); + std::fs::create_dir_all(&project_path).expect("..."); + create_test_files(&project_path); + + // After: + let fs = FakeFs::new(cx.background_executor().clone()); + fs.insert_tree("/project", json!({ + "src": { + "main.rs": include_str!("test_content/main.rs"), + "lib.rs": include_str!("test_content/lib.rs"), + }, + "Cargo.toml": include_str!("test_content/Cargo.toml"), + "README.md": include_str!("test_content/README.md"), + })).await; + let project_path = Path::new("/project"); + ``` + +4. **For the test image** (around line 726): + + ```rust + // Before: + let temp_dir = tempfile::tempdir()?; + let project_path = temp_dir.path().join("project"); + std::fs::create_dir_all(&project_path)?; + let image_path = project_path.join("test-image.png"); + std::fs::write(&image_path, EMBEDDED_TEST_IMAGE)?; + + // After: + fs.insert_file("/project/test-image.png", EMBEDDED_TEST_IMAGE.to_vec()).await; + let project_path = Path::new("/project"); + ``` + +5. **Update `init_app_state`** to accept `fs` as a parameter instead of creating it internally. + +### Reference Example + +See `crates/project_panel/src/project_panel_tests.rs` lines 17-62 for a complete example of using `FakeFs` with `insert_tree()`. + +### How to Verify + +1. Run the tests - they should produce identical screenshots +2. Verify no temp directories are created in `/tmp` or equivalent +3. Check that tests run faster + +--- + +## Improvement 3: Remove Dead Code + +### Problem + +After removing print statements, there's leftover dead code: + +```rust +let _ = update_baseline; // Line 63 - was used in removed print + +let mut passed = 0; // Lines 263-265 +let mut failed = 0; +let mut updated = 0; +// ... counters incremented but never read + +let _ = (passed, updated); // Line 327 - silences warning +``` + +### Solution + +Remove the unused code and restructure to not need counters. + +### How to Implement + +1. **Remove the `let _ = update_baseline;`** on line 63 - `update_baseline` is already used later + +2. **Simplify test result handling**: + + ```rust + // Before: + let mut passed = 0; + let mut failed = 0; + let mut updated = 0; + + match test_result { + Ok(TestResult::Passed) => passed += 1, + Ok(TestResult::BaselineUpdated(_)) => updated += 1, + Err(_) => failed += 1, + } + // ... repeat for each test ... + + let _ = (passed, updated); + + if failed > 0 { + std::process::exit(1); + } + + // After: + let mut any_failed = false; + + if run_visual_test("project_panel", ...).await.is_err() { + any_failed = true; + } + + if run_visual_test("workspace_with_editor", ...).await.is_err() { + any_failed = true; + } + + if run_agent_thread_view_test(...).await.is_err() { + any_failed = true; + } + + if any_failed { + std::process::exit(1); + } + ``` + +3. **Or collect results into a Vec**: + + ```rust + let results = vec![ + run_visual_test("project_panel", ...).await, + run_visual_test("workspace_with_editor", ...).await, + run_agent_thread_view_test(...).await, + ]; + + if results.iter().any(|r| r.is_err()) { + std::process::exit(1); + } + ``` + +### How to Verify + +1. Run `cargo clippy -p zed --features visual-tests` - should have no warnings about unused variables +2. Run the tests - behavior should be unchanged + +--- + +## Improvement 4: Consolidate Test Setup Code + +### Problem + +There's significant duplication between: +- Main workspace test setup (lines 106-260) +- Agent panel test setup (lines 713-900) + +Both create: +- Projects with `Project::local()` +- Worktrees with `find_or_create_worktree()` +- Windows with `cx.open_window()` +- Wait for things to settle + +### Solution + +Extract common setup into helper functions. + +### How to Implement + +1. **Create a `TestWorkspace` struct**: + + ```rust + struct TestWorkspace { + window: WindowHandle, + project: Entity, + } + + impl TestWorkspace { + async fn new( + app_state: Arc, + size: Size, + project_path: &Path, + cx: &mut AsyncApp, + ) -> Result { + let project = cx.update(|cx| { + 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 worktree + let add_task = project.update(cx, |project, cx| { + project.find_or_create_worktree(project_path, true, cx) + })?; + add_task.await?; + + // Create window + let bounds = Bounds { + origin: point(px(0.0), px(0.0)), + size, + }; + + let window = 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) + }) + }, + ) + })??; + + cx.background_executor().run_until_parked(); + + Ok(Self { window, project }) + } + } + ``` + +2. **Create a `setup_project_panel` helper**: + + ```rust + async fn setup_project_panel( + workspace: &TestWorkspace, + cx: &mut AsyncApp, + ) -> Result> { + let panel_task = workspace.window.update(cx, |_ws, window, cx| { + let weak = cx.weak_entity(); + window.spawn(cx, async move |cx| ProjectPanel::load(weak, cx).await) + })?; + + let panel = panel_task.await?; + + workspace.window.update(cx, |ws, window, cx| { + ws.add_panel(panel.clone(), window, cx); + ws.open_panel::(window, cx); + })?; + + cx.background_executor().run_until_parked(); + + Ok(panel) + } + ``` + +3. **Use helpers in tests**: + + ```rust + // Test 1 + let workspace = TestWorkspace::new( + app_state.clone(), + size(px(1280.0), px(800.0)), + &project_path, + &mut cx, + ).await?; + + setup_project_panel(&workspace, &mut cx).await?; + open_file(&workspace, "src/main.rs", &mut cx).await?; + + run_visual_test("project_panel", workspace.window.into(), &mut cx, update_baseline).await?; + ``` + +### How to Verify + +1. Tests should produce identical screenshots +2. Code should be shorter and more readable +3. Adding new tests should be easier + +--- + +## Improvement 5: Exercise More Production Code + +### Problem + +Some tests use minimal stubs where real production code could run deterministically. + +### Current State (Good) + +The agent thread test already runs the **real** `ReadFileTool`: +```rust +let tool = Arc::new(agent::ReadFileTool::new(...)); +tool.clone().run(input, event_stream, cx).await?; +``` + +This is great! It exercises real tool execution. + +### Opportunities for More Coverage + +1. **Syntax highlighting**: Register real language grammars so the editor shows colored code + + ```rust + // Currently just: + let languages = Arc::new(LanguageRegistry::new(cx.background_executor().clone())); + + // Could add: + languages.register_native_grammars([ + tree_sitter_rust::LANGUAGE.into(), + tree_sitter_markdown::LANGUAGE.into(), + ]); + ``` + +2. **File icons**: The project panel could show real file icons by registering file types + +3. **Theme loading**: Currently uses `LoadThemes::JustBase`. Could load a full theme: + ```rust + theme::init(theme::LoadThemes::All, cx); + ``` + (But this might make tests slower - evaluate trade-off) + +4. **More tool types**: Test other tools like `ListFilesTool`, `GrepTool` that don't need network + +### How to Implement Syntax Highlighting + +1. Add language dependencies to `Cargo.toml`: + ```toml + tree-sitter-rust = { workspace = true } + tree-sitter-markdown = { workspace = true } + ``` + +2. In `init_app_state` or test setup: + ```rust + let languages = Arc::new(LanguageRegistry::new(cx.background_executor().clone())); + + // Register Rust grammar + languages.register_native_grammars([ + ("rust", tree_sitter_rust::LANGUAGE.into()), + ]); + + // Register the Rust language config + languages.register_available_language( + LanguageConfig { + name: "Rust".into(), + grammar: Some("rust".into()), + matcher: LanguageMatcher { + path_suffixes: vec!["rs".into()], + ..Default::default() + }, + ..Default::default() + }, + tree_sitter_rust::LANGUAGE.into(), + vec![], // No LSP adapters needed for visual tests + ); + ``` + +### How to Verify + +1. Update baselines after adding syntax highlighting +2. Screenshots should show colored code instead of plain text +3. Tests should still be deterministic (same colors every time) + +--- + +## Improvement 6: Better Test Organization + +### Problem + +Tests are numbered in comments but the structure is ad-hoc: +```rust +// Run Test 1: Project Panel +// Run Test 2: Workspace with Editor +// Run Test 3: Agent Thread View +``` + +### Solution + +Create a test registry or use a more structured approach. + +### How to Implement + +1. **Define tests as structs**: + + ```rust + trait VisualTest { + fn name(&self) -> &'static str; + async fn setup(&self, cx: &mut AsyncApp) -> Result; + async fn run(&self, window: AnyWindowHandle, cx: &mut AsyncApp, update_baseline: bool) -> Result; + } + + struct ProjectPanelTest; + struct WorkspaceEditorTest; + struct AgentThreadTest; + + impl VisualTest for ProjectPanelTest { + fn name(&self) -> &'static str { "project_panel" } + // ... + } + ``` + +2. **Run all tests from a registry**: + + ```rust + let tests: Vec> = vec![ + Box::new(ProjectPanelTest), + Box::new(WorkspaceEditorTest), + Box::new(AgentThreadTest), + ]; + + let mut failed = false; + for test in tests { + match test.run(...).await { + Ok(_) => {}, + Err(_) => failed = true, + } + } + ``` + +This makes it easy to: +- Add new tests (just add to the vec) +- Run specific tests (filter by name) +- See all tests in one place + +--- + +## Improvement 7: Constants and Configuration + +### Problem + +Magic numbers scattered throughout: +```rust +size(px(1280.0), px(800.0)) // Window size for workspace tests +size(px(500.0), px(900.0)) // Window size for agent panel +Duration::from_millis(500) // Various delays +const MATCH_THRESHOLD: f64 = 0.99; // Already a constant, good! +``` + +### Solution + +Move all configuration to constants at the top of the file. + +### How to Implement + +```rust +// Window sizes +const WORKSPACE_WINDOW_SIZE: Size = size(px(1280.0), px(800.0)); +const AGENT_PANEL_WINDOW_SIZE: Size = size(px(500.0), px(900.0)); + +// Timing (if any delays are still needed after Improvement 1) +const FRAME_DELAY: Duration = Duration::from_millis(16); + +// Image comparison +const MATCH_THRESHOLD: f64 = 0.99; +const PIXEL_TOLERANCE: i16 = 2; // Currently hardcoded in pixels_are_similar() +``` + +--- + +## Summary: Recommended Order of Implementation + +1. **Remove dead code** (Improvement 3) - Quick win, low risk +2. **Add constants** (Improvement 7) - Quick win, improves readability +3. **Use FakeFs** (Improvement 2) - Medium effort, big determinism win +4. **Replace timers** (Improvement 1) - Medium effort, biggest determinism win +5. **Consolidate setup** (Improvement 4) - Medium effort, maintainability win +6. **Exercise more production code** (Improvement 5) - Lower priority, nice to have +7. **Better test organization** (Improvement 6) - Lower priority, nice to have for many tests + +## Testing Your Changes + +After each change: + +```bash +# Build to check for compile errors +cargo build -p zed --features visual-tests + +# Run clippy for warnings +cargo clippy -p zed --features visual-tests + +# Run the tests +cargo run -p zed --bin zed_visual_test_runner --features visual-tests + +# If screenshots changed intentionally, update baselines +UPDATE_BASELINE=1 cargo run -p zed --bin zed_visual_test_runner --features visual-tests +``` + +Baseline images are stored in `crates/zed/test_fixtures/visual_tests/`. + +## Questions? + +If you get stuck: +1. Look at existing tests in `crates/project_panel/src/project_panel_tests.rs` for examples of `FakeFs` and `run_until_parked()` +2. Look at `crates/gpui/src/app/test_context.rs` for `VisualTestContext` documentation +3. The CLAUDE.md file at the repo root has Rust coding guidelines