mirror of
https://relay.ngit.dev/npub15qydau2hjma6ngxkl2cyar74wzyjshvl65za5k5rl69264ar2exs5cyejr/ngit-grasp.git
synced 2026-10-05 15:08:24 +00:00
test: survive hostile git config and non-reaping PID 1 in CI
CI runs the suite under a deliberately hostile global git configuration
and inside an act container whose PID 1 does not reap orphans. Seven
lib tests relied on a friendly environment and failed there:
Five tests built git fixtures with bare Command::new("git"), so the
hostile failing pre-commit hook (core.hooksPath) broke fixture commits;
most fixture steps also ignored exit status, so the tests failed later
at an unrelated assertion. Fixtures now go through
grasp_audit::git_command(), which masks user and system git config, and
every step asserts success with stderr in the failure message. The
now-redundant local gpgsign toggles are dropped: hermetic fixtures see
no signing config at all.
Two process-group tests probed descendant death with kill(pid, 0),
which still succeeds for an unreaped zombie. The group kill orphans the
descendant, and without a reaping ancestor it stays a zombie forever,
so the probes timed out. The probe now reads /proc/<pid>/stat and also
treats state Z as terminated; the group-kill behaviour under test is
unchanged.
Both failure modes were reproduced locally with the workflow's hostile
git config (via GIT_CONFIG_GLOBAL) and a non-reaping
PR_SET_CHILD_SUBREAPER wrapper, matching the CI panics exactly; with
these changes the seven tests pass under both conditions, and cargo
fmt, clippy -D warnings, the full workspace suite, and the grasp-audit
suite pass. Four tests/sync.rs timing flakes seen under full parallel
local load pass in isolation and predate this change; they are out of
scope.
This commit is contained in:
DanConwayDev
+34
-56
@@ -573,16 +573,30 @@ mod tests {
|
||||
use std::fs;
|
||||
use tempfile::TempDir;
|
||||
|
||||
/// Run one fixture git step hermetically (immune to ambient git
|
||||
/// configuration such as hooks, signing, and templates) and fail loudly
|
||||
/// on error instead of letting a later assertion fail obscurely.
|
||||
fn run_git(dir: Option<&Path>, args: &[&str]) -> String {
|
||||
let mut command = grasp_audit::git_command();
|
||||
if let Some(dir) = dir {
|
||||
command.current_dir(dir);
|
||||
}
|
||||
let output = command.args(args).output().unwrap();
|
||||
assert!(
|
||||
output.status.success(),
|
||||
"git {args:?} failed: {}",
|
||||
String::from_utf8_lossy(&output.stderr)
|
||||
);
|
||||
String::from_utf8_lossy(&output.stdout).trim().to_string()
|
||||
}
|
||||
|
||||
/// Create a test bare repository with optional commits
|
||||
fn create_test_repo() -> (TempDir, PathBuf) {
|
||||
let temp_dir = TempDir::new().unwrap();
|
||||
let repo_path = temp_dir.path().join("test.git");
|
||||
|
||||
// Initialize bare repository
|
||||
Command::new("git")
|
||||
.args(["init", "--bare", repo_path.to_str().unwrap()])
|
||||
.output()
|
||||
.unwrap();
|
||||
run_git(None, &["init", "--bare", repo_path.to_str().unwrap()]);
|
||||
|
||||
(temp_dir, repo_path)
|
||||
}
|
||||
@@ -594,76 +608,40 @@ mod tests {
|
||||
let bare_repo = temp_dir.path().join("test.git");
|
||||
|
||||
// Initialize bare repository
|
||||
Command::new("git")
|
||||
.args([
|
||||
run_git(
|
||||
None,
|
||||
&[
|
||||
"init",
|
||||
"--bare",
|
||||
"--initial-branch=main",
|
||||
bare_repo.to_str().unwrap(),
|
||||
])
|
||||
.output()
|
||||
.unwrap();
|
||||
],
|
||||
);
|
||||
|
||||
// Clone to working directory
|
||||
Command::new("git")
|
||||
.args([
|
||||
run_git(
|
||||
None,
|
||||
&[
|
||||
"clone",
|
||||
bare_repo.to_str().unwrap(),
|
||||
work_dir.to_str().unwrap(),
|
||||
])
|
||||
.output()
|
||||
.unwrap();
|
||||
],
|
||||
);
|
||||
|
||||
// Configure git for commits
|
||||
Command::new("git")
|
||||
.args(["config", "user.email", "test@test.com"])
|
||||
.current_dir(&work_dir)
|
||||
.output()
|
||||
.unwrap();
|
||||
Command::new("git")
|
||||
.args(["config", "user.name", "Test"])
|
||||
.current_dir(&work_dir)
|
||||
.output()
|
||||
.unwrap();
|
||||
// Disable GPG signing for tests (prevents yubikey prompts)
|
||||
Command::new("git")
|
||||
.args(["config", "commit.gpgsign", "false"])
|
||||
.current_dir(&work_dir)
|
||||
.output()
|
||||
.unwrap();
|
||||
Command::new("git")
|
||||
.args(["config", "tag.gpgsign", "false"])
|
||||
.current_dir(&work_dir)
|
||||
.output()
|
||||
.unwrap();
|
||||
run_git(Some(&work_dir), &["config", "user.email", "test@test.com"]);
|
||||
run_git(Some(&work_dir), &["config", "user.name", "Test"]);
|
||||
|
||||
// Create a file and commit
|
||||
fs::write(work_dir.join("README.md"), "# Test").unwrap();
|
||||
Command::new("git")
|
||||
.args(["add", "README.md"])
|
||||
.current_dir(&work_dir)
|
||||
.output()
|
||||
.unwrap();
|
||||
Command::new("git")
|
||||
.args(["commit", "-m", "Initial commit"])
|
||||
.current_dir(&work_dir)
|
||||
.output()
|
||||
.unwrap();
|
||||
run_git(Some(&work_dir), &["add", "README.md"]);
|
||||
run_git(Some(&work_dir), &["commit", "-m", "Initial commit"]);
|
||||
|
||||
// Get commit hash
|
||||
let output = Command::new("git")
|
||||
.args(["rev-parse", "HEAD"])
|
||||
.current_dir(&work_dir)
|
||||
.output()
|
||||
.unwrap();
|
||||
let commit_hash = String::from_utf8_lossy(&output.stdout).trim().to_string();
|
||||
let commit_hash = run_git(Some(&work_dir), &["rev-parse", "HEAD"]);
|
||||
|
||||
// Push to bare repo
|
||||
Command::new("git")
|
||||
.args(["push", "origin", "main"])
|
||||
.current_dir(&work_dir)
|
||||
.output()
|
||||
.unwrap();
|
||||
run_git(Some(&work_dir), &["push", "origin", "main"]);
|
||||
|
||||
(temp_dir, bare_repo, commit_hash)
|
||||
}
|
||||
|
||||
+29
-67
@@ -611,99 +611,61 @@ mod tests {
|
||||
// can_apply_state tests
|
||||
// =========================================================================
|
||||
|
||||
/// Run one fixture git step hermetically (immune to ambient git
|
||||
/// configuration such as hooks, signing, and templates) and fail loudly
|
||||
/// on error instead of letting a later assertion fail obscurely.
|
||||
fn run_git(dir: &std::path::Path, args: &[&str]) -> String {
|
||||
let output = grasp_audit::git_command()
|
||||
.current_dir(dir)
|
||||
.args(args)
|
||||
.output()
|
||||
.unwrap();
|
||||
assert!(
|
||||
output.status.success(),
|
||||
"git {args:?} failed: {}",
|
||||
String::from_utf8_lossy(&output.stderr)
|
||||
);
|
||||
String::from_utf8_lossy(&output.stdout).trim().to_string()
|
||||
}
|
||||
|
||||
/// Helper to create a temporary bare git repository with a commit.
|
||||
/// Returns (temp_dir, commit_hash) where commit_hash is Some if a commit was created.
|
||||
fn create_test_repo_with_commit() -> (tempfile::TempDir, Option<String>) {
|
||||
use std::process::Command;
|
||||
|
||||
let temp_dir = tempfile::tempdir().unwrap();
|
||||
let bare_path = temp_dir.path();
|
||||
|
||||
// Initialize bare repo
|
||||
Command::new("git")
|
||||
.args(["init", "--bare"])
|
||||
.current_dir(bare_path)
|
||||
.output()
|
||||
.expect("Failed to init bare git repo");
|
||||
run_git(bare_path, &["init", "--bare"]);
|
||||
|
||||
// Create a working repo to generate a commit
|
||||
let work_dir = tempfile::tempdir().unwrap();
|
||||
|
||||
Command::new("git")
|
||||
.args(["init"])
|
||||
.current_dir(work_dir.path())
|
||||
.output()
|
||||
.expect("Failed to init work repo");
|
||||
|
||||
Command::new("git")
|
||||
.args(["config", "user.email", "test@test.com"])
|
||||
.current_dir(work_dir.path())
|
||||
.output()
|
||||
.expect("Failed to set email");
|
||||
|
||||
Command::new("git")
|
||||
.args(["config", "user.name", "Test"])
|
||||
.current_dir(work_dir.path())
|
||||
.output()
|
||||
.expect("Failed to set name");
|
||||
|
||||
// Disable GPG signing for tests (prevents yubikey prompts)
|
||||
Command::new("git")
|
||||
.args(["config", "commit.gpgsign", "false"])
|
||||
.current_dir(work_dir.path())
|
||||
.output()
|
||||
.expect("Failed to disable commit.gpgsign");
|
||||
|
||||
Command::new("git")
|
||||
.args(["config", "tag.gpgsign", "false"])
|
||||
.current_dir(work_dir.path())
|
||||
.output()
|
||||
.expect("Failed to disable tag.gpgsign");
|
||||
run_git(work_dir.path(), &["init"]);
|
||||
run_git(work_dir.path(), &["config", "user.email", "test@test.com"]);
|
||||
run_git(work_dir.path(), &["config", "user.name", "Test"]);
|
||||
|
||||
// Create a commit
|
||||
std::fs::write(work_dir.path().join("file.txt"), "content").unwrap();
|
||||
Command::new("git")
|
||||
.args(["add", "."])
|
||||
.current_dir(work_dir.path())
|
||||
.output()
|
||||
.expect("Failed to add");
|
||||
|
||||
Command::new("git")
|
||||
.args(["commit", "-m", "test"])
|
||||
.current_dir(work_dir.path())
|
||||
.output()
|
||||
.expect("Failed to commit");
|
||||
run_git(work_dir.path(), &["add", "."]);
|
||||
run_git(work_dir.path(), &["commit", "-m", "test"]);
|
||||
|
||||
// Get the commit hash from the working repo
|
||||
let output = Command::new("git")
|
||||
.args(["rev-parse", "HEAD"])
|
||||
.current_dir(work_dir.path())
|
||||
.output()
|
||||
.expect("Failed to get commit hash");
|
||||
|
||||
let commit_hash = String::from_utf8_lossy(&output.stdout).trim().to_string();
|
||||
let commit_hash = run_git(work_dir.path(), &["rev-parse", "HEAD"]);
|
||||
|
||||
// Push to bare repo
|
||||
Command::new("git")
|
||||
.args(["push", bare_path.to_str().unwrap(), "HEAD:refs/heads/main"])
|
||||
.current_dir(work_dir.path())
|
||||
.output()
|
||||
.expect("Failed to push");
|
||||
run_git(
|
||||
work_dir.path(),
|
||||
&["push", bare_path.to_str().unwrap(), "HEAD:refs/heads/main"],
|
||||
);
|
||||
|
||||
(temp_dir, Some(commit_hash))
|
||||
}
|
||||
|
||||
/// Helper to create an empty bare git repository (no commits).
|
||||
fn create_empty_test_repo() -> tempfile::TempDir {
|
||||
use std::process::Command;
|
||||
|
||||
let temp_dir = tempfile::tempdir().unwrap();
|
||||
|
||||
Command::new("git")
|
||||
.args(["init", "--bare"])
|
||||
.current_dir(temp_dir.path())
|
||||
.output()
|
||||
.expect("Failed to init bare git repo");
|
||||
run_git(temp_dir.path(), &["init", "--bare"]);
|
||||
|
||||
temp_dir
|
||||
}
|
||||
|
||||
@@ -1232,39 +1232,62 @@ impl SyncContext for RealSyncContext {
|
||||
mod fetch_helper_tests {
|
||||
use super::*;
|
||||
|
||||
/// Hermetic git for these tests, immune to ambient git configuration
|
||||
/// such as hooks, signing, and templates.
|
||||
fn fixture_git() -> std::process::Command {
|
||||
grasp_audit::git_command()
|
||||
}
|
||||
|
||||
/// True once the descendant with this pid no longer runs. A process-group
|
||||
/// kill leaves the orphaned descendant as an unreaped zombie whenever no
|
||||
/// reaping ancestor is present (CI containers lack a reaping PID 1), and
|
||||
/// `kill(pid, 0)` still succeeds for zombies, so an unreaped zombie must
|
||||
/// also count as terminated.
|
||||
fn descendant_terminated(pid: i32) -> bool {
|
||||
match std::fs::read_to_string(format!("/proc/{pid}/stat")) {
|
||||
Err(_) => true,
|
||||
// The state field follows the parenthesised, possibly
|
||||
// space-containing command name.
|
||||
Ok(stat) => match stat.rsplit_once(") ") {
|
||||
Some((_, rest)) => rest.starts_with('Z'),
|
||||
None => false,
|
||||
},
|
||||
}
|
||||
}
|
||||
|
||||
fn create_source_repo(root: &Path, name: &str, contents: &str) -> (PathBuf, String) {
|
||||
let path = root.join(name);
|
||||
assert!(std::process::Command::new("git")
|
||||
assert!(fixture_git()
|
||||
.args(["init", "--quiet", path.to_str().unwrap()])
|
||||
.status()
|
||||
.unwrap()
|
||||
.success());
|
||||
assert!(std::process::Command::new("git")
|
||||
assert!(fixture_git()
|
||||
.current_dir(&path)
|
||||
.args(["config", "user.name", "Test"])
|
||||
.status()
|
||||
.unwrap()
|
||||
.success());
|
||||
assert!(std::process::Command::new("git")
|
||||
assert!(fixture_git()
|
||||
.current_dir(&path)
|
||||
.args(["config", "user.email", "test@example.com"])
|
||||
.status()
|
||||
.unwrap()
|
||||
.success());
|
||||
std::fs::write(path.join("payload"), contents).unwrap();
|
||||
assert!(std::process::Command::new("git")
|
||||
assert!(fixture_git()
|
||||
.current_dir(&path)
|
||||
.args(["add", "payload"])
|
||||
.status()
|
||||
.unwrap()
|
||||
.success());
|
||||
assert!(std::process::Command::new("git")
|
||||
assert!(fixture_git()
|
||||
.current_dir(&path)
|
||||
.args(["commit", "--quiet", "-m", "fixture"])
|
||||
.status()
|
||||
.unwrap()
|
||||
.success());
|
||||
let oid = std::process::Command::new("git")
|
||||
let oid = fixture_git()
|
||||
.current_dir(&path)
|
||||
.args(["rev-parse", "HEAD"])
|
||||
.output()
|
||||
@@ -1389,9 +1412,7 @@ mod fetch_helper_tests {
|
||||
let pid: i32 = pid.parse().unwrap();
|
||||
let deadline = tokio::time::Instant::now() + Duration::from_secs(1);
|
||||
loop {
|
||||
// Signal zero observes existence without changing process state.
|
||||
let alive = unsafe { libc::kill(pid, 0) } == 0;
|
||||
if !alive {
|
||||
if descendant_terminated(pid) {
|
||||
break;
|
||||
}
|
||||
assert!(
|
||||
@@ -1444,7 +1465,7 @@ mod fetch_helper_tests {
|
||||
|
||||
tokio::time::timeout(Duration::from_secs(1), async {
|
||||
loop {
|
||||
if unsafe { libc::kill(pid, 0) } != 0 {
|
||||
if descendant_terminated(pid) {
|
||||
break;
|
||||
}
|
||||
tokio::task::yield_now().await;
|
||||
@@ -1477,14 +1498,14 @@ mod fetch_helper_tests {
|
||||
let (first_source, first_oid) = create_source_repo(temp.path(), "first", "first");
|
||||
let (second_source, second_oid) = create_source_repo(temp.path(), "second", "second");
|
||||
let target = temp.path().join("target.git");
|
||||
assert!(std::process::Command::new("git")
|
||||
assert!(fixture_git()
|
||||
.args(["init", "--bare", "--quiet", target.to_str().unwrap()])
|
||||
.status()
|
||||
.unwrap()
|
||||
.success());
|
||||
|
||||
let fetch = |source: PathBuf, oid: String, role| {
|
||||
let mut command = Command::new("git");
|
||||
let mut command = Command::from(fixture_git());
|
||||
command
|
||||
.current_dir(&target)
|
||||
.args(["fetch", "--no-write-fetch-head", "--no-auto-maintenance"])
|
||||
@@ -1502,7 +1523,7 @@ mod fetch_helper_tests {
|
||||
assert!(first.unwrap().status.success());
|
||||
assert!(second.unwrap().status.success());
|
||||
assert!(!target.join("FETCH_HEAD").exists());
|
||||
let refs = std::process::Command::new("git")
|
||||
let refs = fixture_git()
|
||||
.current_dir(&target)
|
||||
.args(["for-each-ref", "--format=%(refname)"])
|
||||
.output()
|
||||
@@ -1511,14 +1532,14 @@ mod fetch_helper_tests {
|
||||
assert!(refs.stdout.is_empty(), "object fetch must not update refs");
|
||||
|
||||
for oid in [first_oid, second_oid] {
|
||||
assert!(std::process::Command::new("git")
|
||||
assert!(fixture_git()
|
||||
.current_dir(&target)
|
||||
.args(["cat-file", "-e", &format!("{oid}^{{commit}}")])
|
||||
.status()
|
||||
.unwrap()
|
||||
.success());
|
||||
}
|
||||
let fsck = std::process::Command::new("git")
|
||||
let fsck = fixture_git()
|
||||
.current_dir(&target)
|
||||
.args(["fsck", "--no-dangling"])
|
||||
.output()
|
||||
|
||||
Reference in New Issue
Block a user