Skip to content

Commit d82dedc

Browse files
committed
docs(cli): correct why the single-run-ID invariant is untested
The WARNING on `runID := v1.GenerateRunID()` and the matching test comment claimed the Action's two consumer sites "sit on mutually exclusive code paths". They do not: with neither --snapshot nor --no-cluster, the live-capture branch and runValidation both consume the same id in one invocation — which is precisely the case the invariant protects. What blocks a unit test is live-cluster I/O with no injectable seam in the Action. This comment is the only protection for an untested invariant, so a wrong mental model in it is worse than no comment. Signed-off-by: Alex Yuskauskas <ayuskauskas@nvidia.com>
1 parent a6e0d41 commit d82dedc

2 files changed

Lines changed: 13 additions & 10 deletions

File tree

pkg/cli/validate.go

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -859,12 +859,14 @@ constraint (e.g. K8s version) is not met — --fail-on-error scopes to phase che
859859
// parseValidateAgentConfig call below, or for the
860860
// validationConfig{runID: ...} literal further down — silently
861861
// splits a single `aicr validate` invocation back into two
862-
// uncorrelated runs. The command's two consumer sites
863-
// (live-capture agent vs. validator Jobs) sit on mutually
864-
// exclusive code paths (--no-cluster requires --snapshot, which
865-
// skips the live-capture branch entirely), so no unit test can
866-
// observe both consumers receiving the SAME value in one
867-
// invocation without a live cluster; see
862+
// uncorrelated runs. The two consumer sites are NOT mutually
863+
// exclusive: with neither --snapshot nor --no-cluster, the
864+
// live-capture branch below and runValidation both consume this
865+
// id in the same invocation — which is exactly the case the
866+
// invariant protects. What blocks a unit test is that reaching
867+
// both requires live-cluster I/O (deployAgentForValidation
868+
// deploys a Job; runValidation deploys validator Jobs) with no
869+
// injectable seam here to fake it. See
868870
// TestValidateAgentConfig_ToAgentConfig_ForwardsRunID and
869871
// TestParseValidateAgentConfig_ForwardsCallerRunID in
870872
// validate_test.go for what IS covered (passthrough at each

pkg/cli/validate_test.go

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -513,10 +513,11 @@ func TestValidateAgentConfig_ToAgentConfig_ForwardsRunID(t *testing.T) {
513513
// single-generation invariant (one v1.GenerateRunID() call feeding BOTH
514514
// the live-capture agent and the validator Jobs). See the WARNING comment
515515
// on that call site in validate.go: no automated test in this package
516-
// enforces single-generation, because the Action's two consumer sites sit
517-
// on mutually exclusive code paths (--no-cluster requires --snapshot,
518-
// which skips the live-capture branch entirely) and cannot both be
519-
// exercised in one invocation without a live cluster.
516+
// enforces single-generation. The two consumer sites are NOT mutually
517+
// exclusive — with neither --snapshot nor --no-cluster, the live-capture
518+
// branch and runValidation both consume the id in one invocation. What
519+
// blocks the test is that exercising both requires live-cluster I/O (both
520+
// branches deploy Jobs) with no injectable seam in the Action to fake it.
520521
func TestParseValidateAgentConfig_ForwardsCallerRunID(t *testing.T) {
521522
const wantRunID = "20260821-142233-9f3a1c0b7e2d4a55"
522523

0 commit comments

Comments
 (0)