Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,73 @@
<!-- SPDX-License-Identifier: Apache-2.0 OR LicenseRef-MIND-UCAL-1.0 -->
<!-- © James Ross Ω FLYING•ROBOTS <https://github.com/flyingrobots> -->

# `wesley-gen` test consumer-crate builders are ~40-line copy-paste

Legend: `PLATFORM`

## The smell

`crates/echo-wesley-gen/tests/generation.rs` has four consumer-crate
builder functions:

- `write_basic_generated_crate`
- `write_consumer_smoke_crate`
- `write_contract_host_smoke_crate`
- `write_optic_binding_smoke_crate`

Each one writes a `Cargo.toml` + `src/lib.rs` + `src/generated.rs` and
uses essentially the same boilerplate:

- the same `[workspace]` isolation block
- the same `path = "{}"` dep declaration pattern for
`echo-wasm-abi` and `echo-registry-api`
- the same `default-features = false, features = ["alloc"]` switching
on a `no_std` flag
- the same `serde` dep with no_std/std variants
- the same `#![no_std] / extern crate alloc;` vs `mod generated;`
lib.rs branching

All four were also keyed off `std::process::id()` for the crate dir
until the `chore/test-loop-speedup` PR landed; the PID segment is now
inert (the shared `CARGO_TARGET_DIR` made it moot) but still litters
the disk during a test run.

## Why this matters

- A new generated-output test path means a fifth copy of the same
boilerplate. Easy to drift one config knob (a feature flag, a
serde version, a path) silently.
- The PID-segment leftovers create + remove directories every test
for no benefit now that the target dir is shared elsewhere.
- The PR #383 speedup work touched all four sites identically when
adding `.env("CARGO_TARGET_DIR", ...)` — every future change to
the harness will pay the same cost.

## Fix shape

Extract a single `ConsumerCrateBuilder` (or a free function with an
options struct) that owns:

- the `[workspace]` boilerplate
- the dep declarations (no_std vs std switching)
- the lib.rs shim selection
- the crate-dir layout (dropping the PID segment changes path
semantics and is safe only if deterministic unique labels or
explicit tempdir guarantees preserve parallel-test isolation)

Each call site then declares only what's unique to it: the label,
the no_std flag, the generated source, and any extra deps.

This is a structural cleanup whose path-layout change is
behavior-preserving only when uniqueness and isolation invariants
are retained. Dropping the PID segment can otherwise change
collision behavior under parallel runs or label reuse. After the
extraction, adding a fifth generated-output test path should be a
~5-line call, not a ~40-line copy of the previous one.

## Out of scope here

- Sharing `CARGO_TARGET_DIR` (already landed in PR #383).
- The `[workspace]` isolation itself, which is required because the
generated crates must not pull in the parent workspace's
`Cargo.lock`. Keep that constraint.
Original file line number Diff line number Diff line change
@@ -0,0 +1,75 @@
<!-- SPDX-License-Identifier: Apache-2.0 OR LicenseRef-MIND-UCAL-1.0 -->
<!-- © James Ross Ω FLYING•ROBOTS <https://github.com/flyingrobots> -->

# Pre-commit hook should run prettier before cargo verify

Legend: `PLATFORM`

## What hurts

The PR #383 auto-stage fix already eliminated the most painful case
(prettier rewrites → abort → manual restage → cargo verify again from
scratch). But the underlying ordering is still backwards.

Current hook order:

1. cargo fmt check (auto-stage if rewrites)
2. toolchain pin check
3. PRNG / task list / lockfile guards
4. **`scripts/verify-local.sh pre-commit`** ← can take minutes cold
5. SPDX guard
6. **markdown prettier + lint** ← cheap, runs LAST

The asymmetry: the expensive cargo step runs first, and the cheap
prettier-and-markdownlint step runs last. If a commit only touches
markdown but a previous touch left the cargo cache cold, you pay the
full cargo verify cost for nothing. If your commit touches both Rust
and markdown and prettier rewrites the markdown, the auto-stage path
now means you only pay cargo once — but that one pay is still cold
because cargo ran before prettier had a chance to decide whether the
hook can short-circuit.

## The flip

Run the cheap checks first, expensive last:

1. toolchain / PRNG / lockfile guards (already cheap)
2. **prettier auto-format + markdownlint** ← move up
3. cargo fmt auto-stage
4. `scripts/verify-local.sh pre-commit`
5. SPDX guard

This costs nothing when cargo would have run anyway (the user is
committing Rust changes and the cache happens to be cold). It saves
the entire cargo cost when:

- the user is committing only markdown changes
- the user is committing markdown + Rust changes and prettier is
going to fail in a way the user wants to see before they wait for
cargo

It also makes the hook's failure modes more user-friendly: prettier
errors arrive in seconds instead of after a multi-minute cargo build.

## Risk

- The current order may be intentional if cargo fmt sometimes
rewrites markdown-adjacent files (e.g. `include_str!` doctests).
Worth a quick audit of which files cargo fmt touches before
flipping.
- If prettier auto-stages markdown that's then expected by a
downstream cargo check (unlikely, but possible if a markdown file
is included via `include_str!`), the flip needs care.

## Expected delta

For markdown-only commits on a cold cargo cache: minutes → seconds.
For mixed Rust + markdown commits where prettier rewrites: same wall
time as today, but earlier visibility of formatting errors. For
Rust-only commits: no change.

## Related

- `docs/method/backlog/cool-ideas/PLATFORM_wesley-gen-test-loop-speedup.md`
(the original speedup backlog, win #5 — this is the spiritual
successor now that win #5 partially landed in PR #383).
Loading