Skip to content

test(skills): stop leaking TREQ_APP_DATA_DIR into parallel tests #694

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Ziinc merged 3 commits into from
Oct 3, 2026
Merged

test(skills): stop leaking TREQ_APP_DATA_DIR into parallel tests #694

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
1216a1c
test(skills): stop leaking TREQ_APP_DATA_DIR into parallel tests
claude Sep 30, 2026
a40f701
test(skills): keep the test override comment on one line
claude Sep 30, 2026
89dbd89
test(core): resolve app db path tests without mutating process env
claude Oct 3, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
68 changes: 41 additions & 27 deletions src-tauri/src/core/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,56 +71,70 @@ pub fn resolve_conflict_marker_style(db: &std::sync::Mutex<crate::db::Database>)
}

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<String>) -> 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<Option<PathBuf>> =
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<PathBuf> {
app_data_dir_with(|key| std::env::var(key).ok())
}

fn env_lock() -> &'static Mutex<()> {
static LOCK: OnceLock<Mutex<()>> = OnceLock::new();
LOCK.get_or_init(|| Mutex::new(()))
fn app_data_dir_with(env: impl Fn(&str) -> Option<String>) -> Option<PathBuf> {
#[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"));
}
}
47 changes: 19 additions & 28 deletions src-tauri/src/core/skills.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -133,12 +133,9 @@ pub fn read_bytes_from_locator(locator: &str) -> Result<Vec<u8>, String> {
}

pub fn app_skills_root() -> Result<PathBuf, String> {
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 {
Expand Down Expand Up @@ -551,25 +548,19 @@ mod tests {
}
}

struct EnvGuard {
key: &'static str,
previous: Option<String>,
}
/// 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);
}
}

Expand All @@ -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)
Expand All @@ -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;
Expand All @@ -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());
Expand All @@ -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(
Expand All @@ -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");
Expand All @@ -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");
Expand Down Expand Up @@ -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");
Expand Down Expand Up @@ -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");
Expand Down
Loading