test(desktop): keep persistence tests out of the real user config

Desktop tests exercised agent_connect_store against the real
~/.openpencil, so a parallel test run could leave the user's actual
reconnect list in whichever state the last writer happened to save.
Add a process-level user-root override to op-config-store (a OnceLock
seam, not an env var — set_var races parallel test threads) and a
guard at the desktop store entry points that redirects every test
write to a per-pid scratch root, so isolation does not depend on
which test runs first.
This commit is contained in:
Fini 2026-07-28 00:29:39 +08:00
parent e6667b77c1
commit 864753b950
5 changed files with 97 additions and 0 deletions

View file

@ -7,6 +7,11 @@ use serde::{de::DeserializeOwned, Serialize};
use std::ffi::OsString;
use std::io::{Error, ErrorKind, Write};
use std::path::{Component, Path, PathBuf};
use std::sync::OnceLock;
/// Process-wide replacement for the `~/.openpencil` root. See
/// [`redirect_user_root_for_tests`].
static USER_ROOT_OVERRIDE: OnceLock<PathBuf> = OnceLock::new();
/// Name of the per-user config directory under the home directory
/// (`~/.openpencil`). Exposed for callers that must resolve the
@ -105,7 +110,34 @@ pub fn write_json_path<T: Serialize>(path: &Path, value: &T) -> std::io::Result<
Ok(())
}
/// Redirect [`ConfigStore::user`] — and everything built on it
/// (`read_json` / `write_json` / [`openpencil_dir`]) — at `root` for the
/// remainder of THIS PROCESS.
///
/// Exists so a test binary can point the whole crate graph at a scratch
/// directory instead of the developer's real `~/.openpencil`. Without it
/// a test that exercises a persistence path rewrites live user config:
/// `cargo test -p op-host-desktop` was measured clobbering
/// `agents.json`, the file that decides which agent providers
/// auto-reconnect at launch, with whichever value the last parallel test
/// happened to write.
///
/// Deliberately NOT an environment variable. The harness runs cases in
/// parallel threads of a single process, where `set_var` races every
/// concurrent reader — and is `unsafe` on current Rust besides.
///
/// First call wins and later calls are no-ops, so every test can install
/// the same root unconditionally with no ordering rules between them.
/// Returns the root actually in force.
#[doc(hidden)]
pub fn redirect_user_root_for_tests(root: impl Into<PathBuf>) -> &'static Path {
USER_ROOT_OVERRIDE.get_or_init(|| root.into()).as_path()
}
fn default_openpencil_dir() -> std::io::Result<PathBuf> {
if let Some(root) = USER_ROOT_OVERRIDE.get() {
return Ok(root.clone());
}
home_dir().map(|home| home.join(OPENPENCIL_DIR_NAME))
}
@ -244,6 +276,29 @@ mod tests {
assert!(store.write_json("/tmp/bad.json", &json!({})).is_err());
}
#[test]
fn user_root_redirect_covers_the_process_level_helpers_and_wins_once() {
let root = temp_root("redirect");
assert_eq!(redirect_user_root_for_tests(&root), root);
assert_eq!(ConfigStore::user().unwrap().root(), root);
assert_eq!(openpencil_dir().unwrap(), root);
// The free helpers are what callers actually reach for, and they
// must land in the redirected root, not the real home directory.
write_json("probe.json", &json!({ "ok": true })).unwrap();
assert!(root.join("probe.json").is_file());
let back: Option<serde_json::Value> = read_json("probe.json").unwrap();
assert_eq!(back, Some(json!({ "ok": true })));
assert_ne!(root, home_dir().unwrap().join(".openpencil"));
// Idempotent: a second install does not move the root, so tests
// may all call it in any order.
assert_eq!(
redirect_user_root_for_tests(temp_root("redirect-again")),
root
);
}
#[test]
fn well_known_files_preserve_existing_names() {
assert_eq!(well_known::GIT_AUTH, "git-auth.json");

View file

@ -23,6 +23,7 @@ struct PersistedAgentConnections {
/// Persist the currently-connected provider ids. Best-effort: a failed
/// write must never break the probe flow.
pub(crate) fn save(connected: &[bool; 6]) {
crate::test_config_root::guard_user_config();
let value = PersistedAgentConnections {
connected: AgentProvider::ALL
.iter()
@ -39,6 +40,7 @@ pub(crate) fn save(connected: &[bool; 6]) {
/// Providers remembered as connected from the previous session, in
/// `AgentProvider::ALL` order.
pub(crate) fn load() -> Vec<AgentProvider> {
crate::test_config_root::guard_user_config();
let Ok(Some(value)) = op_config_store::read_json::<PersistedAgentConnections>(FILE) else {
return Vec::new();
};

View file

@ -80,6 +80,7 @@ mod single_instance;
mod sub_agent_session;
mod sub_agent_spawn_error;
mod tcc_selftest;
mod test_config_root;
mod theme_preset_host;
mod ui_prefs;
mod update_check;

View file

@ -0,0 +1,37 @@
//! Keeps test runs off the user's real `~/.openpencil`.
//!
//! `agent_connect_store` and `ui_prefs` persist through `op_config_store`'s
//! process-level helpers, which resolve the real user directory. Exercising
//! either from a test therefore rewrites live config — measured: a
//! `cargo test -p op-host-desktop` run rewrote `~/.openpencil/agents.json`,
//! the file that decides which agent providers auto-reconnect at launch,
//! and since the harness runs cases in parallel whichever one finished last
//! decided its contents.
//!
//! The guard sits at the persistence entry points rather than in a test
//! helper on purpose: a redirect installed by the tests themselves only
//! protects whatever runs after it, and nothing orders the harness's
//! threads. Guarding where the user root is resolved makes the isolation
//! independent of which test happens to run first.
/// Point the config store at a scratch directory the first time a desktop
/// persistence path runs inside a TEST binary.
///
/// Idempotent and shared process-wide (`redirect_user_root_for_tests` is
/// first-caller-wins), so every entry point may call it unconditionally.
/// One shared directory is fine because no test asserts on a stored file's
/// contents — they assert on in-memory state, and the only goal here is to
/// keep the writes off the user's real config.
#[cfg(test)]
pub(crate) fn guard_user_config() {
let root = std::env::temp_dir().join(format!(
"op-host-desktop-test-config-{}",
std::process::id()
));
op_config_store::redirect_user_root_for_tests(root);
}
/// Real builds must resolve the real `~/.openpencil` — compiles away.
#[cfg(not(test))]
#[inline]
pub(crate) fn guard_user_config() {}

View file

@ -12,6 +12,7 @@ struct UiPrefs {
}
pub(crate) fn save_pencil_cursor(style: PencilCursorStyle) {
crate::test_config_root::guard_user_config();
let value = UiPrefs {
pencil_cursor: Some(style.id().to_string()),
};
@ -21,6 +22,7 @@ pub(crate) fn save_pencil_cursor(style: PencilCursorStyle) {
}
pub(crate) fn load_pencil_cursor() -> Option<PencilCursorStyle> {
crate::test_config_root::guard_user_config();
let value: UiPrefs = op_config_store::read_json(FILE).ok().flatten()?;
PencilCursorStyle::from_id(value.pencil_cursor.as_deref()?)
}