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..a5c212e0 100644 --- a/src/session/execution_tests.rs +++ b/src/session/execution_tests.rs @@ -64,6 +64,7 @@ echo '{{"type":"step_start","sessionID":"s1","part":{{}}}}' .args(["--exact", test, "--nocapture"]) .env(FIXTURE, self.0.path()) .env("PATH", path); + crate::test_support::isolate_git_config(&mut command); command } @@ -105,7 +106,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 +369,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() @@ -440,6 +441,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) { diff --git a/src/test_support.rs b/src/test_support.rs index c8d0d059..fb9e5dd5 100644 --- a/src/test_support.rs +++ b/src/test_support.rs @@ -11,6 +11,22 @@ 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"); + 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"); +} + /// 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 +60,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"] {