From c44f13cca02632b2e3159a6af78b66701ef9a2e8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E1=B4=80=E1=B4=8D=E1=B4=9B=E1=B4=8F=E1=B4=80=E1=B4=87?= =?UTF-8?q?=CA=80?= Date: Wed, 18 Mar 2026 21:11:08 +0800 Subject: [PATCH] git_ui: Improve delete branch tests to verify underlying repository state (#51825) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit While fixing the tests in the previous PR (#48338), I noticed that the `delete_branch` tests only verified the UI state changes rather than the actual state of the underlying repository. Currently, if `delete_branch` were a no-op that returned Ok(), the tests would still pass incorrectly. This PR addresses that gap by ensuring the branch is actually removed from the repository. |Before|After| |--|--| |图片|CleanShot 2026-03-18 at 17 51
36@2x| Before you mark this PR as ready for review, make sure that you have: - [x] Added a solid test coverage and/or screenshots from doing manual testing - [x] Done a self-review taking into account security and performance aspects - [ ] Aligned any UI changes with the [UI checklist](https://github.com/zed-industries/zed/blob/main/CONTRIBUTING.md#uiux-checklist) Release Notes: - N/A *or* Added/Fixed/Improved ... --- crates/fs/src/fake_git_repo.rs | 2 + crates/git_ui/src/branch_picker.rs | 74 ++++++++++++++++++++++-------- 2 files changed, 57 insertions(+), 19 deletions(-) diff --git a/crates/fs/src/fake_git_repo.rs b/crates/fs/src/fake_git_repo.rs index dd8442f7c01f0535354edc83106e0a97ed8949a2..91103953f0da1f1cdcf99fec90f40f92526282bf 100644 --- a/crates/fs/src/fake_git_repo.rs +++ b/crates/fs/src/fake_git_repo.rs @@ -392,6 +392,8 @@ impl GitRepository for FakeGitRepository { .map(|branch_name| { let ref_name = if branch_name.starts_with("refs/") { branch_name.into() + } else if branch_name.contains('/') { + format!("refs/remotes/{branch_name}").into() } else { format!("refs/heads/{branch_name}").into() }; diff --git a/crates/git_ui/src/branch_picker.rs b/crates/git_ui/src/branch_picker.rs index cc5f70e2d0f355e94f402f74c0ceaa6121c448d7..329f8e91e9e8a0994b2c3502b7c6c2013f28a936 100644 --- a/crates/git_ui/src/branch_picker.rs +++ b/crates/git_ui/src/branch_picker.rs @@ -1509,6 +1509,30 @@ mod tests { }); cx.run_until_parked(); + let expected_branches = ["main", "feature-auth", "feature-ui", "develop"] + .into_iter() + .filter(|name| name != &branch_to_delete) + .collect::>(); + let repo_branches = branch_list + .update(cx, |branch_list, cx| { + branch_list.picker.update(cx, |picker, cx| { + picker + .delegate + .repo + .as_ref() + .unwrap() + .update(cx, |repo, _cx| repo.branches()) + }) + }) + .await + .unwrap() + .unwrap(); + let repo_branches = repo_branches + .iter() + .map(|b| b.name()) + .collect::>(); + assert_eq!(&repo_branches, &expected_branches); + branch_list.update(cx, move |branch_list, cx| { branch_list.picker.update(cx, move |picker, _cx| { assert_eq!(picker.delegate.matches.len(), 3); @@ -1518,13 +1542,7 @@ mod tests { .iter() .map(|be| be.name()) .collect::>(); - assert_eq!( - branches, - ["main", "feature-auth", "feature-ui", "develop"] - .into_iter() - .filter(|name| name != &branch_to_delete) - .collect::>() - ); + assert_eq!(branches, expected_branches); }) }); } @@ -1577,6 +1595,35 @@ mod tests { }); cx.run_until_parked(); + let expected_branches = [ + "origin/main", + "origin/feature-auth", + "fork/feature-ui", + "private/develop", + ] + .into_iter() + .filter(|name| name != &branch_to_delete) + .collect::>(); + let repo_branches = branch_list + .update(cx, |branch_list, cx| { + branch_list.picker.update(cx, |picker, cx| { + picker + .delegate + .repo + .as_ref() + .unwrap() + .update(cx, |repo, _cx| repo.branches()) + }) + }) + .await + .unwrap() + .unwrap(); + let repo_branches = repo_branches + .iter() + .map(|b| b.name()) + .collect::>(); + assert_eq!(&repo_branches, &expected_branches); + // Check matches, it should match one less branch than before branch_list.update(cx, move |branch_list, cx| { branch_list.picker.update(cx, move |picker, _cx| { @@ -1587,18 +1634,7 @@ mod tests { .iter() .map(|be| be.name()) .collect::>(); - assert_eq!( - branches, - [ - "origin/main", - "origin/feature-auth", - "fork/feature-ui", - "private/develop" - ] - .into_iter() - .filter(|name| name != &branch_to_delete) - .collect::>() - ); + assert_eq!(branches, expected_branches); }) }); }