Isolate tests from global git config to prevent gpg-agent/pinentry#531
Merged
wesm merged 2 commits intoroborev-dev:mainfrom Mar 17, 2026
Merged
Isolate tests from global git config to prevent gpg-agent/pinentry#531wesm merged 2 commits intoroborev-dev:mainfrom
wesm merged 2 commits intoroborev-dev:mainfrom
Conversation
Add GIT_CONFIG_GLOBAL=/dev/null, GIT_CONFIG_NOSYSTEM=1, and GIT_TERMINAL_PROMPT=0 to RunIsolatedMain() so tests never read the user's global git config or prompt for input. Add TestMain to internal/git, internal/prompt, and internal/worktree packages that create git commits in tests but were missing it. Fix the EnsureAbsoluteHooksPath test that simulates global config by explicitly setting GIT_CONFIG_GLOBAL to its fake gitconfig file. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
roborev: Combined Review (
|
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
roborev: Combined Review (
|
wesm
pushed a commit
that referenced
this pull request
Mar 18, 2026
## Summary - **Isolate agent, config, and review tests from global git config** — extends #531 to three more packages. Adds `TestMain` calling `testenv.RunIsolatedMain()` so `git` operations in tests never trigger the host's `gpg-agent`/`pinentry` (which can add 1-2s per invocation on machines with GPG signing configured). - **Make worker pool idle sleeps responsive to shutdown** — replaces `time.Sleep` in the worker idle loop with a `select` on a shutdown channel, so tests that stop the daemon don't block waiting for the sleep to finish. - **Stub `isPROpenFn` in CI poller test harness** — avoids shelling out to `gh` CLI during tests. Removes a source of ~0.5s latency and flaky failures when `gh` isn't authenticated. - **Parallelize daemon subtests** — marks 21 independent daemon tests as `t.Parallel()`, including `TestIsDaemonAliveLegacyStatusCodes` subtests and the bulk of the worker/failover/cancellation tests. Drops `internal/daemon` from ~7.2s to ~4.2s. - **Tighten `requireNever` timeout** — the invalid-config watcher test was waiting 500ms for a "never" assertion; 100ms is sufficient and shaves ~400ms. - **Fix `configureSubprocess` to always set `Cancel` on `exec.Cmd`** — when called on a plain `exec.Command` (no context), `cmd.Cancel` is nil, so the wrapper's `cancel()` call panicked. Now sets a default `Kill` cancel before wrapping. This is a correctness fix surfaced by the timing changes below. - **Speed up agent subprocess tests (8s → 1s)** — two changes: 1. Override `subprocessWaitDelay` from 5s to 50ms in tests that don't need the production grace period. 2. Add a Go 1.25 ETXTBSY probe guard (`case "$1" in *etxtbsy*) exit 0;; esac`) to shell-script test fixtures. ### Go 1.25 ETXTBSY probe — why the guard is necessary Go 1.25 added a built-in [ETXTBSY probe](https://go.dev/cl/528438) to `os/exec`: every `Cmd.Start()` call forks a child that runs `execve(path, [path, "--help-probe-etxtbsy"])`. If `execve` succeeds (file isn't busy), the probe child **actually executes the script** — it isn't killed, and the parent waits for it to exit before proceeding. For shell scripts, this means the script runs **twice**: once as the probe (with `$1 = "--help-probe-etxtbsy"`), and once as the real invocation. If the script ignores `$1` and does something slow (like `sleep 5`), the probe child runs the full body, and Go waits for it — adding the script's entire runtime as invisible overhead to `Cmd.Start()`. This was confirmed via `strace -f`: two child PIDs ran the same `clock_nanosleep(5s)`, one from the probe fork (`CLONE_VFORK|CLONE_PIDFD`) and one from the real execution. The probe child was waited on via `waitid(P_PIDFD)`. The guard pattern — `case "$1" in *etxtbsy*) exit 0;; esac` as the first line after the shebang — makes the probe child exit immediately. This is only needed in test fixtures; real agent binaries already handle unknown flags gracefully. Further details are documented in `internal/agent/agent_test_helpers.go` on the `writeTempCommand` function. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
GIT_CONFIG_GLOBAL=/dev/null,GIT_CONFIG_NOSYSTEM=1, andGIT_TERMINAL_PROMPT=0totestenv.RunIsolatedMain()so tests never read global/system git config or prompt for input — preventscommit.gpgsign=truefrom triggering gpg-agent/pinentry during test commitsTestMaincallingRunIsolatedMain()tointernal/git,internal/prompt, andinternal/worktreepackages that create git commits in tests but were missing itTestEnsureAbsoluteHooksPathsubtest that simulates global config by explicitly settingGIT_CONFIG_GLOBALto its fake gitconfig file (since/dev/nulloverride would otherwise shadow it)🤖 Generated with Claude Code