FIx tests

Mikayla Maki created

Change summary

crates/agent_ui/src/thread_metadata_store.rs  |   2 
crates/sidebar/src/sidebar.rs                 |  31 ++-
crates/sidebar/src/sidebar_tests.rs           |  79 +++++++---
crates/workspace/src/multi_workspace.rs       |  12 
crates/workspace/src/multi_workspace_tests.rs | 154 ---------------------
5 files changed, 78 insertions(+), 200 deletions(-)

Detailed changes

crates/sidebar/src/sidebar.rs 🔗

@@ -425,19 +425,7 @@ fn worktree_info_from_thread_paths(
     // folder paths don't all share the same short name, prefix each
     // linked worktree chip with its main project name so the user knows
     // which project it belongs to.
-    // All root paths (main + linked) resolve to the same short name.
-    let first_name = linked_short_names
-        .first()
-        .map(|(name, _)| name)
-        .or_else(|| infos.first().map(|i| &i.name));
-    let all_same_name = first_name.is_some_and(|first| {
-        infos.iter().all(|i| i.name == *first)
-            && main_paths.iter().all(|mp| {
-                mp.file_name()
-                    .and_then(|n| n.to_str())
-                    .is_some_and(|n| n == first.as_ref())
-            })
-    });
+    let all_same_name = infos.len() > 1 && infos.iter().all(|i| i.name == infos[0].name);
 
     if main_paths.len() > 1 && !all_same_name {
         for (info, (_short_name, project_name)) in infos
@@ -537,7 +525,7 @@ impl Sidebar {
                 }
                 MultiWorkspaceEvent::ProjectGroupKeyChanged { old_key, new_key } => {
                     ThreadMetadataStore::global(cx).update(cx, |store, cx| {
-                        store.migrate_main_worktree_paths(
+                        store.update_main_worktree_paths(
                             old_key.path_list(),
                             new_key.path_list().clone(),
                             cx,
@@ -1036,6 +1024,18 @@ impl Sidebar {
                         }
                     };
 
+                let main_path_list = group_key.path_list();
+
+                // Skip threads whose folder_paths include a linked worktree
+                // that hasn't been discovered by any workspace's git state.
+                let has_unbacked_linked_worktree = |row: &ThreadMetadata| -> bool {
+                    let main_paths = main_path_list.paths();
+                    row.folder_paths.paths().iter().any(|path| {
+                        let is_main = main_paths.iter().any(|mp| mp.as_path() == path.as_path());
+                        !is_main && !linked_to_main.contains_key(&**path)
+                    })
+                };
+
                 // Main code path: one query per group via main_worktree_paths.
                 // The main_worktree_paths column is set on all new threads and
                 // points to the group's canonical paths regardless of which
@@ -1048,6 +1048,9 @@ impl Sidebar {
                     if !seen_session_ids.insert(row.session_id.clone()) {
                         continue;
                     }
+                    if has_unbacked_linked_worktree(&row) {
+                        continue;
+                    }
                     let workspace = resolve_workspace(&row);
                     threads.push(make_thread_entry(row, workspace));
                 }

crates/sidebar/src/sidebar_tests.rs 🔗

@@ -251,6 +251,32 @@ fn save_thread_metadata(
     cx.run_until_parked();
 }
 
+fn save_thread_metadata_with_main_paths(
+    session_id: &str,
+    title: &str,
+    folder_paths: PathList,
+    main_worktree_paths: PathList,
+    cx: &mut TestAppContext,
+) {
+    let session_id = acp::SessionId::new(Arc::from(session_id));
+    let title = SharedString::from(title.to_string());
+    let updated_at = chrono::TimeZone::with_ymd_and_hms(&Utc, 2024, 1, 1, 0, 0, 0).unwrap();
+    let metadata = ThreadMetadata {
+        session_id,
+        agent_id: agent::ZED_AGENT_ID.clone(),
+        title,
+        updated_at,
+        created_at: None,
+        folder_paths,
+        main_worktree_paths,
+        archived: false,
+    };
+    cx.update(|cx| {
+        ThreadMetadataStore::global(cx).update(cx, |store, cx| store.save_manually(metadata, cx));
+    });
+    cx.run_until_parked();
+}
+
 fn focus_sidebar(sidebar: &Entity<Sidebar>, cx: &mut gpui::VisualTestContext) {
     sidebar.update_in(cx, |_, window, cx| {
         cx.focus_self(window);
@@ -2789,14 +2815,15 @@ async fn test_worktree_add_key_collision_removes_duplicate_workspace(cx: &mut Te
     cx.run_until_parked();
 
     // Both project groups should be visible.
-    let entries = visible_entries_as_strings(&sidebar, cx);
-    assert!(
-        entries.iter().any(|e| e.contains("[project-a]")),
-        "should show project-a group: {entries:?}"
-    );
-    assert!(
-        entries.iter().any(|e| e.contains("[project-a, project-b]")),
-        "should show project-a,b group: {entries:?}"
+    assert_eq!(
+        visible_entries_as_strings(&sidebar, cx),
+        vec![
+            //
+            "v [project-a, project-b]",
+            "  Thread B",
+            "v [project-a]",
+            "  Thread A",
+        ]
     );
 
     let workspace_b_id = workspace_b.entity_id();
@@ -2839,18 +2866,14 @@ async fn test_worktree_add_key_collision_removes_duplicate_workspace(cx: &mut Te
     sidebar.update_in(cx, |sidebar, _window, cx| sidebar.update_entries(cx));
     cx.run_until_parked();
 
-    let entries = visible_entries_as_strings(&sidebar, cx);
-    assert!(
-        entries.iter().any(|e| e.contains("Thread A")),
-        "Thread A should be visible: {entries:?}"
-    );
-    assert!(
-        entries.iter().any(|e| e.contains("Thread B")),
-        "Thread B should be visible: {entries:?}"
-    );
-    assert!(
-        entries.iter().filter(|e| e.contains("[project-a")).count() == 1,
-        "should have exactly 1 project header: {entries:?}"
+    assert_eq!(
+        visible_entries_as_strings(&sidebar, cx),
+        vec![
+            //
+            "v [project-a, project-b]",
+            "  Thread A",
+            "  Thread B",
+        ]
     );
 }
 
@@ -3412,13 +3435,21 @@ async fn test_git_worktree_added_live_updates_sidebar(cx: &mut TestAppContext) {
         cx.add_window_view(|window, cx| MultiWorkspace::test_new(project.clone(), window, cx));
     let sidebar = setup_sidebar(&multi_workspace, cx);
 
-    // Save a thread against a worktree path that doesn't exist yet.
-    save_named_thread_metadata("wt-thread", "Worktree Thread", &worktree_project, cx).await;
+    // Save a thread against a worktree path with the correct main
+    // worktree association (as if the git state had been resolved).
+    save_thread_metadata_with_main_paths(
+        "wt-thread",
+        "Worktree Thread",
+        PathList::new(&[PathBuf::from("/wt/rosewood")]),
+        PathList::new(&[PathBuf::from("/project")]),
+        cx,
+    );
 
     multi_workspace.update_in(cx, |_, _window, cx| cx.notify());
     cx.run_until_parked();
 
-    // Thread is not visible yet — no worktree knows about this path.
+    // Thread is not visible yet — the linked worktree hasn't been
+    // discovered by any workspace's git state.
     assert_eq!(
         visible_entries_as_strings(&sidebar, cx),
         vec![
@@ -3699,7 +3730,7 @@ async fn test_multi_worktree_thread_shows_multiple_chips(cx: &mut TestAppContext
         vec![
             //
             "v [project_a, project_b]",
-            "  Cross Worktree Thread {olivetti}, {selectric}",
+            "  Cross Worktree Thread {project_a:olivetti}, {project_b:selectric}",
         ]
     );
 }

crates/workspace/src/multi_workspace.rs 🔗

@@ -563,12 +563,9 @@ impl MultiWorkspace {
         cx.subscribe_in(&project, window, {
             let workspace = workspace.downgrade();
             move |this, _project, event, _window, cx| match event {
-                project::Event::WorktreeAdded(_) | project::Event::WorktreeRemoved(_) => {
-                    if let Some(workspace) = workspace.upgrade() {
-                        this.handle_workspace_key_change(&workspace, cx);
-                    }
-                }
-                project::Event::WorktreeUpdatedRootRepoCommonDir(_) => {
+                project::Event::WorktreeAdded(_)
+                | project::Event::WorktreeRemoved(_)
+                | project::Event::WorktreeUpdatedRootRepoCommonDir(_) => {
                     if let Some(workspace) = workspace.upgrade() {
                         this.handle_workspace_key_change(&workspace, cx);
                     }
@@ -1407,7 +1404,8 @@ impl MultiWorkspace {
         );
 
         let cached_ids: HashSet<EntityId> = self.workspace_group_keys.keys().copied().collect();
-        let workspace_ids: HashSet<EntityId> = self.workspaces().map(|ws| ws.entity_id()).collect();
+        let workspace_ids: HashSet<EntityId> =
+            self.workspaces.iter().map(|ws| ws.entity_id()).collect();
         anyhow::ensure!(
             cached_ids == workspace_ids,
             "workspace_group_keys entity IDs don't match workspaces.\n\

crates/workspace/src/multi_workspace_tests.rs 🔗

@@ -185,157 +185,3 @@ async fn test_project_group_keys_duplicate_not_added(cx: &mut TestAppContext) {
         );
     });
 }
-
-#[gpui::test]
-async fn test_project_group_keys_on_worktree_added(cx: &mut TestAppContext) {
-    init_test(cx);
-    let fs = FakeFs::new(cx.executor());
-    fs.insert_tree("/root_a", json!({ "file.txt": "" })).await;
-    fs.insert_tree("/root_b", json!({ "file.txt": "" })).await;
-    let project = Project::test(fs, ["/root_a".as_ref()], cx).await;
-
-    let initial_key = project.read_with(cx, |p, cx| p.project_group_key(cx));
-
-    let (multi_workspace, cx) =
-        cx.add_window_view(|window, cx| MultiWorkspace::test_new(project.clone(), window, cx));
-
-    multi_workspace.update(cx, |mw, cx| {
-        mw.open_sidebar(cx);
-    });
-
-    // Add a second worktree to the same project.
-    let (worktree, _) = project
-        .update(cx, |project, cx| {
-            project.find_or_create_worktree("/root_b", true, cx)
-        })
-        .await
-        .unwrap();
-    worktree
-        .read_with(cx, |tree, _| tree.as_local().unwrap().scan_complete())
-        .await;
-    cx.run_until_parked();
-
-    let updated_key = project.read_with(cx, |p, cx| p.project_group_key(cx));
-    assert_ne!(
-        initial_key, updated_key,
-        "key should change after adding a worktree"
-    );
-
-    multi_workspace.read_with(cx, |mw, _cx| {
-        let keys: Vec<&ProjectGroupKey> = mw.project_group_keys().collect();
-        assert_eq!(
-            keys.len(),
-            2,
-            "should have both the original and updated key"
-        );
-        assert_eq!(*keys[0], updated_key);
-        assert_eq!(*keys[1], initial_key);
-    });
-}
-
-#[gpui::test]
-async fn test_project_group_keys_on_worktree_removed(cx: &mut TestAppContext) {
-    init_test(cx);
-    let fs = FakeFs::new(cx.executor());
-    fs.insert_tree("/root_a", json!({ "file.txt": "" })).await;
-    fs.insert_tree("/root_b", json!({ "file.txt": "" })).await;
-    let project = Project::test(fs, ["/root_a".as_ref(), "/root_b".as_ref()], cx).await;
-
-    let initial_key = project.read_with(cx, |p, cx| p.project_group_key(cx));
-
-    let (multi_workspace, cx) =
-        cx.add_window_view(|window, cx| MultiWorkspace::test_new(project.clone(), window, cx));
-
-    multi_workspace.update(cx, |mw, cx| {
-        mw.open_sidebar(cx);
-    });
-
-    // Remove one worktree.
-    let worktree_b_id = project.read_with(cx, |project, cx| {
-        project
-            .worktrees(cx)
-            .find(|wt| wt.read(cx).root_name().as_unix_str() == "root_b")
-            .unwrap()
-            .read(cx)
-            .id()
-    });
-    project.update(cx, |project, cx| {
-        project.remove_worktree(worktree_b_id, cx);
-    });
-    cx.run_until_parked();
-
-    let updated_key = project.read_with(cx, |p, cx| p.project_group_key(cx));
-    assert_ne!(
-        initial_key, updated_key,
-        "key should change after removing a worktree"
-    );
-
-    multi_workspace.read_with(cx, |mw, _cx| {
-        let keys: Vec<&ProjectGroupKey> = mw.project_group_keys().collect();
-        assert_eq!(
-            keys.len(),
-            2,
-            "should accumulate both the original and post-removal key"
-        );
-        assert_eq!(*keys[0], updated_key);
-        assert_eq!(*keys[1], initial_key);
-    });
-}
-
-#[gpui::test]
-async fn test_project_group_keys_across_multiple_workspaces_and_worktree_changes(
-    cx: &mut TestAppContext,
-) {
-    init_test(cx);
-    let fs = FakeFs::new(cx.executor());
-    fs.insert_tree("/root_a", json!({ "file.txt": "" })).await;
-    fs.insert_tree("/root_b", json!({ "file.txt": "" })).await;
-    fs.insert_tree("/root_c", json!({ "file.txt": "" })).await;
-    let project_a = Project::test(fs.clone(), ["/root_a".as_ref()], cx).await;
-    let project_b = Project::test(fs.clone(), ["/root_b".as_ref()], cx).await;
-
-    let key_a = project_a.read_with(cx, |p, cx| p.project_group_key(cx));
-    let key_b = project_b.read_with(cx, |p, cx| p.project_group_key(cx));
-
-    let (multi_workspace, cx) =
-        cx.add_window_view(|window, cx| MultiWorkspace::test_new(project_a.clone(), window, cx));
-
-    multi_workspace.update(cx, |mw, cx| {
-        mw.open_sidebar(cx);
-    });
-
-    multi_workspace.update_in(cx, |mw, window, cx| {
-        mw.test_add_workspace(project_b, window, cx);
-    });
-
-    multi_workspace.read_with(cx, |mw, _cx| {
-        assert_eq!(mw.project_group_keys().count(), 2);
-    });
-
-    // Now add a worktree to project_a. This should produce a third key.
-    let (worktree, _) = project_a
-        .update(cx, |project, cx| {
-            project.find_or_create_worktree("/root_c", true, cx)
-        })
-        .await
-        .unwrap();
-    worktree
-        .read_with(cx, |tree, _| tree.as_local().unwrap().scan_complete())
-        .await;
-    cx.run_until_parked();
-
-    let key_a_updated = project_a.read_with(cx, |p, cx| p.project_group_key(cx));
-    assert_ne!(key_a, key_a_updated);
-
-    multi_workspace.read_with(cx, |mw, _cx| {
-        let keys: Vec<&ProjectGroupKey> = mw.project_group_keys().collect();
-        assert_eq!(
-            keys.len(),
-            3,
-            "should have key_a, key_b, and the updated key_a with root_c"
-        );
-        assert_eq!(*keys[0], key_a_updated);
-        assert_eq!(*keys[1], key_b);
-        assert_eq!(*keys[2], key_a);
-    });
-}