[huskcass]

Summary

Every unit and envtest case now lives in its own YAML file under a testdata/ directory next to its package. The Go test code keeps only the loaders, runners, fixtures and assertions. The in-Go case tables (guardHoldCases, workloadStateCases, targetGuardCases, the helper tables, the LLMScaler DescribeTable entries, and others) are gone. Kind e2e under test/e2e is unchanged.

internal/testutil/cases is the shared loader. It reads every *.yaml under a directory and decodes each one strictly with sigs.k8s.io/yaml, so an unknown field, a duplicate key or a wrong type fails the run. Files run in relative-path order, and an empty directory is an error. cases.Run starts one t.Run per file and cases.Entries builds one Ginkgo Entry per file.

Layout

internal/testutil/cases/            loader + its own fixtures
internal/controller/autoscaling/testdata/
  guards/{hold,order,workload-state,target}/
  helpers/{defer-timeout,past-defer-timeout,deferral-requeue-after,forced-write-pace,
           stabilization,parse-target-value,query-prometheus-scalar,compute-desired,
           scale-guard,decision-*,recommend-*,...}/
  controller/{metric-scaling,rollout-guard,settle-guard,stabilization-and-step,
              deferral-timeout,deferral-status,forced-write-pacing,
              custom-reconcile,custom-admission,crd-default,custom-defaults}/
internal/controller/serving/testdata/{controller,predicate,repo,values/{merge,hash}}/
internal/chart/testdata/{version/*,repo/<case>/}
internal/helm/testdata/{guard/{namespace,apply},client/<case>/}

The file name is the case name and the directory is the suite or table. Each document has name (optional; if set, it must match the file name), description (the subtest or spec text), inputs and expected.

Adding a case

Copy a nearby file in the right testdata/<suite>/ directory, change description, inputs and expected, then run make test. See "File-based tests" in DEVELOPMENT.md. Quote keys such as n, y, on and off, because YAML 1.1 reads them as booleans.

flowchart LR
  A[testdata/&lt;suite&gt;/&lt;case&gt;.yaml] --> B[cases loader<br/>strict decode, sorted]
  B --> C[runner<br/>t.Run / Ginkgo Entry]
  C --> D[fixtures<br/>envtest, fake Prometheus, fake helm]
  D --> E[assertions]

Loading

Case counts (before → after)

Table	Before	After
autoscaling guards: hold / order / workload-state / target	14 / 3 / 10 / 7	14 / 3 / 10 / 7
helpers: defer-timeout / past-defer / requeue / forced-write / stabilization	3 / 5 / 10 / 5 / 3	same
helpers: parse-target / prometheus-scalar / compute-desired / scale-guard	10 / 4 / 5 / 6	same
custom provider: decision replicas / sample / request / non-200 / misconfigured	13 / 1 / 4 / 1 / 2	same
recommend custom / recommend prometheus / deferral-memory / reason	5 / 1 / 1 / 1	same
LLMScaler envtest: metric / rollout / settle / step / deferral-timeout / deferral-status / pacing	5 / 1 / 4 / 2 / 4 / 3 / 3	same
LLMScaler envtest: decision server / admission / CRD default / custom defaults	2 / 2 / 1 / 1	same
LLMService envtest specs	30	30
serving: predicate / repo / values merge / values hash	6 / 9 / 2 / 1	same
chart: bare / parse / match / resolve / resolve-refuses / repo	2 / 3 / 2 / 6 / 3 / 7	same
helm: namespace / apply / client sequences	10 / 3 / 4	same

The Ginkgo spec texts are unchanged: 28 autoscaling specs and 30 serving specs, though they now run in file order. Cases that used to loop inside a single test function now show up as named subtests. Every subtest name from before is still there.

Coverage

Package	Before	After
internal/chart	86.9%	86.9%
internal/controller/autoscaling	81.8%	81.8%
internal/controller/serving	82.1%	82.1%
internal/helm	46.3%	46.3%
internal/testutil/cases	—	85.9%
total	58.0%	59.5%

make test, go vet and golangci-lint are clean. make manifests generate leaves no diff.

[huskcass]

Blocking: cases.LoadFiles accepts an empty, comment-only, null or multi-document YAML file without an error (see inline). A blank file dropped into four existing suites ran as a passing subtest, so the claim that strict decoding stops a broken case file from passing does not hold yet.

Verdict: request changes

— Review Golden

[huskcass]

UnmarshalStrict only checks what is in the document. It does not check that a document exists, or that there is only one:

• An empty file, a comment-only file, null or a bare --- decodes to the zero T with err == nil. I added zz-empty.yaml (0 bytes) to testdata/helpers/scale-guard, helpers/decision-misconfigured and helpers/stabilization, and a ----only file to helpers/compute-desired. All four ran as passing subtests: scaleGuard(0,0,0)==0; strings.Contains(err, "") is always true; there are no ops; and NaN-style "no metric" matches the zero expectation.
• a: 1\n---\nbogus: 2 decodes only the first document. The second is dropped without any strict check.

Please have the loader reject a file with no document or a null document, and a file with more than one document. For example, decode with a yaml.NewYAMLOrJSONDecoder loop and require exactly one non-null object. A testdata/ fixture for each case in cases_test.go would keep it covered.

[huskcass]

Fixed in 6391612 (new commit on top of 03f5a70).

Blocking: loader

• A case file must hold exactly one non-null YAML mapping. Each of these now fails the load, and the error names the file:
  • empty, comment-only, null/~, or a lone ---: no document / document is null
  • a non-mapping document: document must be a mapping
  • more than one document: N documents, want 1
• cases_test.go covers each case with fixtures written to t.TempDir(), including a multi-doc file whose second document has an unknown field.
• I checked it by hand: dropping a 0-byte file into helpers/scale-guard, --- into helpers/compute-desired, a comment-only file into helpers/decision-misconfigured, and a two-doc file into helpers/stabilization makes all four tests fail with those errors.

Lows

• .yml files fail the load with "rename it to .yaml". Only .yaml is a case. This is documented in the package doc and in DEVELOPMENT.md.
• Ginkgo flags: Entries moved to internal/testutil/cases/ginkgocases. The core cases package no longer imports Ginkgo, so go test ./internal/chart -args -ginkgo.v is an unknown flag again.
• Key case: keys must match the json name exactly. After the strict decode, keys.go walks the generic tree against the target type. Self-decoding types such as Duration, Quantity and IntOrString are treated as leaves.
• Durations: the docs now match behavior. Only a pointer duration may be omitted or null; a non-pointer null fails, and there's a test for that.
• Omitted fields:
  • Decision-server cases need status; the existing files now say status: 200.
  • Error cases need errorContains.
  • Chart repo cases need allowed (an empty list means allow all) or nilAllowlist: true.
  • An empty entry in any substring list (contains, messageContains, events, notContains, errorNotContains) fails the case and names the field.
• Conditions: message and observedGeneration are asserted when set.
• Dead schema: capture: generation and update.labels are removed; no case file used them.

Checks

• make test, go vet and golangci-lint are clean. make manifests generate gives no diff.
• -race passes, and -ginkgo.randomize-all passes with several seeds.
• There are still 235 case files, and spec and subtest names are unchanged.
• Coverage is unchanged for chart, autoscaling, serving and helm. The loader is at 87.7%, and ginkgocases is at 90.9%.
• CI is green.

[huskcass]

Verdict: approve

— Review Golden