test(gate): assert the real plan's (name, command) pair, not one half
The lane exists to guarantee two things --- that the crdt sweep is preceded by the build producing its binary, and that a build failure is attributed to `build-crdt` rather than to `sweep-crdt`. It shipped with neither guaranteed, because NEITHER WITNESS COULD SEE A NAME. --print-plan renders `emit_plan | cut -f2-`, so the ordering test compared commands and never saw the names beside them. --self-test hardcodes the string `build-crdt` inside its OWN synthetic plan, so it proves things about the runner and nothing about the real emitter. Review demonstrated the consequence: renaming the real build step to `sweep-crdt` left both tests passing --- a plan that would report a build failure under the sweep's name, sitting green, which is exactly the misattribution the separate step exists to prevent. --print-plan-named prints emit_plan VERBATIM: the same `name<TAB>command` text the runner reads back from PLAN_FILE. The new assertion compares WHOLE LINES against it, so name and command are pinned together and a rename of either step fails. The sweep's own pair is asserted too --- asserting only the build's name leaves the identical hole open in the other direction. WHY A RENDERING AND NOT A SEAM. PLAN_FILE stays uninjectable: a test that supplied the runner's plan would turn its `eval` into a general command executor, the same class of defect this script's own review caught in --acceptance and fixed with a parse-time refusal. Re-deriving the plan test-side would be a second implementation of the thing under test, which is the failure being repaired one level up. A distinct mode rather than a --with-names modifier leaves --print-plan's contract --- runnable lines --- exactly as it was, and defines no flag combination that has no meaning. --self-test STAYS. It witnesses the runner: failure naming, the FAILED: list, log paths, non-zero exit, and continuation past a failure via the sentinel. That is a different thing from attributing the real step, and what it may no longer do is stand in for it. A second test pins that the two renderings are one plan --- the stripped one is the named one minus its names --- so a later edit giving either mode its own text is caught rather than leaving an assertion on a name the runner never uses. It also pins the `name<TAB>command` shape the runner's `IFS=<tab> read` depends on. Both new tests stay on the no-gates paths, so the suite stays cheap. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016bqGA6s9tTUFzYpbeW3tai
This commit is contained in:
parent
48120f2a80
commit
677fd25bcd
59
scripts/gate
59
scripts/gate
|
|
@ -3,14 +3,15 @@
|
||||||
# directory, with isolated ambient roots and durable logs.
|
# directory, with isolated ambient roots and durable logs.
|
||||||
#
|
#
|
||||||
# scripts/gate [--acceptance SUITE]... [--protocol] [--print-plan]
|
# scripts/gate [--acceptance SUITE]... [--protocol] [--print-plan]
|
||||||
|
# scripts/gate [--acceptance SUITE]... [--protocol] --print-plan-named
|
||||||
# scripts/gate --print-target-dir
|
# scripts/gate --print-target-dir
|
||||||
# scripts/gate --init
|
# scripts/gate --init
|
||||||
# scripts/gate --prune [--force]
|
# scripts/gate --prune [--force]
|
||||||
# scripts/gate --self-test
|
# scripts/gate --self-test
|
||||||
#
|
#
|
||||||
# Framing: docs/gate-script-framing.md (revision 4, approved), and
|
# Framing: docs/gate-script-framing.md (revision 4, approved), and
|
||||||
# docs/gate-protocol-build-framing.md (revision 3, approved) for the
|
# docs/gate-protocol-build-framing.md (revision 4, approved) for the
|
||||||
# crdt build step and --self-test.
|
# crdt build step, --self-test and --print-plan-named.
|
||||||
#
|
#
|
||||||
# WHY A PER-WORKTREE TARGET DIRECTORY. This machine exports one
|
# WHY A PER-WORKTREE TARGET DIRECTORY. This machine exports one
|
||||||
# CARGO_TARGET_DIR for every checkout, and cargo takes an EXCLUSIVE LOCK
|
# CARGO_TARGET_DIR for every checkout, and cargo takes an EXCLUSIVE LOCK
|
||||||
|
|
@ -40,6 +41,7 @@ set -eu
|
||||||
usage() {
|
usage() {
|
||||||
cat >&2 <<'EOF'
|
cat >&2 <<'EOF'
|
||||||
usage: scripts/gate [--acceptance SUITE]... [--protocol] [--print-plan]
|
usage: scripts/gate [--acceptance SUITE]... [--protocol] [--print-plan]
|
||||||
|
scripts/gate [--acceptance SUITE]... [--protocol] --print-plan-named
|
||||||
scripts/gate --print-target-dir
|
scripts/gate --print-target-dir
|
||||||
scripts/gate --init
|
scripts/gate --init
|
||||||
scripts/gate --prune [--force]
|
scripts/gate --prune [--force]
|
||||||
|
|
@ -54,7 +56,13 @@ usage: scripts/gate [--acceptance SUITE]... [--protocol] [--print-plan]
|
||||||
--protocol the change touches PROTOCOL_VERSION; adds the CRDT
|
--protocol the change touches PROTOCOL_VERSION; adds the CRDT
|
||||||
workspace sweep on top of the default one, plus
|
workspace sweep on top of the default one, plus
|
||||||
the build that sweep needs (see build-crdt below).
|
the build that sweep needs (see build-crdt below).
|
||||||
--print-plan print the exact gate commands and exit.
|
--print-plan print the exact gate commands and exit. Names are
|
||||||
|
stripped, so every line is runnable as printed.
|
||||||
|
--print-plan-named print the plan as `name<TAB>command` lines and exit
|
||||||
|
--- the same text the runner reads. Exists because
|
||||||
|
a step's NAME is half its contract (a build failure
|
||||||
|
must be attributed to build-crdt, not sweep-crdt)
|
||||||
|
and --print-plan cannot show it.
|
||||||
--print-target-dir print this worktree's build directory and exit.
|
--print-target-dir print this worktree's build directory and exit.
|
||||||
Creates nothing.
|
Creates nothing.
|
||||||
--init create the build directory and ownership marker,
|
--init create the build directory and ownership marker,
|
||||||
|
|
@ -407,6 +415,7 @@ while [ $# -gt 0 ]; do
|
||||||
shift 2 ;;
|
shift 2 ;;
|
||||||
--protocol) PROTOCOL=1; shift ;;
|
--protocol) PROTOCOL=1; shift ;;
|
||||||
--print-plan) MODE=plan; shift ;;
|
--print-plan) MODE=plan; shift ;;
|
||||||
|
--print-plan-named) MODE=plannamed; shift ;;
|
||||||
--print-target-dir) MODE=printdir; shift ;;
|
--print-target-dir) MODE=printdir; shift ;;
|
||||||
--init) MODE=init; shift ;;
|
--init) MODE=init; shift ;;
|
||||||
--prune) MODE=prune; shift ;;
|
--prune) MODE=prune; shift ;;
|
||||||
|
|
@ -417,10 +426,54 @@ while [ $# -gt 0 ]; do
|
||||||
esac
|
esac
|
||||||
done
|
done
|
||||||
|
|
||||||
|
# ---------------------------------------------------------------------
|
||||||
|
# Mode dispatch.
|
||||||
|
#
|
||||||
|
# TWO RENDERINGS OF ONE PLAN, and the second exists because the first
|
||||||
|
# hid a defect. `--print-plan` pipes through `cut -f2-` so every printed
|
||||||
|
# line is a command a reader can copy and run --- and that same cut is
|
||||||
|
# why the emitted NAMES never reached a test. Not cosmetic: the entire
|
||||||
|
# reason build-crdt is a separate step is that a build failure must be
|
||||||
|
# attributed to `build-crdt` rather than to `sweep-crdt`, and with the
|
||||||
|
# names stripped, RENAMING THE REAL BUILD STEP TO `sweep-crdt` left the
|
||||||
|
# ordering assertion green. The witness could not reach the step it
|
||||||
|
# named. --self-test could not either --- it hardcodes the string
|
||||||
|
# `build-crdt` in its own synthetic plan, which proves things about the
|
||||||
|
# RUNNER and nothing about this emitter.
|
||||||
|
#
|
||||||
|
# --print-plan-named prints emit_plan VERBATIM: the same
|
||||||
|
# `name<TAB>command` text the runner reads back from PLAN_FILE, so a
|
||||||
|
# test can assert both halves of a real step together and a rename
|
||||||
|
# cannot pass.
|
||||||
|
#
|
||||||
|
# WHY THIS RATHER THAN THE TWO ALTERNATIVES.
|
||||||
|
#
|
||||||
|
# Injecting PLAN_FILE would let a test hand the runner a plan and read
|
||||||
|
# the names back, and it would turn the runner's `eval` into a general
|
||||||
|
# command executor --- the same class of defect this script's own
|
||||||
|
# review caught in --acceptance and fixed with a parse-time refusal.
|
||||||
|
# Declined there; declined here for the same reason.
|
||||||
|
#
|
||||||
|
# Re-deriving the plan test-side (sourcing this file, or parsing
|
||||||
|
# emit_plan out of it) would be a SECOND implementation of the thing
|
||||||
|
# under test, which is the exact failure being repaired one level up.
|
||||||
|
#
|
||||||
|
# A DISTINCT MODE, not a modifier on --print-plan: there is then no
|
||||||
|
# `--with-names` without `--print-plan` whose behaviour has to be
|
||||||
|
# defined, and --print-plan's contract --- runnable lines --- is left
|
||||||
|
# exactly as it was. Both modes call emit_plan, and so does the runner,
|
||||||
|
# so neither rendering can drift from what actually executes;
|
||||||
|
# tests/gate_script_acceptance.rs pins that the stripped rendering is
|
||||||
|
# the named one minus its names, so this stays true by test and not
|
||||||
|
# only by reading.
|
||||||
|
# ---------------------------------------------------------------------
|
||||||
case $MODE in
|
case $MODE in
|
||||||
plan)
|
plan)
|
||||||
emit_plan | cut -f2-
|
emit_plan | cut -f2-
|
||||||
exit 0 ;;
|
exit 0 ;;
|
||||||
|
plannamed)
|
||||||
|
emit_plan
|
||||||
|
exit 0 ;;
|
||||||
printdir)
|
printdir)
|
||||||
target_dir_for "$(canon "$(worktree_root)")"
|
target_dir_for "$(canon "$(worktree_root)")"
|
||||||
echo
|
echo
|
||||||
|
|
|
||||||
|
|
@ -146,6 +146,13 @@ fn the_crdt_workspace_sweep_is_added_by_protocol_and_absent_without_it() {
|
||||||
/// the right place under the right name and leave the gate exactly as
|
/// the right place under the right name and leave the gate exactly as
|
||||||
/// unsound: the crdt sweep needs *those* features, and the wrong ones
|
/// unsound: the crdt sweep needs *those* features, and the wrong ones
|
||||||
/// produce a binary the sweep cannot use.
|
/// produce a binary the sweep cannot use.
|
||||||
|
///
|
||||||
|
/// **What this test cannot see: the names.** `--print-plan` strips them
|
||||||
|
/// (`emit_plan | cut -f2-`), so everything below is an assertion about
|
||||||
|
/// *commands in an order* — renaming the real build step to `sweep-crdt`
|
||||||
|
/// leaves it green. The step's **name** is asserted by
|
||||||
|
/// `the_crdt_build_step_carries_its_own_name_and_its_exact_command`
|
||||||
|
/// below, which reads the plan in the form the runner reads it.
|
||||||
#[test]
|
#[test]
|
||||||
fn the_crdt_sweep_is_immediately_preceded_by_the_build_that_produces_its_binary() {
|
fn the_crdt_sweep_is_immediately_preceded_by_the_build_that_produces_its_binary() {
|
||||||
let root = tempfile::tempdir().expect("tempdir");
|
let root = tempfile::tempdir().expect("tempdir");
|
||||||
|
|
@ -183,6 +190,139 @@ fn the_crdt_sweep_is_immediately_preceded_by_the_build_that_produces_its_binary(
|
||||||
);
|
);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// **The witness that reaches the step it names**, and the reason this
|
||||||
|
/// lane needed a second round.
|
||||||
|
///
|
||||||
|
/// This lane exists to guarantee two things: that the crdt sweep is
|
||||||
|
/// preceded by the build producing its binary, and that a build failure
|
||||||
|
/// is attributed to **`build-crdt`** rather than to `sweep-crdt`. The
|
||||||
|
/// first round shipped with neither guaranteed, because **neither
|
||||||
|
/// witness could see a name**:
|
||||||
|
///
|
||||||
|
/// - `--print-plan` renders `emit_plan | cut -f2-`, so the ordering test
|
||||||
|
/// above compares commands and never sees the names beside them.
|
||||||
|
/// - `--self-test` hardcodes the string `build-crdt` inside its **own
|
||||||
|
/// synthetic** plan, so it proves things about the *runner* and
|
||||||
|
/// nothing about the real emitter.
|
||||||
|
///
|
||||||
|
/// Review demonstrated the consequence directly: **renaming the real
|
||||||
|
/// build step to `sweep-crdt` left both tests passing** — a plan that
|
||||||
|
/// reports a build failure under the sweep's name, which is exactly the
|
||||||
|
/// misattribution the separate step exists to prevent, sitting green.
|
||||||
|
///
|
||||||
|
/// So the pair is asserted **together, as one emitted line**, against
|
||||||
|
/// `--print-plan-named` — the plan in the form the runner reads it back
|
||||||
|
/// from `PLAN_FILE`. Name and command in the same `assert`, from the
|
||||||
|
/// real emitter, is what makes a rename unable to pass; either half
|
||||||
|
/// alone lets the other drift.
|
||||||
|
///
|
||||||
|
/// The mode is a *rendering*, not a seam: `PLAN_FILE` stays
|
||||||
|
/// uninjectable, because a test that supplied the runner's plan would
|
||||||
|
/// turn its `eval` into a general command executor — the defect the
|
||||||
|
/// `--acceptance` refusal below exists to prevent.
|
||||||
|
#[test]
|
||||||
|
fn the_crdt_build_step_carries_its_own_name_and_its_exact_command() {
|
||||||
|
let root = tempfile::tempdir().expect("tempdir");
|
||||||
|
let build = "build-crdt\tcargo build --workspace --no-default-features --features luajit,crdt";
|
||||||
|
let sweep =
|
||||||
|
"sweep-crdt\tcargo test --workspace --features crdt --no-fail-fast -- --skip basedpyright";
|
||||||
|
|
||||||
|
let (plan, err, ok) = run(root.path(), &["--protocol", "--print-plan-named"]);
|
||||||
|
assert!(
|
||||||
|
ok,
|
||||||
|
"--protocol --print-plan-named must succeed; stderr:\n{err}"
|
||||||
|
);
|
||||||
|
let lines: Vec<&str> = plan.lines().collect();
|
||||||
|
|
||||||
|
// Whole-line equality, not `contains`: the name, the tab, and the
|
||||||
|
// command with nothing appended. A step is its (name, command) pair
|
||||||
|
// and the plan is where both are decided.
|
||||||
|
let b = lines.iter().position(|l| *l == build).unwrap_or_else(|| {
|
||||||
|
panic!(
|
||||||
|
"no plan line is exactly:\n {build}\nA build step under a \
|
||||||
|
different NAME misattributes its own failure; a build step \
|
||||||
|
with different FEATURES hands the sweep a binary it cannot \
|
||||||
|
use. Plan was:\n{plan}"
|
||||||
|
)
|
||||||
|
});
|
||||||
|
// The sweep's own pair, for the same reason in the other direction:
|
||||||
|
// asserting only the build's name lets a rename of the SWEEP slip
|
||||||
|
// through the identical hole.
|
||||||
|
let s = lines
|
||||||
|
.iter()
|
||||||
|
.position(|l| *l == sweep)
|
||||||
|
.unwrap_or_else(|| panic!("no plan line is exactly:\n {sweep}\nPlan was:\n{plan}"));
|
||||||
|
|
||||||
|
assert_eq!(
|
||||||
|
s,
|
||||||
|
b + 1,
|
||||||
|
"the build must be the step IMMEDIATELY before the crdt sweep — a \
|
||||||
|
build merely somewhere earlier could be separated from it by a \
|
||||||
|
step that rewrites the same target directory. Plan was:\n{plan}"
|
||||||
|
);
|
||||||
|
|
||||||
|
// Conditionality, on this rendering too: an ordinary lane must not
|
||||||
|
// carry the step at all, not merely not carry its command.
|
||||||
|
let (default_plan, _, ok) = run(root.path(), &["--print-plan-named"]);
|
||||||
|
assert!(ok, "--print-plan-named must succeed");
|
||||||
|
assert!(
|
||||||
|
!default_plan.contains("build-crdt"),
|
||||||
|
"the default sweep never builds pmacs-gpu and never needs it, so no \
|
||||||
|
ordinary lane may pay for a workspace build; plan was:\n{default_plan}"
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
|
/// **The new rendering must be the same plan, or the assertion above
|
||||||
|
/// pins a string only the test ever reads.**
|
||||||
|
///
|
||||||
|
/// `--print-plan-named` and `--print-plan` both call one emitter, and
|
||||||
|
/// the runner writes that same emitter to `PLAN_FILE` — so today they
|
||||||
|
/// cannot disagree. This pins that from outside, where a later edit
|
||||||
|
/// giving either mode its own plan text would be caught rather than
|
||||||
|
/// producing a witness that asserts a name the runner never uses.
|
||||||
|
///
|
||||||
|
/// It also pins the **shape** the runner depends on: the loop reads each
|
||||||
|
/// line with `IFS=<tab> read -r name cmd`, so a plan line without its
|
||||||
|
/// tab would silently run under an empty command.
|
||||||
|
#[test]
|
||||||
|
fn the_named_plan_is_the_printed_plan_with_its_names_removed() {
|
||||||
|
let root = tempfile::tempdir().expect("tempdir");
|
||||||
|
|
||||||
|
for flags in [
|
||||||
|
vec![],
|
||||||
|
vec!["--protocol"],
|
||||||
|
vec!["--acceptance", "m4_acceptance"],
|
||||||
|
] {
|
||||||
|
let mut named_args = flags.clone();
|
||||||
|
named_args.push("--print-plan-named");
|
||||||
|
let mut plain_args = flags.clone();
|
||||||
|
plain_args.push("--print-plan");
|
||||||
|
|
||||||
|
let (named, err, ok_named) = run(root.path(), &named_args);
|
||||||
|
assert!(ok_named, "{named_args:?} must succeed; stderr:\n{err}");
|
||||||
|
let (plain, err, ok_plain) = run(root.path(), &plain_args);
|
||||||
|
assert!(ok_plain, "{plain_args:?} must succeed; stderr:\n{err}");
|
||||||
|
|
||||||
|
let mut stripped = String::new();
|
||||||
|
for l in named.lines() {
|
||||||
|
let (_name, cmd) = l.split_once('\t').unwrap_or_else(|| {
|
||||||
|
panic!(
|
||||||
|
"every plan line must be `name<TAB>command` — the runner \
|
||||||
|
splits on that tab, so a line without one runs an empty \
|
||||||
|
command under the whole line's name. Line was:\n {l:?}"
|
||||||
|
)
|
||||||
|
});
|
||||||
|
stripped.push_str(cmd);
|
||||||
|
stripped.push('\n');
|
||||||
|
}
|
||||||
|
|
||||||
|
assert_eq!(
|
||||||
|
stripped, plain,
|
||||||
|
"the two renderings must be one plan; with {flags:?} they diverged"
|
||||||
|
);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
/// **Conditionality, settled by measurement rather than by reading** —
|
/// **Conditionality, settled by measurement rather than by reading** —
|
||||||
/// which is the whole methodological point of this lane, since the
|
/// which is the whole methodological point of this lane, since the
|
||||||
/// defect it repairs was a precondition nobody checked.
|
/// defect it repairs was a precondition nobody checked.
|
||||||
|
|
|
||||||
Loading…
Reference in New Issue