Skip to content

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

Closed
kelos-bot[bot] wants to merge 1 commit into
mainfrom
kelos-config-update-20260527-1800
Closed

self-development: assert byte-equality in stream-forwarding tests#1223
kelos-bot[bot] wants to merge 1 commit into
mainfrom
kelos-config-update-20260527-1800

Conversation

@kelos-bot

@kelos-bot kelos-bot Bot commented May 27, 2026

Copy link
Copy Markdown
Contributor

🤖 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

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.

Written for commit 873004b. Summary will update on new commits. Review in cubic

Adds a new convention to the agent rule list in AGENTS.md (with CLAUDE.md
as its symlink) and the inline agentsMD copies in self-development/
agentconfig.yaml and kelos-workers.yaml: when a function both forwards
bytes from input to output and parses metadata along the way, tests must
assert byte-for-byte equality (out.String() == want) instead of
per-line strings.Contains. Substring checks still pass when the
forwarder drops a partial line, reorders writes, or duplicates output.

Motivated by repeated reviewer feedback on PR #1189, where both Cubic
[P2] and the Kelos reviewer agent [P3] independently flagged
assertForwarded in internal/capture/usage_test.go for using
strings.Contains(out, line+"\n") per input line and missing the
drop/reorder/duplicate failure modes that the streaming refactor
introduced. Three rounds of feedback ended with the test being rewritten
to byte-equality against the normalized input.
@github-actions github-actions Bot added needs-triage needs-kind Indicates an issue or PR lacks a kind/* label needs-priority kind/cleanup needs-actor needs-release-note Indicates a PR lacks a release-note block and removed needs-kind Indicates an issue or PR lacks a kind/* label labels May 27, 2026
@gjkim42 gjkim42 closed this May 30, 2026
@gjkim42 gjkim42 deleted the kelos-config-update-20260527-1800 branch May 30, 2026 01:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant