Uh oh!
There was an error while loading. Please reload this page.
test(installer): one table of cluster states, both twins, identical verdicts (#772) - #773
Conversation
…erdicts (#772) client#766 gave the two installers a shared CONTRACT (envelope_contract.json) and that fixed arithmetic agreement. It did not give them shared CONTROL FLOW, and every divergence since has landed in that gap -- five across backend#2220: 1. node ordering (memory,cpu) in bash vs (cpu,memory) in the CLI/ps1, so the two anchored on DIFFERENT nodes on a heterogeneous cluster 2. [math]::Max(0,..) binding Int32 on ps1, swallowed by a bare catch{} -- machine sizing was silently DEAD on Windows and nothing failed 3. ps1 coerced unparseable allocatable to 0 and RANKED the node; bash skipped it 4. split provenance lookup: on a failed read ps1 said `installer` (invites a ladder to overrule a human) while bash said `unknown` (permanently strands an installer-sized edge). Opposite directions, so neither twin's behavior told you anything about the other's. 5. bash INFERRED "too small" from a second `kubectl get nodes -o name` probe, so unparseable or not-yet-Ready nodes tripped a warning about a machine never measured; ps1 gated on a parsed node. Each was caught one at a time, by review or Bugbot, after the code shipped. That worked, but it is luck-shaped: #2 had been live and silently disabling machine sizing on Windows, and surfaced only because someone happened to write a full-matrix replay. Track A adds more logic to this same path, so the divergence surface is about to grow. The golden vectors cover the arithmetic. What they cannot cover is behavior on everything that is NOT a clean measurement -- a node that will not parse, a values read that fails, a remainder too small to request. Those are exactly the states all five findings lived in, and each twin decided them alone. So: fixtures/installer_parity.json, 18 cluster states, each declaring four verdicts (size, provenance, undersized, unschedulable). installer-parity.bats drives _resolve_training_size; installer-parity.Tests.ps1 drives Get-TrainingResources/Get-TrainingProvenance. One table, two readers -- a row added to the JSON forces BOTH languages to answer it. Both suites stub at the same boundary (the two external commands, kubectl and helm) rather than stubbing the installers' own helpers, so the two sides cannot drift in what they actually exercise -- which would reintroduce the very problem this closes. PowerShell reads the JSON directly; bash reads a generated table, because jq is not a prerequisite -- the same split envelope_vectors.bash already uses. PROVEN to catch the class, in both directions, rather than assumed: * reverted divergence 3 into ps1 only -> ps1 failed on exactly one-unparseable-one-valid and all-nodes-unparseable; bash stayed green * reverted divergence 1 into bash only -> bash failed on exactly heterogeneous-incomparable and node-order-reversed; ps1 stayed green Both installers were then restored; this commit does not touch either. The fixture also declares excluded_from_parity WITH reasons, and Pester asserts those reasons exist. Parity that quietly skips the awkward states is worse than no parity, because it reads as coverage. The one exclusion is "kubectl binary absent": bash gates on its own `has` helper while ps1 infers absence from $LASTEXITCODE, so comparing them would compare two different questions. No CI wiring needed: installer-tests.yaml already runs `Run.Path = scripts/tests` and standard-checks runs `bats scripts/tests/*.bats`, so both halves are discovered, and gen-installer-parity.sh --check rides in as bats test 1. Verified: parity 4/4 both sides, all 18 rows; Pester 761 passed / 0 failed (+3, and the two .Tests.ps1 files coexist); bats 1293 ok with the same 2 pre-existing failures clean develop has (assess.bats:40, install-bootstrap.bats:88 -- confirmed by stashing); bats-hygiene 18/18; make lint clean over 48 files. Purely additive -- 5 new files, no installer touched, no manifest change (dev tooling and tests are not on the signed file surface). Closes#772 Refs: backend#2220, client#766, client#768, backend#664 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
saadqbal
left a comment
There was a problem hiding this comment.
Approving. Green (35 passing), no open threads, purely additive — neither installer touched.
This is the right response to that table of five, and I'd say so even if it caught nothing today. Rows 4 and 5 are findings from review on #768 and #769, rows 1–3 from #764/#766, and the honest observation in the description is the one that matters: "Every one was caught individually, by review or Bugbot, after the code shipped. That worked — but it's luck-shaped." Row 2 is the proof — machine sizing silently dead on Windows, live, surfacing only because someone happened to write a full-matrix replay. A class that has produced five instances and is about to gain more surface deserves a guard rather than a sixth catch.
And it does constrain the installers, which is the only thing that matters. I re-created two of the five in the bash twin and both reddened:
# row 5 — the 'unschedulable' arm made silent
not ok 3 installer parity: every cluster state produces the declared verdict
too-small-to-request: unschedulable want '1' got '0'
# row 4 — failed provenance read yielding 'installer' instead of 'unknown'
not ok 3 installer parity: every cluster state produces the declared verdict
The four fields compared — size, provenance, undersized, unschedulable — are exactly the state variables all five divergences lived in, so the table isn't a generic parity check that happens to overlap; it's aimed.
Three structural things I checked rather than assumed:
- The generated bash table can't drift from the JSON.
gen-installer-parity.sh --checkexists andinstaller-parity.bats:84runs it, so "the fixture and its generated table agree" is a real gate. Without that, the two twins could end up testing different tables — which would be a worse failure than the one being fixed. - The ps1 driver reads the JSON directly (
ConvertFrom-Json,:21-22) rather than the generated form, with the reason stated. So bash tests the derived copy, ps1 tests the source, and--checkties them together. Same trade asgen-envelope-embed.shand the right one. - The failure message carries the cross-twin logic: "The ps1 twin is asserted against the SAME fixture — if only one side fails, the twins have diverged, which is what this file exists to catch." That's what stops someone fixing one half and moving on, which is precisely how row 5 was born.
"The table is not empty" as its own case is the fail-closed half — a guard that silently covers nothing is the shape this repo keeps removing.
One limitation: Pester won't load in my environment, so I exercised the bash half myself and the ps1 half is covered by the two green Pester (ubuntu-latest) and Pester (windows-latest) jobs rather than by me. I also can't read branch protection with this token, so I can't tell you whether those are required contexts — worth knowing if the ps1 side of this guarantee needs to block rather than advise.
Uh oh!
There was an error while loading. Please reload this page.
Closes#772. Purely additive — 5 new files, neither installer touched.
Why
client#766gave the two installers a shared contract (envelope_contract.json). That fixed arithmetic agreement, and it works. It did not give them shared control flow, and all five divergences in backend#2220 landed in that gap:(memory, cpu)(cpu, memory)— different anchor node on a heterogeneous cluster[math]::Max(0,…)boundInt32, threw >2³¹, swallowed by a barecatch {}→ machine sizing silently dead on Windows, nothing failed0and ranked themunknown→ permanently strands an installer-sized edgeinstaller→ invites a ladder to overrule a humankubectl get nodes -o nameprobe → fabricated warnings about unmeasured machinesNote row 4: the halves fail in opposite directions, so neither twin's behaviour told you anything about the other's. And row 5 was introduced by fixing only one twin.
Every one was caught individually, by review or Bugbot, after the code shipped. That worked — but it's luck-shaped. Row 2 had been live, silently disabling machine sizing on Windows, and surfaced only because someone happened to write a full-matrix replay. Track A adds more logic to this same path, so the surface is about to grow.
The golden vectors can't catch this class. They cover arithmetic. What they don't cover is behaviour on everything that isn't a clean measurement — a node that won't parse, a read that fails, a remainder too small to request. That's exactly where all five lived.
What
fixtures/installer_parity.json— 18 cluster states, each declaring four verdicts:size,provenance,undersized,unschedulable.installer-parity.batsdrives_resolve_training_sizeinstaller-parity.Tests.ps1drivesGet-TrainingResources/Get-TrainingProvenanceOne table, two readers. A row added to the JSON forces both languages to answer it.
Two deliberate design choices:
kubectl,helm) — never the installers' own helpers. Stubbing our own helpers would let the suites drift in what they actually exercise, reintroducing the exact problem this closes.envelope_vectors.bashalready uses.Proven to catch the class — both directions
Not assumed. I reverted a real divergence into one twin at a time:
one-unparseable-one-valid+all-nodes-unparseable; bash stayed greenheterogeneous-incomparable+node-order-reversed; ps1 stayed greenBoth installers were then restored —
git diffagainst them is empty in this PR.Exclusions are declared, not silent
The fixture carries
excluded_from_paritywith reasons, and Pester asserts those reasons exist. Parity that quietly skips the awkward states is worse than no parity, because it reads as coverage.The one exclusion: kubectl binary absent. bash gates on its own
hashelper; ps1 infers absence from$LASTEXITCODE. Comparing those would compare two different questions, so it stays covered per-twin instead.No CI wiring needed
installer-tests.yamlalready runsRun.Path = "scripts/tests"andstandard-checks.ymlrunsbats scripts/tests/*.bats, so both halves are auto-discovered.gen-installer-parity.sh --checkrides in as bats test 1, so fixture/table drift fails the normal test job.Verification
.Tests.ps1files coexist fine)develophas (assess.bats:40,install-bootstrap.bats:88) — confirmed by stashingbats-hygiene18/18;make lintclean over 48 filesRefs: backend#2220, client#766, client#768, backend#664
🤖 Generated with Claude Code
Note
Cursor Bugbot is generating a summary for commit 5bd97f6. Configure here.