From 29f3459bf388f4d083d0d5ab2e83670fa8279590 Mon Sep 17 00:00:00 2001 From: Richard Feldman Date: Tue, 31 Mar 2026 13:57:17 -0400 Subject: [PATCH] Fix visual test runner panics and add visual tests to macOS CI Three pre-existing issues in the visual test runner were never caught because visual tests aren't run in CI. Fix all three and add visual tests to the macOS CI job so they stay healthy going forward. 1. Re-entrant read panic in sidebar creation (3 places): Sidebar::new reads the MultiWorkspace entity, so creating it inside a multi_workspace.update() closure causes a double-borrow panic. Fix by using update_window with root_view.downcast() instead. 2. Tooltip entity leak in breakpoint_hover test: The tooltip system has an Rc cycle (ActiveTooltip::Visible holds a check_visible_and_update closure that captures the Rc>>). This cycle is normally broken when the tooltip is dismissed, but the test closed the window while the tooltip was still visible. Fix by simulating a click before cleanup to dismiss the tooltip. 3. Visual tests not in CI: Add a cargo_run_visual_tests step to the macOS test job via xtask so these tests run on every PR. --- .github/workflows/release.yml | 2 ++ .github/workflows/run_tests.yml | 2 ++ crates/zed/src/visual_test_runner.rs | 18 ++++++++++-------- tooling/xtask/src/tasks/workflows/run_tests.rs | 3 +++ tooling/xtask/src/tasks/workflows/steps.rs | 4 ++++ 5 files changed, 21 insertions(+), 8 deletions(-) diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index b651e7046bc7d603a7a829ce1b59fcf0468bdd3b..3d157ef75d4f9c8d86537a044d42469f84b83ec0 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -43,6 +43,8 @@ jobs: SCCACHE_BUCKET: sccache-zed - name: steps::cargo_nextest run: cargo nextest run --workspace --no-fail-fast --no-tests=warn + - name: steps::cargo_run_visual_tests + run: cargo run -p zed --bin zed_visual_test_runner --features visual-tests - name: steps::show_sccache_stats run: sccache --show-stats || true - name: steps::cleanup_cargo_config diff --git a/.github/workflows/run_tests.yml b/.github/workflows/run_tests.yml index a1a0569f5df00cfa9e93e311681a98c90d6ce067..7c69034b4270dff67bb5baae414c066872cb558c 100644 --- a/.github/workflows/run_tests.yml +++ b/.github/workflows/run_tests.yml @@ -440,6 +440,8 @@ jobs: SCCACHE_BUCKET: sccache-zed - name: steps::cargo_nextest run: cargo nextest run --workspace --no-fail-fast --no-tests=warn${{ needs.orchestrate.outputs.changed_packages && format(' -E "{0}"', needs.orchestrate.outputs.changed_packages) || '' }} + - name: steps::cargo_run_visual_tests + run: cargo run -p zed --bin zed_visual_test_runner --features visual-tests - name: steps::show_sccache_stats run: sccache --show-stats || true - name: steps::cleanup_cargo_config diff --git a/crates/zed/src/visual_test_runner.rs b/crates/zed/src/visual_test_runner.rs index e5713e90df397a01af850af55338897f9d437e55..27776eba84a10c5314c5e28fab430ad61e65bbf2 100644 --- a/crates/zed/src/visual_test_runner.rs +++ b/crates/zed/src/visual_test_runner.rs @@ -2646,10 +2646,11 @@ fn run_multi_workspace_sidebar_visual_tests( cx.run_until_parked(); - // Create the sidebar and register it on the MultiWorkspace - let sidebar = multi_workspace_window - .update(cx, |_multi_workspace, window, cx| { - let multi_workspace_handle = cx.entity(); + // Create the sidebar outside the MultiWorkspace update to avoid a + // re-entrant read panic (Sidebar::new reads the MultiWorkspace). + let sidebar = cx + .update_window(multi_workspace_window.into(), |root_view, window, cx| { + let multi_workspace_handle: Entity = root_view.downcast().unwrap(); cx.new(|cx| sidebar::Sidebar::new(multi_workspace_handle, window, cx)) }) .context("Failed to create sidebar")?; @@ -3178,10 +3179,11 @@ edition = "2021" cx.run_until_parked(); - // Create and register the workspace sidebar - let sidebar = workspace_window - .update(cx, |_multi_workspace, window, cx| { - let multi_workspace_handle = cx.entity(); + // Create the sidebar outside the MultiWorkspace update to avoid a + // re-entrant read panic (Sidebar::new reads the MultiWorkspace). + let sidebar = cx + .update_window(workspace_window.into(), |root_view, window, cx| { + let multi_workspace_handle: Entity = root_view.downcast().unwrap(); cx.new(|cx| sidebar::Sidebar::new(multi_workspace_handle, window, cx)) }) .context("Failed to create sidebar")?; diff --git a/tooling/xtask/src/tasks/workflows/run_tests.rs b/tooling/xtask/src/tasks/workflows/run_tests.rs index 52338e30ff15f9862c802f8515980642a9ba490d..9c0027d6d5db802ae09339718c4643c5f284813c 100644 --- a/tooling/xtask/src/tasks/workflows/run_tests.rs +++ b/tooling/xtask/src/tasks/workflows/run_tests.rs @@ -596,6 +596,9 @@ fn run_platform_tests_impl(platform: Platform, filter_packages: bool) -> NamedJo .when(!filter_packages, |job| { job.add_step(steps::cargo_nextest(platform)) }) + .when(platform == Platform::Mac, |job| { + job.add_step(steps::cargo_run_visual_tests()) + }) .add_step(steps::show_sccache_stats(platform)) .add_step(steps::cleanup_cargo_config(platform)), } diff --git a/tooling/xtask/src/tasks/workflows/steps.rs b/tooling/xtask/src/tasks/workflows/steps.rs index ebdd9b30538eb389a267a1c2fdb1822eec1d3a54..de4bcfca79c164af74733cd5946fa5c79e63b7e0 100644 --- a/tooling/xtask/src/tasks/workflows/steps.rs +++ b/tooling/xtask/src/tasks/workflows/steps.rs @@ -254,6 +254,10 @@ pub fn setup_sccache(platform: Platform) -> Step { .add_env(("SCCACHE_BUCKET", SCCACHE_R2_BUCKET)) } +pub fn cargo_run_visual_tests() -> Step { + named::bash("cargo run -p zed --bin zed_visual_test_runner --features visual-tests") +} + pub fn show_sccache_stats(platform: Platform) -> Step { match platform { // Use $env:RUSTC_WRAPPER (absolute path) because GITHUB_PATH changes