Skip to content

Commit 8e34039

Browse files
branchseerclaude
andcommitted
test(e2e): add argv step spawn mode
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 reads/writes work when fixture_path is on a remote filesystem (e.g. WebDAV via cargo-xtest) - Remove INSTA_REQUIRE_FULL_MATCH (incompatible with remote execution where snapshot metadata differs) - Document Conventional Commits format for PR titles in CLAUDE.md Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent 2295d41 commit 8e34039

File tree

2 files changed

+66
-15
lines changed

2 files changed

+66
-15
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 use [Conventional Commits](https://www.conventionalcommits.org) format: `type(scope): summary` (scope is optional), e.g. `feat(cache): add LRU eviction`, `fix: 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: 64 additions & 15 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,12 @@ 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
222+
// Point insta at the staged copy's snapshot dir. The original fixture_path
223+
// may be on a read-only/remote filesystem (e.g. WebDAV via cargo-xtest).
224+
// The stage_path is created by run_case_inner's copy_tree call.
225+
let stage_snapshot_path = tmpdir.as_path().join(fixture_name).join("snapshots");
191226
let mut settings = insta::Settings::clone_current();
192-
settings.set_snapshot_path(fixture_path.join("snapshots"));
227+
settings.set_snapshot_path(stage_snapshot_path);
193228
settings.set_prepend_module_to_snapshot(false);
194229
settings.remove_snapshot_suffix();
195230

@@ -276,10 +311,27 @@ fn run_case_inner(tmpdir: &AbsolutePath, fixture_path: &std::path::Path, fixture
276311

277312
let mut e2e_outputs = String::new();
278313
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);
314+
let step_display = step.display_command();
315+
let mut cmd = match step.spawn_mode() {
316+
StepSpawn::Shell(command) => {
317+
let mut cmd = CommandBuilder::new(&shell_exe);
318+
cmd.arg("-c");
319+
cmd.arg(command);
320+
cmd
321+
}
322+
StepSpawn::Direct(argv) => {
323+
// Resolve the program from CARGO_BIN_EXE_<name> if available,
324+
// since CommandBuilder doesn't do PATH lookup on all platforms.
325+
let program = argv[0].as_str();
326+
let exe_env = vite_str::format!("CARGO_BIN_EXE_{program}");
327+
let resolved = env::var_os(exe_env.as_str()).unwrap_or_else(|| program.into());
328+
let mut cmd = CommandBuilder::new(resolved);
329+
for arg in &argv[1..] {
330+
cmd.arg(arg.as_str());
331+
}
332+
cmd
333+
}
334+
};
283335
cmd.env_clear();
284336
cmd.env("PATH", &e2e_env_path);
285337
cmd.env("NO_COLOR", "1");
@@ -390,7 +442,7 @@ fn run_case_inner(tmpdir: &AbsolutePath, fixture_path: &std::path::Path, fixture
390442
}
391443

392444
e2e_outputs.push_str("> ");
393-
e2e_outputs.push_str(step_command);
445+
e2e_outputs.push_str(&step_display);
394446
e2e_outputs.push('\n');
395447

396448
e2e_outputs.push_str(&redact_e2e_output(output, e2e_stage_path_str));
@@ -412,9 +464,6 @@ fn run_case_inner(tmpdir: &AbsolutePath, fixture_path: &std::path::Path, fixture
412464
}
413465

414466
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") };
417-
418467
let filter = std::env::args().nth(1);
419468

420469
let tmp_dir = tempfile::tempdir().unwrap();
@@ -430,7 +479,7 @@ fn main() {
430479

431480
// Copy .node-version to the tmp dir so version manager shims can resolve the correct
432481
// Node.js binary when running task commands.
433-
let repo_root = manifest_dir.join("../..").canonicalize().unwrap();
482+
let repo_root = manifest_dir.parent().unwrap().parent().unwrap();
434483
std::fs::copy(repo_root.join(".node-version"), tmp_dir.path().join(".node-version"))
435484
.unwrap();
436485

0 commit comments

Comments
 (0)