self-development: assert byte-equality in stream-forwarding tests

Closed
KE
5mo ago
+3-0

🤖 Kelos Agent @gjkim42

What type of PR is this?

/kind cleanup

What this PR does / why we need it:

Adds one new convention to AGENTS.md (with CLAUDE.md as its symlink) and the inline agentsMD copies in self-development/agentconfig.yaml and self-development/kelos-workers.yaml:

Assert byte-equality, not Contains, in stream-forwarding tests. When a function both forwards bytes from an input to an output and parses metadata along the way (a StreamUsage-style tee/parse loop), assert out.String() == want against the normalized expected bytes, not strings.Contains(out, line+"\n") per input line. A per-line substring check still passes when the forwarder drops a partial line, reorders writes, or duplicates output — exactly the regressions a streaming refactor risks. If the same package already has a stricter byte-equality test (e.g., for unterminated input), reuse its shape across the table-driven cases instead of weakening to Contains.

Motivating review feedback

Both reviewers on PR #1189 (Stream agent output through kelos-capture instead of writing to /tmp) independently flagged the same test-style issue on internal/capture/usage_test.go:169-179:

  • Cubic [P2] at internal/capture/usage_test.go:175: "Forwarding check too weak. Contains can miss dropped/reordered/duplicated lines. Compare normalized full output instead."
  • Kelos Reviewer Agent [P3] at the same line: "assertForwarded only checks strings.Contains(out, line+"\n") for each non-empty input line, so the assertion still passes if the forwarder duplicates, reorders, or interleaves extra output. TestStreamUsageForwardsUnterminatedLine already does the stricter byte-level comparison; the same shape (normalize input to end in \n, then if out.String() != want) could be reused across the table-driven cases for a stronger forwarder invariant."

The same PR also bounced through three rounds of reviewer feedback partly because the weak Contains assertion masked the very regressions the refactor introduced (oversized-line handling, deferred-flush error). Codifying the byte-equality rule up-front would have shortened the review cycle.

The convention complements — but does not duplicate — the existing Avoid vacuous substring assertions in printer/formatter tests rule. That rule covers label: value collisions in CLI-output tests; this one covers byte-fidelity in stream-forwarding tests. Different domains, same underlying failure mode: strings.Contains is too permissive a check.

Scope notes

  • CLAUDE.md is a symlink to AGENTS.md, so a single edit covers both surfaces.
  • The same bullet is added to the inline agentsMD in self-development/agentconfig.yaml and self-development/kelos-workers.yaml, which is the existing pattern for keeping agentsMD in sync with AGENTS.md.
  • No changes to other self-development/*.yaml spawners — they consume the convention list via the shared agentsMD; only the two files that inline that list need editing.
  • No code or behavior change — text-only update to three files.
  • Existing open config-update PRs (#1162, #1214, #1219) each add distinct conventions on different themes; this PR's finding (byte-equality forwarding tests) is not duplicated in any of them.

Which issue(s) this PR is related to:

N/A

Special notes for your reviewer:

N/A

Does this PR introduce a user-facing change?

+ "```" +release-note
NONE
+ "```" +

Summary by cubic

Add a testing convention to assert byte-for-byte equality in stream-forwarding tests instead of per-line strings.Contains checks. Update AGENTS.md and the inline agentsMD in self-development/agentconfig.yaml and self-development/kelos-workers.yaml to avoid tests that miss dropped, reordered, or duplicated output; no code changes.