From c23529953f632e84bda9fde6e78aa9356807d200 Mon Sep 17 00:00:00 2001 From: MK Date: Mon, 21 Sep 2026 21:54:37 +0800 Subject: [PATCH 01/10] test(snapshots): share package preparation and retain diagnostics --- .github/workflows/ci.yml | 25 +++ .../tests/cli_snapshots/README.md | 53 +++-- .../tests/cli_snapshots/main.rs | 144 ++++++++++---- .../tests/cli_snapshots/registry_pack.rs | 125 ++++++++++++ .../tests/cli_snapshots/report.rs | 129 +++++++++++++ .../tests/registry_pack_unit.rs | 182 ++++++++++++++++++ crates/vp_cli_snapshots/tests/report_unit.rs | 62 ++++++ 7 files changed, 665 insertions(+), 55 deletions(-) create mode 100644 crates/vp_cli_snapshots/tests/cli_snapshots/registry_pack.rs create mode 100644 crates/vp_cli_snapshots/tests/cli_snapshots/report.rs create mode 100644 crates/vp_cli_snapshots/tests/registry_pack_unit.rs create mode 100644 crates/vp_cli_snapshots/tests/report_unit.rs diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 431c7f46dd..916ca0e6f5 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -970,6 +970,9 @@ jobs: - name: Build snapshot test binaries run: cargo test -p vp_cli_snapshots --no-run --message-format=json > "$RUNNER_TEMP/snapshot-test-build.json" + - name: Test snapshot runner utilities + run: cargo test -p vp_cli_snapshots --test redact_unit --test shard_unit --test registry_pack_unit --test report_unit + - name: Package CLI test binaries run: | artifact_dir="$(mktemp -d)" @@ -1110,6 +1113,16 @@ jobs: CARGO_MANIFEST_DIR: ${{ github.workspace }}/crates/vp_cli_snapshots CARGO_BIN_EXE_vpt: ${{ github.workspace }}/target/snapshot-tests/vpt VP_SNAP_SHARD: ${{ matrix.shard }}/3 + VP_SNAP_ARTIFACTS_DIR: ${{ runner.temp }}/snapshot-artifacts + + - name: Upload snapshot diagnostics and timings + if: ${{ !cancelled() }} + uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1 + with: + name: snapshot-diagnostics-${{ matrix.target }}-${{ matrix.shard }} + path: ${{ runner.temp }}/snapshot-artifacts + if-no-files-found: ignore + retention-days: 7 # Runs the PTY snapshot suite (crates/vp_cli_snapshots) on Windows with # BOTH vp flavors, without a Rust toolchain on the runner: the test binary @@ -1256,9 +1269,21 @@ jobs: exit "$test_exit" env: RUST_BACKTRACE: '1' + # Share immutable tarballs across nextest processes in this job. + VP_SNAP_PACKAGES_DIR: ${{ runner.temp }}/snapshot-packages + VP_SNAP_ARTIFACTS_DIR: ${{ runner.temp }}/snapshot-artifacts # Keep Windows env parity with the `test` recipe in justfile. __COMPAT_LAYER: RunAsInvoker + - name: Upload snapshot diagnostics and timings + if: ${{ !cancelled() }} + uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1 + with: + name: snapshot-diagnostics-windows-${{ matrix.shard }} + path: ${{ runner.temp }}/snapshot-artifacts + if-no-files-found: ignore + retention-days: 7 + cli-e2e-test-musl: name: CLI E2E test (Linux x64 musl) needs: diff --git a/crates/vp_cli_snapshots/tests/cli_snapshots/README.md b/crates/vp_cli_snapshots/tests/cli_snapshots/README.md index 9129433673..9e8a0f31da 100644 --- a/crates/vp_cli_snapshots/tests/cli_snapshots/README.md +++ b/crates/vp_cli_snapshots/tests/cli_snapshots/README.md @@ -64,19 +64,46 @@ nextest runner with `--partition hash:1/3` (then `2/3` and `3/3`). Environment overrides, mainly for CI: -| Variable | Effect | -| --------------------------- | ------------------------------------------------------------------- | -| `VP_SNAP_GLOBAL_VP` | Path to a prebuilt global `vp` binary (skips the target-dir lookup) | -| `VP_SNAP_LOCAL_CLI_BIN_DIR` | Local CLI bin dir (default `/packages/cli/bin`) | -| `VP_SNAP_JS_RUNTIME_DIR` | Provisioned managed runtime to seed case homes with | -| `VP_SNAP_SH_BIN` | POSIX `sh` binary for cases that execute the generated `env` file | -| `VP_SNAP_BASH_BIN` | Bash binary for cases that execute the generated `env` file | -| `VP_SNAP_ZSH_BIN` | Zsh binary for cases that execute the generated `env` file | -| `VP_SNAP_CMD_BIN` | System cmd.exe for cases that execute generated batch files | -| `VP_SNAP_FISH_BIN` | Fish binary for cases that execute generated `env.fish` files | -| `VP_SNAP_NU_BIN` | Nushell binary for cases that execute generated `env.nu` files | -| `VP_SNAP_PWSH_BIN` | PowerShell binary for cases that execute generated `env.ps1` files | -| `VP_SNAP_SKIP_FLAVORS` | Comma-separated flavors to skip registering (e.g. `local`) | +| Variable | Effect | +| --------------------------- | ----------------------------------------------------------------------------- | +| `VP_SNAP_GLOBAL_VP` | Path to a prebuilt global `vp` binary (skips the target-dir lookup) | +| `VP_SNAP_LOCAL_CLI_BIN_DIR` | Local CLI bin dir (default `/packages/cli/bin`) | +| `VP_SNAP_JS_RUNTIME_DIR` | Provisioned managed runtime to seed case homes with | +| `VP_SNAP_SH_BIN` | POSIX `sh` binary for cases that execute the generated `env` file | +| `VP_SNAP_BASH_BIN` | Bash binary for cases that execute the generated `env` file | +| `VP_SNAP_ZSH_BIN` | Zsh binary for cases that execute the generated `env` file | +| `VP_SNAP_CMD_BIN` | System cmd.exe for cases that execute generated batch files | +| `VP_SNAP_FISH_BIN` | Fish binary for cases that execute generated `env.fish` files | +| `VP_SNAP_NU_BIN` | Nushell binary for cases that execute generated `env.nu` files | +| `VP_SNAP_PWSH_BIN` | PowerShell binary for cases that execute generated `env.ps1` files | +| `VP_SNAP_SKIP_FLAVORS` | Comma-separated flavors to skip registering (e.g. `local`) | +| `VP_SNAP_PACKAGES_DIR` | Run-scoped directory for sharing packed packages across test processes | +| `VP_SNAP_ARTIFACTS_DIR` | Directory for phase timings and failure diagnostics; unset disables artifacts | + +`VP_SNAP_PACKAGES_DIR` packs the checkout on the first registry case, under a +cross-process file lock. Later cases reuse the completed tarballs. Use a fresh +directory for each test run and keep the build unchanged while tests run. The +runner checks the checkout path and input/tarball file sizes and modification +times; a changed build fails instead of silently testing stale packages. This +is a per-run preparation directory, not a persistent content-addressed cache. +Each case still has its own registry server, overlays, and mutable caches. +Windows CI uses this to avoid repacking in every nextest process. + +With `VP_SNAP_ARTIFACTS_DIR` set, each runner process creates a unique `run-*` +directory. `runner/timing.json` records discovery, test execution, and final +cleanup. `cases///timing.json` records gate waiting, +provisioning, package preparation/reuse, registry startup, individual steps, +rendering, comparison, and cleanup. `registry-pack` appears only in the process +that actually packs; it is nested inside `registry-pack-or-reuse`. Phase +durations are milliseconds; nested phases must not be added together. The +existing console timings still exclude gate waiting. + +Failed cases also write `error.txt`, `expected.md` (when present), `actual.md` +(when a complete or partial snapshot was rendered), and `output.txt`. The output +contains the last 1 MiB of unredacted rendered step output, including successful +hidden steps; it is not the raw PTY byte stream. Artifacts survive temporary +workspace cleanup. CI uploads these files and timings on successful and failed +runs. Listing tests with `--list` neither packs packages nor creates artifacts. ## Case reference diff --git a/crates/vp_cli_snapshots/tests/cli_snapshots/main.rs b/crates/vp_cli_snapshots/tests/cli_snapshots/main.rs index 52c59b9c3b..8d9c81e55c 100644 --- a/crates/vp_cli_snapshots/tests/cli_snapshots/main.rs +++ b/crates/vp_cli_snapshots/tests/cli_snapshots/main.rs @@ -18,6 +18,8 @@ mod exit_code; mod flavor; mod redact; +mod registry_pack; +mod report; mod shard; use std::{ @@ -1193,11 +1195,13 @@ fn run_case( runtime: &FlavorRuntime, snapshot_name: &str, local_registry_pack: Option<&Path>, + report: &report::Report, ) -> Result<(), String> { let snapshots = snapshot_test::Snapshots::new(fixture_path.join("snapshots")); // Copy the fixture to a per-case staging directory so the test runs in // isolation and workspace-root discovery doesn't walk past the fixture. + let staging_phase = report.phase("fixture-staging"); let case_root = tmpdir.join(format!("{fixture_name}_case_{case_index}_{}", flavor.as_str())); let stage = case_root.join("workspace"); std::fs::create_dir_all(&stage).unwrap(); @@ -1211,8 +1215,11 @@ fn run_case( .copy_tree(fixture_path, &stage) .unwrap(); + drop(staging_phase); + let setup_phase = report.phase("case-setup"); let case_home = CaseHome::provision(&case_root, case.seed_runtime); let case_install = case_home.provision_vite_plus(flavor, runtime)?; + drop(setup_phase); let mut case_env = baseline_env(&case_home, &case_install); for key in &case.unset_env { @@ -1232,7 +1239,8 @@ fn run_case( // fold its registry env into every step. Held to the end of the case so // its teardown removes the throwaway package-manager caches. Registry env // never sets PATH, so folding it in after `case_path` is derived is safe. - let _registry = if case.local_registry { + let registry_phase = report.phase("registry-startup"); + let registry = if case.local_registry { let pack_dir = local_registry_pack .ok_or("internal error: local-registry case reached run_case without a packed dir")?; // Prefer the seed runtime's real node binary over the case's node @@ -1251,6 +1259,8 @@ fn run_case( None }; + drop(registry_phase); + // Installs through the local registry are slower than pure vp commands, so // local-registry steps get a 120s default (still overridable per step); // everything else keeps the standard per-step default. @@ -1292,6 +1302,11 @@ fn run_case( let step_env: &BTreeMap = &step_env; let timeout = step.timeout(step_default_timeout); + let execution_phase = report.phase(format!( + "step-{}: {}", + step_index + 1, + step.display_command_line(&case.cwd) + )); let (termination_state, raw_output) = if step.tty { 'tty: { let mut cmd = CommandBuilder::new(&program); @@ -1430,6 +1445,10 @@ fn run_case( (state, block) }; + drop(execution_phase); + let rendering_phase = report.phase(format!("render-step-{}", step_index + 1)); + report.capture(&step.display_command_line(&case.cwd), &raw_output); + // Blank line separator before every `##`. doc.push('\n'); doc.push_str("## `"); @@ -1478,6 +1497,8 @@ fn run_case( doc.push_str(&redacted); } + drop(rendering_phase); + // Shell-like `&&` semantics with line boundaries: a failing step // skips the rest of its line, up to and including the next // continue-on-failure step, then the following line resumes. @@ -1504,6 +1525,8 @@ fn run_case( step_index += 1; } + report.actual(&doc); + let cleanup_phase = report.phase("case-cleanup"); // Cleanup steps: best-effort, never snapshotted. Per-step envs apply // here too: cleanup often depends on the same PATH/prefix overrides as // the step it tears down. @@ -1529,11 +1552,15 @@ fn run_case( } } + drop(registry); + drop(cleanup_phase); + // Deferred so the cleanup above always runs, even for hung steps. if let Some(error) = timeout_error { return Err(error); } + let _comparison_phase = report.phase("snapshot-comparison"); snapshots.check_snapshot(snapshot_name, &doc) } @@ -1614,6 +1641,24 @@ fn case_needs_isolation(case: &Case) -> bool { } fn main() { + let args = libtest_mimic::Arguments::from_args(); + // A unique process directory avoids collisions between nextest workers and + // repeated trials (for example the two Windows PowerShell versions in CI). + let artifacts = if args.list { + None + } else { + std::env::var_os("VP_SNAP_ARTIFACTS_DIR").map(|root| { + std::fs::create_dir_all(&root).expect("failed to create snapshot artifact directory"); + tempfile::Builder::new() + .prefix("run-") + .tempdir_in(root) + .expect("failed to create process artifact directory") + .keep() + }) + }; + let run_report = + report::Report::new(artifacts.as_ref().map(|dir| dir.join("runner")), "runner".into()); + let discovery_phase = run_report.phase("discovery"); let tmp_dir = tempfile::tempdir().unwrap(); // dunce, not std: std's canonicalize returns a `\\?\` verbatim path on // Windows, and CMD.EXE (which runs the local flavor's .cmd shims) @@ -1654,8 +1699,6 @@ fn main() { // run in isolation; see `EXECUTION_GATE`. This replaces the old // Linux-wide `--test-threads=1`, which serialized the entire suite to // protect a handful of ctrl-c cases. - let args = libtest_mimic::Arguments::from_args(); - // `VP_SNAP_SKIP_FLAVORS=local` (comma-separated) skips registering trials // for a flavor entirely; CI legs that don't build the JS CLI use it. let skip_flavors: Vec = std::env::var("VP_SNAP_SKIP_FLAVORS") @@ -1670,9 +1713,9 @@ fn main() { run_root: Arc, global: std::sync::OnceLock>, local: std::sync::OnceLock>, - /// Packed checkout packages (vite-plus, @voidzero-dev/vite-plus-core), - /// produced once on the first local-registry case and shared by every - /// per-case registry via `SNAP_LOCAL_VP_PACKAGES_DIR`. + /// Packed checkout packages, prepared once per process by default. + /// VP_SNAP_PACKAGES_DIR also shares them across nextest processes; + /// this cell avoids revalidating the directory for each local trial. local_registry_pack: std::sync::OnceLock, String>>, } impl LazyRuntimes { @@ -1687,31 +1730,36 @@ fn main() { /// Packs the checkout packages once and returns the shared dir. Reuses /// `local-npm-registry.ts --pack-to` so the pack logic lives in one /// place (the same helper the tool and ecosystem-ci use). - fn local_registry_pack(&self) -> Result, String> { + fn local_registry_pack(&self, report: &report::Report) -> Result, String> { self.local_registry_pack .get_or_init(|| { let node = which::which("node") .map_err(|e| format!("`node` not found on PATH (needed to pack): {e}"))?; let repo_root = flavor::repo_root(); let script = repo_root.join("packages/tools/src/local-npm-registry.ts"); - let dest = self.run_root.join("local-registry-packages"); - std::fs::create_dir_all(&dest) - .map_err(|e| format!("failed to create pack dir: {e}"))?; - // Inherit the runner's environment so the packer finds - // `pnpm` and `node` the same way a developer would. - let output = std::process::Command::new(&node) - .arg(&script) - .arg("--pack-to") - .arg(&dest) - .current_dir(&repo_root) - .output() - .map_err(|e| format!("failed to run local-registry pack: {e}"))?; - if !output.status.success() { - return Err(format!( - "packing checkout packages failed:\n{}", - String::from_utf8_lossy(&output.stderr) - )); - } + let root = std::env::var_os("VP_SNAP_PACKAGES_DIR").map_or_else( + || self.run_root.join("local-registry-packages"), + PathBuf::from, + ); + let dest = registry_pack::get_or_prepare(&root, &repo_root, |dest| { + let _packing_phase = report.phase("registry-pack"); + // Inherit the runner environment so pnpm and node resolve + // just as they do for a developer's normal package build. + let output = std::process::Command::new(&node) + .arg(&script) + .arg("--pack-to") + .arg(dest) + .current_dir(&repo_root) + .output() + .map_err(|e| format!("failed to run local-registry pack: {e}"))?; + if !output.status.success() { + return Err(format!( + "packing checkout packages failed:\n{}", + String::from_utf8_lossy(&output.stderr) + )); + } + Ok(()) + })?; Ok(Arc::from(dest.as_path())) }) .clone() @@ -1775,29 +1823,29 @@ fn main() { let isolated = case_needs_isolation(&case); let timings = Arc::clone(&timings); let timing_name = trial_name.clone(); + let artifact_dir = artifacts + .as_ref() + .map(|dir| dir.join("cases").join(&*fixture_name).join(&snapshot_name)); tests.push( libtest_mimic::Trial::test(trial_name, move || { - // Hold the execution lease for the whole case: shared - // (parallel) unless the case needs isolation. Acquired - // before timing so the reported duration is the case's - // own work, not time spent waiting for the lease. + let report = report::Report::new(artifact_dir, timing_name.clone()); + let gate_phase = report.phase("gate-wait"); + // Keep the console's case duration independent of lock + // waiting; the artifact records that wait separately. let _gate = acquire_gate(isolated); + drop(gate_phase); let started = std::time::Instant::now(); - let result = (|| -> Result<(), libtest_mimic::Failed> { - let runtime = match runtimes.get(flavor) { - Ok(runtime) => runtime, - Err(message) => return Err(message.clone().into()), - }; - // Pack the checkout once, lazily, only when a - // local-registry case actually runs. + let result = (|| -> Result<(), String> { + let runtime_phase = report.phase("flavor-setup"); + let runtime = runtimes.get(flavor).as_ref().map_err(Clone::clone)?; + drop(runtime_phase); + let pack_phase = report.phase("registry-pack-or-reuse"); let local_registry_pack = if case.local_registry { - match runtimes.local_registry_pack() { - Ok(dir) => Some(dir), - Err(message) => return Err(message.into()), - } + Some(runtimes.local_registry_pack(&report)?) } else { None }; + drop(pack_phase); run_case( &tmp_dir_path, &fixture_path, @@ -1808,11 +1856,15 @@ fn main() { runtime, &snapshot_name, local_registry_pack.as_deref(), + &report, ) - .map_err(Into::into) })(); timings.lock().unwrap().push((timing_name, started.elapsed())); - result + let artifact_result = report.finish( + result.as_ref().err().map(String::as_str), + Some(&fixture_path.join("snapshots").join(&snapshot_name)), + ); + result.and(artifact_result).map_err(Into::into) }) .with_ignored_flag(ignored), ); @@ -1825,7 +1877,10 @@ fn main() { tests = shard::select(tests, shard).unwrap_or_else(|error| panic!("{error}")); } + drop(discovery_phase); + let execution_phase = run_report.phase("tests"); let conclusion = libtest_mimic::run(&args, tests); + drop(execution_phase); // Report each case's wall time (slowest first). Skipped for `--list`, // which runs nothing. @@ -1842,6 +1897,11 @@ fn main() { // exit() never returns, so the staged run tree must be dropped first or // every run would leave its full tempdir behind. + let cleanup_phase = run_report.phase("run-cleanup"); drop(tmp_dir); + drop(cleanup_phase); + run_report + .finish(conclusion.has_failed().then_some("one or more snapshot tests failed"), None) + .expect("failed to write runner timing artifact"); conclusion.exit(); } diff --git a/crates/vp_cli_snapshots/tests/cli_snapshots/registry_pack.rs b/crates/vp_cli_snapshots/tests/cli_snapshots/registry_pack.rs new file mode 100644 index 0000000000..09bb98d332 --- /dev/null +++ b/crates/vp_cli_snapshots/tests/cli_snapshots/registry_pack.rs @@ -0,0 +1,125 @@ +//! Share immutable package tarballs across nextest processes within one test run. +use std::{ + fs::{File, OpenOptions}, + path::{Path, PathBuf}, + time::UNIX_EPOCH, +}; + +/// Filesystem stamps avoid re-reading and compressing the package bytes in every +/// worker. The directory is run-scoped, not a persistent cache across builds. +fn stamp(path: &Path) -> Result { + let metadata = path.metadata().map_err(|error| format!("{}: {error}", path.display()))?; + let modified = metadata + .modified() + .and_then(|time| time.duration_since(UNIX_EPOCH).map_err(std::io::Error::other)) + .map_err(|error| format!("{}: {error}", path.display()))?; + Ok(serde_json::json!({ + "bytes": metadata.len(), + "modified_ns": modified.as_nanos().to_string(), + })) +} + +fn collect_inputs( + root: &Path, + relative: &Path, + inputs: &mut std::collections::BTreeMap, + ancestors: &mut Vec, +) -> Result<(), String> { + let path = root.join(relative); + if path.is_dir() { + let canonical = dunce::canonicalize(&path).map_err(|error| error.to_string())?; + if ancestors.contains(&canonical) { + return Err(format!("cycle in package inputs at {}", path.display())); + } + ancestors.push(canonical); + for entry in path.read_dir().map_err(|error| format!("{}: {error}", path.display()))? { + let entry = entry.map_err(|error| error.to_string())?; + // Installed dependencies and Cargo output are not package inputs. + if matches!(entry.file_name().to_str(), Some("node_modules" | "target" | ".git")) { + continue; + } + collect_inputs(root, &relative.join(entry.file_name()), inputs, ancestors)?; + } + ancestors.pop(); + } else { + inputs.insert(relative.to_string_lossy().into_owned(), stamp(&path)?); + } + Ok(()) +} + +fn build_stamp(repo: &Path) -> Result { + let repo = dunce::canonicalize(repo).map_err(|error| error.to_string())?; + let mut inputs = std::collections::BTreeMap::new(); + for path in + ["package.json", "pnpm-workspace.yaml", "pnpm-lock.yaml", "packages/cli", "packages/core"] + { + collect_inputs(&repo, Path::new(path), &mut inputs, &mut Vec::new())?; + } + Ok(serde_json::json!({ "repo": repo, "inputs": inputs })) +} + +fn archives(directory: &Path) -> Result { + let mut archives = std::collections::BTreeMap::new(); + for entry in directory.read_dir().map_err(|error| error.to_string())? { + let entry = entry.map_err(|error| error.to_string())?; + if entry.path().extension().is_some_and(|extension| extension == "tgz") { + archives + .insert(entry.file_name().to_string_lossy().into_owned(), stamp(&entry.path())?); + } + } + if archives.len() != 2 { + return Err(format!( + "expected two packed packages in {}, found {}", + directory.display(), + archives.len() + )); + } + Ok(serde_json::json!(archives)) +} + +/// Only the lock holder prepares the packages. Publish the complete directory +/// atomically, so a failed pack can be retried without exposing partial tarballs. +/// A stale directory fails explicitly rather than testing a previous build. +pub fn get_or_prepare( + root: &Path, + repo: &Path, + prepare: impl FnOnce(&Path) -> Result<(), String>, +) -> Result { + std::fs::create_dir_all(root).map_err(|error| error.to_string())?; + let lock = OpenOptions::new() + .create(true) + .truncate(false) + .read(true) + .write(true) + .open(root.join("pack.lock")) + .map_err(|error| error.to_string())?; + File::lock(&lock).map_err(|error| error.to_string())?; + let packages = root.join("packages"); + let build = build_stamp(repo)?; + if packages.exists() { + let manifest: serde_json::Value = serde_json::from_slice( + &std::fs::read(packages.join("build.json")).map_err(|error| error.to_string())?, + ) + .map_err(|error| error.to_string())?; + if manifest["build"] != build || manifest["archives"] != archives(&packages)? { + return Err(format!( + "prepared snapshot packages in {} are stale; use a fresh VP_SNAP_PACKAGES_DIR for this run", + root.display() + )); + } + return Ok(packages); + } + let staging = tempfile::tempdir_in(root).map_err(|error| error.to_string())?; + prepare(staging.path())?; + if build != build_stamp(repo)? { + return Err( + "checkout changed while packing snapshot packages; retry after the build finishes" + .into(), + ); + } + let manifest = serde_json::json!({ "build": build, "archives": archives(staging.path())? }); + std::fs::write(staging.path().join("build.json"), serde_json::to_vec(&manifest).unwrap()) + .map_err(|error| error.to_string())?; + std::fs::rename(staging.path(), &packages).map_err(|error| error.to_string())?; + Ok(packages) +} diff --git a/crates/vp_cli_snapshots/tests/cli_snapshots/report.rs b/crates/vp_cli_snapshots/tests/cli_snapshots/report.rs new file mode 100644 index 0000000000..7a3b0a8093 --- /dev/null +++ b/crates/vp_cli_snapshots/tests/cli_snapshots/report.rs @@ -0,0 +1,129 @@ +//! Optional per-process artifacts. Each trial owns its files, including under nextest. +use std::{ + cell::{Cell, RefCell}, + path::PathBuf, + time::{Duration, Instant}, +}; + +const OUTPUT_LIMIT: usize = 1024 * 1024; + +pub struct Report { + directory: Option, + name: String, + started: Instant, + phases: RefCell>, + output: RefCell, + output_truncated: Cell, + actual: RefCell>, +} + +impl Report { + pub fn new(directory: Option, name: String) -> Self { + Self { + directory, + name, + started: Instant::now(), + phases: RefCell::new(Vec::new()), + output: RefCell::new(String::new()), + output_truncated: Cell::new(false), + actual: RefCell::new(None), + } + } + + pub fn phase(&self, name: impl Into) -> Phase<'_> { + Phase { report: self, name: name.into(), started: Instant::now() } + } + + pub fn record(&self, name: &str, elapsed: Duration) { + if self.directory.is_some() { + self.phases.borrow_mut().push(serde_json::json!({ + "name": name, + "duration_ms": elapsed.as_secs_f64() * 1000.0, + })); + } + } + + /// Keep bounded, unredacted rendered output, including successful hidden steps. + /// It is only written when the trial fails; this is not the raw PTY byte stream. + pub fn capture(&self, command: &str, output: &str) { + if self.directory.is_none() { + return; + } + let mut captured = self.output.borrow_mut(); + captured.push_str(&format!("\n$ {command}\n")); + captured.push_str(output); + if captured.len() > OUTPUT_LIMIT { + self.output_truncated.set(true); + let mut start = captured.len() - OUTPUT_LIMIT; + while !captured.is_char_boundary(start) { + start += 1; + } + captured.drain(..start); + } + } + + pub fn actual(&self, contents: &str) { + if self.directory.is_some() { + *self.actual.borrow_mut() = Some(contents.to_owned()); + } + } + + pub fn finish( + &self, + error: Option<&str>, + expected: Option<&std::path::Path>, + ) -> Result<(), String> { + let Some(directory) = &self.directory else { return Ok(()) }; + let write = || -> std::io::Result<()> { + std::fs::create_dir_all(directory)?; + let timings = serde_json::json!({ + "schema_version": 1, + "name": self.name, + "output_truncated": self.output_truncated.get(), + "status": if error.is_some() { "failed" } else { "passed" }, + "duration_ms": self.started.elapsed().as_secs_f64() * 1000.0, + "phases": *self.phases.borrow(), + }); + std::fs::write(directory.join("timing.json"), serde_json::to_vec_pretty(&timings)?)?; + if let Some(error) = error { + std::fs::write(directory.join("error.txt"), error)?; + std::fs::write(directory.join("output.txt"), self.output.borrow().as_bytes())?; + if let Some(actual) = self.actual.borrow().as_ref() { + std::fs::write(directory.join("actual.md"), actual)?; + } + if let Some(expected) = expected { + match std::fs::read(expected) { + Ok(contents) => std::fs::write(directory.join("expected.md"), contents)?, + Err(error) if error.kind() == std::io::ErrorKind::NotFound => {} + Err(error) => return Err(error), + } + } + } + Ok(()) + }; + write().map_err(|error| { + format!("failed to write artifacts to {}: {error}", directory.display()) + }) + } +} + +impl Drop for Report { + fn drop(&mut self) { + if std::thread::panicking() { + // Do not mask the original panic if saving diagnostics also fails. + let _ = self.finish(Some("snapshot runner panicked; see the test log"), None); + } + } +} + +pub struct Phase<'a> { + report: &'a Report, + name: String, + started: Instant, +} + +impl Drop for Phase<'_> { + fn drop(&mut self) { + self.report.record(&self.name, self.started.elapsed()); + } +} diff --git a/crates/vp_cli_snapshots/tests/registry_pack_unit.rs b/crates/vp_cli_snapshots/tests/registry_pack_unit.rs new file mode 100644 index 0000000000..78089543b8 --- /dev/null +++ b/crates/vp_cli_snapshots/tests/registry_pack_unit.rs @@ -0,0 +1,182 @@ +#![expect(clippy::disallowed_types, reason = "standalone test uses std types")] +#![expect(clippy::disallowed_macros, reason = "standalone test uses std macros")] + +#[path = "cli_snapshots/registry_pack.rs"] +mod registry_pack; + +use std::{ + path::Path, + sync::atomic::{AtomicUsize, Ordering}, +}; + +fn repo(root: &Path) { + for package in ["cli", "core"] { + let dir = root.join("packages").join(package).join("dist"); + std::fs::create_dir_all(&dir).unwrap(); + std::fs::write(dir.join("index.js"), "built output").unwrap(); + } + for file in ["package.json", "pnpm-workspace.yaml", "pnpm-lock.yaml"] { + std::fs::write(root.join(file), "fixture").unwrap(); + } +} + +fn pack(dest: &Path) -> Result<(), String> { + for file in ["vite-plus.tgz", "core.tgz"] { + std::fs::write(dest.join(file), "packed output").unwrap(); + } + Ok(()) +} + +#[test] +fn concurrent_workers_prepare_once() { + let tmp = tempfile::tempdir().unwrap(); + repo(tmp.path()); + let cache = tmp.path().join("cache"); + let calls = AtomicUsize::new(0); + std::thread::scope(|scope| { + for _ in 0..8 { + scope.spawn(|| { + let prepared = registry_pack::get_or_prepare(&cache, tmp.path(), |dir| { + calls.fetch_add(1, Ordering::SeqCst); + pack(dir) + }) + .unwrap(); + assert_eq!( + std::fs::read_to_string(prepared.join("core.tgz")).unwrap(), + "packed output" + ); + }); + } + }); + assert_eq!(calls.load(Ordering::SeqCst), 1); +} + +#[test] +fn rejects_changed_build_and_changed_archives() { + for change in ["input", "archive", "new-input"] { + let tmp = tempfile::tempdir().unwrap(); + repo(tmp.path()); + let cache = tmp.path().join("cache"); + let prepared = registry_pack::get_or_prepare(&cache, tmp.path(), pack).unwrap(); + let path = match change { + "input" => tmp.path().join("packages/cli/dist/index.js"), + "new-input" => tmp.path().join("packages/core/dist/new.js"), + _ => prepared.join("core.tgz"), + }; + std::fs::write(path, "different package content").unwrap(); + let error = registry_pack::get_or_prepare(&cache, tmp.path(), |_| { + panic!("must not repack in-use files") + }) + .unwrap_err(); + assert!(error.contains("stale"), "{error}"); + } +} + +#[test] +fn failed_pack_is_not_published_and_can_be_retried() { + let tmp = tempfile::tempdir().unwrap(); + repo(tmp.path()); + let cache = tmp.path().join("cache"); + assert!( + registry_pack::get_or_prepare(&cache, tmp.path(), |dir| { + std::fs::write(dir.join("partial.tgz"), "incomplete").unwrap(); + Err("pack failed".into()) + }) + .is_err() + ); + assert!(!cache.join("packages").exists()); + registry_pack::get_or_prepare(&cache, tmp.path(), pack).unwrap(); +} + +#[test] +fn rejects_checkout_changes_during_pack() { + let tmp = tempfile::tempdir().unwrap(); + repo(tmp.path()); + let cache = tmp.path().join("cache"); + let error = registry_pack::get_or_prepare(&cache, tmp.path(), |dir| { + pack(dir)?; + std::fs::write(tmp.path().join("packages/cli/dist/index.js"), "rebuilt while packing") + .unwrap(); + Ok(()) + }) + .unwrap_err(); + assert!(error.contains("checkout changed")); + assert!(!cache.join("packages").exists()); +} + +#[test] +fn installed_dependencies_do_not_invalidate_preparation() { + let tmp = tempfile::tempdir().unwrap(); + repo(tmp.path()); + let cache = tmp.path().join("cache"); + registry_pack::get_or_prepare(&cache, tmp.path(), pack).unwrap(); + let modules = tmp.path().join("packages/cli/node_modules"); + std::fs::create_dir(&modules).unwrap(); + std::fs::write(modules.join("dependency"), "not packed").unwrap(); + registry_pack::get_or_prepare(&cache, tmp.path(), |_| panic!("must reuse packed files")) + .unwrap(); +} + +#[test] +fn separate_processes_prepare_once() { + let tmp = tempfile::tempdir().unwrap(); + repo(tmp.path()); + let cache = tmp.path().join("cache"); + let workers: Vec<_> = (0..4) + .map(|_| { + std::process::Command::new(std::env::current_exe().unwrap()) + .args(["--ignored", "--exact", "pack_worker"]) + .env("VP_SNAP_TEST_REPO", tmp.path()) + .env("VP_SNAP_TEST_PACKAGES", &cache) + .stdout(std::process::Stdio::piped()) + .stderr(std::process::Stdio::piped()) + .spawn() + .unwrap() + }) + .collect(); + for worker in workers { + let output = worker.wait_with_output().unwrap(); + assert!( + output.status.success(), + "{}\n{}", + String::from_utf8_lossy(&output.stdout), + String::from_utf8_lossy(&output.stderr) + ); + } + assert_eq!(std::fs::read_to_string(cache.join("preparations")).unwrap(), "packed\n"); +} + +#[test] +#[ignore = "subprocess helper for separate_processes_prepare_once"] +fn pack_worker() { + use std::io::Write as _; + let Some(repo) = std::env::var_os("VP_SNAP_TEST_REPO") else { return }; + let cache = std::path::PathBuf::from(std::env::var_os("VP_SNAP_TEST_PACKAGES").unwrap()); + registry_pack::get_or_prepare(&cache, Path::new(&repo), |dir| { + let mut calls = std::fs::OpenOptions::new() + .create(true) + .append(true) + .open(cache.join("preparations")) + .unwrap(); + calls.write_all(b"packed\n").unwrap(); + pack(dir) + }) + .unwrap(); +} + +#[cfg(unix)] +#[test] +fn linked_package_inputs_are_validated_without_following_cycles() { + let tmp = tempfile::tempdir().unwrap(); + repo(tmp.path()); + let docs = tmp.path().join("docs"); + std::fs::create_dir(&docs).unwrap(); + std::fs::write(docs.join("guide.md"), "original guide").unwrap(); + std::os::unix::fs::symlink(&docs, tmp.path().join("packages/cli/docs")).unwrap(); + let cache = tmp.path().join("cache"); + registry_pack::get_or_prepare(&cache, tmp.path(), pack).unwrap(); + std::fs::write(docs.join("guide.md"), "updated packaged guide").unwrap(); + assert!(registry_pack::get_or_prepare(&cache, tmp.path(), pack).unwrap_err().contains("stale")); + std::os::unix::fs::symlink(&docs, docs.join("cycle")).unwrap(); + assert!(registry_pack::get_or_prepare(&cache, tmp.path(), pack).unwrap_err().contains("cycle")); +} diff --git a/crates/vp_cli_snapshots/tests/report_unit.rs b/crates/vp_cli_snapshots/tests/report_unit.rs new file mode 100644 index 0000000000..8d95c1192b --- /dev/null +++ b/crates/vp_cli_snapshots/tests/report_unit.rs @@ -0,0 +1,62 @@ +#![expect(clippy::disallowed_types, reason = "standalone test uses std types")] +#![expect(clippy::disallowed_macros, reason = "standalone test uses std macros")] + +#[path = "cli_snapshots/report.rs"] +mod report; + +#[test] +fn successful_cases_only_write_timings() { + let tmp = tempfile::tempdir().unwrap(); + let report = report::Report::new(Some(tmp.path().to_path_buf()), "fixture::case".into()); + { + let _phase = report.phase("command"); + } + report.capture("vpt print", "not a failure"); + report.actual("# snapshot\n"); + report.finish(None, None).unwrap(); + assert!(!tmp.path().join("output.txt").exists()); + assert!(!tmp.path().join("actual.md").exists()); + let timing: serde_json::Value = + serde_json::from_slice(&std::fs::read(tmp.path().join("timing.json")).unwrap()).unwrap(); + assert_eq!(timing["name"], "fixture::case"); + assert_eq!(timing["status"], "passed"); + assert_eq!(timing["phases"][0]["name"], "command"); +} + +#[test] +fn failures_keep_expected_actual_and_bounded_unicode_output() { + let tmp = tempfile::tempdir().unwrap(); + let expected = tmp.path().join("baseline.md"); + std::fs::write(&expected, "expected snapshot").unwrap(); + let dir = tmp.path().join("artifacts"); + let report = report::Report::new(Some(dir.clone()), "fixture::case".into()); + report.capture("setup", &"界".repeat(400_000)); + report.capture("vp check", "diagnostic tail"); + report.actual("actual snapshot"); + report.finish(Some("snapshot mismatch"), Some(&expected)).unwrap(); + let output = std::fs::read_to_string(dir.join("output.txt")).unwrap(); + assert!(output.len() <= 1024 * 1024); + let timing: serde_json::Value = + serde_json::from_slice(&std::fs::read(dir.join("timing.json")).unwrap()).unwrap(); + assert_eq!(timing["output_truncated"], true); + assert!(output.ends_with("$ vp check\ndiagnostic tail")); + assert_eq!(std::fs::read_to_string(dir.join("expected.md")).unwrap(), "expected snapshot"); + assert_eq!(std::fs::read_to_string(dir.join("actual.md")).unwrap(), "actual snapshot"); + assert_eq!(std::fs::read_to_string(expected).unwrap(), "expected snapshot"); +} + +#[test] +fn panics_preserve_phase_timings() { + let tmp = tempfile::tempdir().unwrap(); + let dir = tmp.path().to_path_buf(); + let result = std::panic::catch_unwind(|| { + let report = report::Report::new(Some(dir), "panicked".into()); + let _phase = report.phase("spawn"); + panic!("simulated runner failure"); + }); + assert!(result.is_err()); + let timing: serde_json::Value = + serde_json::from_slice(&std::fs::read(tmp.path().join("timing.json")).unwrap()).unwrap(); + assert_eq!(timing["status"], "failed"); + assert_eq!(timing["phases"][0]["name"], "spawn"); +} From 5e68c0d9322568347979d82968ee57aa06e4caf4 Mon Sep 17 00:00:00 2001 From: MK Date: Mon, 21 Sep 2026 23:00:35 +0800 Subject: [PATCH 02/10] test(snapshots): reduce worker waits and cleanup overhead --- .github/workflows/ci.yml | 19 ++++++-- .../tests/cli_snapshots/README.md | 24 ++++++++++ .../tests/cli_snapshots/main.rs | 48 +++++++++++++++---- .../tests/cli_snapshots/schedule.rs | 15 ++++++ .../vp_cli_snapshots/tests/schedule_unit.rs | 27 +++++++++++ 5 files changed, 119 insertions(+), 14 deletions(-) create mode 100644 crates/vp_cli_snapshots/tests/cli_snapshots/schedule.rs create mode 100644 crates/vp_cli_snapshots/tests/schedule_unit.rs diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 916ca0e6f5..7578a7a37f 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -971,7 +971,7 @@ jobs: run: cargo test -p vp_cli_snapshots --no-run --message-format=json > "$RUNNER_TEMP/snapshot-test-build.json" - name: Test snapshot runner utilities - run: cargo test -p vp_cli_snapshots --test redact_unit --test shard_unit --test registry_pack_unit --test report_unit + run: cargo test -p vp_cli_snapshots --test redact_unit --test shard_unit --test registry_pack_unit --test report_unit --test schedule_unit - name: Package CLI test binaries run: | @@ -1105,7 +1105,7 @@ jobs: VP_SNAP_JS_RUNTIME_DIR="$HOME/.vite-plus/js_runtime" \ VP_SNAP_FISH_BIN="$(command -v fish)" \ VP_SNAP_NU_BIN="$(command -v nu)" \ - ./target/snapshot-tests/cli_snapshots + ./target/snapshot-tests/cli_snapshots --test-threads 8 env: RUST_BACKTRACE: '1' # Relocate fixture and helper paths while keeping the same test @@ -1246,24 +1246,33 @@ jobs: # Keep pwsh at its canonical installation path; copying pwsh.exe # alone can break its adjacent runtime dependencies. export VP_SNAP_PWSH_BIN="$(cygpath -w "$(command -v pwsh.exe)")" + # Reserve all nextest workers for cases that need the execution gate. + # Generate the overrides from the fixtures, including automatic Ctrl-C + # isolation, so this configuration cannot drift from the runner. + VP_SNAP_NEXTEST_CONFIG="$RUNNER_TEMP/snapshot-nextest.toml" \ + cargo-nextest nextest list --archive-file windows-snapshot-tests.tar.zst --workspace-remap . \ + > "$RUNNER_TEMP/snapshot-test-list.txt" # --no-fail-fast: on a snapshot suite every diff is diagnostic # signal; cancelling on the first failure hides the rest. test_exit=0 # This fixture requires case-sensitive directory support, which the # default temp directory on these runners does not provide. - cargo-nextest nextest run --archive-file windows-snapshot-tests.tar.zst --workspace-remap . --no-fail-fast --partition hash:${{ matrix.shard }}/3 \ + cargo-nextest nextest run --test-threads 8 --config-file "$RUNNER_TEMP/snapshot-nextest.toml" \ + --archive-file windows-snapshot-tests.tar.zst --workspace-remap . --no-fail-fast --partition hash:${{ matrix.shard }}/3 \ -E 'not test(windows_case_sensitive_shims)' || test_exit=$? if [[ '${{ matrix.shard }}' == '1' ]]; then # Run both flavors on NTFS once. Rust's GetTempPath2 ignores # TEMP/TMP under the SYSTEM account, so override SystemTemp. SystemTemp='${{ steps.snapshot-temp.outputs.directory }}' \ - cargo-nextest nextest run --archive-file windows-snapshot-tests.tar.zst --workspace-remap . --no-fail-fast \ + cargo-nextest nextest run --test-threads 8 --config-file "$RUNNER_TEMP/snapshot-nextest.toml" \ + --archive-file windows-snapshot-tests.tar.zst --workspace-remap . --no-fail-fast \ -E 'test(windows_case_sensitive_shims)' || test_exit=$? # Exercise the same environment wrapper under Windows PowerShell 5.1. VP_SNAP_PWSH_BIN="$(cygpath -w "$(command -v powershell.exe)")" \ - cargo-nextest nextest run --archive-file windows-snapshot-tests.tar.zst --workspace-remap . --no-fail-fast \ + cargo-nextest nextest run --test-threads 8 --config-file "$RUNNER_TEMP/snapshot-nextest.toml" \ + --archive-file windows-snapshot-tests.tar.zst --workspace-remap . --no-fail-fast \ -E 'test(command_env_powershell)' || test_exit=$? fi exit "$test_exit" diff --git a/crates/vp_cli_snapshots/tests/cli_snapshots/README.md b/crates/vp_cli_snapshots/tests/cli_snapshots/README.md index 9e8a0f31da..4b336c8293 100644 --- a/crates/vp_cli_snapshots/tests/cli_snapshots/README.md +++ b/crates/vp_cli_snapshots/tests/cli_snapshots/README.md @@ -61,6 +61,15 @@ CI runs three shards per platform. Linux and macOS use `VP_SNAP_SHARD=1/3` happens before name filtering, so filtered runs keep the same assignment. Leave the variable unset to run the whole suite. Windows uses the existing nextest runner with `--partition hash:1/3` (then `2/3` and `3/3`). +Each CI shard uses eight workers so process and network waits can overlap. +Local runs retain libtest's default worker count; override it with +`--test-threads `. + +Within each shard, the in-process runner executes parallel cases before isolated +cases. This keeps workers available for ready work instead of blocking them on +the execution gate. Shard membership and test listing order stay unchanged. +The existing gate still protects `serial = true` and Ctrl-C cases, including +against other runner processes. Environment overrides, mainly for CI: @@ -79,6 +88,17 @@ Environment overrides, mainly for CI: | `VP_SNAP_SKIP_FLAVORS` | Comma-separated flavors to skip registering (e.g. `local`) | | `VP_SNAP_PACKAGES_DIR` | Run-scoped directory for sharing packed packages across test processes | | `VP_SNAP_ARTIFACTS_DIR` | Directory for phase timings and failure diagnostics; unset disables artifacts | +| `VP_SNAP_NEXTEST_CONFIG` | With `--list`, write nextest overrides for the discovered isolated cases | + +Windows CI generates its nextest configuration from the same case definitions. +The overrides reserve all test workers for an isolated case and schedule these +cases last. The file lock remains a fallback when running without the generated +configuration. To use these overrides locally: + +```bash +VP_SNAP_NEXTEST_CONFIG=target/snapshot-nextest.toml cargo nextest list -p vp_cli_snapshots +cargo nextest run -p vp_cli_snapshots --config-file target/snapshot-nextest.toml +``` `VP_SNAP_PACKAGES_DIR` packs the checkout on the first registry case, under a cross-process file lock. Later cases reuse the completed tarballs. Use a fresh @@ -98,6 +118,10 @@ that actually packs; it is nested inside `registry-pack-or-reuse`. Phase durations are milliseconds; nested phases must not be added together. The existing console timings still exclude gate waiting. +`workspace-cleanup` removes the case workspace after comparison, while other +workers can still run tests. The final `run-cleanup` removes shared run files and +any case files left by a panic or an unsuccessful earlier cleanup attempt. + Failed cases also write `error.txt`, `expected.md` (when present), `actual.md` (when a complete or partial snapshot was rendered), and `output.txt`. The output contains the last 1 MiB of unredacted rendered step output, including successful diff --git a/crates/vp_cli_snapshots/tests/cli_snapshots/main.rs b/crates/vp_cli_snapshots/tests/cli_snapshots/main.rs index 8d9c81e55c..66a173c6ed 100644 --- a/crates/vp_cli_snapshots/tests/cli_snapshots/main.rs +++ b/crates/vp_cli_snapshots/tests/cli_snapshots/main.rs @@ -20,10 +20,11 @@ mod flavor; mod redact; mod registry_pack; mod report; +mod schedule; mod shard; use std::{ - collections::{BTreeMap, hash_map::DefaultHasher}, + collections::{BTreeMap, BTreeSet, hash_map::DefaultHasher}, ffi::OsString, fs::{File, OpenOptions}, hash::{Hash, Hasher}, @@ -1187,9 +1188,8 @@ fn start_local_registry( )] fn run_case( tmpdir: &Path, + case_root: &Path, fixture_path: &Path, - fixture_name: &str, - case_index: usize, case: &Case, flavor: Flavor, runtime: &FlavorRuntime, @@ -1202,7 +1202,6 @@ fn run_case( // Copy the fixture to a per-case staging directory so the test runs in // isolation and workspace-root discovery doesn't walk past the fixture. let staging_phase = report.phase("fixture-staging"); - let case_root = tmpdir.join(format!("{fixture_name}_case_{case_index}_{}", flavor.as_str())); let stage = case_root.join("workspace"); std::fs::create_dir_all(&stage).unwrap(); if case.link_node_modules { @@ -1217,7 +1216,7 @@ fn run_case( drop(staging_phase); let setup_phase = report.phase("case-setup"); - let case_home = CaseHome::provision(&case_root, case.seed_runtime); + let case_home = CaseHome::provision(case_root, case.seed_runtime); let case_install = case_home.provision_vite_plus(flavor, runtime)?; drop(setup_phase); @@ -1785,6 +1784,7 @@ fn main() { let local_build_present = flavor::repo_root().join("packages/cli/dist/bin.js").is_file(); let mut tests: Vec = Vec::new(); + let mut isolated_trials = BTreeSet::new(); for fixture_path in fixture_paths { let fixture_path: Arc = Arc::from(fixture_path.as_path()); let fixture_name: Arc = Arc::from(fixture_path.file_name().unwrap().to_str().unwrap()); @@ -1821,6 +1821,9 @@ fn main() { || required_tool_missing || (case.local_registry && !local_build_present); let isolated = case_needs_isolation(&case); + if isolated { + isolated_trials.insert(trial_name.clone()); + } let timings = Arc::clone(&timings); let timing_name = trial_name.clone(); let artifact_dir = artifacts @@ -1846,18 +1849,29 @@ fn main() { None }; drop(pack_phase); - run_case( + let case_root = tmp_dir_path.join(format!( + "{fixture_name}_case_{case_index}_{}", + flavor.as_str() + )); + let result = run_case( &tmp_dir_path, + &case_root, &fixture_path, - &fixture_name, - case_index, &case, flavor, runtime, &snapshot_name, local_registry_pack.as_deref(), &report, - ) + ); + // Reclaim each workspace while other workers can + // still make progress, instead of deleting every + // installed dependency in one serial tail. The run + // TempDir remains a fallback after panics or files + // that are still open during this first attempt. + let _cleanup_phase = report.phase("workspace-cleanup"); + let _ = std::fs::remove_dir_all(&case_root); + result })(); timings.lock().unwrap().push((timing_name, started.elapsed())); let artifact_result = report.finish( @@ -1872,11 +1886,27 @@ fn main() { } } + if args.list + && let Some(path) = std::env::var_os("VP_SNAP_NEXTEST_CONFIG") + { + std::fs::write(&path, schedule::nextest_config(isolated_trials.iter().map(String::as_str))) + .unwrap_or_else(|error| { + panic!("failed to write nextest config {}: {error}", Path::new(&path).display()) + }); + } + if let Some(shard) = std::env::var_os("VP_SNAP_SHARD") { let shard = shard.to_str().expect("VP_SNAP_SHARD must be valid UTF-8"); tests = shard::select(tests, shard).unwrap_or_else(|error| panic!("{error}")); } + if !args.list { + // A worker waiting for exclusive access cannot run another ready case. + // Finish parallel work first, then take the existing exclusive leases. + // Keep discovery and shard membership independent of execution order. + tests.sort_by_key(|trial| isolated_trials.contains(trial.name())); + } + drop(discovery_phase); let execution_phase = run_report.phase("tests"); let conclusion = libtest_mimic::run(&args, tests); diff --git a/crates/vp_cli_snapshots/tests/cli_snapshots/schedule.rs b/crates/vp_cli_snapshots/tests/cli_snapshots/schedule.rs new file mode 100644 index 0000000000..34059476ee --- /dev/null +++ b/crates/vp_cli_snapshots/tests/cli_snapshots/schedule.rs @@ -0,0 +1,15 @@ +/// Tell nextest about the same exclusive cases that the runner protects with +/// its execution gate. Reserving all workers before starting a case avoids +/// occupying one worker with a process that only waits for the gate. +pub fn nextest_config<'a>(isolated: impl IntoIterator) -> String { + let mut config = String::from("# Generated from snapshot case isolation requirements.\n"); + for name in isolated { + config.push_str(&format!( + "\n[[profile.default.overrides]]\n\ + filter = 'binary(cli_snapshots) & test(={name})'\n\ + threads-required = 'num-test-threads'\n\ + priority = -100\n" + )); + } + config +} diff --git a/crates/vp_cli_snapshots/tests/schedule_unit.rs b/crates/vp_cli_snapshots/tests/schedule_unit.rs new file mode 100644 index 0000000000..bbdc2382cb --- /dev/null +++ b/crates/vp_cli_snapshots/tests/schedule_unit.rs @@ -0,0 +1,27 @@ +#![expect(clippy::disallowed_macros, reason = "standalone test uses std macros")] +#![expect(clippy::disallowed_types, reason = "standalone test uses std types")] + +#[path = "cli_snapshots/schedule.rs"] +mod schedule; + +#[test] +fn nextest_reserves_all_workers_for_exact_case_names() { + let names = ["ctrlc_isolation::explicit_serial", "app_root_listing::picker_cancel::global"]; + let config: toml::Value = toml::from_str(&schedule::nextest_config(names)).unwrap(); + let overrides = config["profile"]["default"]["overrides"].as_array().unwrap(); + assert_eq!(overrides.len(), names.len()); + for (entry, name) in overrides.iter().zip(names) { + assert_eq!( + entry["filter"].as_str().unwrap(), + format!("binary(cli_snapshots) & test(={name})") + ); + assert_eq!(entry["threads-required"].as_str().unwrap(), "num-test-threads"); + assert_eq!(entry["priority"].as_integer().unwrap(), -100); + } +} + +#[test] +fn no_isolated_cases_does_not_change_nextest_defaults() { + let config: toml::Value = toml::from_str(&schedule::nextest_config([])).unwrap(); + assert!(config.as_table().unwrap().is_empty()); +} From 30afd71ab98ced034023ec5e119a899aaa6566b3 Mon Sep 17 00:00:00 2001 From: MK Date: Mon, 21 Sep 2026 23:20:04 +0800 Subject: [PATCH 03/10] test(snapshots): prioritize installs and trim exact discovery --- .github/workflows/ci.yml | 6 ++-- .../tests/cli_snapshots/README.md | 9 +++-- .../packages/utils/vite.config.ts | 4 +++ .../tests/cli_snapshots/main.rs | 36 +++++++++++++++---- .../tests/cli_snapshots/schedule.rs | 12 ++++++- .../vp_cli_snapshots/tests/schedule_unit.rs | 15 ++++++-- 6 files changed, 67 insertions(+), 15 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 7578a7a37f..8b3e07e8a0 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -1257,7 +1257,7 @@ jobs: test_exit=0 # This fixture requires case-sensitive directory support, which the # default temp directory on these runners does not provide. - cargo-nextest nextest run --test-threads 8 --config-file "$RUNNER_TEMP/snapshot-nextest.toml" \ + cargo-nextest nextest run --test-threads 4 --config-file "$RUNNER_TEMP/snapshot-nextest.toml" \ --archive-file windows-snapshot-tests.tar.zst --workspace-remap . --no-fail-fast --partition hash:${{ matrix.shard }}/3 \ -E 'not test(windows_case_sensitive_shims)' || test_exit=$? @@ -1265,13 +1265,13 @@ jobs: # Run both flavors on NTFS once. Rust's GetTempPath2 ignores # TEMP/TMP under the SYSTEM account, so override SystemTemp. SystemTemp='${{ steps.snapshot-temp.outputs.directory }}' \ - cargo-nextest nextest run --test-threads 8 --config-file "$RUNNER_TEMP/snapshot-nextest.toml" \ + cargo-nextest nextest run --test-threads 4 --config-file "$RUNNER_TEMP/snapshot-nextest.toml" \ --archive-file windows-snapshot-tests.tar.zst --workspace-remap . --no-fail-fast \ -E 'test(windows_case_sensitive_shims)' || test_exit=$? # Exercise the same environment wrapper under Windows PowerShell 5.1. VP_SNAP_PWSH_BIN="$(cygpath -w "$(command -v powershell.exe)")" \ - cargo-nextest nextest run --test-threads 8 --config-file "$RUNNER_TEMP/snapshot-nextest.toml" \ + cargo-nextest nextest run --test-threads 4 --config-file "$RUNNER_TEMP/snapshot-nextest.toml" \ --archive-file windows-snapshot-tests.tar.zst --workspace-remap . --no-fail-fast \ -E 'test(command_env_powershell)' || test_exit=$? fi diff --git a/crates/vp_cli_snapshots/tests/cli_snapshots/README.md b/crates/vp_cli_snapshots/tests/cli_snapshots/README.md index 4b336c8293..590b30108c 100644 --- a/crates/vp_cli_snapshots/tests/cli_snapshots/README.md +++ b/crates/vp_cli_snapshots/tests/cli_snapshots/README.md @@ -61,7 +61,8 @@ CI runs three shards per platform. Linux and macOS use `VP_SNAP_SHARD=1/3` happens before name filtering, so filtered runs keep the same assignment. Leave the variable unset to run the whole suite. Windows uses the existing nextest runner with `--partition hash:1/3` (then `2/3` and `3/3`). -Each CI shard uses eight workers so process and network waits can overlap. +Unix CI shards use eight workers so process and network waits can overlap. +Windows uses four workers to limit CPU and filesystem contention. Local runs retain libtest's default worker count; override it with `--test-threads `. @@ -88,9 +89,13 @@ Environment overrides, mainly for CI: | `VP_SNAP_SKIP_FLAVORS` | Comma-separated flavors to skip registering (e.g. `local`) | | `VP_SNAP_PACKAGES_DIR` | Run-scoped directory for sharing packed packages across test processes | | `VP_SNAP_ARTIFACTS_DIR` | Directory for phase timings and failure diagnostics; unset disables artifacts | -| `VP_SNAP_NEXTEST_CONFIG` | With `--list`, write nextest overrides for the discovered isolated cases | +| `VP_SNAP_NEXTEST_CONFIG` | With `--list`, write nextest isolation and registry scheduling overrides | Windows CI generates its nextest configuration from the same case definitions. +Registry cases start before other parallel cases so long installs do not leave +workers idle near the end of the run. Exact nextest runs read only the selected +fixture; listing and native shard assignment still discover all cases. + The overrides reserve all test workers for an isolated case and schedule these cases last. The file lock remains a fallback when running without the generated configuration. To use these overrides locally: diff --git a/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/migration_pnpm_vite_identity/packages/utils/vite.config.ts b/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/migration_pnpm_vite_identity/packages/utils/vite.config.ts index eb27f1af40..8e9e453ef6 100644 --- a/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/migration_pnpm_vite_identity/packages/utils/vite.config.ts +++ b/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/migration_pnpm_vite_identity/packages/utils/vite.config.ts @@ -1,6 +1,10 @@ import { defineConfig } from 'vite-plus'; export default defineConfig({ + test: { + // Keep passing test details independent of machine load in snapshots. + slowTestThreshold: 60_000, + }, run: { tasks: { test: { command: 'vp test run' }, diff --git a/crates/vp_cli_snapshots/tests/cli_snapshots/main.rs b/crates/vp_cli_snapshots/tests/cli_snapshots/main.rs index 66a173c6ed..35e64ba8dc 100644 --- a/crates/vp_cli_snapshots/tests/cli_snapshots/main.rs +++ b/crates/vp_cli_snapshots/tests/cli_snapshots/main.rs @@ -1683,12 +1683,21 @@ fn main() { let fixtures_dir = flavor::manifest_dir().join("tests/cli_snapshots/fixtures"); + // nextest starts a process with one exact trial. Avoid reading every TOML + // file in each child. Native sharding still needs the complete trial list + // because its round-robin assignment precedes libtest filtering. + let exact_fixture = if args.exact && !args.list && std::env::var_os("VP_SNAP_SHARD").is_none() { + args.filter.as_deref().and_then(|name| name.split_once("::")).map(|(fixture, _)| fixture) + } else { + None + }; let mut fixture_paths = std::fs::read_dir(&fixtures_dir) .unwrap_or_else(|e| panic!("failed to read {}: {e}", fixtures_dir.display())) .map(|entry| entry.unwrap().path()) .filter(|p| { - p.is_dir() - && p.file_name().and_then(|n| n.to_str()).is_some_and(|n| !n.starts_with('.')) + p.file_name().and_then(|n| n.to_str()).is_some_and(|name| { + !name.starts_with('.') && exact_fixture.is_none_or(|fixture| name == fixture) + }) && p.is_dir() }) .collect::>(); fixture_paths.sort(); @@ -1785,6 +1794,7 @@ fn main() { let mut tests: Vec = Vec::new(); let mut isolated_trials = BTreeSet::new(); + let mut registry_trials = BTreeSet::new(); for fixture_path in fixture_paths { let fixture_path: Arc = Arc::from(fixture_path.as_path()); let fixture_name: Arc = Arc::from(fixture_path.file_name().unwrap().to_str().unwrap()); @@ -1823,6 +1833,8 @@ fn main() { let isolated = case_needs_isolation(&case); if isolated { isolated_trials.insert(trial_name.clone()); + } else if case.local_registry { + registry_trials.insert(trial_name.clone()); } let timings = Arc::clone(&timings); let timing_name = trial_name.clone(); @@ -1889,10 +1901,16 @@ fn main() { if args.list && let Some(path) = std::env::var_os("VP_SNAP_NEXTEST_CONFIG") { - std::fs::write(&path, schedule::nextest_config(isolated_trials.iter().map(String::as_str))) - .unwrap_or_else(|error| { - panic!("failed to write nextest config {}: {error}", Path::new(&path).display()) - }); + std::fs::write( + &path, + schedule::nextest_config( + isolated_trials.iter().map(String::as_str), + registry_trials.iter().map(String::as_str), + ), + ) + .unwrap_or_else(|error| { + panic!("failed to write nextest config {}: {error}", Path::new(&path).display()) + }); } if let Some(shard) = std::env::var_os("VP_SNAP_SHARD") { @@ -1904,7 +1922,11 @@ fn main() { // A worker waiting for exclusive access cannot run another ready case. // Finish parallel work first, then take the existing exclusive leases. // Keep discovery and shard membership independent of execution order. - tests.sort_by_key(|trial| isolated_trials.contains(trial.name())); + // Registry cases usually install dependencies or start several tools. + // Start them early so shorter cases can fill the remaining worker slots. + tests.sort_by_key(|trial| { + (isolated_trials.contains(trial.name()), !registry_trials.contains(trial.name())) + }); } drop(discovery_phase); diff --git a/crates/vp_cli_snapshots/tests/cli_snapshots/schedule.rs b/crates/vp_cli_snapshots/tests/cli_snapshots/schedule.rs index 34059476ee..9e25496cb7 100644 --- a/crates/vp_cli_snapshots/tests/cli_snapshots/schedule.rs +++ b/crates/vp_cli_snapshots/tests/cli_snapshots/schedule.rs @@ -1,7 +1,10 @@ /// Tell nextest about the same exclusive cases that the runner protects with /// its execution gate. Reserving all workers before starting a case avoids /// occupying one worker with a process that only waits for the gate. -pub fn nextest_config<'a>(isolated: impl IntoIterator) -> String { +pub fn nextest_config<'a>( + isolated: impl IntoIterator, + registry: impl IntoIterator, +) -> String { let mut config = String::from("# Generated from snapshot case isolation requirements.\n"); for name in isolated { config.push_str(&format!( @@ -11,5 +14,12 @@ pub fn nextest_config<'a>(isolated: impl IntoIterator) -> String priority = -100\n" )); } + for name in registry { + config.push_str(&format!( + "\n[[profile.default.overrides]]\n\ + filter = 'binary(cli_snapshots) & test(={name})'\n\ + priority = 50\n" + )); + } config } diff --git a/crates/vp_cli_snapshots/tests/schedule_unit.rs b/crates/vp_cli_snapshots/tests/schedule_unit.rs index bbdc2382cb..630ca06117 100644 --- a/crates/vp_cli_snapshots/tests/schedule_unit.rs +++ b/crates/vp_cli_snapshots/tests/schedule_unit.rs @@ -7,7 +7,7 @@ mod schedule; #[test] fn nextest_reserves_all_workers_for_exact_case_names() { let names = ["ctrlc_isolation::explicit_serial", "app_root_listing::picker_cancel::global"]; - let config: toml::Value = toml::from_str(&schedule::nextest_config(names)).unwrap(); + let config: toml::Value = toml::from_str(&schedule::nextest_config(names, [])).unwrap(); let overrides = config["profile"]["default"]["overrides"].as_array().unwrap(); assert_eq!(overrides.len(), names.len()); for (entry, name) in overrides.iter().zip(names) { @@ -22,6 +22,17 @@ fn nextest_reserves_all_workers_for_exact_case_names() { #[test] fn no_isolated_cases_does_not_change_nextest_defaults() { - let config: toml::Value = toml::from_str(&schedule::nextest_config([])).unwrap(); + let config: toml::Value = toml::from_str(&schedule::nextest_config([], [])).unwrap(); assert!(config.as_table().unwrap().is_empty()); } + +#[test] +fn registry_priority_does_not_override_exclusive_reservations() { + let name = "browser::packed"; + let config: toml::Value = toml::from_str(&schedule::nextest_config([name], [name])).unwrap(); + let overrides = config["profile"]["default"]["overrides"].as_array().unwrap(); + // nextest uses the first matching override for each setting. + assert_eq!(overrides[0]["priority"].as_integer().unwrap(), -100); + assert_eq!(overrides[1]["priority"].as_integer().unwrap(), 50); + assert!(overrides[1].get("threads-required").is_none()); +} From 74a67d61aedc8cd0d442048da18a405f5811dda1 Mon Sep 17 00:00:00 2001 From: MK Date: Mon, 21 Sep 2026 23:30:03 +0800 Subject: [PATCH 04/10] test(snapshots): keep dependency installs spread through the run --- .../tests/cli_snapshots/README.md | 7 +++--- .../tests/cli_snapshots/main.rs | 23 ++++--------------- .../tests/cli_snapshots/schedule.rs | 12 +--------- .../vp_cli_snapshots/tests/schedule_unit.rs | 15 ++---------- 4 files changed, 11 insertions(+), 46 deletions(-) diff --git a/crates/vp_cli_snapshots/tests/cli_snapshots/README.md b/crates/vp_cli_snapshots/tests/cli_snapshots/README.md index 590b30108c..e45ba8983b 100644 --- a/crates/vp_cli_snapshots/tests/cli_snapshots/README.md +++ b/crates/vp_cli_snapshots/tests/cli_snapshots/README.md @@ -89,12 +89,11 @@ Environment overrides, mainly for CI: | `VP_SNAP_SKIP_FLAVORS` | Comma-separated flavors to skip registering (e.g. `local`) | | `VP_SNAP_PACKAGES_DIR` | Run-scoped directory for sharing packed packages across test processes | | `VP_SNAP_ARTIFACTS_DIR` | Directory for phase timings and failure diagnostics; unset disables artifacts | -| `VP_SNAP_NEXTEST_CONFIG` | With `--list`, write nextest isolation and registry scheduling overrides | +| `VP_SNAP_NEXTEST_CONFIG` | With `--list`, write nextest overrides for the discovered isolated cases | Windows CI generates its nextest configuration from the same case definitions. -Registry cases start before other parallel cases so long installs do not leave -workers idle near the end of the run. Exact nextest runs read only the selected -fixture; listing and native shard assignment still discover all cases. +Exact nextest runs read only the selected fixture; listing and native shard +assignment still discover all cases. The overrides reserve all test workers for an isolated case and schedule these cases last. The file lock remains a fallback when running without the generated diff --git a/crates/vp_cli_snapshots/tests/cli_snapshots/main.rs b/crates/vp_cli_snapshots/tests/cli_snapshots/main.rs index 35e64ba8dc..0f9ce01eec 100644 --- a/crates/vp_cli_snapshots/tests/cli_snapshots/main.rs +++ b/crates/vp_cli_snapshots/tests/cli_snapshots/main.rs @@ -1794,7 +1794,6 @@ fn main() { let mut tests: Vec = Vec::new(); let mut isolated_trials = BTreeSet::new(); - let mut registry_trials = BTreeSet::new(); for fixture_path in fixture_paths { let fixture_path: Arc = Arc::from(fixture_path.as_path()); let fixture_name: Arc = Arc::from(fixture_path.file_name().unwrap().to_str().unwrap()); @@ -1833,8 +1832,6 @@ fn main() { let isolated = case_needs_isolation(&case); if isolated { isolated_trials.insert(trial_name.clone()); - } else if case.local_registry { - registry_trials.insert(trial_name.clone()); } let timings = Arc::clone(&timings); let timing_name = trial_name.clone(); @@ -1901,16 +1898,10 @@ fn main() { if args.list && let Some(path) = std::env::var_os("VP_SNAP_NEXTEST_CONFIG") { - std::fs::write( - &path, - schedule::nextest_config( - isolated_trials.iter().map(String::as_str), - registry_trials.iter().map(String::as_str), - ), - ) - .unwrap_or_else(|error| { - panic!("failed to write nextest config {}: {error}", Path::new(&path).display()) - }); + std::fs::write(&path, schedule::nextest_config(isolated_trials.iter().map(String::as_str))) + .unwrap_or_else(|error| { + panic!("failed to write nextest config {}: {error}", Path::new(&path).display()) + }); } if let Some(shard) = std::env::var_os("VP_SNAP_SHARD") { @@ -1922,11 +1913,7 @@ fn main() { // A worker waiting for exclusive access cannot run another ready case. // Finish parallel work first, then take the existing exclusive leases. // Keep discovery and shard membership independent of execution order. - // Registry cases usually install dependencies or start several tools. - // Start them early so shorter cases can fill the remaining worker slots. - tests.sort_by_key(|trial| { - (isolated_trials.contains(trial.name()), !registry_trials.contains(trial.name())) - }); + tests.sort_by_key(|trial| isolated_trials.contains(trial.name())); } drop(discovery_phase); diff --git a/crates/vp_cli_snapshots/tests/cli_snapshots/schedule.rs b/crates/vp_cli_snapshots/tests/cli_snapshots/schedule.rs index 9e25496cb7..34059476ee 100644 --- a/crates/vp_cli_snapshots/tests/cli_snapshots/schedule.rs +++ b/crates/vp_cli_snapshots/tests/cli_snapshots/schedule.rs @@ -1,10 +1,7 @@ /// Tell nextest about the same exclusive cases that the runner protects with /// its execution gate. Reserving all workers before starting a case avoids /// occupying one worker with a process that only waits for the gate. -pub fn nextest_config<'a>( - isolated: impl IntoIterator, - registry: impl IntoIterator, -) -> String { +pub fn nextest_config<'a>(isolated: impl IntoIterator) -> String { let mut config = String::from("# Generated from snapshot case isolation requirements.\n"); for name in isolated { config.push_str(&format!( @@ -14,12 +11,5 @@ pub fn nextest_config<'a>( priority = -100\n" )); } - for name in registry { - config.push_str(&format!( - "\n[[profile.default.overrides]]\n\ - filter = 'binary(cli_snapshots) & test(={name})'\n\ - priority = 50\n" - )); - } config } diff --git a/crates/vp_cli_snapshots/tests/schedule_unit.rs b/crates/vp_cli_snapshots/tests/schedule_unit.rs index 630ca06117..bbdc2382cb 100644 --- a/crates/vp_cli_snapshots/tests/schedule_unit.rs +++ b/crates/vp_cli_snapshots/tests/schedule_unit.rs @@ -7,7 +7,7 @@ mod schedule; #[test] fn nextest_reserves_all_workers_for_exact_case_names() { let names = ["ctrlc_isolation::explicit_serial", "app_root_listing::picker_cancel::global"]; - let config: toml::Value = toml::from_str(&schedule::nextest_config(names, [])).unwrap(); + let config: toml::Value = toml::from_str(&schedule::nextest_config(names)).unwrap(); let overrides = config["profile"]["default"]["overrides"].as_array().unwrap(); assert_eq!(overrides.len(), names.len()); for (entry, name) in overrides.iter().zip(names) { @@ -22,17 +22,6 @@ fn nextest_reserves_all_workers_for_exact_case_names() { #[test] fn no_isolated_cases_does_not_change_nextest_defaults() { - let config: toml::Value = toml::from_str(&schedule::nextest_config([], [])).unwrap(); + let config: toml::Value = toml::from_str(&schedule::nextest_config([])).unwrap(); assert!(config.as_table().unwrap().is_empty()); } - -#[test] -fn registry_priority_does_not_override_exclusive_reservations() { - let name = "browser::packed"; - let config: toml::Value = toml::from_str(&schedule::nextest_config([name], [name])).unwrap(); - let overrides = config["profile"]["default"]["overrides"].as_array().unwrap(); - // nextest uses the first matching override for each setting. - assert_eq!(overrides[0]["priority"].as_integer().unwrap(), -100); - assert_eq!(overrides[1]["priority"].as_integer().unwrap(), 50); - assert!(overrides[1].get("threads-required").is_none()); -} From daaacdc1ddee9545ae974da733fff44b6467b24d Mon Sep 17 00:00:00 2001 From: MK Date: Mon, 21 Sep 2026 23:43:02 +0800 Subject: [PATCH 05/10] test(snapshots): retain eight workers for Windows throughput --- .github/workflows/ci.yml | 6 +++--- crates/vp_cli_snapshots/tests/cli_snapshots/README.md | 3 +-- 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 8b3e07e8a0..7578a7a37f 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -1257,7 +1257,7 @@ jobs: test_exit=0 # This fixture requires case-sensitive directory support, which the # default temp directory on these runners does not provide. - cargo-nextest nextest run --test-threads 4 --config-file "$RUNNER_TEMP/snapshot-nextest.toml" \ + cargo-nextest nextest run --test-threads 8 --config-file "$RUNNER_TEMP/snapshot-nextest.toml" \ --archive-file windows-snapshot-tests.tar.zst --workspace-remap . --no-fail-fast --partition hash:${{ matrix.shard }}/3 \ -E 'not test(windows_case_sensitive_shims)' || test_exit=$? @@ -1265,13 +1265,13 @@ jobs: # Run both flavors on NTFS once. Rust's GetTempPath2 ignores # TEMP/TMP under the SYSTEM account, so override SystemTemp. SystemTemp='${{ steps.snapshot-temp.outputs.directory }}' \ - cargo-nextest nextest run --test-threads 4 --config-file "$RUNNER_TEMP/snapshot-nextest.toml" \ + cargo-nextest nextest run --test-threads 8 --config-file "$RUNNER_TEMP/snapshot-nextest.toml" \ --archive-file windows-snapshot-tests.tar.zst --workspace-remap . --no-fail-fast \ -E 'test(windows_case_sensitive_shims)' || test_exit=$? # Exercise the same environment wrapper under Windows PowerShell 5.1. VP_SNAP_PWSH_BIN="$(cygpath -w "$(command -v powershell.exe)")" \ - cargo-nextest nextest run --test-threads 4 --config-file "$RUNNER_TEMP/snapshot-nextest.toml" \ + cargo-nextest nextest run --test-threads 8 --config-file "$RUNNER_TEMP/snapshot-nextest.toml" \ --archive-file windows-snapshot-tests.tar.zst --workspace-remap . --no-fail-fast \ -E 'test(command_env_powershell)' || test_exit=$? fi diff --git a/crates/vp_cli_snapshots/tests/cli_snapshots/README.md b/crates/vp_cli_snapshots/tests/cli_snapshots/README.md index e45ba8983b..bbef446eab 100644 --- a/crates/vp_cli_snapshots/tests/cli_snapshots/README.md +++ b/crates/vp_cli_snapshots/tests/cli_snapshots/README.md @@ -61,8 +61,7 @@ CI runs three shards per platform. Linux and macOS use `VP_SNAP_SHARD=1/3` happens before name filtering, so filtered runs keep the same assignment. Leave the variable unset to run the whole suite. Windows uses the existing nextest runner with `--partition hash:1/3` (then `2/3` and `3/3`). -Unix CI shards use eight workers so process and network waits can overlap. -Windows uses four workers to limit CPU and filesystem contention. +Each CI shard uses eight workers so process and network waits can overlap. Local runs retain libtest's default worker count; override it with `--test-threads `. From 03bc434ea923bb29fc58ba8dfbac832b1450316f Mon Sep 17 00:00:00 2001 From: MK Date: Mon, 21 Sep 2026 23:59:19 +0800 Subject: [PATCH 06/10] docs(snapshots): use an absolute path for nextest config --- crates/vp_cli_snapshots/tests/cli_snapshots/README.md | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/crates/vp_cli_snapshots/tests/cli_snapshots/README.md b/crates/vp_cli_snapshots/tests/cli_snapshots/README.md index bbef446eab..6c3157a65e 100644 --- a/crates/vp_cli_snapshots/tests/cli_snapshots/README.md +++ b/crates/vp_cli_snapshots/tests/cli_snapshots/README.md @@ -67,7 +67,8 @@ Local runs retain libtest's default worker count; override it with Within each shard, the in-process runner executes parallel cases before isolated cases. This keeps workers available for ready work instead of blocking them on -the execution gate. Shard membership and test listing order stay unchanged. +the execution gate. Scheduling happens after partitioning and does not change +shard membership or test listing order. The existing gate still protects `serial = true` and Ctrl-C cases, including against other runner processes. @@ -99,7 +100,7 @@ cases last. The file lock remains a fallback when running without the generated configuration. To use these overrides locally: ```bash -VP_SNAP_NEXTEST_CONFIG=target/snapshot-nextest.toml cargo nextest list -p vp_cli_snapshots +VP_SNAP_NEXTEST_CONFIG="$PWD/target/snapshot-nextest.toml" cargo nextest list -p vp_cli_snapshots cargo nextest run -p vp_cli_snapshots --config-file target/snapshot-nextest.toml ``` From 10c72ac9a73f00543df6a211b0e33f9ffc80cbe0 Mon Sep 17 00:00:00 2001 From: MK Date: Tue, 22 Sep 2026 11:44:42 +0800 Subject: [PATCH 07/10] refactor(snapshots): simplify runner and CI setup --- .github/workflows/ci.yml | 13 +++--- .../tests/cli_snapshots/main.rs | 25 +++++------ .../tests/cli_snapshots/registry_pack.rs | 45 ++++++++++--------- .../tests/cli_snapshots/report.rs | 33 ++++++-------- 4 files changed, 56 insertions(+), 60 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 7578a7a37f..48a1cea46b 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -1252,27 +1252,28 @@ jobs: VP_SNAP_NEXTEST_CONFIG="$RUNNER_TEMP/snapshot-nextest.toml" \ cargo-nextest nextest list --archive-file windows-snapshot-tests.tar.zst --workspace-remap . \ > "$RUNNER_TEMP/snapshot-test-list.txt" + nextest_run_args=( + --test-threads 8 --config-file "$RUNNER_TEMP/snapshot-nextest.toml" + --archive-file windows-snapshot-tests.tar.zst --workspace-remap . --no-fail-fast + ) # --no-fail-fast: on a snapshot suite every diff is diagnostic # signal; cancelling on the first failure hides the rest. test_exit=0 # This fixture requires case-sensitive directory support, which the # default temp directory on these runners does not provide. - cargo-nextest nextest run --test-threads 8 --config-file "$RUNNER_TEMP/snapshot-nextest.toml" \ - --archive-file windows-snapshot-tests.tar.zst --workspace-remap . --no-fail-fast --partition hash:${{ matrix.shard }}/3 \ + cargo-nextest nextest run "${nextest_run_args[@]}" --partition hash:${{ matrix.shard }}/3 \ -E 'not test(windows_case_sensitive_shims)' || test_exit=$? if [[ '${{ matrix.shard }}' == '1' ]]; then # Run both flavors on NTFS once. Rust's GetTempPath2 ignores # TEMP/TMP under the SYSTEM account, so override SystemTemp. SystemTemp='${{ steps.snapshot-temp.outputs.directory }}' \ - cargo-nextest nextest run --test-threads 8 --config-file "$RUNNER_TEMP/snapshot-nextest.toml" \ - --archive-file windows-snapshot-tests.tar.zst --workspace-remap . --no-fail-fast \ + cargo-nextest nextest run "${nextest_run_args[@]}" \ -E 'test(windows_case_sensitive_shims)' || test_exit=$? # Exercise the same environment wrapper under Windows PowerShell 5.1. VP_SNAP_PWSH_BIN="$(cygpath -w "$(command -v powershell.exe)")" \ - cargo-nextest nextest run --test-threads 8 --config-file "$RUNNER_TEMP/snapshot-nextest.toml" \ - --archive-file windows-snapshot-tests.tar.zst --workspace-remap . --no-fail-fast \ + cargo-nextest nextest run "${nextest_run_args[@]}" \ -E 'test(command_env_powershell)' || test_exit=$? fi exit "$test_exit" diff --git a/crates/vp_cli_snapshots/tests/cli_snapshots/main.rs b/crates/vp_cli_snapshots/tests/cli_snapshots/main.rs index 0f9ce01eec..01c34820a3 100644 --- a/crates/vp_cli_snapshots/tests/cli_snapshots/main.rs +++ b/crates/vp_cli_snapshots/tests/cli_snapshots/main.rs @@ -1301,11 +1301,8 @@ fn run_case( let step_env: &BTreeMap = &step_env; let timeout = step.timeout(step_default_timeout); - let execution_phase = report.phase(format!( - "step-{}: {}", - step_index + 1, - step.display_command_line(&case.cwd) - )); + let command_line = step.display_command_line(&case.cwd); + let execution_phase = report.phase(format!("step-{}: {command_line}", step_index + 1)); let (termination_state, raw_output) = if step.tty { 'tty: { let mut cmd = CommandBuilder::new(&program); @@ -1446,12 +1443,12 @@ fn run_case( drop(execution_phase); let rendering_phase = report.phase(format!("render-step-{}", step_index + 1)); - report.capture(&step.display_command_line(&case.cwd), &raw_output); + report.capture(&command_line, &raw_output); // Blank line separator before every `##`. doc.push('\n'); doc.push_str("## `"); - doc.push_str(&step.display_command_line(&case.cwd)); + doc.push_str(&command_line); doc.push_str("`\n\n"); if let Some(comment) = step.comment.as_deref() { @@ -1466,8 +1463,7 @@ fn run_case( if matches!(termination_state, TerminationState::TimedOut) { let redacted = redact_output(raw_output, &redactions, !step.formatted_snapshot); timeout_error = Some(format!( - "step `{}` timed out after {timeout:?}; partial output:\n{redacted}", - step.display_command_line(&case.cwd), + "step `{command_line}` timed out after {timeout:?}; partial output:\n{redacted}", )); break; } @@ -1694,10 +1690,13 @@ fn main() { let mut fixture_paths = std::fs::read_dir(&fixtures_dir) .unwrap_or_else(|e| panic!("failed to read {}: {e}", fixtures_dir.display())) .map(|entry| entry.unwrap().path()) - .filter(|p| { - p.file_name().and_then(|n| n.to_str()).is_some_and(|name| { - !name.starts_with('.') && exact_fixture.is_none_or(|fixture| name == fixture) - }) && p.is_dir() + .filter(|path| { + let Some(name) = path.file_name().and_then(|name| name.to_str()) else { + return false; + }; + !name.starts_with('.') + && exact_fixture.is_none_or(|fixture| name == fixture) + && path.is_dir() }) .collect::>(); fixture_paths.sort(); diff --git a/crates/vp_cli_snapshots/tests/cli_snapshots/registry_pack.rs b/crates/vp_cli_snapshots/tests/cli_snapshots/registry_pack.rs index 09bb98d332..25ed1d6dfc 100644 --- a/crates/vp_cli_snapshots/tests/cli_snapshots/registry_pack.rs +++ b/crates/vp_cli_snapshots/tests/cli_snapshots/registry_pack.rs @@ -1,5 +1,6 @@ //! Share immutable package tarballs across nextest processes within one test run. use std::{ + collections::BTreeMap, fs::{File, OpenOptions}, path::{Path, PathBuf}, time::UNIX_EPOCH, @@ -22,34 +23,34 @@ fn stamp(path: &Path) -> Result { fn collect_inputs( root: &Path, relative: &Path, - inputs: &mut std::collections::BTreeMap, + inputs: &mut BTreeMap, ancestors: &mut Vec, ) -> Result<(), String> { let path = root.join(relative); - if path.is_dir() { - let canonical = dunce::canonicalize(&path).map_err(|error| error.to_string())?; - if ancestors.contains(&canonical) { - return Err(format!("cycle in package inputs at {}", path.display())); - } - ancestors.push(canonical); - for entry in path.read_dir().map_err(|error| format!("{}: {error}", path.display()))? { - let entry = entry.map_err(|error| error.to_string())?; - // Installed dependencies and Cargo output are not package inputs. - if matches!(entry.file_name().to_str(), Some("node_modules" | "target" | ".git")) { - continue; - } - collect_inputs(root, &relative.join(entry.file_name()), inputs, ancestors)?; - } - ancestors.pop(); - } else { + if !path.is_dir() { inputs.insert(relative.to_string_lossy().into_owned(), stamp(&path)?); + return Ok(()); + } + let canonical = dunce::canonicalize(&path).map_err(|error| error.to_string())?; + if ancestors.contains(&canonical) { + return Err(format!("cycle in package inputs at {}", path.display())); + } + ancestors.push(canonical); + for entry in path.read_dir().map_err(|error| format!("{}: {error}", path.display()))? { + let name = entry.map_err(|error| error.to_string())?.file_name(); + // Installed dependencies and Cargo output are not package inputs. + if matches!(name.to_str(), Some("node_modules" | "target" | ".git")) { + continue; + } + collect_inputs(root, &relative.join(name), inputs, ancestors)?; } + ancestors.pop(); Ok(()) } fn build_stamp(repo: &Path) -> Result { let repo = dunce::canonicalize(repo).map_err(|error| error.to_string())?; - let mut inputs = std::collections::BTreeMap::new(); + let mut inputs = BTreeMap::new(); for path in ["package.json", "pnpm-workspace.yaml", "pnpm-lock.yaml", "packages/cli", "packages/core"] { @@ -59,12 +60,12 @@ fn build_stamp(repo: &Path) -> Result { } fn archives(directory: &Path) -> Result { - let mut archives = std::collections::BTreeMap::new(); + let mut archives = BTreeMap::new(); for entry in directory.read_dir().map_err(|error| error.to_string())? { let entry = entry.map_err(|error| error.to_string())?; - if entry.path().extension().is_some_and(|extension| extension == "tgz") { - archives - .insert(entry.file_name().to_string_lossy().into_owned(), stamp(&entry.path())?); + let path = entry.path(); + if path.extension().is_some_and(|extension| extension == "tgz") { + archives.insert(entry.file_name().to_string_lossy().into_owned(), stamp(&path)?); } } if archives.len() != 2 { diff --git a/crates/vp_cli_snapshots/tests/cli_snapshots/report.rs b/crates/vp_cli_snapshots/tests/cli_snapshots/report.rs index 7a3b0a8093..377d565eec 100644 --- a/crates/vp_cli_snapshots/tests/cli_snapshots/report.rs +++ b/crates/vp_cli_snapshots/tests/cli_snapshots/report.rs @@ -1,7 +1,7 @@ //! Optional per-process artifacts. Each trial owns its files, including under nextest. use std::{ cell::{Cell, RefCell}, - path::PathBuf, + path::{Path, PathBuf}, time::{Duration, Instant}, }; @@ -34,7 +34,7 @@ impl Report { Phase { report: self, name: name.into(), started: Instant::now() } } - pub fn record(&self, name: &str, elapsed: Duration) { + fn record(&self, name: &str, elapsed: Duration) { if self.directory.is_some() { self.phases.borrow_mut().push(serde_json::json!({ "name": name, @@ -68,11 +68,7 @@ impl Report { } } - pub fn finish( - &self, - error: Option<&str>, - expected: Option<&std::path::Path>, - ) -> Result<(), String> { + pub fn finish(&self, error: Option<&str>, expected: Option<&Path>) -> Result<(), String> { let Some(directory) = &self.directory else { return Ok(()) }; let write = || -> std::io::Result<()> { std::fs::create_dir_all(directory)?; @@ -85,18 +81,17 @@ impl Report { "phases": *self.phases.borrow(), }); std::fs::write(directory.join("timing.json"), serde_json::to_vec_pretty(&timings)?)?; - if let Some(error) = error { - std::fs::write(directory.join("error.txt"), error)?; - std::fs::write(directory.join("output.txt"), self.output.borrow().as_bytes())?; - if let Some(actual) = self.actual.borrow().as_ref() { - std::fs::write(directory.join("actual.md"), actual)?; - } - if let Some(expected) = expected { - match std::fs::read(expected) { - Ok(contents) => std::fs::write(directory.join("expected.md"), contents)?, - Err(error) if error.kind() == std::io::ErrorKind::NotFound => {} - Err(error) => return Err(error), - } + let Some(error) = error else { return Ok(()) }; + std::fs::write(directory.join("error.txt"), error)?; + std::fs::write(directory.join("output.txt"), self.output.borrow().as_bytes())?; + if let Some(actual) = self.actual.borrow().as_ref() { + std::fs::write(directory.join("actual.md"), actual)?; + } + if let Some(expected) = expected { + match std::fs::read(expected) { + Ok(contents) => std::fs::write(directory.join("expected.md"), contents)?, + Err(error) if error.kind() == std::io::ErrorKind::NotFound => {} + Err(error) => return Err(error), } } Ok(()) From f7e0f23852044953063c3dff7057a3eade3d1188 Mon Sep 17 00:00:00 2001 From: MK Date: Tue, 22 Sep 2026 15:32:54 +0800 Subject: [PATCH 08/10] test(snapshots): measure case initialization phases --- .../tests/cli_snapshots/README.md | 5 +++++ .../vp_cli_snapshots/tests/cli_snapshots/main.rs | 16 +++++++++++++--- 2 files changed, 18 insertions(+), 3 deletions(-) diff --git a/crates/vp_cli_snapshots/tests/cli_snapshots/README.md b/crates/vp_cli_snapshots/tests/cli_snapshots/README.md index 6c3157a65e..c80fc804b5 100644 --- a/crates/vp_cli_snapshots/tests/cli_snapshots/README.md +++ b/crates/vp_cli_snapshots/tests/cli_snapshots/README.md @@ -122,6 +122,11 @@ that actually packs; it is nested inside `registry-pack-or-reuse`. Phase durations are milliseconds; nested phases must not be added together. The existing console timings still exclude gate waiting. +The `case-setup/*` phases divide `case-setup` into home creation, binary and +package installation, `vp env setup`, and `vp env on pm`. These phases are +nested inside `case-setup`; use them to identify preparation costs without +counting the parent duration twice. + `workspace-cleanup` removes the case workspace after comparison, while other workers can still run tests. The final `run-cleanup` removes shared run files and any case files left by a panic or an unsuccessful earlier cleanup attempt. diff --git a/crates/vp_cli_snapshots/tests/cli_snapshots/main.rs b/crates/vp_cli_snapshots/tests/cli_snapshots/main.rs index 01c34820a3..dec1963156 100644 --- a/crates/vp_cli_snapshots/tests/cli_snapshots/main.rs +++ b/crates/vp_cli_snapshots/tests/cli_snapshots/main.rs @@ -581,7 +581,9 @@ impl CaseHome { &self, flavor: Flavor, runtime: &FlavorRuntime, + report: &report::Report, ) -> Result { + let binaries_phase = report.phase("case-setup/install-binaries"); let current_bin = self.vp_home().join("current").join("bin"); std::fs::create_dir_all(¤t_bin) .map_err(|e| format!("failed to create current/bin dir: {e}"))?; @@ -605,6 +607,8 @@ impl CaseHome { flavor::install_file(¤t_bin.join("vp-shim.exe"), &shim, "global vp-shim.exe")?; } + drop(binaries_phase); + let package_phase = report.phase("case-setup/install-package"); let package_dir = self.vp_home().join("current").join("node_modules").join("vite-plus"); Self::install_case_package(&runtime.cli_package_dir, &package_dir)?; let local_bin_dir = local_package_bin_dir(&package_dir); @@ -612,7 +616,8 @@ impl CaseHome { if flavor == Flavor::Local { self.write_local_package_cmd_shims(&package_dir, &local_bin_dir)?; } - self.run_env_setup(&vp)?; + drop(package_phase); + self.run_env_setup(&vp, report)?; let vp_bin_dir = self.vp_home().join("bin"); let mut tool_dirs = match flavor { @@ -696,8 +701,9 @@ impl CaseHome { Ok(()) } - fn run_env_setup(&self, vp: &Path) -> Result<(), String> { + fn run_env_setup(&self, vp: &Path, report: &report::Report) -> Result<(), String> { let env = self.base_env(compose_path_env(&[])); + let setup_phase = report.phase("case-setup/env-setup"); let output = std::process::Command::new(vp) .args(["env", "setup", "--refresh"]) .env_clear() @@ -713,6 +719,8 @@ impl CaseHome { )); } + drop(setup_phase); + let _preferences_phase = report.phase("case-setup/env-on-pm"); // Cases start with explicit package-manager preferences, as fresh installations do. let output = std::process::Command::new(vp) .args(["env", "on", "pm"]) @@ -1216,8 +1224,10 @@ fn run_case( drop(staging_phase); let setup_phase = report.phase("case-setup"); + let home_phase = report.phase("case-setup/home"); let case_home = CaseHome::provision(case_root, case.seed_runtime); - let case_install = case_home.provision_vite_plus(flavor, runtime)?; + drop(home_phase); + let case_install = case_home.provision_vite_plus(flavor, runtime, report)?; drop(setup_phase); let mut case_env = baseline_env(&case_home, &case_install); From 013f64dfa56773c092443b4844d4565f69f9f69f Mon Sep 17 00:00:00 2001 From: MK Date: Tue, 22 Sep 2026 15:45:00 +0800 Subject: [PATCH 09/10] test(snapshots): initialize managed shims in one pass --- .../tests/cli_snapshots/README.md | 8 +- .../fixtures/snapshot_setup/package.json | 4 + .../fixtures/snapshot_setup/snapshots.toml | 6 ++ .../prepared_home_matches_cli_setup.global.md | 10 ++ .../prepared_home_matches_cli_setup.local.md | 10 ++ .../fixtures/snapshot_setup/verify.mjs | 95 +++++++++++++++++++ .../tests/cli_snapshots/main.rs | 40 ++++---- 7 files changed, 152 insertions(+), 21 deletions(-) create mode 100644 crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/snapshot_setup/package.json create mode 100644 crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/snapshot_setup/snapshots.toml create mode 100644 crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/snapshot_setup/snapshots/prepared_home_matches_cli_setup.global.md create mode 100644 crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/snapshot_setup/snapshots/prepared_home_matches_cli_setup.local.md create mode 100644 crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/snapshot_setup/verify.mjs diff --git a/crates/vp_cli_snapshots/tests/cli_snapshots/README.md b/crates/vp_cli_snapshots/tests/cli_snapshots/README.md index c80fc804b5..383af0a7ce 100644 --- a/crates/vp_cli_snapshots/tests/cli_snapshots/README.md +++ b/crates/vp_cli_snapshots/tests/cli_snapshots/README.md @@ -123,7 +123,7 @@ durations are milliseconds; nested phases must not be added together. The existing console timings still exclude gate waiting. The `case-setup/*` phases divide `case-setup` into home creation, binary and -package installation, `vp env setup`, and `vp env on pm`. These phases are +package installation, preference seeding, and `vp env setup`. These phases are nested inside `case-setup`; use them to identify preparation costs without counting the parent duration twice. @@ -170,6 +170,12 @@ flavor exposes sibling `.cmd` shims under one trial and one snapshot per flavor; use it for parity cases (help output, routing, error messages) where both surfaces must agree. +Each case starts with explicit managed-mode preferences for npm, pnpm, Yarn, +and Bun. The runner writes these preferences before `vp env setup` so setup +creates the shims in their final locations in one pass. Tests of preference +inference or an empty installation must create a separate home, as the +`shim_package_manager_setup` fixture does. + A step is a bare argv array or a table: ```toml diff --git a/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/snapshot_setup/package.json b/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/snapshot_setup/package.json new file mode 100644 index 0000000000..5b71b291bb --- /dev/null +++ b/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/snapshot_setup/package.json @@ -0,0 +1,4 @@ +{ + "name": "snapshot-setup", + "private": true +} diff --git a/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/snapshot_setup/snapshots.toml b/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/snapshot_setup/snapshots.toml new file mode 100644 index 0000000000..b87bafcd33 --- /dev/null +++ b/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/snapshot_setup/snapshots.toml @@ -0,0 +1,6 @@ +[[case]] +name = "prepared_home_matches_cli_setup" +vp = ["global", "local"] +steps = [ + { argv = ["node", "verify.mjs"], comment = "The prepared home has the same preferences, environment files, and shims as setup followed by enabling package-manager management." }, +] diff --git a/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/snapshot_setup/snapshots/prepared_home_matches_cli_setup.global.md b/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/snapshot_setup/snapshots/prepared_home_matches_cli_setup.global.md new file mode 100644 index 0000000000..50b67471f0 --- /dev/null +++ b/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/snapshot_setup/snapshots/prepared_home_matches_cli_setup.global.md @@ -0,0 +1,10 @@ +# prepared_home_matches_cli_setup + +## `node verify.mjs` + +The prepared home has the same preferences, environment files, and shims as setup followed by enabling package-manager management. + +``` +Prepared preferences, environment files, and shims match the CLI setup sequence. +Configuration changes remain isolated to their own home. +``` diff --git a/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/snapshot_setup/snapshots/prepared_home_matches_cli_setup.local.md b/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/snapshot_setup/snapshots/prepared_home_matches_cli_setup.local.md new file mode 100644 index 0000000000..50b67471f0 --- /dev/null +++ b/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/snapshot_setup/snapshots/prepared_home_matches_cli_setup.local.md @@ -0,0 +1,10 @@ +# prepared_home_matches_cli_setup + +## `node verify.mjs` + +The prepared home has the same preferences, environment files, and shims as setup followed by enabling package-manager management. + +``` +Prepared preferences, environment files, and shims match the CLI setup sequence. +Configuration changes remain isolated to their own home. +``` diff --git a/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/snapshot_setup/verify.mjs b/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/snapshot_setup/verify.mjs new file mode 100644 index 0000000000..739e2626b1 --- /dev/null +++ b/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/snapshot_setup/verify.mjs @@ -0,0 +1,95 @@ +import assert from 'node:assert/strict'; +import { spawnSync } from 'node:child_process'; +import { createHash } from 'node:crypto'; +import fs from 'node:fs'; +import path from 'node:path'; + +const prepared = fs.realpathSync(process.env.VP_HOME); +const referenceHome = path.resolve('reference-home'); +const reference = path.join(referenceHome, '.vite-plus'); +const windows = process.platform === 'win32'; +const binaryName = windows ? 'vp.exe' : 'vp'; +const bin = path.join(reference, 'current', 'bin'); +fs.mkdirSync(bin, { recursive: true }); +for (const name of windows ? [binaryName, 'vp-shim.exe'] : [binaryName]) { + fs.copyFileSync(path.join(prepared, 'current', 'bin', name), path.join(bin, name)); +} + +const env = { ...process.env }; +for (const name of Object.keys(env)) { + if (name.startsWith('VP_') || name.startsWith('XDG_')) { + delete env[name]; + } +} +Object.assign(env, { + VP_HOME: reference, + VP_CLI_TEST: '1', + HOME: referenceHome, + USERPROFILE: referenceHome, + PATH: windows + ? (process.env.PATH ?? '') + .split(path.delimiter) + .filter((entry) => !entry.startsWith(path.dirname(prepared))) + .join(path.delimiter) + : '/usr/bin:/bin:/usr/sbin:/sbin', +}); +for (const args of [ + ['env', 'setup', '--refresh'], + ['env', 'on', 'pm'], +]) { + const result = spawnSync(path.join(bin, binaryName), args, { + env, + encoding: 'utf8', + timeout: 30000, + }); + assert.equal(result.status, 0, result.error?.message ?? result.stdout + result.stderr); +} + +function installation(home) { + const files = {}; + const normalize = (value) => value.replaceAll(home, ''); + function visit(relative) { + const file = path.join(home, relative); + const stat = fs.lstatSync(file); + if (stat.isSymbolicLink()) { + files[relative] = { target: normalize(fs.readlinkSync(file)) }; + } else if (stat.isDirectory()) { + files[relative] = 'directory'; + for (const name of fs.readdirSync(file).sort()) { + visit(path.join(relative, name)); + } + } else if (file.endsWith('.exe')) { + files[relative] = { + sha256: createHash('sha256').update(fs.readFileSync(file)).digest('hex'), + }; + } else { + files[relative] = { contents: normalize(fs.readFileSync(file, 'utf8')) }; + } + } + for (const name of fs.readdirSync(home).sort()) { + if ( + name === 'config.json' || + name === 'bin' || + name === 'fallback-bin' || + /^env(?:\.|$)/.test(name) + ) { + visit(name); + } + } + return files; +} + +const actual = installation(prepared); +const expected = installation(reference); +assert.deepEqual(Object.keys(actual), Object.keys(expected)); +for (const name of Object.keys(expected)) { + assert.deepEqual(actual[name], expected[name], name); +} +console.log('Prepared preferences, environment files, and shims match the CLI setup sequence.'); + +// Mutating the reference must not change the case's own configuration. +const config = path.join(prepared, 'config.json'); +const original = fs.readFileSync(config, 'utf8'); +fs.writeFileSync(path.join(reference, 'config.json'), '{}'); +assert.equal(fs.readFileSync(config, 'utf8'), original); +console.log('Configuration changes remain isolated to their own home.'); diff --git a/crates/vp_cli_snapshots/tests/cli_snapshots/main.rs b/crates/vp_cli_snapshots/tests/cli_snapshots/main.rs index dec1963156..e8f862356b 100644 --- a/crates/vp_cli_snapshots/tests/cli_snapshots/main.rs +++ b/crates/vp_cli_snapshots/tests/cli_snapshots/main.rs @@ -702,37 +702,37 @@ impl CaseHome { } fn run_env_setup(&self, vp: &Path, report: &report::Report) -> Result<(), String> { + // Every case starts with managed package-manager shims. Set these + // preferences before setup so it creates each shim once, without a + // second process that moves inferred system-first shims afterward. + let preferences_phase = report.phase("case-setup/preferences"); + let preferences = serde_json::json!({ + "packageManagerShimModes": { + "bun": "managed", + "npm": "managed", + "pnpm": "managed", + "yarn": "managed", + }, + }); + std::fs::write( + self.vp_home().join("config.json"), + serde_json::to_vec_pretty(&preferences).unwrap(), + ) + .map_err(|e| format!("failed to write case package-manager preferences: {e}"))?; + drop(preferences_phase); let env = self.base_env(compose_path_env(&[])); - let setup_phase = report.phase("case-setup/env-setup"); + let _setup_phase = report.phase("case-setup/env-setup"); let output = std::process::Command::new(vp) .args(["env", "setup", "--refresh"]) .env_clear() .envs(&env) .output() .map_err(|e| format!("failed to run `vp env setup`: {e}"))?; - if !output.status.success() { - return Err(format!( - "`vp env setup` failed with status {}\nstdout:\n{}\nstderr:\n{}", - output.status, - String::from_utf8_lossy(&output.stdout), - String::from_utf8_lossy(&output.stderr) - )); - } - - drop(setup_phase); - let _preferences_phase = report.phase("case-setup/env-on-pm"); - // Cases start with explicit package-manager preferences, as fresh installations do. - let output = std::process::Command::new(vp) - .args(["env", "on", "pm"]) - .env_clear() - .envs(&env) - .output() - .map_err(|e| format!("failed to run `vp env on pm`: {e}"))?; if output.status.success() { return Ok(()); } Err(format!( - "`vp env on pm` failed with status {}\nstdout:\n{}\nstderr:\n{}", + "`vp env setup` failed with status {}\nstdout:\n{}\nstderr:\n{}", output.status, String::from_utf8_lossy(&output.stdout), String::from_utf8_lossy(&output.stderr) From 32cc707675903a2db98c20535af2643980fa4e83 Mon Sep 17 00:00:00 2001 From: MK Date: Tue, 22 Sep 2026 15:59:43 +0800 Subject: [PATCH 10/10] test(snapshots): run case installation setup once --- .../tests/cli_snapshots/README.md | 14 +++++++++----- .../fixtures/snapshot_setup/verify.mjs | 16 ++++++++++++++++ .../tests/cli_snapshots/main.rs | 19 +++++++++++++------ 3 files changed, 38 insertions(+), 11 deletions(-) diff --git a/crates/vp_cli_snapshots/tests/cli_snapshots/README.md b/crates/vp_cli_snapshots/tests/cli_snapshots/README.md index 383af0a7ce..bba56cf886 100644 --- a/crates/vp_cli_snapshots/tests/cli_snapshots/README.md +++ b/crates/vp_cli_snapshots/tests/cli_snapshots/README.md @@ -123,7 +123,7 @@ durations are milliseconds; nested phases must not be added together. The existing console timings still exclude gate waiting. The `case-setup/*` phases divide `case-setup` into home creation, binary and -package installation, preference seeding, and `vp env setup`. These phases are +package installation, preference seeding, and first-start setup. These phases are nested inside `case-setup`; use them to identify preparation costs without counting the parent duration twice. @@ -162,7 +162,7 @@ after = [ ... ] # cleanup steps, never snapshotted `vp` picks which CLI runs the case. Both flavors install the built Rust binary into the case's `VP_HOME/current/bin`, install the checkout package under that -case home, and run `vp env setup` before steps. `"global"` exposes only +case home, and run first-start setup before steps. `"global"` exposes only `VP_HOME/bin`; `"local"` also exposes the case-local `VP_HOME/current/node_modules/vite-plus/bin` package bin. On Windows, local flavor exposes sibling `.cmd` shims under @@ -171,10 +171,14 @@ one trial and one snapshot per flavor; use it for parity cases (help output, routing, error messages) where both surfaces must agree. Each case starts with explicit managed-mode preferences for npm, pnpm, Yarn, -and Bun. The runner writes these preferences before `vp env setup` so setup -creates the shims in their final locations in one pass. Tests of preference -inference or an empty installation must create a separate home, as the +and Bun. The runner writes these preferences before starting the unmarked +`vp` binary with no arguments. First-start setup creates the environment files +and shims, then exits. This avoids a second `vp env setup --refresh`. Tests of +preference inference or an empty installation must create a separate home, as the `shim_package_manager_setup` fixture does. +The preparation command sets `VP_SELF_SETUP_NO_MODIFY_PATH=1` so Windows +self-setup does not add temporary case homes to the user's persistent `PATH`. +This override applies only to runner preparation, not to fixture commands. A step is a bare argv array or a table: diff --git a/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/snapshot_setup/verify.mjs b/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/snapshot_setup/verify.mjs index 739e2626b1..0032f4112a 100644 --- a/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/snapshot_setup/verify.mjs +++ b/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/snapshot_setup/verify.mjs @@ -8,6 +8,21 @@ const prepared = fs.realpathSync(process.env.VP_HOME); const referenceHome = path.resolve('reference-home'); const reference = path.join(referenceHome, '.vite-plus'); const windows = process.platform === 'win32'; +assert.equal(process.env.VP_SELF_SETUP_NO_MODIFY_PATH, undefined); +if (windows) { + const result = spawnSync( + 'powershell.exe', + [ + '-NoProfile', + '-NonInteractive', + '-Command', + '[Environment]::GetEnvironmentVariable("Path", "User")', + ], + { encoding: 'utf8', timeout: 30000 }, + ); + assert.equal(result.status, 0, result.error?.message ?? result.stdout + result.stderr); + assert.ok(!result.stdout.toLowerCase().includes(prepared.toLowerCase())); +} const binaryName = windows ? 'vp.exe' : 'vp'; const bin = path.join(reference, 'current', 'bin'); fs.mkdirSync(bin, { recursive: true }); @@ -24,6 +39,7 @@ for (const name of Object.keys(env)) { Object.assign(env, { VP_HOME: reference, VP_CLI_TEST: '1', + VP_SELF_SETUP_NO_MODIFY_PATH: '1', HOME: referenceHome, USERPROFILE: referenceHome, PATH: windows diff --git a/crates/vp_cli_snapshots/tests/cli_snapshots/main.rs b/crates/vp_cli_snapshots/tests/cli_snapshots/main.rs index e8f862356b..12fff375b2 100644 --- a/crates/vp_cli_snapshots/tests/cli_snapshots/main.rs +++ b/crates/vp_cli_snapshots/tests/cli_snapshots/main.rs @@ -617,7 +617,7 @@ impl CaseHome { self.write_local_package_cmd_shims(&package_dir, &local_bin_dir)?; } drop(package_phase); - self.run_env_setup(&vp, report)?; + self.run_initial_setup(&vp, report)?; let vp_bin_dir = self.vp_home().join("bin"); let mut tool_dirs = match flavor { @@ -701,7 +701,7 @@ impl CaseHome { Ok(()) } - fn run_env_setup(&self, vp: &Path, report: &report::Report) -> Result<(), String> { + fn run_initial_setup(&self, vp: &Path, report: &report::Report) -> Result<(), String> { // Every case starts with managed package-manager shims. Set these // preferences before setup so it creates each shim once, without a // second process that moves inferred system-first shims afterward. @@ -721,18 +721,25 @@ impl CaseHome { .map_err(|e| format!("failed to write case package-manager preferences: {e}"))?; drop(preferences_phase); let env = self.base_env(compose_path_env(&[])); - let _setup_phase = report.phase("case-setup/env-setup"); + let _setup_phase = report.phase("case-setup/first-start"); + // This fresh installation has no setup marker. Its first invocation + // creates the environment files and shims, then exits with no args. + // Passing `env setup --refresh` would run setup a second time. let output = std::process::Command::new(vp) - .args(["env", "setup", "--refresh"]) .env_clear() .envs(&env) + // The unmarked case binary runs self-setup first. Its Windows + // handoff must not start PowerShell to add this temporary home + // to the real user's persistent PATH. Case commands do not + // inherit this override, so self-setup fixtures retain coverage. + .env("VP_SELF_SETUP_NO_MODIFY_PATH", "1") .output() - .map_err(|e| format!("failed to run `vp env setup`: {e}"))?; + .map_err(|e| format!("failed to run first-start setup: {e}"))?; if output.status.success() { return Ok(()); } Err(format!( - "`vp env setup` failed with status {}\nstdout:\n{}\nstderr:\n{}", + "first-start setup failed with status {}\nstdout:\n{}\nstderr:\n{}", output.status, String::from_utf8_lossy(&output.stdout), String::from_utf8_lossy(&output.stderr)