diff --git a/src-tauri/src/core/mod.rs b/src-tauri/src/core/mod.rs index 0db008c3..554f650d 100644 --- a/src-tauri/src/core/mod.rs +++ b/src-tauri/src/core/mod.rs @@ -71,56 +71,70 @@ pub fn resolve_conflict_marker_style(db: &std::sync::Mutex) } pub fn resolve_app_db_path(repo_path: &str) -> PathBuf { - if let Ok(explicit_db_path) = std::env::var("TREQ_APP_DB_PATH") { + resolve_app_db_path_with(repo_path, |key| std::env::var(key).ok()) +} + +/// `resolve_app_db_path` reading env vars through `env`, so tests need not mutate the process env. +fn resolve_app_db_path_with(repo_path: &str, env: impl Fn(&str) -> Option) -> PathBuf { + if let Some(explicit_db_path) = env("TREQ_APP_DB_PATH") { let trimmed = explicit_db_path.trim(); if !trimmed.is_empty() { return PathBuf::from(trimmed); } } - if let Ok(app_data_dir) = std::env::var("TREQ_APP_DATA_DIR") { - let trimmed = app_data_dir.trim(); - if !trimmed.is_empty() { - return Path::new(trimmed).join("treq.db"); - } + if let Some(app_data_dir) = app_data_dir_with(env) { + return app_data_dir.join("treq.db"); } Path::new(repo_path).join(".treq").join("treq.db") } #[cfg(test)] -mod tests { - use super::resolve_app_db_path; - use std::sync::{Mutex, OnceLock}; +thread_local! { + /// Per-thread app data dir for tests; the env var would leak across parallel tests. + pub(crate) static TEST_APP_DATA_DIR: std::cell::RefCell> = + const { std::cell::RefCell::new(None) }; +} + +/// The app data dir from `TREQ_APP_DATA_DIR`, or a test's per-thread override. +pub(crate) fn app_data_dir() -> Option { + app_data_dir_with(|key| std::env::var(key).ok()) +} - fn env_lock() -> &'static Mutex<()> { - static LOCK: OnceLock> = OnceLock::new(); - LOCK.get_or_init(|| Mutex::new(())) +fn app_data_dir_with(env: impl Fn(&str) -> Option) -> Option { + #[cfg(test)] + if let Some(dir) = TEST_APP_DATA_DIR.with(|dir| dir.borrow().clone()) { + return Some(dir); } + env("TREQ_APP_DATA_DIR") + .map(|dir| dir.trim().to_string()) + .filter(|dir| !dir.is_empty()) + .map(PathBuf::from) +} + +#[cfg(test)] +mod tests { + use super::resolve_app_db_path_with; + use std::path::Path; #[test] fn resolve_app_db_path_prefers_explicit_db_path() { - let _guard = env_lock().lock().unwrap(); - std::env::set_var("TREQ_APP_DB_PATH", "/tmp/explicit-treq.db"); - std::env::set_var("TREQ_APP_DATA_DIR", "/tmp/ignored-dir"); + let env = |key: &str| match key { + "TREQ_APP_DB_PATH" => Some("/tmp/explicit-treq.db".to_string()), + "TREQ_APP_DATA_DIR" => Some("/tmp/ignored-dir".to_string()), + _ => None, + }; - let resolved = resolve_app_db_path("/repo/path"); + let resolved = resolve_app_db_path_with("/repo/path", env); assert_eq!(resolved.to_string_lossy(), "/tmp/explicit-treq.db"); - - std::env::remove_var("TREQ_APP_DB_PATH"); - std::env::remove_var("TREQ_APP_DATA_DIR"); } #[test] fn resolve_app_db_path_falls_back_to_app_data_dir() { - let _guard = env_lock().lock().unwrap(); - std::env::remove_var("TREQ_APP_DB_PATH"); - std::env::set_var("TREQ_APP_DATA_DIR", "/tmp/app-data"); - - let resolved = resolve_app_db_path("/repo/path"); - let expected = std::path::Path::new("/tmp/app-data").join("treq.db"); - assert_eq!(resolved, expected); + let env = |key: &str| (key == "TREQ_APP_DATA_DIR").then(|| "/tmp/app-data".to_string()); - std::env::remove_var("TREQ_APP_DATA_DIR"); + let resolved = resolve_app_db_path_with("/repo/path", env); + assert_eq!(resolved, Path::new("/tmp/app-data").join("treq.db")); } } diff --git a/src-tauri/src/core/skills.rs b/src-tauri/src/core/skills.rs index e4ca521a..791952f6 100644 --- a/src-tauri/src/core/skills.rs +++ b/src-tauri/src/core/skills.rs @@ -133,12 +133,9 @@ pub fn read_bytes_from_locator(locator: &str) -> Result, String> { } pub fn app_skills_root() -> Result { - let dir = std::env::var("TREQ_APP_DATA_DIR") - .ok() - .map(|s| s.trim().to_string()) - .filter(|s| !s.is_empty()) - .ok_or_else(|| "TREQ_APP_DATA_DIR is not set".to_string())?; - Ok(PathBuf::from(dir).join("skills")) + let dir = + crate::core::app_data_dir().ok_or_else(|| "TREQ_APP_DATA_DIR is not set".to_string())?; + Ok(dir.join("skills")) } pub fn repo_skills_root(repo_path: &str) -> PathBuf { @@ -551,25 +548,19 @@ mod tests { } } - struct EnvGuard { - key: &'static str, - previous: Option, - } + /// Points this test thread's app data dir at `dir` until dropped. + struct AppDataDirGuard; - impl EnvGuard { - fn set(key: &'static str, value: &str) -> Self { - let previous = std::env::var(key).ok(); - std::env::set_var(key, value); - Self { key, previous } + impl AppDataDirGuard { + fn set(dir: &Path) -> Self { + crate::core::TEST_APP_DATA_DIR.with(|d| *d.borrow_mut() = Some(dir.to_path_buf())); + Self } } - impl Drop for EnvGuard { + impl Drop for AppDataDirGuard { fn drop(&mut self) { - match &self.previous { - Some(value) => std::env::set_var(self.key, value), - None => std::env::remove_var(self.key), - } + crate::core::TEST_APP_DATA_DIR.with(|d| *d.borrow_mut() = None); } } @@ -592,7 +583,7 @@ mod tests { #[test] fn install_skill_files_rejects_checksum_mismatch() { let app = TempDir::new().unwrap(); - let _guard = EnvGuard::set("TREQ_APP_DATA_DIR", app.path().to_str().unwrap()); + let _guard = AppDataDirGuard::set(app.path()); let files = sample_files(); let entry = sample_entry("deadbeef".to_string()); let err = install_skill_files(&entry, &files, SkillInstallScope::Application, None) @@ -603,7 +594,7 @@ mod tests { #[test] fn install_skill_files_records_computed_checksum_when_catalog_omits_it() { let app = TempDir::new().unwrap(); - let _guard = EnvGuard::set("TREQ_APP_DATA_DIR", app.path().to_str().unwrap()); + let _guard = AppDataDirGuard::set(app.path()); let files = sample_files(); let mut entry = sample_entry(skill_checksum(&files)); entry.checksum = None; @@ -615,7 +606,7 @@ mod tests { #[test] fn install_skill_files_writes_application_pack() { let app = TempDir::new().unwrap(); - let _guard = EnvGuard::set("TREQ_APP_DATA_DIR", app.path().to_str().unwrap()); + let _guard = AppDataDirGuard::set(app.path()); let files = sample_files(); let checksum = skill_checksum(&files); let entry = sample_entry(checksum.clone()); @@ -637,7 +628,7 @@ mod tests { fn install_skill_files_writes_repository_pack() { let app = TempDir::new().unwrap(); let repo = TempDir::new().unwrap(); - let _guard = EnvGuard::set("TREQ_APP_DATA_DIR", app.path().to_str().unwrap()); + let _guard = AppDataDirGuard::set(app.path()); let files = sample_files(); let entry = sample_entry(skill_checksum(&files)); install_skill_files( @@ -664,7 +655,7 @@ mod tests { fn set_skill_install_scope_moves_pack() { let app = TempDir::new().unwrap(); let repo = TempDir::new().unwrap(); - let _guard = EnvGuard::set("TREQ_APP_DATA_DIR", app.path().to_str().unwrap()); + let _guard = AppDataDirGuard::set(app.path()); let files = sample_files(); let entry = sample_entry(skill_checksum(&files)); install_skill_files(&entry, &files, SkillInstallScope::Application, None).expect("install"); @@ -687,7 +678,7 @@ mod tests { let app = TempDir::new().unwrap(); let repo = TempDir::new().unwrap(); let workspace = TempDir::new().unwrap(); - let _guard = EnvGuard::set("TREQ_APP_DATA_DIR", app.path().to_str().unwrap()); + let _guard = AppDataDirGuard::set(app.path()); let files = sample_files(); let entry = sample_entry(skill_checksum(&files)); install_skill_files(&entry, &files, SkillInstallScope::Application, None).expect("install"); @@ -715,7 +706,7 @@ mod tests { let app = TempDir::new().unwrap(); let repo = TempDir::new().unwrap(); let workspace = TempDir::new().unwrap(); - let _guard = EnvGuard::set("TREQ_APP_DATA_DIR", app.path().to_str().unwrap()); + let _guard = AppDataDirGuard::set(app.path()); let files = sample_files(); let entry = sample_entry(skill_checksum(&files)); install_skill_files(&entry, &files, SkillInstallScope::Application, None).expect("install"); @@ -747,7 +738,7 @@ mod tests { #[test] fn merge_catalog_marks_installed_skills() { let app = TempDir::new().unwrap(); - let _guard = EnvGuard::set("TREQ_APP_DATA_DIR", app.path().to_str().unwrap()); + let _guard = AppDataDirGuard::set(app.path()); let files = sample_files(); let entry = sample_entry(skill_checksum(&files)); install_skill_files(&entry, &files, SkillInstallScope::Application, None).expect("install");