skills/max-sixty__worktrunk
1
0
Fork 0
mirror of https://github.com/max-sixty/worktrunk.git synced 2026-09-14 20:00:38 +08:00
Code Issues Packages Projects Releases Wiki Activity

refactor(tests): give the PTY test environment one home (#3618)

Browse Source 
Follow-up to #3616, which fixed a PTY snapshot flake by adding one env
knob — and to add it I had to touch three separate env builders, none of
which knew about the others. This consolidates that surface.

## What was there

The environment a test subprocess runs in was assembled at nine sites:
five copies of the PTY prologue (`env_clear`, HOME, PATH, the Windows
block, coverage passthrough) and four partial restatements of the
determinism knobs. There was no rule for which belonged where, so adding
a knob meant finding every copy, and a missed copy surfaced later as a
flake somewhere unrelated.

## What's there now

Three named layers, each with one home in `src/testing/mod.rs`:

| Layer | Home | Contents |
|---|---|---|
| Baseline | `STATIC_TEST_ENV_VARS` | knobs every child needs, whatever
it's attached to |
| Terminal | `PTY_TEST_ENV_VARS` (new) | knobs only a TTY triggers —
`WORKTRUNK_TEST_SPINNERS=0` |
| Fixture | `pty_env_vars(TestEnvPaths { … })` (new) | the paths that
vary per fixture |

`configure_cli_command` and `configure_pty_command` apply them by
transport. That turns the standing `// NOTE: TERM is intentionally NOT
in STATIC_TEST_ENV_VARS` comment into a consequence rather than an
exception: `TERM` is transport-level, so it can't sit in a baseline both
transports share.

`WORKTRUNK_TEST_SPINNERS` stays out of the shared baseline deliberately.
It's inert on a pipe, and insta-cmd records the whole environment into
every snapshot it writes (`Info::from_std_command` builds it
unconditionally from `cmd.get_envs()` — there's no hook to suppress it),
so putting it there would add a no-op line to 1043 snapshot files.

`configure_pty_command` is now the only place a PTY child's isolation is
set up. `shell_command`, `execute_shell_script`,
`configure_pty_environment`, `exec_in_pty_shell`,
`exec_bash_truly_interactive` and two `wt switch` spawns all delegate to
it. `shell_wrapper`'s `STANDARD_TEST_ENV` and `bare_repository`'s
hand-rolled `test_env_vars` are gone, as are `configure_shell`'s
hand-copied knobs and four redundant `CLICOLOR_FORCE` lines in
`switch_picker`. Net −149 lines.

One spawn stays outside: the Windows ConPTY smoke test, which runs
PowerShell against a deliberately bare environment and isn't a wt child
at all.

## Reviewing

Start at `src/testing/mod.rs` — the three layers and their doc comments
are the whole design. Everything under `tests/` is deletion plus a
delegation call.

Two snapshots change: `install_preview_with_gutter` and
`install_preview_declined` now carry ANSI, because those two tests
previously ran without `CLICOLOR_FORCE`. Text is identical. Arguably a
fix — the test named "with_gutter" couldn't see the gutter (a
background-color block), while its own prompt line was already colored,
so the file was internally inconsistent.

## Testing

Full `wt hook pre-merge --yes` green: 4601 tests, `--features
shell-integration-tests`, `RUSTFLAGS='-D warnings'`, `insta --check`.

The knob's delivery path was verified by probe rather than by
inspection. With `sleep 6` in the mock `llm`,
`test_readme_example_hooks_pre_merge` passes; flipping
`PTY_TEST_ENV_VARS` to `"1"` reproduces the original failure byte for
byte:

```
+␛[1G␛[J␛[2m↳␛[22m ␛[2mWaiting for the commit generation command (4s)␛[22m␛[1G␛[J␛[2m↳␛[22m ␛[2mWaiting for the commit generation command (5s)␛[22m
```

So the knob reaches the shell-wrapper PTY child through the shared
setup, not through a surviving copy. Both probe edits are reverted.

> _This was written by Claude Code on behalf of max_

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
Maximilian Roos
2026-07-26 14:59:48 -07:00
committed by GitHub
parent d6ef23d573
commit 9032308400
12 changed files with 207 additions and 356 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/testing/mod.rs
+99 -73
View File
@@ -271,19 +271,22 @@ pub fn allow_network_transports(cmd: &mut Command) {
cmd.env_remove("GIT_ALLOW_PROTOCOL");
}
/// Static environment variables shared by all test isolation helpers.
/// Determinism knobs every isolated wt subprocess needs, whatever it's
/// attached to.
///
/// These are used by both `configure_cli_command()` (for Command-based tests)
/// and `TestRepo::test_env_vars()` (for PTY tests). Adding a variable here
/// ensures consistency across both test infrastructure paths.
/// A test child's environment is three layers, each with one home: this
/// baseline, the fixture's paths ([`pty_env_vars`]), and whatever the child's
/// transport needs — [`configure_cli_command`] for a piped child,
/// [`PTY_TEST_ENV_VARS`] for one on a terminal. A knob both transports need
/// belongs here, so adding it once reaches every path.
///
/// NOTE: Path-dependent variables (HOME, WORKTRUNK_CONFIG_PATH, GIT_CONFIG_*)
/// are NOT included here because they depend on the TestRepo instance.
/// `TERM` is transport-level rather than baseline, which is why it's absent
/// here: a piped child gets `TERM=alacritty` so hyperlink detection has
/// something to key on, while a PTY child needs a `TERM` with real terminfo —
/// macOS CI carries no alacritty entry, and skim fails without one.
pub const STATIC_TEST_ENV_VARS: &[(&str, &str)] = &[
("CLICOLOR_FORCE", "1"),
// Deny network git transports (see GIT_ALLOWED_PROTOCOLS). Host-independent,
// so it belongs here rather than in each builder's path-dependent block —
// which is what reaches the hand-rolled PTY env builders too.
// Deny network git transports (see GIT_ALLOWED_PROTOCOLS)
("GIT_ALLOW_PROTOCOL", GIT_ALLOWED_PROTOCOLS),
// Terminal width for PTY tests. configure_cli_command() overrides to 500 for longer paths.
("COLUMNS", "150"),
@@ -318,10 +321,83 @@ pub const STATIC_TEST_ENV_VARS: &[(&str, &str)] = &[
("WORKTRUNK_TEST_POWERSHELL_ENV", "0"),
];
// NOTE: TERM is intentionally NOT in STATIC_TEST_ENV_VARS because:
// - configure_cli_command() sets TERM=alacritty for hyperlink detection testing
// - PTY tests (especially skim-based picker tests) need a TERM with valid terminfo
// - macOS CI doesn't have alacritty terminfo, causing skim to fail
/// Determinism knobs for a child whose stderr is a terminal, layered over
/// [`STATIC_TEST_ENV_VARS`].
///
/// Output that appears only once an operation runs past a threshold is a
/// function of machine load rather than of behavior, and a PTY test captures
/// the raw byte stream — so it keeps every in-place redraw frame a terminal
/// would have erased, elapsed-second counter and all. These pin such output to
/// one state, off, so a snapshot records what the command did rather than how
/// fast the machine was. A piped child needs none of them: the TTY half of
/// each gate is already false there, which is why they'd only add noise to the
/// `env:` block every `assert_cmd_snapshot!` records.
pub const PTY_TEST_ENV_VARS: &[(&str, &str)] = &[
// The `Progress` and `Watchdog` spinners (src/progress.rs). Gates the
// render, not the counters.
("WORKTRUNK_TEST_SPINNERS", "0"),
];
/// The paths an isolated wt subprocess is pointed at — everything in its
/// environment that varies per fixture. See [`pty_env_vars`].
pub struct TestEnvPaths<'a> {
/// `GIT_CONFIG_GLOBAL`.
pub git_config: &'a Path,
/// `HOME`, with `XDG_CONFIG_HOME` beneath it.
pub home: &'a Path,
/// `WORKTRUNK_CONFIG_PATH`.
pub wt_config: &'a Path,
/// `WORKTRUNK_APPROVALS_PATH`.
pub approvals: &'a Path,
}
/// Every environment variable a PTY-spawned wt subprocess needs: the
/// [`STATIC_TEST_ENV_VARS`] and [`PTY_TEST_ENV_VARS`] baselines, plus `paths`.
///
/// A PTY child is spawned through `portable_pty::CommandBuilder`, which takes
/// variables one at a time rather than a configured [`Command`] — so its
/// environment has to exist as a value, which is what separates this from
/// [`configure_cli_command`]. Every fixture that spawns one builds it here, so
/// a new variable reaches all of them.
pub fn pty_env_vars(paths: TestEnvPaths<'_>) -> Vec<(String, String)> {
let mut vars: Vec<(String, String)> = STATIC_TEST_ENV_VARS
.iter()
.chain(PTY_TEST_ENV_VARS)
.map(|&(k, v)| (k.to_string(), v.to_string()))
.collect();
vars.extend(
[
("GIT_CONFIG_GLOBAL", paths.git_config.display().to_string()),
("GIT_CONFIG_SYSTEM", NULL_DEVICE.to_string()),
("GIT_AUTHOR_DATE", "2025-01-01T00:00:00Z".to_string()),
("GIT_COMMITTER_DATE", "2025-01-01T00:00:00Z".to_string()),
// Prevent git from prompting for credentials when running under a TTY
("GIT_TERMINAL_PROMPT", "0".to_string()),
("HOME", paths.home.display().to_string()),
(
"XDG_CONFIG_HOME",
paths.home.join(".config").display().to_string(),
),
("WORKTRUNK_TEST_EPOCH", TEST_EPOCH.to_string()),
(
"WORKTRUNK_CONFIG_PATH",
paths.wt_config.display().to_string(),
),
(
"WORKTRUNK_SYSTEM_CONFIG_PATH",
DEFAULT_ISOLATED_SYSTEM_CONFIG.to_string(),
),
(
"WORKTRUNK_APPROVALS_PATH",
paths.approvals.display().to_string(),
),
]
.map(|(key, value)| (key.to_string(), value)),
);
vars
}
/// Null device path, platform-appropriate.
/// Use this for GIT_CONFIG_SYSTEM to disable system config in tests.
@@ -1064,69 +1140,19 @@ impl TestRepo {
configure_git_cmd(cmd, &self.git_config_path);
}
/// Get standard test environment variables as a vector.
/// This repo's environment for a PTY-spawned wt subprocess.
///
/// This is useful for PTY tests and other cases where you need environment variables
/// as a vector rather than setting them on a Command.
///
/// ## Related: `configure_cli_command()`
///
/// Command-based tests use `configure_cli_command()`. Both functions share common
/// variables via `STATIC_TEST_ENV_VARS`. See that function's docs for differences.
/// Thin wrapper over [`pty_env_vars`], which documents the layering and is
/// where a new variable goes. Command-based tests use
/// [`configure_cli_command`] instead.
#[cfg_attr(windows, allow(dead_code))] // Used only by unix PTY tests
pub fn test_env_vars(&self) -> Vec<(String, String)> {
// Start with shared static env vars
let mut vars: Vec<(String, String)> = STATIC_TEST_ENV_VARS
.iter()
.map(|&(k, v)| (k.to_string(), v.to_string()))
.collect();
// Add path-dependent variables specific to this TestRepo
vars.extend([
(
"GIT_CONFIG_GLOBAL".to_string(),
self.git_config_path.display().to_string(),
),
("GIT_CONFIG_SYSTEM".to_string(), NULL_DEVICE.to_string()),
(
"GIT_AUTHOR_DATE".to_string(),
"2025-01-01T00:00:00Z".to_string(),
),
(
"GIT_COMMITTER_DATE".to_string(),
"2025-01-01T00:00:00Z".to_string(),
),
// Prevent git from prompting for credentials when running under a TTY
("GIT_TERMINAL_PROMPT".to_string(), "0".to_string()),
// Use test-specific home directory for isolation
("HOME".to_string(), self.home_path().display().to_string()),
(
"XDG_CONFIG_HOME".to_string(),
self.home_path().join(".config").display().to_string(),
),
// Suppress the in-place TTY spinners (`Progress`, `Watchdog`). A
// PTY capture keeps every redraw frame the terminal would have
// erased, so an operation that load pushes past a startup delay
// adds frames — with their elapsed seconds — to the snapshot. Only
// the PTY path needs this: `configure_cli_command()` pipes stdout
// and stderr, so the TTY half of the gate is already false there.
("WORKTRUNK_TEST_SPINNERS".to_string(), "0".to_string()),
("WORKTRUNK_TEST_EPOCH".to_string(), TEST_EPOCH.to_string()),
(
"WORKTRUNK_CONFIG_PATH".to_string(),
self.test_config_path().display().to_string(),
),
(
"WORKTRUNK_SYSTEM_CONFIG_PATH".to_string(),
"/etc/xdg/worktrunk/config.toml".to_string(),
),
(
"WORKTRUNK_APPROVALS_PATH".to_string(),
self.test_approvals_path().display().to_string(),
),
]);
vars
pty_env_vars(TestEnvPaths {
git_config: &self.git_config_path,
home: self.home_path(),
wt_config: self.test_config_path(),
approvals: self.test_approvals_path(),
})
}
/// Configure shell integration for test environment.
tests/CLAUDE.md
+16 -9
View File
@@ -101,6 +101,17 @@ call `.current_dir(...)` explicitly.
| `wt_command()` | `Command` | Running wt without a TestRepo (free function) |
| `repo.git_command()` | `Cmd` | Running git commands (use `.run()` not `.output()`) |
### Where a new environment variable goes
A test child's environment is three layers, each with one home in
`src/testing/mod.rs`: `STATIC_TEST_ENV_VARS` for a determinism knob every child
needs, `PTY_TEST_ENV_VARS` for one only a terminal triggers, and `pty_env_vars`
for a path that varies per fixture. `configure_cli_command` and
`configure_pty_command` apply them by transport, so a variable added to the
right layer reaches every test that spawns `wt`. A per-builder copy reaches only
the tests that happen to use that builder, and the ones it misses fail later,
somewhere else.
## Config Isolation for In-Process Unit Tests
`repo.wt_command()` / `wt_command()` isolate *subprocess* tests (above). An
@@ -254,7 +265,7 @@ Two traps:
Output that appears only once an operation runs past a threshold is a function of machine load, not of behavior: the `Progress` and `Watchdog` spinners (`src/progress.rs`), `Cmd::delayed_stream`'s progress line, the picker's placeholder reveal. A PTY test captures the raw byte stream, so it keeps every in-place redraw frame a terminal would have erased, elapsed-second counter and all — frames that show up when the whole suite runs together and not when the test runs alone.
Each threshold has an env override the PTY env builders pin (`WORKTRUNK_TEST_SPINNERS=0`, `WORKTRUNK_TEST_DELAYED_STREAM_MS=-1`, `WORKTRUNK_PLACEHOLDER_REVEAL_MS=0`), so the output is present or absent by construction rather than by timing; a new builder or a new threshold needs the same. Filtering the frames out of the capture afterwards is the weaker fix: the filter has to model cursor movement, and a block that redraws with a cursor-up spans lines a line-scoped filter can't follow.
Each threshold has an env override pinning it, so the output is present or absent by construction rather than by timing. A new one goes in the baseline that matches its scope — `PTY_TEST_ENV_VARS` when only a terminal triggers it (`WORKTRUNK_TEST_SPINNERS=0`), `STATIC_TEST_ENV_VARS` when a pipe does too (`WORKTRUNK_TEST_DELAYED_STREAM_MS=-1`) — and reaches every PTY child from there, rather than being added per builder. Filtering the frames out of the capture afterwards is the weaker fix: the filter has to model cursor movement, and a block that redraws with a cursor-up spans lines a line-scoped filter can't follow.
## No Retries
@@ -401,17 +412,13 @@ The PTY approach is specifically for **user-facing output documentation**. It's
## Coverage in PTY Tests
PTY tests use `cmd.env_clear()` for isolation. To enable coverage, pass through LLVM env vars:
`configure_pty_command` clears the child's environment, so an instrumented
binary would lose the LLVM vars that tell it where to write coverage data. It
passes them back through, which is one more reason every PTY test starts there:
```rust
// Standard setup (most PTY tests)
crate::common::configure_pty_command(&mut cmd);
// Custom env setup (shell tests needing USER, SHELL, ZDOTDIR)
cmd.env_clear();
cmd.env("HOME", ...);
// ... custom env ...
crate::common::pass_coverage_env_to_pty_cmd(&mut cmd);
// ... test-specific env (USER, SHELL, ZDOTDIR, the fixture's paths) ...
```
## No Global State Mutations in Tests
tests/common/mod.rs
+41 -25
View File
@@ -407,16 +407,35 @@ pub fn open_pty_with_size(rows: u16, cols: u16) -> portable_pty::PtyPair {
/// Configure a PTY CommandBuilder with isolated environment for testing.
///
/// This is the PTY equivalent of `configure_cli_command()`. It:
/// The PTY equivalent of `configure_cli_command()`, and the one place a PTY
/// child's isolation is set up:
/// 1. Clears all inherited environment variables
/// 2. Sets minimal required vars (HOME, PATH)
/// 3. Passes through LLVM coverage profiling vars so subprocess coverage works
/// 2. Sets the minimal vars a shell or binary needs to run (HOME, PATH, and
/// the Windows equivalents)
/// 3. Applies the `STATIC_TEST_ENV_VARS` and `PTY_TEST_ENV_VARS` determinism
/// baselines
/// 4. Passes through LLVM coverage profiling vars so subprocess coverage works
///
/// Call this early in PTY test setup, then add any test-specific env vars after.
/// It supplies no fixture paths, having no fixture to read them from; a caller
/// with one adds `TestRepo::test_env_vars()` on top, which carries the
/// baselines again at the same values. `HOME` points at the developer's real
/// home, since a shell needs a plausible one to start in — a caller that
/// wants the fixture's overrides it.
pub fn configure_pty_command(cmd: &mut portable_pty::CommandBuilder) {
// Clear inherited environment for test isolation
cmd.env_clear();
for &(key, value) in worktrunk::testing::STATIC_TEST_ENV_VARS
.iter()
.chain(worktrunk::testing::PTY_TEST_ENV_VARS)
{
cmd.env(key, value);
}
cmd.env(
"WORKTRUNK_TEST_EPOCH",
worktrunk::testing::TEST_EPOCH.to_string(),
);
// Minimal environment for shells/binaries to function
let home_dir = home::home_dir().unwrap().to_string_lossy().to_string();
cmd.env("HOME", &home_dir);
@@ -474,8 +493,10 @@ pub fn configure_pty_command(cmd: &mut portable_pty::CommandBuilder) {
/// [`worktrunk::testing::default_llvm_profile_file`] for the
/// inherit-or-temp-dir resolution.
///
/// Use `configure_pty_command()` for the full setup, or call this directly if you
/// need custom env_clear handling (e.g., shell-specific env vars).
/// [`configure_pty_command`] calls this, so a test that spawns `wt` through it
/// needs nothing further. It stays separate for the one spawn that isn't a wt
/// child at all — the ConPTY smoke test, which runs PowerShell against a
/// deliberately bare environment.
pub fn pass_coverage_env_to_pty_cmd(cmd: &mut portable_pty::CommandBuilder) {
cmd.env(
"LLVM_PROFILE_FILE",
@@ -490,11 +511,11 @@ pub fn pass_coverage_env_to_pty_cmd(cmd: &mut portable_pty::CommandBuilder) {
/// Create a CommandBuilder for running a shell in PTY tests.
///
/// Handles all shell-specific setup:
/// - env_clear + HOME + PATH (with optional bin_dir prefix)
/// [`configure_pty_command`] for the isolated environment, plus the
/// shell-specific parts on top:
/// - `bin_dir` prepended to PATH, for tests that shadow a binary with a mock
/// - Shell-specific env vars (ZDOTDIR for zsh)
/// - Shell-specific isolation flags (--norc, --no-rcs, --no-config)
/// - Coverage passthrough
///
/// Returns a CommandBuilder ready for `.arg("-c")` and `.arg(&script)`.
#[cfg(unix)]
@@ -503,22 +524,18 @@ pub fn shell_command(
bin_dir: Option<&std::path::Path>,
) -> portable_pty::CommandBuilder {
let mut cmd = portable_pty::CommandBuilder::new(shell);
cmd.env_clear();
configure_pty_command(&mut cmd);
cmd.env(
"HOME",
home::home_dir().unwrap().to_string_lossy().to_string(),
);
let path = match bin_dir {
Some(dir) => format!(
"{}:{}",
dir.display(),
std::env::var("PATH").unwrap_or_else(|_| "/usr/bin:/bin".to_string())
),
None => std::env::var("PATH").unwrap_or_else(|_| "/usr/bin:/bin".to_string()),
};
cmd.env("PATH", path);
if let Some(dir) = bin_dir {
cmd.env(
"PATH",
format!(
"{}:{}",
dir.display(),
std::env::var("PATH").unwrap_or_else(|_| "/usr/bin:/bin".to_string())
),
);
}
// Shell-specific setup
match shell {
@@ -540,7 +557,6 @@ pub fn shell_command(
_ => {}
}
pass_coverage_env_to_pty_cmd(&mut cmd);
cmd
}
tests/common/progressive_output.rs
+2 -26
View File
@@ -536,18 +536,8 @@ pub fn capture_progressive_output(
/// Configure PTY command with test environment variables
fn configure_pty_environment(cmd: &mut CommandBuilder, repo: &TestRepo) {
// Clear environment
cmd.env_clear();
// Basic environment
cmd.env(
"HOME",
home::home_dir().unwrap().to_string_lossy().to_string(),
);
cmd.env(
"PATH",
std::env::var("PATH").unwrap_or_else(|_| "/usr/bin:/bin".to_string()),
);
// Isolated environment (env_clear, HOME, PATH, determinism baselines, coverage)
super::configure_pty_command(cmd);
// Test environment (from TestRepo::test_env_vars)
for (key, value) in repo.test_env_vars() {
@@ -558,20 +548,6 @@ fn configure_pty_environment(cmd: &mut CommandBuilder, repo: &TestRepo) {
// loading indicator see it on every render — otherwise fast runs finish
// before the deferred tick fires and dots never appear.
cmd.env("WORKTRUNK_PLACEHOLDER_REVEAL_MS", "0");
// Pass through LLVM coverage profiling environment for subprocess coverage collection.
// When running under cargo-llvm-cov, spawned binaries need LLVM_PROFILE_FILE to record
// their coverage data; otherwise, point it at a temp-dir default so an
// instrumented child can't write `default_*.profraw` into the repo root.
cmd.env(
"LLVM_PROFILE_FILE",
worktrunk::testing::default_llvm_profile_file(),
);
for key in worktrunk::testing::COVERAGE_ENV_VARS {
if let Ok(val) = std::env::var(key) {
cmd.env(key, val);
}
}
}
#[cfg(test)]
tests/common/pty.rs
-4
View File
@@ -308,10 +308,6 @@ pub fn build_pty_command(
);
#[cfg(windows)]
cmd.env("USERPROFILE", home.to_string_lossy().to_string());
// Suppress nushell auto-detection for deterministic PTY tests.
// Other shell-installed defaults are picked up via STATIC_TEST_ENV_VARS
// in callers that pass env_vars from TestRepo::test_env_vars.
cmd.env("WORKTRUNK_TEST_NUSHELL_ENV", "0");
}
cmd
tests/common/shell.rs
+9 -25
View File
@@ -23,11 +23,16 @@ pub fn execute_shell_script(repo: &TestRepo, shell: &str, script: &str) -> Strin
let mut cmd = CommandBuilder::new(shell_binary(shell));
// Clear inherited environment for test isolation
cmd.env_clear();
// Isolated environment (env_clear, PATH, determinism baselines, coverage)
super::configure_pty_command(&mut cmd);
cmd.env("USER", "testuser");
cmd.env("SHELL", shell_binary(shell));
// Set minimal required environment for shells to function
cmd.env("HOME", repo.home_path().to_string_lossy().to_string());
// The repo's own environment: git config, worktrunk config, and a HOME
// under the test's temp dir rather than the developer's
for (key, value) in repo.test_env_vars() {
cmd.env(key, value);
}
// Windows: Also set USERPROFILE for PowerShell and Git Bash
#[cfg(windows)]
cmd.env(
@@ -35,24 +40,6 @@ pub fn execute_shell_script(repo: &TestRepo, shell: &str, script: &str) -> Strin
repo.home_path().to_string_lossy().to_string(),
);
// Use platform-appropriate PATH
#[cfg(unix)]
let default_path = "/usr/bin:/bin";
#[cfg(windows)]
let default_path = std::env::var("PATH").unwrap_or_default();
cmd.env(
"PATH",
std::env::var("PATH").unwrap_or_else(|_| default_path.to_string()),
);
cmd.env("USER", "testuser");
cmd.env("SHELL", shell_binary(shell));
// Add repo's test environment (git config, worktrunk config, etc.)
for (key, value) in repo.test_env_vars() {
cmd.env(key, value);
}
// Add shell-specific no-config flags
match shell {
"bash" => {
@@ -95,9 +82,6 @@ pub fn execute_shell_script(repo: &TestRepo, shell: &str, script: &str) -> Strin
}
cmd.cwd(repo.root_path());
// Pass through LLVM coverage env vars for subprocess coverage collection
super::pass_coverage_env_to_pty_cmd(&mut cmd);
let mut child = pair.slave.spawn_command(cmd).unwrap();
drop(pair.slave); // Close slave in parent
tests/integration_tests/bare_repository.rs
+9 -51
View File
@@ -1498,64 +1498,22 @@ impl NestedBareRepoTest {
.env_remove("CLICOLOR_FORCE");
}
/// Get test environment variables as a vector for PTY tests.
/// This fixture's environment for a PTY-spawned wt subprocess, built from
/// the same layers as `TestRepo::test_env_vars`.
#[cfg(all(unix, feature = "shell-integration-tests"))]
fn test_env_vars(&self) -> Vec<(String, String)> {
use crate::common::{NULL_DEVICE, STATIC_TEST_ENV_VARS, TEST_EPOCH};
let mut vars: Vec<(String, String)> = STATIC_TEST_ENV_VARS
.iter()
.map(|&(k, v)| (k.to_string(), v.to_string()))
.collect();
use crate::common::{TestEnvPaths, pty_env_vars};
// HOME and XDG_CONFIG_HOME are needed for config lookups in env_clear'd PTY
let home = self.temp_dir.path().join("home");
std::fs::create_dir_all(&home).ok();
vars.extend([
(
"GIT_CONFIG_GLOBAL".to_string(),
self.git_config_path.display().to_string(),
),
("GIT_CONFIG_SYSTEM".to_string(), NULL_DEVICE.to_string()),
(
"GIT_AUTHOR_DATE".to_string(),
"2025-01-01T00:00:00Z".to_string(),
),
(
"GIT_COMMITTER_DATE".to_string(),
"2025-01-01T00:00:00Z".to_string(),
),
("GIT_TERMINAL_PROMPT".to_string(), "0".to_string()),
("HOME".to_string(), home.display().to_string()),
(
"XDG_CONFIG_HOME".to_string(),
home.join(".config").display().to_string(),
),
// Suppress the in-place TTY spinners; a PTY capture keeps every
// redraw frame a terminal would have erased. Mirrors
// `TestRepo::test_env_vars`, which explains why.
("WORKTRUNK_TEST_SPINNERS".to_string(), "0".to_string()),
("WORKTRUNK_TEST_EPOCH".to_string(), TEST_EPOCH.to_string()),
(
"WORKTRUNK_CONFIG_PATH".to_string(),
self.test_config_path.display().to_string(),
),
(
"WORKTRUNK_SYSTEM_CONFIG_PATH".to_string(),
"/etc/xdg/worktrunk/config.toml".to_string(),
),
(
"WORKTRUNK_APPROVALS_PATH".to_string(),
self.temp_dir
.path()
.join("test-approvals.toml")
.display()
.to_string(),
),
]);
vars
pty_env_vars(TestEnvPaths {
git_config: &self.git_config_path,
home: &home,
wt_config: &self.test_config_path,
approvals: &self.temp_dir.path().join("test-approvals.toml"),
})
}
}
tests/integration_tests/configure_shell.rs
+5 -22
View File
@@ -2184,20 +2184,11 @@ mod pty_tests {
configure_pty_command(&mut cmd);
cmd.env("HOME", temp_home.path());
cmd.env("XDG_CONFIG_HOME", temp_home.path().join(".config"));
// Treat shells as not installed by default; the test exercises the
// single-zsh path. Mirrors the STATIC_TEST_ENV_VARS values used by
// Command-based tests, applied explicitly here because
// configure_pty_command intentionally keeps the PTY env minimal.
cmd.env("WORKTRUNK_TEST_BASH_INSTALLED", "0");
cmd.env("WORKTRUNK_TEST_ZSH_INSTALLED", "0");
cmd.env("WORKTRUNK_TEST_FISH_INSTALLED", "0");
cmd.env("WORKTRUNK_TEST_POWERSHELL_INSTALLED", "0");
cmd.env("WORKTRUNK_TEST_NUSHELL_ENV", "0");
// Disable the process-tree shell walk so detection keys on SHELL:
// the real ancestry here is the test harness and whatever shell runs
// it (zsh on a dev box, bash on CI), which would flip the zsh-only
// compinit/restart output between environments.
cmd.env("WORKTRUNK_TEST_PARENT_SHELL", "");
// Detection keys on SHELL alone, which puts the test on the single-zsh
// path: the baseline's "not installed" and empty-parent-shell defaults
// rule out the host's process ancestry — the test harness and whatever
// shell ran it, zsh on a dev box and bash on CI, flipping the zsh-only
// compinit/restart output between them.
cmd.env("SHELL", "/bin/zsh");
// Skip the compinit probe and force the advisory to appear. The probe spawns
// `zsh -ic` which triggers global zshrc configs that can produce "insecure
@@ -2314,14 +2305,6 @@ mod pty_tests {
configure_pty_command(&mut cmd);
cmd.env("HOME", temp_home.path());
cmd.env("XDG_CONFIG_HOME", temp_home.path().join(".config"));
cmd.env("WORKTRUNK_TEST_BASH_INSTALLED", "0");
cmd.env("WORKTRUNK_TEST_ZSH_INSTALLED", "0");
cmd.env("WORKTRUNK_TEST_FISH_INSTALLED", "0");
cmd.env("WORKTRUNK_TEST_POWERSHELL_INSTALLED", "0");
cmd.env("WORKTRUNK_TEST_NUSHELL_ENV", "0");
// Disable the process-tree shell walk so detection keys on SHELL
// (see exec_install_in_pty).
cmd.env("WORKTRUNK_TEST_PARENT_SHELL", "");
cmd.env("SHELL", "/bin/zsh");
let (output, exit_code) = exec_cmd_in_pty_prompted(cmd, &["n\n"], "[y/N");
tests/integration_tests/shell_wrapper.rs
+20 -111
View File
@@ -371,57 +371,8 @@ fn exec_in_pty_shell(
let shell_binary = shell_binary(shell);
let mut cmd = CommandBuilder::new(shell_binary);
// Clear inherited environment for test isolation
cmd.env_clear();
// Set minimal required environment for shells to function
let home_dir = home::home_dir().unwrap().to_string_lossy().to_string();
cmd.env("HOME", &home_dir);
// Windows-specific env vars required for processes to run
#[cfg(windows)]
{
// USERPROFILE is Windows equivalent of HOME
cmd.env("USERPROFILE", &home_dir);
// SystemRoot is critical - many DLLs and system components need this
if let Ok(val) = std::env::var("SystemRoot") {
cmd.env("SystemRoot", &val);
cmd.env("windir", &val); // Alias used by some programs
}
// SystemDrive (usually C:)
if let Ok(val) = std::env::var("SystemDrive") {
cmd.env("SystemDrive", val);
}
// TEMP/TMP directories
if let Ok(val) = std::env::var("TEMP") {
cmd.env("TEMP", &val);
cmd.env("TMP", val);
}
// COMSPEC (cmd.exe path) - needed by some programs
if let Ok(val) = std::env::var("COMSPEC") {
cmd.env("COMSPEC", val);
}
// PSModulePath for PowerShell
if let Ok(val) = std::env::var("PSModulePath") {
cmd.env("PSModulePath", val);
}
}
// Use platform-appropriate default PATH
#[cfg(unix)]
let default_path = "/usr/bin:/bin";
#[cfg(windows)]
let default_path = std::env::var("PATH").unwrap_or_default();
cmd.env(
"PATH",
std::env::var("PATH").unwrap_or_else(|_| default_path.to_string()),
);
// Isolated environment (env_clear, HOME, PATH, determinism baselines, coverage)
crate::common::configure_pty_command(&mut cmd);
cmd.env("USER", "testuser");
cmd.env("SHELL", shell_binary);
@@ -555,18 +506,8 @@ fn exec_bash_truly_interactive(
cmd.arg("--noprofile");
cmd.arg("-i");
// Clear inherited environment for test isolation
cmd.env_clear();
// Set minimal required environment for shells to function
cmd.env(
"HOME",
home::home_dir().unwrap().to_string_lossy().to_string(),
);
cmd.env(
"PATH",
std::env::var("PATH").unwrap_or_else(|_| "/usr/bin:/bin".to_string()),
);
// Isolated environment (env_clear, HOME, PATH, determinism baselines, coverage)
crate::common::configure_pty_command(&mut cmd);
cmd.env("USER", "testuser");
cmd.env("SHELL", "bash");
@@ -579,9 +520,6 @@ fn exec_bash_truly_interactive(
cmd.env(key, value);
}
// Pass through LLVM coverage env vars for subprocess coverage collection
crate::common::pass_coverage_env_to_pty_cmd(&mut cmd);
let mut child = pair.slave.spawn_command(cmd).unwrap();
drop(pair.slave); // Close slave in parent
@@ -763,7 +701,6 @@ fn exec_through_wrapper_with_env(
let approvals_path = repo.test_approvals_path().to_string_lossy().to_string();
let mut env_vars = build_test_env_vars(&config_path, &approvals_path);
env_vars.push(("CLICOLOR_FORCE", "1"));
// Add extra env vars (these can override defaults if needed)
env_vars.extend(extra_env.iter().copied());
@@ -776,46 +713,29 @@ fn exec_through_wrapper_with_env(
}
}
/// Standard test environment variables (static parts that don't depend on test state)
/// Build the per-test half of a shell-wrapper PTY environment.
///
/// These are used by tests that build custom scripts and call `exec_in_pty_interactive` directly.
/// For tests using `exec_through_wrapper*`, these are already applied.
const STANDARD_TEST_ENV: &[(&str, &str)] = &[
("TERM", "xterm"),
("GIT_AUTHOR_NAME", "Test User"),
("GIT_AUTHOR_EMAIL", "test@example.com"),
("GIT_COMMITTER_NAME", "Test User"),
("GIT_COMMITTER_EMAIL", "test@example.com"),
("GIT_AUTHOR_DATE", "2025-01-01T00:00:00Z"),
("GIT_COMMITTER_DATE", "2025-01-01T00:00:00Z"),
("LANG", "C"),
("LC_ALL", "C"),
("WORKTRUNK_TEST_EPOCH", "1735776000"),
// Suppress delayed-stream progress output so git worktree add doesn't
// produce extra lines when the system is under load (>400ms threshold).
("WORKTRUNK_TEST_DELAYED_STREAM_MS", "-1"),
// Same, for the in-place TTY spinners: a PTY capture keeps every redraw
// frame a terminal would have erased, so a command that load pushes past a
// startup delay adds frames — with their elapsed seconds — to the snapshot.
("WORKTRUNK_TEST_SPINNERS", "0"),
];
/// Build standard test env vars with config and approvals paths
///
/// Returns a Vec containing STANDARD_TEST_ENV plus WORKTRUNK_CONFIG_PATH and
/// WORKTRUNK_APPROVALS_PATH. The caller must keep both path strings alive for
/// the duration of the returned Vec's use.
/// The determinism baselines come from [`crate::common::configure_pty_command`],
/// which `exec_in_pty_shell` applies first; these are what it can't supply —
/// the paths pointing wt at this test's fixture, a git identity for the
/// commits the scripts make, and a `TERM` with real terminfo. The caller must
/// keep both path strings alive for the duration of the returned Vec's use.
#[cfg(test)]
fn build_test_env_vars<'a>(
config_path: &'a str,
approvals_path: &'a str,
) -> Vec<(&'a str, &'a str)> {
let mut env_vars: Vec<(&str, &str)> = vec![
vec![
("WORKTRUNK_CONFIG_PATH", config_path),
("WORKTRUNK_APPROVALS_PATH", approvals_path),
];
env_vars.extend_from_slice(STANDARD_TEST_ENV);
env_vars
("TERM", "xterm"),
("GIT_AUTHOR_NAME", "Test User"),
("GIT_AUTHOR_EMAIL", "test@example.com"),
("GIT_COMMITTER_NAME", "Test User"),
("GIT_COMMITTER_EMAIL", "test@example.com"),
("GIT_AUTHOR_DATE", "2025-01-01T00:00:00Z"),
("GIT_COMMITTER_DATE", "2025-01-01T00:00:00Z"),
]
}
// =============================================================================
@@ -2753,22 +2673,11 @@ fi
cmd.cwd(repo.root_path());
// Set environment
cmd.env_clear();
cmd.env(
"HOME",
home::home_dir().unwrap().to_string_lossy().to_string(),
);
cmd.env(
"PATH",
std::env::var("PATH").unwrap_or_else(|_| "/usr/bin:/bin".to_string()),
);
crate::common::configure_pty_command(&mut cmd);
for (key, value) in repo.test_env_vars() {
cmd.env(key, value);
}
// Pass through LLVM coverage env vars for subprocess coverage collection
crate::common::pass_coverage_env_to_pty_cmd(&mut cmd);
let mut child = pair.slave.spawn_command(cmd).unwrap();
drop(pair.slave);
tests/integration_tests/switch_picker.rs
-4
View File
@@ -241,7 +241,6 @@ fn boot_picker_pty(
// Isolated environment with coverage passthrough
crate::common::configure_pty_command(&mut cmd);
cmd.env("CLICOLOR_FORCE", "1");
cmd.env("TERM", "xterm-256color");
// Test-specific environment variables
@@ -2783,7 +2782,6 @@ fn drive_alt_x_then_switch(
}
cmd.cwd(working_dir);
crate::common::configure_pty_command(&mut cmd);
cmd.env("CLICOLOR_FORCE", "1");
cmd.env("TERM", "xterm-256color");
for (key, value) in env_vars {
cmd.env(key, value);
@@ -3301,7 +3299,6 @@ fn test_switch_picker_alt_x_morphs_removed_worktree_in_place(mut repo: TestRepo)
cmd.arg("switch");
cmd.cwd(repo.root_path());
crate::common::configure_pty_command(&mut cmd);
cmd.env("CLICOLOR_FORCE", "1");
cmd.env("TERM", "xterm-256color");
for (key, value) in &env_vars {
cmd.env(key, value);
@@ -3413,7 +3410,6 @@ fn test_switch_picker_alt_x_keeps_current_worktree(mut repo: TestRepo) {
cmd.arg("switch");
cmd.cwd(&wt_path); // launched from inside the worktree → it's the current one
crate::common::configure_pty_command(&mut cmd);
cmd.env("CLICOLOR_FORCE", "1");
cmd.env("TERM", "xterm-256color");
for (key, value) in &env_vars {
cmd.env(key, value);
tests/snapshots/integration__integration_tests__configure_shell__pty_tests__install_preview_declined.snap
+1 -1
View File
@@ -3,4 +3,4 @@ source: tests/integration_tests/configure_shell.rs
expression: "output.trim_start_matches('\\n')"
---
❯ Install shell integration? [y/N/?]
✗ Cancelled by user
✗ Cancelled by user
tests/snapshots/integration__integration_tests__configure_shell__pty_tests__install_preview_with_gutter.snap
+5 -5
View File
@@ -3,9 +3,9 @@ source: tests/integration_tests/configure_shell.rs
expression: "output.trim_start_matches('\\n')"
---
❯ Install shell integration? [y/N/?]
✓ Added shell extension & completions for zsh @ ~/.zshrc
✓ Added shell extension & completions for zsh @ ~/.zshrc
✓ Configured 1 shell
▲ Completions require compinit; add to ~/.zshrc before the wt line:
autoload -Uz compinit && compinit
↳ Restart shell to activate shell integration
✓ Configured 1 shell
▲ Completions require compinit; add to ~/.zshrc before the wt line:
  autoload -Uz compinit && compinit
↳ Restart shell to activate shell integration