Skip to content

Commit 5781dac

Browse files
branchseerclaude
andcommitted
Add argv step spawn mode and cross-platform snapshot support for e2e tests
Add `argv` field to e2e step config that spawns the process directly without a shell wrapper (`sh -c`). This avoids shell interference with signal handling and exit codes on Windows where bash intercepts CTRL_C. The program is resolved from `CARGO_BIN_EXE_<name>` env vars since CommandBuilder doesn't do PATH lookup on all platforms. Also: - Use staged temp dir for insta snapshot paths so writes work on read-only filesystems (e.g. WebDAV mount during cargo-xtest) - Skip INSTA_REQUIRE_FULL_MATCH when INSTA_UPDATE is set, allowing cross-platform runs to accept metadata-only snapshot changes Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent 2295d41 commit 5781dac

File tree

2 files changed

+76
-14
lines changed

2 files changed

+76
-14
lines changed

CLAUDE.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,8 @@ just doc # Documentation generation
3838

3939
If `gt` (Graphite CLI) is available in PATH, use it instead of `gh` to create pull requests.
4040

41+
PR titles must follow the format `type(area): summary`, e.g. `feat(cache): add LRU eviction`, `fix(fspy): handle symlink loops`, `test(e2e): add ctrl-c propagation test`.
42+
4143
## Tests
4244

4345
```bash

crates/vite_task_bin/tests/e2e_snapshots/main.rs

Lines changed: 74 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -64,16 +64,48 @@ enum Step {
6464
#[derive(serde::Deserialize, Debug)]
6565
#[serde(deny_unknown_fields)]
6666
struct StepConfig {
67-
command: Str,
67+
/// Shell command string (run via `sh -c`).
68+
#[serde(default)]
69+
command: Option<Str>,
70+
/// Argument vector — spawned directly without a shell wrapper.
71+
#[serde(default)]
72+
argv: Option<Vec<Str>>,
6873
#[serde(default)]
6974
interactions: Vec<Interaction>,
7075
}
7176

77+
/// How to spawn a step: either via shell or directly.
78+
enum StepSpawn<'a> {
79+
/// Run through `sh -c "<command>"`.
80+
Shell(&'a str),
81+
/// Spawn directly with the given argv (first element is the program).
82+
Direct(&'a [Str]),
83+
}
84+
7285
impl Step {
73-
fn command(&self) -> &str {
86+
fn spawn_mode(&self) -> StepSpawn<'_> {
7487
match self {
75-
Self::Command(command) => command.as_str(),
76-
Self::Detailed(config) => config.command.as_str(),
88+
Self::Command(command) => StepSpawn::Shell(command.as_str()),
89+
Self::Detailed(config) => config.argv.as_deref().map_or_else(
90+
|| {
91+
StepSpawn::Shell(
92+
config
93+
.command
94+
.as_ref()
95+
.expect("step must have either 'command' or 'argv'")
96+
.as_str(),
97+
)
98+
},
99+
StepSpawn::Direct,
100+
),
101+
}
102+
}
103+
104+
#[expect(clippy::disallowed_types, reason = "String required by join")]
105+
fn display_command(&self) -> String {
106+
match self.spawn_mode() {
107+
StepSpawn::Shell(cmd) => cmd.to_string(),
108+
StepSpawn::Direct(argv) => argv.iter().map(Str::as_str).collect::<Vec<_>>().join(" "),
77109
}
78110
}
79111

@@ -187,9 +219,7 @@ fn run_case(tmpdir: &AbsolutePath, fixture_path: &std::path::Path, filter: Optio
187219
{
188220
println!("{fixture_name}");
189221
}
190-
// Configure insta to write snapshots to fixture directory
191222
let mut settings = insta::Settings::clone_current();
192-
settings.set_snapshot_path(fixture_path.join("snapshots"));
193223
settings.set_prepend_module_to_snapshot(false);
194224
settings.remove_snapshot_suffix();
195225

@@ -214,6 +244,13 @@ fn run_case_inner(tmpdir: &AbsolutePath, fixture_path: &std::path::Path, fixture
214244
let stage_path = tmpdir.join(fixture_name);
215245
CopyOptions::new().copy_tree(fixture_path, stage_path.as_path()).unwrap();
216246

247+
// Point insta at the staged copy's snapshot dir (which is writable). The
248+
// original fixture_path may be on a read-only filesystem (e.g. WebDAV mount
249+
// during cross-platform testing via cargo-xtest).
250+
let mut settings = insta::Settings::clone_current();
251+
settings.set_snapshot_path(stage_path.as_path().join("snapshots"));
252+
let _guard = settings.bind_to_scope();
253+
217254
let (workspace_root, _cwd) = find_workspace_root(&stage_path).unwrap();
218255

219256
assert_eq!(
@@ -276,10 +313,27 @@ fn run_case_inner(tmpdir: &AbsolutePath, fixture_path: &std::path::Path, fixture
276313

277314
let mut e2e_outputs = String::new();
278315
for step in &e2e.steps {
279-
let step_command = step.command();
280-
let mut cmd = CommandBuilder::new(&shell_exe);
281-
cmd.arg("-c");
282-
cmd.arg(step_command);
316+
let step_display = step.display_command();
317+
let mut cmd = match step.spawn_mode() {
318+
StepSpawn::Shell(command) => {
319+
let mut cmd = CommandBuilder::new(&shell_exe);
320+
cmd.arg("-c");
321+
cmd.arg(command);
322+
cmd
323+
}
324+
StepSpawn::Direct(argv) => {
325+
// Resolve the program from CARGO_BIN_EXE_<name> if available,
326+
// since CommandBuilder doesn't do PATH lookup on all platforms.
327+
let program = argv[0].as_str();
328+
let exe_env = vite_str::format!("CARGO_BIN_EXE_{program}");
329+
let resolved = env::var_os(exe_env.as_str()).unwrap_or_else(|| program.into());
330+
let mut cmd = CommandBuilder::new(resolved);
331+
for arg in &argv[1..] {
332+
cmd.arg(arg.as_str());
333+
}
334+
cmd
335+
}
336+
};
283337
cmd.env_clear();
284338
cmd.env("PATH", &e2e_env_path);
285339
cmd.env("NO_COLOR", "1");
@@ -390,7 +444,7 @@ fn run_case_inner(tmpdir: &AbsolutePath, fixture_path: &std::path::Path, fixture
390444
}
391445

392446
e2e_outputs.push_str("> ");
393-
e2e_outputs.push_str(step_command);
447+
e2e_outputs.push_str(&step_display);
394448
e2e_outputs.push('\n');
395449

396450
e2e_outputs.push_str(&redact_e2e_output(output, e2e_stage_path_str));
@@ -412,8 +466,14 @@ fn run_case_inner(tmpdir: &AbsolutePath, fixture_path: &std::path::Path, fixture
412466
}
413467

414468
fn main() {
415-
// SAFETY: Called before any threads are spawned; insta reads this lazily on first assertion.
416-
unsafe { std::env::set_var("INSTA_REQUIRE_FULL_MATCH", "1") };
469+
// Skip INSTA_REQUIRE_FULL_MATCH when running cross-platform (via cargo-xtest): the snapshot
470+
// source metadata changes when the test binary runs on a remote machine, causing insta to
471+
// write .snap.new even when content matches. INSTA_UPDATE=always in this mode accepts
472+
// metadata-only changes.
473+
if std::env::var_os("INSTA_UPDATE").is_none() {
474+
// SAFETY: Called before any threads are spawned; insta reads this lazily on first assertion.
475+
unsafe { std::env::set_var("INSTA_REQUIRE_FULL_MATCH", "1") };
476+
}
417477

418478
let filter = std::env::args().nth(1);
419479

@@ -430,7 +490,7 @@ fn main() {
430490

431491
// Copy .node-version to the tmp dir so version manager shims can resolve the correct
432492
// Node.js binary when running task commands.
433-
let repo_root = manifest_dir.join("../..").canonicalize().unwrap();
493+
let repo_root = manifest_dir.parent().unwrap().parent().unwrap();
434494
std::fs::copy(repo_root.join(".node-version"), tmp_dir.path().join(".node-version"))
435495
.unwrap();
436496

0 commit comments

Comments
 (0)