From 8dda4646b8eea37fb7caa3305c2e1b0e82368ff2 Mon Sep 17 00:00:00 2001 From: James Ross Date: Sat, 30 May 2026 11:09:17 -0700 Subject: [PATCH] chore(dev-loop): kill the slow iteration loop on echo-wesley-gen tests Three targeted fixes that compound: pre-built wesley-gen binary, shared CARGO_TARGET_DIR for generated consumer crates, and auto-staged hook formatters. Observed result on the test that previously dominated wall-clock time (test_toy_contract_no_std_generated_output_checks_in_ consumer_crate): Before this commit (warm cache): ~96s After this commit (warm cache): 1.14s After this commit (cold cache): ~9s + build That is roughly an 85x speedup on the hot path. The mechanics: 1) wesley_gen_binary() -- OnceLock in tests/generation.rs. Builds echo-wesley-gen exactly once per test binary via cargo build with --message-format=json-render-diagnostics, captures the executable path from the compiler-artifact JSON, then exec's the binary directly on every subsequent invocation. Previously every test spawned cargo run -p echo-wesley-gen -- and paid a freshness re-check across the whole dependency graph per test. 2) shared_consumer_target_dir() -- returns target/echo-wesley-gen-test-consumer-cache/ and is wired in via .env("CARGO_TARGET_DIR", ...) on every cargo check/cargo test invocation against a generated consumer crate. Previously each generated crate had its own [workspace] block AND a per-PID parent dir, so echo-wasm-abi, echo-registry-api, and serde compiled from scratch for every single test. Now they compile once and stay cached across both within-run and between-run test invocations. 3) .githooks/pre-commit -- both auto-format paths (cargo fmt and prettier) now auto-stage their output when the working tree was clean before the formatter ran, instead of aborting and forcing a manual git add -A + recommit cycle. Partial staging still aborts safely: if there were unstaged changes before the formatter, we can't tell whether the formatter touched staged hunks, unstaged hunks, or both, so we bail with the same message as before. The common case (working tree clean, you staged everything you wanted) just continues. The cargo verify cost was previously paid twice per typical hook-aborted commit; now it's paid once. The integration test change is purely test-harness; no production code or generated wire formats are affected, so the determinism gates are not at risk. Verified by running test_toy_contract_no_std_generated_output_checks_in_consumer_crate twice back-to-back -- first run 9.06s cold, second run 1.14s warm. Implements wins #1, #2, #5 from docs/method/backlog/cool-ideas/PLATFORM_wesley-gen-test-loop-speedup.md (currently committed on cycle/0025-sessions-as-causal-contexts as part of PR #382). Win #3 (crate-scoped pre-commit) already shipped under scripts/verify-local.sh; win #4 (cargo-nextest) can ride later. --- .githooks/pre-commit | 50 ++++++- crates/echo-wesley-gen/tests/generation.rs | 150 ++++++++++++++++----- 2 files changed, 162 insertions(+), 38 deletions(-) diff --git a/.githooks/pre-commit b/.githooks/pre-commit index 4d156340..0b40ee66 100755 --- a/.githooks/pre-commit +++ b/.githooks/pre-commit @@ -64,16 +64,41 @@ if command -v rustup >/dev/null 2>&1; then fi # 3) Format (auto-fix if opted in) +# +# Auto-stage protocol: when ECHO_AUTO_FMT is on and rustfmt/prettier rewrite +# files, the hook used to abort and force a manual restage-and-recommit cycle +# (paying cargo + lint costs twice). Instead, if the working tree was *clean* +# relative to the index before the formatter ran, the formatted output is +# safe to auto-add: every modified line came from the staged content, so we +# stage the result and continue. Partial staging (any unstaged changes +# present before the formatter) still aborts, because we can't tell whether +# the formatter touched staged hunks, unstaged hunks, or both. _auto_fmt="${ECHO_AUTO_FMT:-1}" + +# Snapshot whether the working tree has unstaged changes BEFORE any +# auto-formatter runs. `git diff --quiet` exits 0 if clean, 1 if dirty. +if git diff --quiet; then + HAD_UNSTAGED_BEFORE_FMT=0 +else + HAD_UNSTAGED_BEFORE_FMT=1 +fi + case "${_auto_fmt}" in 1|true|TRUE|yes|YES|on|ON) echo "pre-commit: ECHO_AUTO_FMT=${_auto_fmt} → checking format" if ! cargo fmt --all -- --check; then echo "pre-commit: running cargo fmt to apply changes" >&2 cargo fmt --all || { echo "pre-commit: cargo fmt failed" >&2; exit 1; } - echo "pre-commit: rustfmt updated files. Aborting commit to preserve index integrity (partial staging safe)." >&2 - echo "Hint: review changes, restage (e.g., 'git add -p' or 'git add -A'), then commit again." >&2 - exit 1 + if [[ "$HAD_UNSTAGED_BEFORE_FMT" -eq 0 ]]; then + echo "pre-commit: rustfmt updated files; auto-staging (working tree was clean)." >&2 + git add -u || { echo "pre-commit: failed to re-stage rustfmt changes" >&2; exit 1; } + # Refresh STAGED so later steps see the post-fmt index. + STAGED=$(git diff --cached --name-only) + else + echo "pre-commit: rustfmt updated files but you had unstaged changes; aborting to preserve partial-staging integrity." >&2 + echo "Hint: review changes, restage (e.g., 'git add -p' or 'git add -A'), then commit again." >&2 + exit 1 + fi fi ;; 0|false|FALSE|no|NO|off|OFF) @@ -139,14 +164,27 @@ if [[ ${#MD_FILES[@]} -gt 0 ]] && command -v npx >/dev/null 2>&1; then : # Already formatted else echo "pre-commit: running prettier on markdown files" >&2 + # Re-snapshot working-tree dirtiness for the markdown path specifically. + # The rustfmt step above may have auto-staged its own changes, so the + # earlier HAD_UNSTAGED_BEFORE_FMT value is stale by this point. + if git diff --quiet -- "${MD_FILES[@]}"; then + MD_HAD_UNSTAGED=0 + else + MD_HAD_UNSTAGED=1 + fi # Write changes; suppress stdout but preserve stderr for errors if ! npx prettier --write "${MD_FILES[@]}" >/dev/null; then echo "pre-commit: prettier failed" >&2 exit 1 fi - echo "pre-commit: prettier updated markdown files. Aborting commit to preserve index integrity." >&2 - echo "Hint: review changes, restage (e.g., 'git add -p' or 'git add -A'), then commit again." >&2 - exit 1 + if [[ "$MD_HAD_UNSTAGED" -eq 0 ]]; then + echo "pre-commit: prettier updated markdown files; auto-staging (markdown working tree was clean)." >&2 + git add -- "${MD_FILES[@]}" || { echo "pre-commit: failed to re-stage prettier changes" >&2; exit 1; } + else + echo "pre-commit: prettier updated markdown files but you had unstaged markdown edits; aborting to preserve partial-staging integrity." >&2 + echo "Hint: review changes, restage (e.g., 'git add -p' or 'git add -A'), then commit again." >&2 + exit 1 + fi fi ;; esac diff --git a/crates/echo-wesley-gen/tests/generation.rs b/crates/echo-wesley-gen/tests/generation.rs index 8dd4aa63..2e8993f5 100644 --- a/crates/echo-wesley-gen/tests/generation.rs +++ b/crates/echo-wesley-gen/tests/generation.rs @@ -1,30 +1,134 @@ // SPDX-License-Identifier: Apache-2.0 // © James Ross Ω FLYING•ROBOTS -#![allow(clippy::unwrap_used, clippy::expect_used)] +#![allow(clippy::unwrap_used, clippy::expect_used, clippy::panic)] //! Integration test for the echo-wesley-gen CLI (Wesley IR -> Rust code). use std::fs; use std::io::Write; use std::path::{Path, PathBuf}; use std::process::{Command, Output, Stdio}; +use std::sync::OnceLock; const TOY_COUNTER_IR: &str = include_str!("fixtures/toy-counter/echo-ir-v1.json"); -/// Spawns `cargo run -p echo-wesley-gen --`, pipes `ir` to stdin, and returns the output. +// --------------------------------------------------------------------------- +// Test harness speedups. +// +// The integration tests in this file used to be 60-120s each because every +// one of them paid two avoidable costs: +// +// 1. `cargo run -p echo-wesley-gen --` on every invocation, which forced +// cargo to re-check freshness across the whole dependency graph even +// when the binary was already current. +// 2. Each generated consumer crate (under `target/echo-wesley-gen-*-smoke +// //...`) had its own independent `target/` directory, so +// `echo-wasm-abi`, `echo-registry-api`, and `serde` were recompiled +// from scratch for every test instead of shared across the suite. +// +// The two helpers below kill both costs: +// +// * `wesley_gen_binary()` builds `echo-wesley-gen` once per test binary +// (`OnceLock`) and returns the path to the compiled binary. All later +// invocations exec the binary directly, skipping cargo's freshness check. +// * `shared_consumer_target_dir()` returns a single stable path under the +// workspace `target/` directory. Every cargo invocation against a +// generated consumer crate runs with `CARGO_TARGET_DIR` set to that +// path, so the heavy upstream dependencies compile exactly once for the +// entire suite and stay cached across `cargo test` runs. +// --------------------------------------------------------------------------- + +/// Build `echo-wesley-gen` once per test binary and reuse the compiled path. +/// +/// The binary is built under the workspace's normal target directory so the +/// dev profile artifacts are shared with day-to-day workflows. Cargo is +/// still invoked once (the first call) so the binary is guaranteed fresh +/// against the current source; subsequent calls return immediately. +fn wesley_gen_binary() -> &'static Path { + static BINARY: OnceLock = OnceLock::new(); + BINARY + .get_or_init(|| { + // Use `--message-format=json` so we can read the compiler- + // executable path directly instead of guessing where the binary + // landed (works across debug/release and custom profiles). + let output = Command::new("cargo") + .args([ + "build", + "-p", + "echo-wesley-gen", + "--bin", + "echo-wesley-gen", + "--message-format=json-render-diagnostics", + ]) + .stderr(Stdio::inherit()) + .output() + .expect("failed to build echo-wesley-gen for tests"); + assert!( + output.status.success(), + "cargo build of echo-wesley-gen failed: {}", + String::from_utf8_lossy(&output.stderr) + ); + for line in output.stdout.split(|b| *b == b'\n') { + if line.is_empty() { + continue; + } + let msg: serde_json::Value = match serde_json::from_slice(line) { + Ok(v) => v, + Err(_) => continue, + }; + if msg.get("reason").and_then(|r| r.as_str()) != Some("compiler-artifact") { + continue; + } + let target = msg.get("target").and_then(|t| t.as_object()); + let is_bin = target + .and_then(|t| t.get("kind")) + .and_then(|k| k.as_array()) + .is_some_and(|kinds| kinds.iter().any(|k| k.as_str() == Some("bin"))); + let name_matches = target.and_then(|t| t.get("name")).and_then(|n| n.as_str()) + == Some("echo-wesley-gen"); + if !(is_bin && name_matches) { + continue; + } + if let Some(exe) = msg.get("executable").and_then(|e| e.as_str()) { + return PathBuf::from(exe); + } + } + panic!( + "cargo build produced no echo-wesley-gen bin artifact in JSON output:\n{}", + String::from_utf8_lossy(&output.stdout) + ); + }) + .as_path() +} + +/// Returns the shared `CARGO_TARGET_DIR` path used for every generated +/// consumer crate. All consumer crates share this directory so upstream +/// deps (`echo-wasm-abi`, `echo-registry-api`, `serde`, …) compile once +/// for the whole suite instead of once per test, both within a single +/// `cargo test` run and across runs while the path stays warm. +fn shared_consumer_target_dir() -> PathBuf { + let dir = workspace_root() + .join("target") + .join("echo-wesley-gen-test-consumer-cache"); + fs::create_dir_all(&dir).expect("failed to create shared consumer target dir"); + dir +} + +/// Spawns the pre-built `echo-wesley-gen` binary, pipes `ir` to stdin, and +/// returns the output. Equivalent to the old `cargo run -p echo-wesley-gen +/// -- ` flow but without re-checking freshness per invocation. fn run_wesley_gen(ir: &str) -> Output { run_wesley_gen_with_args(ir, &[]) } -/// Spawns `cargo run -p echo-wesley-gen -- `, pipes `ir` to stdin, and returns the output. +/// Spawns the pre-built `echo-wesley-gen` binary with extra CLI args. fn run_wesley_gen_with_args(ir: &str, args: &[&str]) -> Output { - let mut child = Command::new("cargo") - .args(["run", "-p", "echo-wesley-gen", "--"]) + let mut child = Command::new(wesley_gen_binary()) .args(args) .stdin(Stdio::piped()) .stdout(Stdio::piped()) .stderr(Stdio::piped()) .spawn() - .expect("failed to spawn cargo run"); + .expect("failed to spawn echo-wesley-gen"); let mut stdin = child.stdin.take().expect("failed to get stdin"); stdin @@ -36,13 +140,13 @@ fn run_wesley_gen_with_args(ir: &str, args: &[&str]) -> Output { } fn run_wesley_gen_schema(schema_path: &Path) -> Output { - Command::new("cargo") - .args(["run", "-p", "echo-wesley-gen", "--", "--schema"]) + Command::new(wesley_gen_binary()) + .args(["--schema"]) .arg(schema_path) .stdout(Stdio::piped()) .stderr(Stdio::piped()) .output() - .expect("failed to run cargo run") + .expect("failed to run echo-wesley-gen") } fn generated_str_const<'a>(source: &'a str, name: &str) -> &'a str { @@ -1283,6 +1387,7 @@ mod tests { fn assert_generated_crate_checks(crate_dir: &Path) { let output = Command::new("cargo") + .env("CARGO_TARGET_DIR", shared_consumer_target_dir()) .args(["check", "--manifest-path"]) .arg(crate_dir.join("Cargo.toml")) .output() @@ -1336,6 +1441,7 @@ fn test_reserved_control_op_id_fails_closed() { fn test_generated_optic_helper_shape_compiles_against_abi() { let crate_dir = write_optic_binding_smoke_crate(); let output = Command::new("cargo") + .env("CARGO_TARGET_DIR", shared_consumer_target_dir()) .args(["test", "--manifest-path"]) .arg(crate_dir.join("Cargo.toml")) .output() @@ -1748,6 +1854,7 @@ fn test_toy_contract_generated_output_compiles_in_consumer_crate() { let generated = String::from_utf8_lossy(&output.stdout); let crate_dir = write_consumer_smoke_crate(&generated); let output = Command::new("cargo") + .env("CARGO_TARGET_DIR", shared_consumer_target_dir()) .args(["test", "--manifest-path"]) .arg(crate_dir.join("Cargo.toml")) .output() @@ -1777,6 +1884,7 @@ fn test_toy_contract_generated_contract_host_query_observer_compiles_in_consumer assert!(generated.contains("pub fn counter_value_observer_vars")); let crate_dir = write_contract_host_smoke_crate(&generated); let output = Command::new("cargo") + .env("CARGO_TARGET_DIR", shared_consumer_target_dir()) .args(["test", "--manifest-path"]) .arg(crate_dir.join("Cargo.toml")) .output() @@ -1844,29 +1952,7 @@ fn test_generate_no_std_minicbor() { "ops": [] }"#; - // Run with flags - let mut child = Command::new("cargo") - .args([ - "run", - "-p", - "echo-wesley-gen", - "--", - "--no-std", - "--minicbor", - ]) - .stdin(Stdio::piped()) - .stdout(Stdio::piped()) - .stderr(Stdio::piped()) - .spawn() - .expect("failed to spawn cargo run"); - - let mut stdin = child.stdin.take().expect("failed to get stdin"); - stdin - .write_all(ir.as_bytes()) - .expect("failed to write to stdin"); - drop(stdin); - - let output = child.wait_with_output().expect("failed to wait on child"); + let output = run_wesley_gen_with_args(ir, &["--no-std", "--minicbor"]); let stdout = String::from_utf8_lossy(&output.stdout); assert!( output.status.success(),