Problem
Files in approvals/ are named kir_test.<TestFunc>.<Scenario>.<stream>.approved.txt (and .input.yaml), e.g. kir_test.TestKind.Pod.stdout.approved.txt. Opening the directory, the eye is drawn to the repeated kir_test.TestKind. Go-test plumbing rather than to the thing that actually varies — the scenario (Pod, Service, WorkloadAndService, …).
Two things are tangled here and worth separating:
- Golden names (
*.approved.txt / *.received.txt) — these are derived by go-approval-tests from t.Name() + the source filename (see stack_trace_namer.go: name = strings.ReplaceAll(t.Name(), "/", ".")). We don't (and shouldn't) hardcode these.
- Input fixtures (
*.input.yaml) — these are ours; we currently spell out the full name at each call site.
Lever A — call sites express only the scenario (code readability)
Make the scenario the subtest label (the library needs a unique t.Name() per case anyway) and derive the rest inside the shared helper:
func verify(t *testing.T) {
file := "kir_test." + strings.ReplaceAll(t.Name(), "/", ".") + ".input.yaml"
// stdout/stderr as today
}
func TestKind(t *testing.T) {
for _, kind := range []string{"Pod", "CronJob" /* … */} {
t.Run(kind, func(t *testing.T) { verify(t) })
}
}
- Pro:
a.b (kir_test.TestKind) and d.e (input.yaml) live once in verify; call sites read as pure scenario.
- Constraints: only works because each case is a subtest (else goldens collide on
TestKind); a descriptive fixture like TestMixed.WorkloadAndService must become the subtest label (t.Run("WorkloadAndService", …)).
- Note: this cleans up the code but the on-disk names still read
kir_test.TestKind.Pod.….
Lever B — files on disk lead with the scenario (directory readability)
The kir_test.<TestFunc>. stem is the library's default namer. To change what lands on disk, opt out of it via the library's hooks (UseFolder, templatedCustomNamer in v1.5.0). Then "how do we name things" is the real question:
- Flat, scenario-only:
approvals/pod.stdout.txt, service-skipped.stderr.txt. Most readable, but scenario names must be globally unique across all tests.
- Per-group folders (preferred):
approvals/kinds/pod.stdout.txt, approvals/errors/service.stderr.txt, approvals/multi-doc/workload-and-service.stdout.txt. Uniqueness is per-folder, browsing shows scenarios grouped by intent, and the folder still threads back to where the test lives.
- Cheapest: keep default names but mark
*.approved.txt as generated (.gitattributes / a short approvals/README) so review UIs de-emphasize them. No code change.
Caveat
The kir_test.<TestFunc> prefix isn't pure noise — it's the thread from a golden back to the test that produced it. Flat scenario-only names lose that; per-group folders keep it while still leading with the scenario.
Suggested path
Ship Lever A as a small standalone change first (improves the code, no rename churn), then consider Lever B (per-group folders) as its own dedicated PR once the current stack has landed, so the mass-rename doesn't tangle with behavior diffs.
Context
Came out of review discussion on the stdout/stderr approval work (#59 and the stacks built on it). Parked deliberately to avoid churning those PRs.
Problem
Files in
approvals/are namedkir_test.<TestFunc>.<Scenario>.<stream>.approved.txt(and.input.yaml), e.g.kir_test.TestKind.Pod.stdout.approved.txt. Opening the directory, the eye is drawn to the repeatedkir_test.TestKind.Go-test plumbing rather than to the thing that actually varies — the scenario (Pod,Service,WorkloadAndService, …).Two things are tangled here and worth separating:
*.approved.txt/*.received.txt) — these are derived bygo-approval-testsfromt.Name()+ the source filename (seestack_trace_namer.go:name = strings.ReplaceAll(t.Name(), "/", ".")). We don't (and shouldn't) hardcode these.*.input.yaml) — these are ours; we currently spell out the full name at each call site.Lever A — call sites express only the scenario (code readability)
Make the scenario the subtest label (the library needs a unique
t.Name()per case anyway) and derive the rest inside the shared helper:a.b(kir_test.TestKind) andd.e(input.yaml) live once inverify; call sites read as pure scenario.TestKind); a descriptive fixture likeTestMixed.WorkloadAndServicemust become the subtest label (t.Run("WorkloadAndService", …)).kir_test.TestKind.Pod.….Lever B — files on disk lead with the scenario (directory readability)
The
kir_test.<TestFunc>.stem is the library's default namer. To change what lands on disk, opt out of it via the library's hooks (UseFolder,templatedCustomNamerin v1.5.0). Then "how do we name things" is the real question:approvals/pod.stdout.txt,service-skipped.stderr.txt. Most readable, but scenario names must be globally unique across all tests.approvals/kinds/pod.stdout.txt,approvals/errors/service.stderr.txt,approvals/multi-doc/workload-and-service.stdout.txt. Uniqueness is per-folder, browsing shows scenarios grouped by intent, and the folder still threads back to where the test lives.*.approved.txtas generated (.gitattributes/ a shortapprovals/README) so review UIs de-emphasize them. No code change.Caveat
The
kir_test.<TestFunc>prefix isn't pure noise — it's the thread from a golden back to the test that produced it. Flat scenario-only names lose that; per-group folders keep it while still leading with the scenario.Suggested path
Ship Lever A as a small standalone change first (improves the code, no rename churn), then consider Lever B (per-group folders) as its own dedicated PR once the current stack has landed, so the mass-rename doesn't tangle with behavior diffs.
Context
Came out of review discussion on the stdout/stderr approval work (#59 and the stacks built on it). Parked deliberately to avoid churning those PRs.