Kunit#30330
Draft
dotnwat wants to merge 6 commits intoredpanda-data:devfrom
Draft
Conversation
Signed-off-by: Noah Watkins <noahwatkins@gmail.com>
Move describe_log_dirs.cc helpers into a new kafka::describe_log_dirs::detail namespace and forward-declare describe_partition (and its log_partition_data return type) in the handler header, so a unit test can call it directly. Adds a fake partition_proxy::impl that returns canned values for the four methods describe_partition reads (ntp, local_size_bytes, offset_lag, cloud_size_bytes) and throws on any other call. The test covers the four cases produced by the include_remote flag crossed with cloud_size_bytes presence, plus offset_lag clamping. This is a starting point for incrementally making more of the kafka server handler surface unit-testable without spinning up a redpanda fixture. The detail namespace pattern is meant to scale across handlers without naming collisions between their helpers. Signed-off-by: Noah Watkins <noahwatkins@gmail.com> Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
collect() previously rebuilt the request's chunked_vector filter into a std::vector so that the map_reduce0 lambda capture was copyable across shards. The conversion only existed because chunked_vector isn't copyable; the filter itself is logically read-only and shared. Pass a const chunked_vector* through collect() and collect_mapper() instead. The filter is owned by the awaiting handle() coroutine, which outlives the map_reduce0, so borrowing across shards is safe for read-only access. Drops the per-request allocation and move loop. Also collapses the optional<container> + null-check pattern into a plain nullable pointer at the boundary, matching the "no filter" sentinel naturally. Signed-off-by: Noah Watkins <noahwatkins@gmail.com> Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Pulls the map_reduce0 reducer in collect() out of an inline lambda into a named free function, merge_partition_dir_sets, exposed via the handler header so it can be tested directly. Behavior is unchanged: update's per-topic partitions are appended onto acc[topic], with new topics inserted as needed. Adds tests for the empty/empty, empty-acc, empty-update, disjoint, and overlapping-topic cases. The overlapping case pins the acc-then-update ordering so future shard reorderings don't silently change the produced partition list order. Also moves the partition_dir_set type alias from the .cc into the detail namespace in the header so tests can construct inputs. Signed-off-by: Noah Watkins <noahwatkins@gmail.com> Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Adds an abstract partition_proxy_source interface with two operations handlers actually need on a single shard: enumerate ktps, and look up a partition_proxy by ktp. The production implementation, partition_manager_proxy_source, wraps cluster::partition_manager and forwards get() to make_partition_proxy. Migrates describe_log_dirs's collect_mapper to take a partition_proxy_source& instead of a cluster::partition_manager&. collect()'s lambda constructs the production source on each shard inside the map_reduce0 fan-out, so behavior is unchanged. Also drops the request_context& parameter from collect() in favor of taking ss::sharded<cluster::partition_manager>& directly, narrowing the function's surface to what it actually uses. The seam exists so handler internals can be unit tested with fakes that don't require a real partition_manager. No tests added in this commit; the next commit extracts the existing fake and adds collect_mapper tests. Other handlers that today call make_partition_proxy(ktp, pm) directly (fetch, list_offsets, delete_records, offset_for_leader_epoch, produce) can migrate to this abstraction over time. Signed-off-by: Noah Watkins <noahwatkins@gmail.com> Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Moves fake_partition_proxy_impl out of describe_log_dirs_test.cc into
a new shared test header (kafka/data/tests/fake_partition_proxy_impl.h)
under namespace tests, alongside a new fake_partition_proxy_source
that produces fresh fake impls on each get(). Both fakes are exposed
through a kafka/data/tests:fake_partition_proxy test_cc_library so
future handler tests can reuse them.
Exposes collect_mapper through the handler header (drops static) and
adds tests covering its branching:
- null filter enumerates every partition the source advertises,
grouped by topic
- empty filter list returns no results
- non-empty filter returns only the requested subset
- filter entries naming a topic the source doesn't know about are
silently dropped (preserving existing behavior)
- filter entries naming a missing partition_id are silently dropped
- the include_remote flag is threaded through to describe_partition
Combined with the existing describe_partition and merge_partition_dir_sets
tests, every internal piece of describe_log_dirs is now unit-testable
without spinning up a redpanda fixture; only the map_reduce0 fan-out
in collect() relies on the seastar runtime.
Signed-off-by: Noah Watkins <noahwatkins@gmail.com>
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Backports Required
Release Notes