Merge pull request gitbutlerapp#9639 from gitbutlerapp/kv-branch-40

krlvi · web-flow · commit b5566f8b2f29 · 2025-07-31T03:21:00.000-07:00
feat: add add_with_path and update_with_path for test data dir control and remove global test dir env var
diff --git a/crates/but-path/src/lib.rs b/crates/but-path/src/lib.rs
@@ -1,9 +1,6 @@
 use std::path::PathBuf;
 
 pub fn app_data_dir() -> anyhow::Result<PathBuf> {
-    if let Ok(test_dir) = std::env::var("TEST_APP_DATA_DIR") {
-        return Ok(PathBuf::from(test_dir));
-    }
     dirs::data_dir()
         .ok_or(anyhow::anyhow!("Could not get app data dir"))
         .map(|dir| dir.join(identifier()))
diff --git a/crates/gitbutler-branch-actions/tests/virtual_branches/amend.rs b/crates/gitbutler-branch-actions/tests/virtual_branches/amend.rs
@@ -17,11 +17,13 @@ fn forcepush_allowed() -> anyhow::Result<()> {
         ..
     } = &Test::default();
 
-    gitbutler_testsupport::set_test_data_dir(data_dir.as_ref().unwrap());
-    gitbutler_project::update(&projects::UpdateRequest {
-        id: *project_id,
-        ..Default::default()
-    })
+    gitbutler_project::update_with_path(
+        data_dir.as_ref().unwrap(),
+        &projects::UpdateRequest {
+            id: *project_id,
+            ..Default::default()
+        },
+    )
     .unwrap();
 
     gitbutler_branch_actions::set_base_branch(
@@ -32,10 +34,13 @@ fn forcepush_allowed() -> anyhow::Result<()> {
     )
     .unwrap();
 
-    gitbutler_project::update(&projects::UpdateRequest {
-        id: *project_id,
-        ..Default::default()
-    })
+    gitbutler_project::update_with_path(
+        data_dir.as_ref().unwrap(),
+        &projects::UpdateRequest {
+            id: *project_id,
+            ..Default::default()
+        },
+    )
     .unwrap();
 
     let stack_entry = gitbutler_branch_actions::create_virtual_branch(
diff --git a/crates/gitbutler-branch-actions/tests/virtual_branches/init.rs b/crates/gitbutler-branch-actions/tests/virtual_branches/init.rs
@@ -9,9 +9,9 @@ fn twice() {
     let test_project = TestProject::default();
 
     {
-        gitbutler_testsupport::set_test_data_dir(data_dir.path());
         let project =
-            gitbutler_project::add(test_project.path(), None, None).expect("failed to add project");
+            gitbutler_project::add_with_path(data_dir.path(), test_project.path(), None, None)
+                .expect("failed to add project");
         let ctx = CommandContext::open(&project, AppSettings::default()).unwrap();
 
         gitbutler_branch_actions::set_base_branch(
@@ -23,11 +23,13 @@ fn twice() {
         .unwrap();
         let stacks = stack_details(&ctx);
         assert_eq!(stacks.len(), 0);
-        gitbutler_project::delete(project.id).unwrap();
+        gitbutler_project::delete_with_path(data_dir.path(), project.id).unwrap();
     }
 
     {
-        let project = gitbutler_project::add(test_project.path(), None, None).unwrap();
+        let project =
+            gitbutler_project::add_with_path(data_dir.path(), test_project.path(), None, None)
+                .unwrap();
         let ctx = CommandContext::open(&project, AppSettings::default()).unwrap();
         gitbutler_branch_actions::set_base_branch(
             &ctx,
diff --git a/crates/gitbutler-branch-actions/tests/virtual_branches/mod.rs b/crates/gitbutler-branch-actions/tests/virtual_branches/mod.rs
@@ -31,9 +31,9 @@ impl Default for Test {
         let data_dir = paths::data_dir();
 
         let test_project = TestProject::default();
-        gitbutler_testsupport::set_test_data_dir(data_dir.as_ref());
         let project =
-            gitbutler_project::add(test_project.path(), None, None).expect("failed to add project");
+            gitbutler_project::add_with_path(data_dir.as_ref(), test_project.path(), None, None)
+                .expect("failed to add project");
         let ctx = CommandContext::open(&project, AppSettings::default()).unwrap();
 
         Self {
diff --git a/crates/gitbutler-branch-actions/tests/virtual_branches/update_commit_message.rs b/crates/gitbutler-branch-actions/tests/virtual_branches/update_commit_message.rs
@@ -136,6 +136,7 @@ fn middle() {
 #[test]
 fn forcepush_allowed() {
     let Test {
+        data_dir,
         repo,
         project_id,
 
@@ -151,10 +152,13 @@ fn forcepush_allowed() {
     )
     .unwrap();
 
-    gitbutler_project::update(&projects::UpdateRequest {
-        id: *project_id,
-        ..Default::default()
-    })
+    gitbutler_project::update_with_path(
+        data_dir.as_ref().unwrap(),
+        &projects::UpdateRequest {
+            id: *project_id,
+            ..Default::default()
+        },
+    )
     .unwrap();
 
     let stack_entry = gitbutler_branch_actions::create_virtual_branch(
diff --git a/crates/gitbutler-cli/src/command/project.rs b/crates/gitbutler-cli/src/command/project.rs
@@ -20,13 +20,13 @@ pub fn list() -> Result<()> {
     Ok(())
 }
 
-pub fn add(path: PathBuf, refname: Option<RemoteRefname>) -> Result<()> {
+pub fn add(data_dir: PathBuf, path: PathBuf, refname: Option<RemoteRefname>) -> Result<()> {
     let path = gix::discover(path)?
         .workdir()
         .context("Only non-bare repositories can be added")?
         .to_owned()
         .canonicalize()?;
-    let project = gitbutler_project::add(path, None, None)?;
+    let project = gitbutler_project::add_with_path(data_dir, path, None, None)?;
     let ctx = CommandContext::open(&project, AppSettings::default())?;
     if let Some(refname) = refname {
         gitbutler_branch_actions::set_base_branch(
diff --git a/crates/gitbutler-cli/src/main.rs b/crates/gitbutler-cli/src/main.rs
@@ -1,4 +1,6 @@
-use anyhow::Result;
+use std::path::PathBuf;
+
+use anyhow::{bail, Context, Result};
 
 mod args;
 use args::Args;
@@ -58,8 +60,8 @@ fn main() -> Result<()> {
             }
         }
         args::Subcommands::Project(project::Platform {
-            app_data_dir: _,
-            app_suffix: _,
+            app_data_dir,
+            app_suffix,
             cmd,
         }) => match cmd {
             Some(project::SubCommands::SwitchToWorkspace { remote_ref_name }) => {
@@ -69,11 +71,42 @@ fn main() -> Result<()> {
             Some(project::SubCommands::Add {
                 switch_to_workspace,
                 path,
-            }) => command::project::add(path, switch_to_workspace),
+            }) => command::project::add(
+                data_dir(app_suffix, app_data_dir)?,
+                path,
+                switch_to_workspace,
+            ),
             None => command::project::list(),
         },
     }
 }
+pub fn data_dir(
+    app_suffix: Option<String>,
+    app_data_dir: Option<PathBuf>,
+) -> anyhow::Result<PathBuf> {
+    let path = if let Some(dir) = app_data_dir {
+        std::fs::create_dir_all(&dir).context("Failed to assure the designated data-dir exists")?;
+        dir
+    } else {
+        dirs_next::data_dir()
+            .map(|dir| {
+                dir.join(format!(
+                    "com.gitbutler.app{}",
+                    app_suffix
+                        .map(|mut suffix| {
+                            suffix.insert(0, '.');
+                            suffix
+                        })
+                        .unwrap_or_default()
+                ))
+            })
+            .context("no data-directory available on this platform")?
+    };
+    if !path.is_dir() {
+        bail!("Path '{}' must be a valid directory", path.display());
+    }
+    Ok(path)
+}
 
 mod trace {
     use tracing::metadata::LevelFilter;
diff --git a/crates/gitbutler-project/src/lib.rs b/crates/gitbutler-project/src/lib.rs
@@ -30,6 +30,12 @@ pub fn get(id: ProjectId) -> anyhow::Result<Project> {
     controller.get(id)
 }
 
+/// Testing purpose only.
+pub fn get_with_path<P: AsRef<Path>>(data_dir: P, id: ProjectId) -> anyhow::Result<Project> {
+    let controller = Controller::from_path(data_dir.as_ref());
+    controller.get(id)
+}
+
 pub fn get_validated(id: ProjectId) -> anyhow::Result<Project> {
     let controller = Controller::from_path(but_path::app_data_dir()?);
     controller.get_validated(id)
@@ -45,6 +51,15 @@ pub fn update(project: &UpdateRequest) -> anyhow::Result<Project> {
     controller.update(project)
 }
 
+/// Testing purpose only.
+pub fn update_with_path<P: AsRef<Path>>(
+    data_dir: P,
+    project: &UpdateRequest,
+) -> anyhow::Result<Project> {
+    let controller = Controller::from_path(data_dir.as_ref());
+    controller.update(project)
+}
+
 pub fn add<P: AsRef<Path>>(
     path: P,
     name: Option<String>,
@@ -54,6 +69,17 @@ pub fn add<P: AsRef<Path>>(
     controller.add(path, name, email)
 }
 
+/// Testing purpose only.
+pub fn add_with_path<P: AsRef<Path>>(
+    data_dir: P,
+    path: P,
+    name: Option<String>,
+    email: Option<String>,
+) -> anyhow::Result<Project> {
+    let controller = Controller::from_path(data_dir.as_ref());
+    controller.add(path, name, email)
+}
+
 pub fn list() -> anyhow::Result<Vec<Project>> {
     let controller = Controller::from_path(but_path::app_data_dir()?);
     controller.list()
@@ -64,6 +90,12 @@ pub fn delete(id: ProjectId) -> anyhow::Result<()> {
     controller.delete(id)
 }
 
+/// Testing purpose only.
+pub fn delete_with_path<P: AsRef<Path>>(data_dir: P, id: ProjectId) -> anyhow::Result<()> {
+    let controller = Controller::from_path(data_dir.as_ref());
+    controller.delete(id)
+}
+
 pub fn assure_app_can_startup_or_fix_it(
     projects: anyhow::Result<Vec<Project>>,
 ) -> anyhow::Result<Vec<Project>> {
diff --git a/crates/gitbutler-project/tests/projects/main.rs b/crates/gitbutler-project/tests/projects/main.rs
@@ -13,8 +13,7 @@ mod add {
         let tmp = paths::data_dir();
         let repository = gitbutler_testsupport::TestProject::default();
         let path = repository.path();
-        gitbutler_testsupport::set_test_data_dir(tmp.path());
-        let project = gitbutler_project::add(path, None, None).unwrap();
+        let project = gitbutler_project::add_with_path(tmp.path(), path, None, None).unwrap();
         assert_eq!(project.path, path);
         assert_eq!(
             project.title,
@@ -30,8 +29,8 @@ mod add {
         fn non_bare_without_worktree() {
             let tmp = paths::data_dir();
             let root = repo_path_at("non-bare-without-worktree");
-            gitbutler_testsupport::set_test_data_dir(tmp.path());
-            let err = gitbutler_project::add(root.as_path(), None, None).unwrap_err();
+            let err = gitbutler_project::add_with_path(tmp.path(), root.as_path(), None, None)
+                .unwrap_err();
             assert_eq!(
                 err.to_string(),
                 "Cannot add non-bare repositories without a workdir"
@@ -42,8 +41,8 @@ mod add {
         fn submodule() {
             let tmp = paths::data_dir();
             let root = repo_path_at("with-submodule").join("submodule");
-            gitbutler_testsupport::set_test_data_dir(tmp.path());
-            let err = gitbutler_project::add(root.as_path(), None, None).unwrap_err();
+            let err = gitbutler_project::add_with_path(tmp.path(), root.as_path(), None, None)
+                .unwrap_err();
             assert_eq!(
                 err.to_string(),
                 "A git-repository without a `.git` directory cannot currently be added"
@@ -54,11 +53,15 @@ mod add {
         fn missing() {
             let data_dir = paths::data_dir();
             let tmp = tempfile::tempdir().unwrap();
-            gitbutler_testsupport::set_test_data_dir(data_dir.path());
             assert_eq!(
-                gitbutler_project::add(tmp.path().join("missing"), None, None)
-                    .unwrap_err()
-                    .to_string(),
+                gitbutler_project::add_with_path(
+                    data_dir.path(),
+                    &tmp.path().join("missing"),
+                    None,
+                    None
+                )
+                .unwrap_err()
+                .to_string(),
                 "path not found"
             );
         }
@@ -69,9 +72,8 @@ mod add {
             let tmp = tempfile::tempdir().unwrap();
             let path = tmp.path();
             std::fs::write(path.join("file.txt"), "hello world").unwrap();
-            gitbutler_testsupport::set_test_data_dir(data_dir.path());
             assert_eq!(
-                gitbutler_project::add(path, None, None)
+                gitbutler_project::add_with_path(data_dir.path(), path, None, None)
                     .unwrap_err()
                     .to_string(),
                 "must be a Git repository"
@@ -82,8 +84,8 @@ mod add {
         fn empty() {
             let data_dir = paths::data_dir();
             let tmp = tempfile::tempdir().unwrap();
-            gitbutler_testsupport::set_test_data_dir(data_dir.path());
-            let err = gitbutler_project::add(tmp.path(), None, None).unwrap_err();
+            let err = gitbutler_project::add_with_path(data_dir.path(), tmp.path(), None, None)
+                .unwrap_err();
             assert_eq!(err.to_string(), "must be a Git repository");
         }
 
@@ -92,10 +94,9 @@ mod add {
             let data_dir = paths::data_dir();
             let repository = gitbutler_testsupport::TestProject::default();
             let path = repository.path();
-            gitbutler_testsupport::set_test_data_dir(data_dir.path());
-            gitbutler_project::add(path, None, None).unwrap();
+            gitbutler_project::add_with_path(data_dir.path(), path, None, None).unwrap();
             assert_eq!(
-                gitbutler_project::add(path, None, None)
+                gitbutler_project::add_with_path(data_dir.path(), path, None, None)
                     .unwrap_err()
                     .to_string(),
                 "project already exists"
@@ -111,8 +112,9 @@ mod add {
             let repo = git2::Repository::init_bare(&repo_dir).unwrap();
             create_initial_commit(&repo);
 
-            gitbutler_testsupport::set_test_data_dir(data_dir.path());
-            let err = gitbutler_project::add(repo_dir.as_path(), None, None).unwrap_err();
+            let err =
+                gitbutler_project::add_with_path(data_dir.path(), repo_dir.as_path(), None, None)
+                    .unwrap_err();
             assert_eq!(err.to_string(), "bare repositories are unsupported");
         }
 
@@ -127,8 +129,9 @@ mod add {
             create_initial_commit(&repo);
 
             let worktree = repo.worktree("feature", &worktree_dir, None).unwrap();
-            gitbutler_testsupport::set_test_data_dir(data_dir.path());
-            let err = gitbutler_project::add(worktree.path(), None, None).unwrap_err();
+            let err =
+                gitbutler_project::add_with_path(data_dir.path(), worktree.path(), None, None)
+                    .unwrap_err();
             assert_eq!(err.to_string(), "can only work in main worktrees");
         }
 
@@ -164,13 +167,12 @@ mod delete {
     #[test]
     fn success() {
         let data_dir = paths::data_dir();
-        gitbutler_testsupport::set_test_data_dir(&data_dir);
         let repository = gitbutler_testsupport::TestProject::default();
         let path = repository.path();
-        let project = gitbutler_project::add(path, None, None).unwrap();
-        assert!(gitbutler_project::delete(project.id).is_ok());
-        assert!(gitbutler_project::delete(project.id).is_ok()); // idempotent
-        assert!(gitbutler_project::get(project.id).is_err());
+        let project = gitbutler_project::add_with_path(data_dir.path(), path, None, None).unwrap();
+        assert!(gitbutler_project::delete_with_path(data_dir.path(), project.id).is_ok());
+        assert!(gitbutler_project::delete_with_path(data_dir.path(), project.id).is_ok()); // idempotent
+        assert!(gitbutler_project::get_with_path(data_dir.path(), project.id).is_err());
         assert!(!project.gb_dir().exists());
     }
 }
diff --git a/crates/gitbutler-testsupport/src/lib.rs b/crates/gitbutler-testsupport/src/lib.rs
diff --git a/crates/gitbutler-testsupport/src/suite.rs b/crates/gitbutler-testsupport/src/suite.rs
