From d25ca950d79de5ab7fa699a05acdea60d23937bc Mon Sep 17 00:00:00 2001 From: Jacob Stephens Date: Mon, 26 Oct 2026 17:14:48 -0400 Subject: [PATCH 1/3] Isolate unit-test Git fixtures from host config (#1392) --- src/git.rs | 14 ++++--- src/launch.rs | 6 +-- src/session/execution_tests.rs | 4 +- src/test_support.rs | 13 +++++- src/worktree.rs | 6 +-- src/worktree/synchronization_tests.rs | 57 +++++++++++++++++++++++++++ 6 files changed, 85 insertions(+), 15 deletions(-) diff --git a/src/git.rs b/src/git.rs index 34a7801f..b4cca10c 100644 --- a/src/git.rs +++ b/src/git.rs @@ -2,7 +2,7 @@ use std::fs::{File, TryLockError}; use std::path::{Path, PathBuf}; -use std::process::{Command, Output}; +use std::process::Output; use std::time::{Duration, Instant}; use anyhow::{Context, Result, bail}; @@ -304,8 +304,12 @@ impl Git { } fn output(&self, args: &[&str]) -> Result { + #[cfg(not(test))] + let mut command = std::process::Command::new("git"); + #[cfg(test)] + let mut command = crate::test_support::git_command(); process::output( - Command::new("git").args(args).current_dir(&self.dir), + command.args(args).current_dir(&self.dir), None, Control { name: &format!("git {}", args.join(" ")), @@ -419,11 +423,9 @@ mod tests { pub(super) fn repo_with_origin() -> (tempfile::TempDir, Git) { let temp = tempfile::TempDir::new().unwrap(); let isolated = |dir: &Path, args: &[&str]| { - let status = Command::new("git") + let status = crate::test_support::git_command() .args(args) .current_dir(dir) - .env("GIT_CONFIG_GLOBAL", "/dev/null") - .env("GIT_CONFIG_NOSYSTEM", "1") .status() .unwrap(); assert!(status.success(), "git {args:?}"); @@ -434,6 +436,8 @@ mod tests { for (key, value) in [ ("user.name", "Test Runner"), ("user.email", "runner@example.com"), + ("commit.gpgsign", "false"), + ("tag.gpgsign", "false"), ("remote.origin.url", "../origin.git"), ] { isolated(&work, &["config", key, value]); diff --git a/src/launch.rs b/src/launch.rs index efa9a2d2..f135f579 100644 --- a/src/launch.rs +++ b/src/launch.rs @@ -456,6 +456,8 @@ mod tests { for (key, value) in [ ("user.name", "Test Runner"), ("user.email", "runner@example.com"), + ("commit.gpgsign", "false"), + ("tag.gpgsign", "false"), ("remote.origin.url", "https://github.com/acme/widgets.git"), ("remote.origin.fetch", "+refs/heads/*:refs/remotes/origin/*"), ( @@ -471,11 +473,9 @@ mod tests { } fn isolated(dir: &Path, args: &[&str]) -> String { - let output = Command::new("git") + let output = crate::test_support::git_command() .args(args) .current_dir(dir) - .env("GIT_CONFIG_GLOBAL", "/dev/null") - .env("GIT_CONFIG_NOSYSTEM", "1") .output() .unwrap(); assert!(output.status.success(), "git {args:?}"); diff --git a/src/session/execution_tests.rs b/src/session/execution_tests.rs index 8a4af382..33a2663f 100644 --- a/src/session/execution_tests.rs +++ b/src/session/execution_tests.rs @@ -105,7 +105,7 @@ fn execute_session(root: &Path) -> anyhow::Error { let worktree = root.join("worktree"); fs::create_dir(&worktree).unwrap(); assert!( - Command::new("git") + crate::test_support::git_command() .args(["init", "-q"]) .arg(&worktree) .status() @@ -368,7 +368,7 @@ fn with_review_sessions( let worktree = root.join("worktree"); fs::create_dir(&worktree).unwrap(); assert!( - Command::new("git") + crate::test_support::git_command() .args(["init", "-q"]) .arg(&worktree) .status() diff --git a/src/test_support.rs b/src/test_support.rs index c8d0d059..ce2f398a 100644 --- a/src/test_support.rs +++ b/src/test_support.rs @@ -11,6 +11,17 @@ use std::process::Command; use crate::interrupt; +/// Git for unit-test fixtures, including commands made after repository setup. +/// Keep this on each child Command so parallel tests never change the process +/// environment and new Git views of a fixture inherit the same isolation. +pub(crate) fn git_command() -> Command { + let mut command = Command::new("git"); + command + .env("GIT_CONFIG_GLOBAL", "/dev/null") + .env("GIT_CONFIG_NOSYSTEM", "1"); + command +} + /// A child that announces Session s1 after writing a native Codex quota record. /// The record may deliberately name another Session/directory or a stale time; /// `current` writes a fresh timestamp. HOME is set only on the child Command. @@ -44,7 +55,7 @@ touch "$HOME/../continued" pub(crate) fn scratch_worktree() -> tempfile::TempDir { let worktree = tempfile::tempdir().unwrap(); assert!( - Command::new("git") + git_command() .args(["init", "-q"]) .arg(worktree.path()) .status() diff --git a/src/worktree.rs b/src/worktree.rs index 2cee57a7..e49a86f0 100644 --- a/src/worktree.rs +++ b/src/worktree.rs @@ -529,7 +529,6 @@ fn lock_launch(launch: &Git) -> Result { #[cfg(test)] mod tests { - use std::process::Command; use std::time::Duration; use super::*; @@ -539,11 +538,9 @@ mod tests { pub(super) fn launch_directory() -> (tempfile::TempDir, Git) { let temp = tempfile::TempDir::new().unwrap(); let git = |dir: &Path, args: &[&str]| { - let status = Command::new("git") + let status = crate::test_support::git_command() .args(args) .current_dir(dir) - .env("GIT_CONFIG_GLOBAL", "/dev/null") - .env("GIT_CONFIG_NOSYSTEM", "1") .status() .unwrap(); assert!(status.success(), "git {args:?}"); @@ -568,6 +565,7 @@ mod tests { ("user.name", "Test Runner"), ("user.email", "runner@example.com"), ("commit.gpgSign", "false"), + ("tag.gpgsign", "false"), ("core.hooksPath", hooks.to_str().unwrap()), ] { git(&work, &["config", key, value]); diff --git a/src/worktree/synchronization_tests.rs b/src/worktree/synchronization_tests.rs index 34ed995c..107faffc 100644 --- a/src/worktree/synchronization_tests.rs +++ b/src/worktree/synchronization_tests.rs @@ -1060,6 +1060,63 @@ fn base_merges_ignore_shadowing_branch_and_tag_names() { } } +#[test] +fn git_fixtures_ignore_global_and_system_config() { + const CHILD: &str = "THIRDSHIFT_TEST_HOST_GIT_CONFIG"; + if std::env::var_os(CHILD).is_some() { + base_merges_ignore_shadowing_branch_and_tag_names(); + foreign_observation_and_merge_ignore_shadowing_branch_and_tag_names(); + spec_catch_up_ignores_shadowing_branch_and_tag_names(); + + let (_temp, launch, worktree) = fixture(); + for git in [launch, Git::new(worktree.path()).completion()] { + for scope in ["global", "system"] { + assert!( + !git.succeeds(&["config", "--get", &format!("fixture.{scope}")]) + .unwrap(), + "fixture read {scope} config" + ); + } + git.run(&["config", "fixture.local", "visible"]).unwrap(); + assert_eq!( + git.run(&["config", "--get", "fixture.local"]).unwrap(), + "visible" + ); + } + return; + } + + let temp = tempfile::tempdir().unwrap(); + for scope in ["global", "system"] { + fs::write( + temp.path().join(scope), + format!( + "[tag]\n gpgsign = true\n[commit]\n gpgsign = true\n[fixture]\n {scope} = visible\n" + ), + ) + .unwrap(); + } + let output = std::process::Command::new(std::env::current_exe().unwrap()) + .args([ + "--exact", + "worktree::synchronization_tests::git_fixtures_ignore_global_and_system_config", + "--nocapture", + ]) + .env(CHILD, "1") + .env("GIT_CONFIG_GLOBAL", temp.path().join("global")) + .env("GIT_CONFIG_SYSTEM", temp.path().join("system")) + .env_remove("GIT_CONFIG_NOSYSTEM") + .env("GIT_EDITOR", "true") + .output() + .unwrap(); + assert!( + output.status.success(), + "{}\n{}", + String::from_utf8_lossy(&output.stdout), + String::from_utf8_lossy(&output.stderr) + ); +} + #[test] fn foreign_observation_and_merge_ignore_shadowing_branch_and_tag_names() { for kind in ["branch", "tag"] { From 059678fe487792af67f64246980bf80586191f3f Mon Sep 17 00:00:00 2001 From: Jacob Stephens Date: Mon, 26 Oct 2026 17:20:05 -0400 Subject: [PATCH 2/3] Isolate Git invoked by Session fixture scripts --- src/session/execution_tests.rs | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/src/session/execution_tests.rs b/src/session/execution_tests.rs index 33a2663f..c330dcfb 100644 --- a/src/session/execution_tests.rs +++ b/src/session/execution_tests.rs @@ -63,6 +63,9 @@ echo '{{"type":"step_start","sessionID":"s1","part":{{}}}}' command .args(["--exact", test, "--nocapture"]) .env(FIXTURE, self.0.path()) + // Fake Harness scripts also invoke Git directly. + .env("GIT_CONFIG_GLOBAL", "/dev/null") + .env("GIT_CONFIG_NOSYSTEM", "1") .env("PATH", path); command } @@ -440,6 +443,31 @@ fn keeps_review_reports_beside_each_harness_session_log() { } } +#[test] +fn shell_git_fixtures_ignore_global_and_system_config() { + let temp = tempfile::tempdir().unwrap(); + let config = temp.path().join("host.gitconfig"); + fs::write(&config, "[core]\n ignoreCase = invalid\n").unwrap(); + let output = Command::new(std::env::current_exe().unwrap()) + .args([ + "--exact", + "session::execution_tests::keeps_review_reports_beside_each_harness_session_log", + "--nocapture", + ]) + .env_remove(FIXTURE) + .env("GIT_CONFIG_GLOBAL", &config) + .env("GIT_CONFIG_SYSTEM", &config) + .env_remove("GIT_CONFIG_NOSYSTEM") + .output() + .unwrap(); + assert!( + output.status.success(), + "{}\n{}", + String::from_utf8_lossy(&output.stdout), + String::from_utf8_lossy(&output.stderr) + ); +} + #[test] fn missing_review_reports_get_a_progress_line_and_the_run_carries_on() { if let Some(root) = std::env::var_os(FIXTURE) { From c6c826321ed1a795aaf3c6bfe5315a355a6f0790 Mon Sep 17 00:00:00 2001 From: Jacob Stephens Date: Mon, 26 Oct 2026 17:22:52 -0400 Subject: [PATCH 3/3] Share fixture Git configuration policy --- src/session/execution_tests.rs | 4 +--- src/test_support.rs | 7 ++++++- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/src/session/execution_tests.rs b/src/session/execution_tests.rs index c330dcfb..a5c212e0 100644 --- a/src/session/execution_tests.rs +++ b/src/session/execution_tests.rs @@ -63,10 +63,8 @@ echo '{{"type":"step_start","sessionID":"s1","part":{{}}}}' command .args(["--exact", test, "--nocapture"]) .env(FIXTURE, self.0.path()) - // Fake Harness scripts also invoke Git directly. - .env("GIT_CONFIG_GLOBAL", "/dev/null") - .env("GIT_CONFIG_NOSYSTEM", "1") .env("PATH", path); + crate::test_support::isolate_git_config(&mut command); command } diff --git a/src/test_support.rs b/src/test_support.rs index ce2f398a..fb9e5dd5 100644 --- a/src/test_support.rs +++ b/src/test_support.rs @@ -16,10 +16,15 @@ use crate::interrupt; /// environment and new Git views of a fixture inherit the same isolation. pub(crate) fn git_command() -> Command { let mut command = Command::new("git"); + isolate_git_config(&mut command); + command +} + +/// Also cover Git invoked by shell scripts in a fixture subprocess. +pub(crate) fn isolate_git_config(command: &mut Command) { command .env("GIT_CONFIG_GLOBAL", "/dev/null") .env("GIT_CONFIG_NOSYSTEM", "1"); - command } /// A child that announces Session s1 after writing a native Codex quota record.