aliehsaeedii opened a new pull request, #22967:
URL: https://github.com/apache/kafka/pull/22967
## Summary
Test-only. Adds direct coverage for the changelog encoding introduced in
#22961, and fills a
pre-existing gap in `ListValueStore` coverage. No production code changes.
> ⚠️ **Stacked on #22961.** Until that merges, the diff shown here against
`trunk` also contains
> #22961's changes. The changes belonging to *this* PR are only the 3 test
files listed below.
>
> This PR must merge **after** #22961: `ListValueStoreUpgradeUtilsTest`
calls the split/join
> methods added there.
## Why this is a separate PR
#22961 deliberately kept itself reviewable as a self-contained format
change, which meant
`ListValueStoreUpgradeUtils` — its largest file — landed with only
transitive coverage through its
two callers, and no negative or byte-level tests. This PR closes that gap,
so it is worth merging
promptly after #22961 rather than letting it drift.
## Changes
- **`ListValueStoreUpgradeUtilsTest`** — direct coverage of the encoding:
- the PLAIN → HEADERS blob conversion, including the right-side value that
used to be silently
truncated (`right` → `ight`) and the left-side value that used to throw
`SerializationException: negative length -1`;
- the `split`/`join` pair, asserting the split output is byte-identical to
what a PLAIN store
would have written — the property that makes downgrade safe;
- the exact wire layout: the **zigzag** `headersSize` varint, and one
`0x00` per element with no
headers;
- duplicate header keys across different list elements (safe because
record headers are a list,
not a map);
- tombstones, empty lists, and absent control headers (the legacy case,
which must be identical
to all-empty prefixes);
- independence from record-header *ordering* —
`ProcessorContextImpl#logChange` appends the
consistency vector clock to the same `Headers` instance the store passes
to `log()`, so the
encoding must not depend on position;
- the error paths: too few prefixes for the element count, trailing prefix
bytes, and a
`headersSize` longer than the element. There were no `assertThrows`
anywhere in the encoding's
coverage before this.
- **`ListValueStoreTest`** — parametrize the existing list/iterator
invariants (ordering,
tombstones, delete-during-iterate, closed-iterator semantics) over both
plain and
headers-embedding value serdes.
- **`ListValueStoreWithHeadersTest`** — assert per-record headers and
timestamps survive a
round-trip through the builder → metered → change-logger →
`ListValueStore` pipeline, including
duplicate keys with different headers and the empty-headers case.
## Testing
42 tests, all passing. Checkstyle passes.
## Relationship to the other PRs
| PR | scope | changes behaviour? |
|---|---|---|
| **#22961** | storage + changelog format | no — nothing constructs it |
| **#22966** | DSL wiring + non-joined emit | yes — the switch |
| **this one** | tests only | no |
Independent of #22966 — the two can be reviewed in parallel and merged in
either order once
#22961 lands.
## Committer Checklist (excluded from commit message)
- [ ] Verify design and implementation
- [ ] Verify test coverage and CI build status
- [ ] Verify documentation (including upgrade notes)
Reviewers: Matthias J. Sax <[email protected]>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]