Skip to content

Commit 3780383

Browse files
committed
fix(test): address Codex round-14 findings
Real-tool resolution runs from the step cwd, so relative PATH entries resolve exactly as the child sees them (after-steps included). The migrator omits only the ROOT steps.json/snap.txt; a project file with the same name in a subdirectory carries over. Windows global provisioning copies vp-shim.exe beside vp.exe when the source build has one, so trampoline-creating cases (vp env setup) work. Claude-Session: https://claude.ai/code/session_01NRgjMi2Vus3iJctudGEWPT
1 parent c060a5d commit 3780383

3 files changed

Lines changed: 26 additions & 11 deletions

File tree

crates/vite_cli_snapshots/tests/cli_snapshots/flavor.rs

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -268,6 +268,15 @@ pub fn provision(flavor: Flavor, run_root: &Path) -> Result<FlavorRuntime, Strin
268268
for name in ["vp", "vpr", "vpx"] {
269269
install_tool(&bin_dir, name, &vp)?;
270270
}
271+
// Windows `vp env setup` looks for the trampoline template
272+
// (vp-shim.exe) beside vp.exe; carry it over when the source
273+
// build has one, so shim-creating cases work.
274+
#[cfg(windows)]
275+
if let Some(shim) = vp.parent().map(|dir| dir.join("vp-shim.exe"))
276+
&& shim.is_file()
277+
{
278+
let _ = std::fs::copy(&shim, bin_dir.join("vp-shim.exe"));
279+
}
271280
Some(repo_root().join("packages/cli/dist"))
272281
}
273282
Flavor::Local => {
@@ -298,6 +307,7 @@ impl FlavorRuntime {
298307
&self,
299308
program: &str,
300309
case_path: &std::ffi::OsStr,
310+
cwd: &Path,
301311
) -> Result<PathBuf, String> {
302312
match program {
303313
"vp" | "vpr" | "vpx" | "oxfmt" | "oxlint" => {
@@ -306,14 +316,14 @@ impl FlavorRuntime {
306316
// on that PATH too, so this is a pure precedence rule; the
307317
// direct bin-dir lookup below only remains as the fallback
308318
// for cases that override PATH entirely.
309-
if let Ok(found) = which::which_in(program, Some(case_path), PathBuf::from(".")) {
319+
if let Ok(found) = which::which_in(program, Some(case_path), cwd) {
310320
return Ok(found);
311321
}
312322
self.bin_dir_tool(program)
313323
}
314324
"vpt" => self.bin_dir_tool(program),
315325
"node" | "git" | "npm" | "pnpm" | "yarn" | "bun" => {
316-
which::which_in(program, Some(case_path), PathBuf::from("."))
326+
which::which_in(program, Some(case_path), cwd)
317327
.map_err(|e| format!("`{program}` not found on the case PATH: {e}"))
318328
}
319329
other => Err(format!(

crates/vite_cli_snapshots/tests/cli_snapshots/main.rs

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -702,11 +702,12 @@ fn run_case(
702702
step_env_override = env;
703703
&step_env_override
704704
};
705-
// Resolution honors a per-step PATH override, so steps testing shims
706-
// or custom prefixes run exactly the tool the child would see.
705+
// Resolution honors a per-step PATH override and runs from the step
706+
// cwd (relative PATH entries resolve as the child would see them),
707+
// so shim and custom-prefix steps run exactly the child's tool.
707708
let step_path = step_env.get("PATH").cloned().unwrap_or_else(|| case_path.clone());
708-
let program = runtime.resolve_program(&argv[0], &step_path)?;
709709
let step_cwd = stage.join(step.cwd().unwrap_or(case.cwd.as_str()));
710+
let program = runtime.resolve_program(&argv[0], &step_path, &step_cwd)?;
710711
let timeout = step.timeout();
711712

712713
let (termination_state, raw_output) = if step.tty() {
@@ -910,12 +911,13 @@ fn run_case(
910911
&after_env_override
911912
};
912913
let after_path = after_env.get("PATH").cloned().unwrap_or_else(|| case_path.clone());
913-
if let Ok(program) = runtime.resolve_program(&argv[0], &after_path) {
914+
let after_cwd = stage.join(step.cwd().unwrap_or(case.cwd.as_str()));
915+
if let Ok(program) = runtime.resolve_program(&argv[0], &after_path, &after_cwd) {
914916
let mut cmd = std::process::Command::new(program);
915917
cmd.args(&argv[1..])
916918
.env_clear()
917919
.envs(after_env)
918-
.current_dir(stage.join(step.cwd().unwrap_or(case.cwd.as_str())))
920+
.current_dir(&after_cwd)
919921
.stdin(std::process::Stdio::null())
920922
.stdout(std::process::Stdio::null())
921923
.stderr(std::process::Stdio::null());

packages/tools/src/migrate-snap-tests.ts

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -576,12 +576,15 @@ function migrateCase(
576576
// Write the fixture: everything except steps.json and snap.txt carries over.
577577
const fixtureDir = path.join(outDir, newName);
578578
fs.mkdirSync(fixtureDir, { recursive: true });
579+
// Only the ROOT metadata files are omitted; a project file that happens
580+
// to be named snap.txt or steps.json in a subdirectory carries over.
581+
const rootMetadata = new Set([
582+
path.resolve(caseDir, 'steps.json'),
583+
path.resolve(caseDir, 'snap.txt'),
584+
]);
579585
fs.cpSync(caseDir, fixtureDir, {
580586
recursive: true,
581-
filter: (src) => {
582-
const base = path.basename(src);
583-
return base !== 'steps.json' && base !== 'snap.txt';
584-
},
587+
filter: (src) => !rootMetadata.has(path.resolve(src)),
585588
});
586589
fs.writeFileSync(path.join(fixtureDir, 'snapshots.toml'), `${lines.join('\n')}\n`);
587590
return report;

0 commit comments

Comments
 (0)