Add visual tests to macOS CI (#52847)

Richard Feldman created 1 week ago

This fixes three pre-existing issues in the visual test runner that were
never caught because visual tests aren't run in CI.

## Fixes

**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 `cannot read while being
updated` panic. Fixed by using `update_window` with
`root_view.downcast()` to get the handle without holding a mutable
borrow.

**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<RefCell<Option<ActiveTooltip>>>`). This cycle is normally broken
when the tooltip is dismissed, but the test closed the window while the
tooltip was still visible, causing a panic from the leak detector. Fixed
by simulating a click before cleanup to dismiss the tooltip.

**3. Visual tests not in CI**

Added a `cargo_run_visual_tests` step to the macOS test job via xtask so
these tests run on every PR and regressions are caught early.

Release Notes:

- N/A

Change summary

.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(-)

Detailed changes

.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_build_visual_tests
+      run: cargo build -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

.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_build_visual_tests
+      run: cargo build -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

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<MultiWorkspace> = 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<MultiWorkspace> = root_view.downcast().unwrap();
             cx.new(|cx| sidebar::Sidebar::new(multi_workspace_handle, window, cx))
         })
         .context("Failed to create sidebar")?;

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_build_visual_tests())
+            })
             .add_step(steps::show_sccache_stats(platform))
             .add_step(steps::cleanup_cargo_config(platform)),
     }

tooling/xtask/src/tasks/workflows/steps.rs

@@ -254,6 +254,10 @@ pub fn setup_sccache(platform: Platform) -> Step<Run> {
         .add_env(("SCCACHE_BUCKET", SCCACHE_R2_BUCKET))
 }
 
+pub fn cargo_build_visual_tests() -> Step<Run> {
+    named::bash("cargo build -p zed --bin zed_visual_test_runner --features visual-tests")
+}
+
 pub fn show_sccache_stats(platform: Platform) -> Step<Run> {
     match platform {
         // Use $env:RUSTC_WRAPPER (absolute path) because GITHUB_PATH changes