mirror of
https://github.com/max-sixty/worktrunk.git
synced 2026-09-14 20:00:38 +08:00
9032308400
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>
166 lines
5.3 KiB
Rust
166 lines
5.3 KiB
Rust
use super::{TestRepo, wt_bin, wt_command};
|
|
|
|
/// Map shell display names to actual binaries.
|
|
pub fn shell_binary(shell: &str) -> &str {
|
|
match shell {
|
|
"nushell" => "nu",
|
|
"powershell" => "pwsh",
|
|
"oil" => "osh",
|
|
_ => shell,
|
|
}
|
|
}
|
|
|
|
/// Execute a script in the given shell with the repo's isolated environment.
|
|
///
|
|
/// Uses a PTY so that stdout appears as a terminal to the shell. This simulates
|
|
/// real terminal behavior for shell wrapper tests (combined stdout/stderr, ANSI codes).
|
|
///
|
|
/// Works on both Unix (bash/zsh/fish) and Windows (PowerShell, Git Bash).
|
|
pub fn execute_shell_script(repo: &TestRepo, shell: &str, script: &str) -> String {
|
|
use portable_pty::CommandBuilder;
|
|
|
|
let pair = super::open_pty();
|
|
|
|
let mut cmd = CommandBuilder::new(shell_binary(shell));
|
|
|
|
// Isolated environment (env_clear, PATH, determinism baselines, coverage)
|
|
super::configure_pty_command(&mut cmd);
|
|
cmd.env("USER", "testuser");
|
|
cmd.env("SHELL", shell_binary(shell));
|
|
|
|
// 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(
|
|
"USERPROFILE",
|
|
repo.home_path().to_string_lossy().to_string(),
|
|
);
|
|
|
|
// Add shell-specific no-config flags
|
|
match shell {
|
|
"bash" => {
|
|
cmd.arg("--noprofile");
|
|
cmd.arg("--norc");
|
|
}
|
|
"zsh" => {
|
|
cmd.arg("--no-globalrcs");
|
|
cmd.arg("-f");
|
|
}
|
|
"fish" => {
|
|
cmd.arg("--no-config");
|
|
}
|
|
"powershell" | "pwsh" => {
|
|
cmd.arg("-NoProfile");
|
|
}
|
|
"xonsh" => {
|
|
cmd.arg("--no-rc");
|
|
}
|
|
"nushell" | "nu" => {
|
|
cmd.arg("--no-config-file");
|
|
}
|
|
_ => {}
|
|
};
|
|
|
|
// PTY combines stdout/stderr at the terminal device level, so we don't need
|
|
// explicit redirection. Redirecting would break the shell wrapper protocol:
|
|
// wt_exec() captures stdout for directives, and stderr must stay separate.
|
|
//
|
|
// PowerShell uses -Command, all other shells use -c
|
|
match shell {
|
|
"powershell" | "pwsh" => {
|
|
cmd.arg("-Command");
|
|
cmd.arg(script);
|
|
}
|
|
_ => {
|
|
cmd.arg("-c");
|
|
cmd.arg(script);
|
|
}
|
|
}
|
|
cmd.cwd(repo.root_path());
|
|
|
|
let mut child = pair.slave.spawn_command(cmd).unwrap();
|
|
drop(pair.slave); // Close slave in parent
|
|
|
|
// Read everything the "terminal" would display. Blocks until child exits &
|
|
// PTY closes; treats Linux's EIO-on-slave-close as a clean EOF (see
|
|
// read_pty_master_to_string).
|
|
let mut reader = pair.master.try_clone_reader().unwrap();
|
|
let buf = super::pty::read_pty_master_to_string(&mut reader);
|
|
|
|
let status = child.wait().unwrap();
|
|
|
|
if !status.success() {
|
|
let exit_info = match status.exit_code() {
|
|
0 => "unknown error".to_string(),
|
|
code => format!("exit code {}", code),
|
|
};
|
|
panic!(
|
|
"Shell script failed ({}):\nshell: {}\noutput: {}",
|
|
exit_info, shell, buf
|
|
);
|
|
}
|
|
|
|
// Check for shell errors in output (command not found, syntax errors, etc.)
|
|
// These indicate problems with our shell integration code
|
|
if buf.contains("command not found") || buf.contains("not defined") {
|
|
panic!(
|
|
"Shell integration error detected:\nshell: {}\noutput: {}",
|
|
shell, buf
|
|
);
|
|
}
|
|
|
|
// Normalize CRLF to LF (PTYs use CRLF on some platforms)
|
|
buf.replace("\r\n", "\n")
|
|
}
|
|
|
|
/// Generate `wt config shell init <shell>` output for the repo.
|
|
pub fn generate_init_code(repo: &TestRepo, shell: &str) -> String {
|
|
let mut cmd = wt_command();
|
|
repo.configure_wt_cmd(&mut cmd);
|
|
|
|
let output = cmd
|
|
.args(["config", "shell", "init", shell])
|
|
.current_dir(repo.root_path())
|
|
.output()
|
|
.unwrap();
|
|
|
|
let stdout = String::from_utf8(output.stdout).unwrap();
|
|
let stderr = String::from_utf8_lossy(&output.stderr);
|
|
|
|
if !output.status.success() && stdout.trim().is_empty() {
|
|
panic!("Failed to generate init code:\nstderr: {}", stderr);
|
|
}
|
|
|
|
// Check for shell errors in the generated init code when it's evaluated
|
|
// This catches issues like missing compdef guards
|
|
if stderr.contains("command not found") || stderr.contains("not defined") {
|
|
panic!(
|
|
"Init code contains errors:\nstderr: {}\nGenerated code:\n{}",
|
|
stderr, stdout
|
|
);
|
|
}
|
|
|
|
stdout
|
|
}
|
|
|
|
/// Format PATH mutation per shell.
|
|
pub fn path_export_syntax(shell: &str, bin_path: &str) -> String {
|
|
match shell {
|
|
"fish" => format!(r#"set -x PATH {} $PATH"#, bin_path),
|
|
"nushell" => format!(r#"$env.PATH = ($env.PATH | prepend "{}")"#, bin_path),
|
|
"powershell" => format!(r#"$env:PATH = "{};$env:PATH""#, bin_path),
|
|
"elvish" => format!(r#"set E:PATH = {}:$E:PATH"#, bin_path),
|
|
"xonsh" => format!(r#"$PATH.insert(0, "{}")"#, bin_path),
|
|
_ => format!(r#"export PATH="{}:$PATH""#, bin_path),
|
|
}
|
|
}
|
|
|
|
/// Helper that returns the `wt` binary directory for PATH injection.
|
|
pub fn wt_bin_dir() -> String {
|
|
wt_bin().parent().unwrap().to_string_lossy().to_string()
|
|
}
|