add tests for get_branch_listing_details()

crates/gitbutler-branch-actions/src/branch.rs

@@ -414,8 +414,8 @@ pub struct VirtualBranchReference {
     pub in_workspace: bool,
 }
 
-/// Takes a list of branch names (the given name, as returned by `BranchListing`) and returns
-/// a list of enriched branch data in the form of `BranchData`.
+/// Takes a list of `branch_names` (the given name, as returned by `BranchListing`) and returns
+/// a list of enriched branch data.
 pub fn get_branch_listing_details(
     ctx: &CommandContext,
     branch_names: impl IntoIterator<Item = impl TryInto<BranchIdentity>>,
@@ -428,18 +428,17 @@ pub fn get_branch_listing_details(
     let repo = ctx.repository();
     let branches = list_branches(ctx, None, Some(branch_names))?;
 
-    let (default_target_upstream_commit_id, default_target_commit_id) = {
-        let default_target = ctx
+    let (default_target_upstream_commit_id, default_target_merge_base) = {
+        let target = ctx
             .project()
             .virtual_branches()
             .get_default_target()
             .context("failed to get default target")?;
-        let default_local_branch =
-            repo.find_branch(default_target.branch.branch(), git2::BranchType::Local)?;
-        let default_branch = default_local_branch.upstream()?;
+        let local_branch = repo.find_branch(target.branch.branch(), git2::BranchType::Local)?;
+        let local_tracking_branch = local_branch.upstream()?;
         (
-            default_branch.get().peel_to_commit()?.id(),
-            default_target.sha,
+            local_tracking_branch.get().peel_to_commit()?.id(),
+            target.sha,
         )
     };
 
@@ -447,7 +446,7 @@ pub fn get_branch_listing_details(
     for branch in branches {
         let other_branch_commit_id = if let Some(virtual_branch) = branch.virtual_branch {
             if virtual_branch.in_workspace {
-                default_target_commit_id
+                default_target_merge_base
             } else {
                 default_target_upstream_commit_id
             }

crates/gitbutler-branch-actions/tests/fixtures/for-details.sh

@@ -0,0 +1,52 @@
+#!/usr/bin/env bash
+set -eu -o pipefail
+CLI=${1:?The first argument is the GitButler CLI}
+
+function tick () {
+  if test -z "${tick+set}"; then
+    tick=1675176957
+  else
+    tick=$(($tick + 60))
+  fi
+  GIT_COMMITTER_DATE="$tick +0100"
+  GIT_AUTHOR_DATE="$tick +0100"
+  export GIT_COMMITTER_DATE GIT_AUTHOR_DATE
+}
+tick
+
+
+git init remote
+(cd remote
+  echo first > file
+  git add . && git commit -m "init"
+)
+
+export GITBUTLER_CLI_DATA_DIR=../user/gitbutler/app-data
+git clone remote complex-repo
+(cd complex-repo
+  for round in $(seq 5); do
+    echo main >> file
+    git commit -am "main-$round"
+  done
+
+  local_tracking_ref="$(git rev-parse --symbolic-full-name @{u})";
+
+  git checkout -b feature main
+  for round in $(seq 100); do
+    echo feature >> file
+    git commit -am "feat-$round"
+  done
+
+  git checkout main
+  $CLI project add --switch-to-integration "$local_tracking_ref"
+  for round in $(seq 10); do
+    echo virtual-main >> file
+    $CLI branch commit --message "virt-$round" main
+  done
+
+  git checkout -b non-virtual-feature main
+  for round in $(seq 50); do
+    echo non-virtual-feature >> file
+    git commit -am "non-virtual-feat-$round"
+  done
+)

crates/gitbutler-branch-actions/tests/virtual_branches/list.rs

@@ -229,4 +229,4 @@ mod util {
         Ok(branches)
     }
 }
-use util::{assert_equal, init_env, list_branches, project_ctx, ExpectedBranchListing};
+pub use util::{assert_equal, init_env, list_branches, project_ctx, ExpectedBranchListing};

crates/gitbutler-branch-actions/tests/virtual_branches/list_details.rs

@@ -0,0 +1,113 @@
+use crate::virtual_branches::list;
+use gitbutler_branch_actions::BranchListingDetails;
+
+#[test]
+fn one_vbranch_on_integration_empty_details() -> anyhow::Result<()> {
+    let list = branch_details(
+        &list::project_ctx("one-vbranch-on-integration")?,
+        Some("virtual"),
+    )?;
+    assert_eq!(list.len(), 1);
+    assert_eq!(
+        list[0],
+        BranchListingDetails {
+            name: "virtual".into(),
+            lines_added: 0,
+            lines_removed: 0,
+            number_of_files: 0,
+            number_of_commits: 0,
+            authors: vec![],
+        }
+    );
+    Ok(())
+}
+
+#[test]
+fn one_vbranch_on_integration_single_commit() -> anyhow::Result<()> {
+    let list = branch_details(
+        &list::project_ctx("one-vbranch-on-integration-one-commit")?,
+        Some("virtual"),
+    )?;
+    assert_eq!(list.len(), 1);
+    assert_eq!(
+        list[0],
+        BranchListingDetails {
+            name: "virtual".into(),
+            lines_added: 2,
+            lines_removed: 0,
+            number_of_files: 2,
+            number_of_commits: 1,
+            authors: vec![default_author()],
+        }
+    );
+    Ok(())
+}
+
+#[test]
+fn many_commits_in_all_branch_types() -> anyhow::Result<()> {
+    let ctx = project_ctx("complex-repo")?;
+    let list = branch_details(&ctx, ["feature", "main", "non-virtual-feature"])?;
+    assert_eq!(list.len(), 3);
+    assert_eq!(
+        list[0],
+        BranchListingDetails {
+            name: "feature".into(),
+            lines_added: 100 + 5,
+            lines_removed: 0,
+            number_of_files: 1,
+            number_of_commits: 100 + 5, /* local tracking branch is merge base */
+            authors: vec![default_author()],
+        }
+    );
+    assert_eq!(
+        list[1],
+        BranchListingDetails {
+            name: "main".into(),
+            lines_added: 15,
+            lines_removed: 0,
+            number_of_files: 1,
+            // TODO(ST): why is it also going against the local tracking branch instead of the local `main`?
+            number_of_commits: 10 + 5,
+            authors: vec![default_author()],
+        }
+    );
+    assert_eq!(
+        list[2],
+        BranchListingDetails {
+            name: "non-virtual-feature".into(),
+            lines_added: 55,
+            lines_removed: 0,
+            number_of_files: 1,
+            number_of_commits: 50 + 5,
+            authors: vec![default_author()],
+        }
+    );
+    Ok(())
+}
+
+mod util {
+    use gitbutler_branch::BranchIdentity;
+    use gitbutler_branch_actions::{Author, BranchListingDetails};
+    use gitbutler_command_context::CommandContext;
+
+    pub fn branch_details(
+        ctx: &CommandContext,
+        branch_names: impl IntoIterator<Item = impl TryInto<BranchIdentity>>,
+    ) -> anyhow::Result<Vec<BranchListingDetails>> {
+        let mut details = gitbutler_branch_actions::get_branch_listing_details(ctx, branch_names)?;
+        details.sort_by(|a, b| a.name.cmp(&b.name));
+        Ok(details)
+    }
+
+    pub fn default_author() -> Author {
+        Author {
+            name: Some("author".into()),
+            email: Some("author@example.com".into()),
+        }
+    }
+
+    pub fn project_ctx(name: &str) -> anyhow::Result<CommandContext> {
+        gitbutler_testsupport::read_only::fixture("for-details.sh", name)
+    }
+}
+use util::{branch_details, default_author, project_ctx};

crates/gitbutler-branch-actions/tests/virtual_branches/mod.rs

@@ -65,6 +65,7 @@ mod delete_virtual_branch;
 mod init;
 mod insert_blank_commit;
 mod list;
+mod list_details;
 mod move_commit_file;
 mod move_commit_to_vbranch;
 mod oplog;

crates/gitbutler-branch/src/target.rs

@@ -3,9 +3,16 @@ use serde::{ser::SerializeStruct, Deserialize, Deserializer, Serialize, Serializ
 
 #[derive(Debug, PartialEq, Clone)]
 pub struct Target {
+    /// The combination of remote name and branch name, i.e. `origin` and `main`.
+    /// The remote name is the one used to fetch from.
     pub branch: RemoteRefname,
+    /// The URL of the remote behind the symbolic name.
     pub remote_url: String,
+    /// The merge-base between `branch` and the current worktree `HEAD`.
+    // TODO(ST): is it safe/correct to rename this to `merge_base_commit_id`?
+    //           It seems like it, but why was it named just `sha` in the first place?
     pub sha: git2::Oid,
+    /// The name of the remote to push to.
     pub push_remote_name: Option<String>,
 }
 

crates/gitbutler-serde/src/bstring.rs

@@ -65,6 +65,13 @@ impl From<&BStr> for BStringForFrontend {
     }
 }
 
+/// Primarily for tests
+impl From<&str> for BStringForFrontend {
+    fn from(value: &str) -> Self {
+        BStringForFrontend(value.into())
+    }
+}
+
 impl PartialEq<&str> for BStringForFrontend {
     fn eq(&self, other: &&str) -> bool {
         self.0.eq(other)