MINOR: Split GroupMetadataManagerTest into per-group-type files#22350
Open
sjhajharia wants to merge 2 commits into
Open
MINOR: Split GroupMetadataManagerTest into per-group-type files#22350sjhajharia wants to merge 2 commits into
sjhajharia wants to merge 2 commits into
Conversation
Resolves conflict in GroupMetadataManagerTest.java caused by trunk commit f7dbf0b (KAFKA-20570) adding two tests for malformed ConsumerProtocol deserialization. Both new tests exercise the classic->consumer migration code path (testConsumerGroupHeartbeatWithStableClassicGroupFailsOnMalformedProtocol and testClassicGroupJoinToConsumerGroupFailsOnMalformedSubscriptionMetadata), so per the split's assignment rule they are placed in GroupMetadataManagerConsumerGroupTest.java alongside the other testClassicGroup*ToConsumerGroup* migration tests, not in the residual GroupMetadataManagerTest.java. Verified: ./gradlew :group-coordinator:test --rerun-tasks - 1,758 tests, 0 failures, 0 errors, 0 skipped - Both newly relocated tests pass
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.
Description:
GroupMetadataManagerTest.javahad grown to 27,879 lines / 406@Testmethods, testing four distinct group types (Classic,Consumer/KIP-848, Share/KIP-932, Streams/KIP-1071) plus cross-cutting
concerns in a single file. This made the file slow to navigate, prone to
merge conflicts between teams, and left contributors with no clear rule
for where new tests should land.
What changed
The file is split, by group type, into five files in the same package:
GroupMetadataManagerTest(residual) | 39 tests |Group-type-agnostic: list/describe, metadata-image plumbing, dynamic
config wiring, cross-type rejection
GroupMetadataManagerConsumerGroupTest| 129 tests | KIP-848consumer-group heartbeat/reconciliation/regex/replay, classic↔consumer
migration paths
GroupMetadataManagerClassicGroupTest| 130 tests | Classic protocoljoin/sync/heartbeat/leave, static membership, rebalance/session timeouts
GroupMetadataManagerShareGroupTest| 41 tests | KIP-932 share-groupheartbeat, init/delete, offset deletion, replay
GroupMetadataManagerStreamsGroupTest| 69 tests | KIP-1071streams-group heartbeat, topology, task assignment, replay
Each new class has a Javadoc stating its scope so future contributors
can pick a destination by reading one sentence: a test goes into the
file matching the group type whose operation it exercises; genuinely
cross-cutting tests go in the residual
GroupMetadataManagerTest.The shared
GroupMetadataManagerTestContextis unchanged. No productioncode is changed. No tests are renamed, deleted, or modified — every
@Testis moved byte-identically into its new home. Helpers that wereonly used by a single bucket move with their callers; nothing required
promotion to
TestContext(verified via call-site analysis).checkstyle/suppressions.xmlwas updated to add the four new file namesalongside the existing
GroupMetadataManagerTestentry (the new filesinherit the same
NPathComplexity,MethodLength,ClassFanOutComplexity,ClassDataAbstractionCoupling,JavaNCSSsuppressions the original already had).