Add tests for single-branch but branch apply

These are the things only modern code can do.

While at it, also see what happens if there is no worktree, i.e. as there is a bare repository.

Author: Byron

crates/but-graph/src/debug.rs

@@ -262,8 +262,13 @@ impl Graph {
         static SUFFIX: AtomicUsize = AtomicUsize::new(0);
         let suffix = SUFFIX.fetch_add(1, std::sync::atomic::Ordering::SeqCst);
         let svg_name = format!("debug-graph-{suffix:02}.svg");
+        let svg_path = std::env::var_os("CARGO_MANIFEST_DIR")
+            .map(std::path::PathBuf::from)
+            .unwrap_or_default()
+            .join(svg_name);
         let mut dot = std::process::Command::new("dot")
-            .args(["-Tsvg", "-o", &svg_name])
+            .args(["-Tsvg", "-o"])
+            .arg(&svg_path)
             .stdin(Stdio::piped())
             .stdout(Stdio::piped())
             .stderr(Stdio::piped())
@@ -284,11 +289,12 @@ impl Graph {
 
         assert!(
             std::process::Command::new("open")
-                .arg(&svg_name)
+                .arg(&svg_path)
                 .status()
                 .unwrap()
                 .success(),
-            "Opening of {svg_name} failed"
+            "Opening of {svg_path} failed",
+            svg_path = svg_path.display()
         );
     }
 

crates/but-graph/src/projection/stack.rs

@@ -95,7 +95,10 @@ impl Stack {
 
 impl Stack {
     /// A one-line string representing the stack itself, without its contents.
-    pub fn debug_string(&self) -> String {
+    ///
+    /// Use `id_override` to have it use this (usually controlled) id instead of what otherwise
+    /// would be a generated one.
+    pub fn debug_string(&self, id_override: Option<StackId>) -> String {
         let mut dbg = self
             .segments
             .first()
@@ -105,7 +108,7 @@ impl Stack {
             dbg.push_str(&base.to_hex_with_len(7).to_string());
         }
         dbg.insert(0, '≡');
-        if let Some(id) = self.id {
+        if let Some(id) = id_override.or(self.id) {
             let id_string = id.to_string().replace("0", "").replace("-", "");
             dbg.push_str(&format!(
                 " {{{}}}",
@@ -122,7 +125,7 @@ impl Stack {
 
 impl std::fmt::Debug for Stack {
     fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result {
-        let mut s = f.debug_struct(&format!("Stack({})", self.debug_string()));
+        let mut s = f.debug_struct(&format!("Stack({})", self.debug_string(None)));
         s.field("segments", &self.segments);
         if let Some(stack_id) = self.id {
             s.field("id", &stack_id);

crates/but-meta/src/legacy.rs

@@ -55,6 +55,9 @@ impl Snapshot {
             if self.content == Default::default() {
                 std::fs::remove_file(&self.path)?;
             } else {
+                if let Some(dir) = self.path.parent() {
+                    std::fs::create_dir_all(dir)?;
+                }
                 fs::write(
                     &self.path,
                     toml::to_string(&self.to_consistent_data(reconcile))?,

crates/but-testsupport/src/graph.rs

@@ -1,5 +1,6 @@
 use std::collections::{BTreeMap, BTreeSet};
 
+use but_core::ref_metadata::StackId;
 use but_graph::{
     EntryPoint, Graph, SegmentIndex, SegmentMetadata, projection::StackCommitDebugFlags,
 };
@@ -9,23 +10,43 @@ type StringTree = Tree<String>;
 
 /// Visualize `graph` as a tree.
 pub fn graph_workspace(workspace: &but_graph::projection::Workspace<'_>) -> StringTree {
+    graph_workspace_inner(workspace, None)
+}
+
+/// Visualize `graph` as a tree, and remap random stack ids to something deterministic.
+pub fn graph_workspace_determinisitcally(
+    workspace: &but_graph::projection::Workspace<'_>,
+) -> StringTree {
+    graph_workspace_inner(workspace, Some(Default::default()))
+}
+
+fn graph_workspace_inner(
+    workspace: &but_graph::projection::Workspace<'_>,
+    mut stack_id_map: Option<BTreeMap<StackId, StackId>>,
+) -> StringTree {
     let commit_flags = if workspace.graph.hard_limit_hit() {
         StackCommitDebugFlags::HardLimitReached
     } else {
         Default::default()
     };
     let mut root = Tree::new(workspace.debug_string());
     for stack in &workspace.stacks {
-        root.push(tree_for_stack(stack, commit_flags));
+        root.push(tree_for_stack(stack, commit_flags, stack_id_map.as_mut()));
     }
     root
 }
 
 fn tree_for_stack(
     stack: &but_graph::projection::Stack,
     commit_flags: StackCommitDebugFlags,
+    stack_id_map: Option<&mut BTreeMap<StackId, StackId>>,
 ) -> StringTree {
-    let mut root = Tree::new(stack.debug_string());
+    let mut root = Tree::new(
+        stack.debug_string(stack.id.zip(stack_id_map).map(|(id, map)| {
+            let next_id = StackId::from_number_for_testing((map.len() + 1) as u128);
+            *map.entry(id).or_insert(next_id)
+        })),
+    );
     for segment in &stack.segments {
         root.push(tree_for_stack_segment(segment, commit_flags));
     }

crates/but-testsupport/src/lib.rs

@@ -592,8 +592,7 @@ pub fn debug_str(input: &dyn std::fmt::Debug) -> String {
 }
 
 mod graph;
-
-pub use graph::{graph_tree, graph_workspace};
+pub use graph::{graph_tree, graph_workspace, graph_workspace_determinisitcally};
 
 mod prepare_cmd_env;
 pub use prepare_cmd_env::isolate_env_std_cmd;

crates/but/src/command/branch/apply.rs

@@ -8,19 +8,25 @@ pub fn apply(ctx: Context, branch_name: &str, out: &mut OutputChannel) -> anyhow
     let repo = ctx.repo.get()?;
 
     let reference = repo.find_reference(branch_name)?;
-    let _outcome = but_api::branch::apply(&ctx, reference.name())?;
+    let mut outcome = but_api::branch::apply(&ctx, reference.name())?;
 
     if let Some(out) = out.for_human() {
-        let short_name = reference.name().shorten();
-        let is_remote_reference = reference
-            .name()
-            .category()
-            .is_some_and(|c| c == Category::RemoteBranch);
-        if is_remote_reference {
-            writeln!(out, "Applied remote branch '{short_name}' to workspace")
-        } else {
-            writeln!(out, "Applied branch '{short_name}' to workspace")
-        }?;
+        // Since `applied_branches` is the actual applied branches, turning remotes into local branches,
+        // hack it into submission while the legacy version exists that it has to match.
+        let special_case_remove_me_once_there_is_no_legacy_apply =
+            outcome.applied_branches.len() == 1;
+        if special_case_remove_me_once_there_is_no_legacy_apply {
+            outcome.applied_branches = vec![reference.name().to_owned()];
+        }
+        for name in outcome.applied_branches {
+            let short_name = name.shorten();
+            let is_remote_reference = name.category().is_some_and(|c| c == Category::RemoteBranch);
+            if is_remote_reference {
+                writeln!(out, "Applied remote branch '{short_name}' to workspace")
+            } else {
+                writeln!(out, "Applied branch '{short_name}' to workspace")
+            }?;
+        }
     } else if let Some(out) = out.for_shell() {
         writeln!(out, "{reference_name}", reference_name = reference.name())?;
     }

crates/but/tests/but/command/branch/apply.rs

@@ -1,6 +1,81 @@
 use crate::utils::{Sandbox, setup_metadata};
 use snapbox::str;
 
+#[cfg(not(feature = "legacy"))]
+#[test]
+fn single_branch() -> anyhow::Result<()> {
+    let env = Sandbox::open_with_default_settings("one-fork")?;
+    insta::assert_snapshot!(env.git_log()?, @r"
+    * bf53300 (A) add A
+    | * b1540e5 (HEAD -> main) M
+    |/  
+    | * 0e391b2 (origin/B) add B
+    |/  
+    * e31e6ca (origin/main, origin/HEAD) add init
+    ");
+
+    env.but("branch apply A")
+        .assert()
+        .success()
+        .stderr_eq(str![])
+        .stdout_eq(str![[r#"
+Applied branch 'main' to workspace
+Applied branch 'A' to workspace
+
+"#]]);
+
+    insta::assert_snapshot!(env.workspace_debug_at_head()?, @r"
+    📕🏘️:0:gitbutler/workspace[🌳] <> ✓!
+    └── ≡📙:1:main {1}
+        ├── 📙:1:main
+        │   └── ·b1540e5 (🏘️)
+        └── 📙:3:A →:2:
+            ├── ·bf53300*
+            └── ·e31e6ca (🏘️)
+    ");
+
+    // TODO: fix this: A should be in workspace. Also: where is our metadata?
+    insta::assert_snapshot!(env.git_log()?, @r"
+    * bf53300 (A) add A
+    | * e97d217 (HEAD -> gitbutler/workspace) GitButler Workspace Commit
+    | * b1540e5 (main) M
+    |/  
+    | * 0e391b2 (origin/B) add B
+    |/  
+    * e31e6ca (origin/main, origin/HEAD) add init
+    ");
+
+    env.but("branch apply origin/B")
+        .assert()
+        .success()
+        .stdout_eq(str![[r#"
+Applied remote branch 'origin/B' to workspace
+
+"#]])
+        .stderr_eq(str![""]);
+    insta::assert_snapshot!(env.workspace_debug_at_head()?, @r"
+    📕🏘️:0:gitbutler/workspace[🌳] <> ✓!
+    └── ≡📙:1:main {1}
+        ├── 📙:1:main
+        │   └── ·b1540e5 (🏘️)
+        └── 📙:4:B <> origin/B →:2:⇡1
+            ├── ·0e391b2*
+            └── ·e31e6ca (🏘️)
+    ");
+
+    // TODO: should be success and create a local tracking branch.
+    insta::assert_snapshot!(env.git_log()?, @r"
+    * bf53300 (A) add A
+    | * 0e391b2 (origin/B, B) add B
+    |/  
+    | * e97d217 (HEAD -> gitbutler/workspace) GitButler Workspace Commit
+    | * b1540e5 (main) M
+    |/  
+    * e31e6ca (origin/main, origin/HEAD) add init
+    ");
+    Ok(())
+}
+
 use crate::command::branch::apply::utils::create_local_branch_with_commit_with_message;
 use utils::create_local_branch_with_commit;
 

crates/but/tests/but/journey.rs

@@ -1,10 +1,29 @@
 //! Tests for various nice user-journeys, from different starting points, performing multiple common steps in sequence.
-use snapbox::{file, str};
+use crate::utils::Sandbox;
+use snapbox::str;
 
-use crate::utils::{Sandbox, setup_metadata};
+#[cfg(not(feature = "legacy"))]
+#[test]
+fn from_unborn() -> anyhow::Result<()> {
+    let env = Sandbox::open_with_default_settings("unborn")?;
+    insta::assert_snapshot!(env.git_log()?, @r"");
+
+    env.but("branch apply main")
+        .assert()
+        .failure()
+        .stderr_eq(str![[r#"
+Error: The reference 'main' did not exist
+
+"#]]);
+
+    // TODO: we should be able to use the CLI to create a commit
+    Ok(())
+}
 
+// TODO: maybe this should be a non-legacy journey only as we start out without workspace?
+#[cfg(feature = "legacy")]
 #[test]
-fn from_scratch_needs_work() -> anyhow::Result<()> {
+fn from_empty() -> anyhow::Result<()> {
     let env = Sandbox::empty()?;
 
     env.but("status").assert().failure().stderr_eq(str![[r#"
@@ -94,8 +113,11 @@ Error: workspace at refs/heads/main is missing a base
     Ok(())
 }
 
+#[cfg(feature = "legacy")]
 #[test]
 fn from_workspace() -> anyhow::Result<()> {
+    use crate::utils::setup_metadata;
+    use snapbox::file;
     let env = Sandbox::init_scenario_with_target_and_default_settings("two-stacks")?;
     insta::assert_snapshot!(env.git_log()?, @r"
     *   c128bce (HEAD -> gitbutler/workspace) GitButler Workspace Commit

crates/but/tests/but/main.rs

@@ -2,6 +2,5 @@ mod command;
 // TODO: Id tests can be on integration level, but shouldn't involve the CLI
 #[cfg(feature = "legacy")]
 mod id;
-#[cfg(feature = "legacy")]
 mod journey;
 pub mod utils;