Context
Surfaced by Copilot review on PR #21 (the list-valued override-directive fix). Two new tests in that PR followed an existing project pattern; the review flagged the new instances. Deferred from #21 for cohesion — bundling the cleanup would have grown the PR past its primary concern.
Problem
Several tests in internal/overrides/overrides_test.go discard the error return from os.ReadFile(path) via the data, _ := os.ReadFile(path) pattern. When the read fails for any reason (filesystem state, permission issue, race in a parallel test environment), data is nil, string(data) is empty, and the subsequent strings.Contains or line-equality assertions all fail with diagnostics pointing at the assertion expectation. The actual root cause — that the test could not read the file at all — is invisible. A future engineer investigating the failure will look at the assertion logic and the override writer first, not at filesystem state.
Current instances:
TestWriteIncludesHeader — internal/overrides/overrides_test.go:67TestWriteSortsKeys — internal/overrides/overrides_test.go:88TestWriteEmitsClearingLineForListValuedKeys — internal/overrides/overrides_test.go:136TestWriteEmitsClearingLineForEachListValuedKey — internal/overrides/overrides_test.go:172
Suggested approach
Replace each data, _ := os.ReadFile(path) with a checked read that calls t.Fatalf on error, matching the style already used for the corresponding Write errors elsewhere in the same tests. Scope the change to internal/overrides/overrides_test.go; the same pattern may exist in other test files but is out of scope here — file separately if found and discrepant.
Context
Surfaced by Copilot review on PR #21 (the list-valued override-directive fix). Two new tests in that PR followed an existing project pattern; the review flagged the new instances. Deferred from #21 for cohesion — bundling the cleanup would have grown the PR past its primary concern.
Problem
Several tests in
internal/overrides/overrides_test.godiscard the error return fromos.ReadFile(path)via thedata, _ := os.ReadFile(path)pattern. When the read fails for any reason (filesystem state, permission issue, race in a parallel test environment),datais nil,string(data)is empty, and the subsequentstrings.Containsor line-equality assertions all fail with diagnostics pointing at the assertion expectation. The actual root cause — that the test could not read the file at all — is invisible. A future engineer investigating the failure will look at the assertion logic and the override writer first, not at filesystem state.Current instances:
TestWriteIncludesHeader—internal/overrides/overrides_test.go:67TestWriteSortsKeys—internal/overrides/overrides_test.go:88TestWriteEmitsClearingLineForListValuedKeys—internal/overrides/overrides_test.go:136TestWriteEmitsClearingLineForEachListValuedKey—internal/overrides/overrides_test.go:172Suggested approach
Replace each
data, _ := os.ReadFile(path)with a checked read that callst.Fatalfon error, matching the style already used for the correspondingWriteerrors elsewhere in the same tests. Scope the change tointernal/overrides/overrides_test.go; the same pattern may exist in other test files but is out of scope here — file separately if found and discrepant.