Skip to content

MINOR: Remove unstable.api.version.enable config and rename corresponding test from StreamsGroupHeartbeatRequestTest #21132

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
chia7712 merged 5 commits into from
Dec 21, 2025
+2 −7
Merged

MINOR: Remove unstable.api.version.enable config and rename corresponding test from StreamsGroupHeartbeatRequestTest #21132

Changes from 2 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
ea0592c
remove disable unstable.api.versions test
lucliu1108 Dec 11, 2025
9ee392d
remove unstable.api.versions config
lucliu1108 Dec 11, 2025
79913ec
rename test
lucliu1108 Dec 19, 2025
ebe57ab
Merge branch 'trunk' into streamsgroup-heartbeat-test-cleanup
lucliu1108 Dec 19, 2025
327e723
align spacing
lucliu1108 Dec 19, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 0 additions & 26 deletions core/src/test/scala/unit/kafka/server/StreamsGroupHeartbeatRequestTest.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,6 @@ import scala.jdk.CollectionConverters._
serverProperties = Array(
new ClusterConfigProperty(key = GroupCoordinatorConfig.OFFSETS_TOPIC_PARTITIONS_CONFIG, value = "1"),
new ClusterConfigProperty(key = GroupCoordinatorConfig.OFFSETS_TOPIC_REPLICATION_FACTOR_CONFIG, value = "1"),
new ClusterConfigProperty(key = "unstable.api.versions.enable", value = "true"),
new ClusterConfigProperty(key = "group.coordinator.rebalance.protocols", value = "classic,consumer,streams"),
new ClusterConfigProperty(key = "group.streams.initial.rebalance.delay.ms", value = "0")
Copy link

Copilot AI Dec 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The removal of unstable.api.versions.enable configuration from this test class may be premature. This configuration is still actively used in other test files including:

  • ApiVersionsRequestTest.scala (lines 32, 42, 72)
  • SaslApiVersionsRequestTest.scala (lines 45, 85)
  • DescribeFeaturesTest.java (lines 49, 54)
  • FeatureCommandTest.java (lines 134, 139)

The configuration constant is also still defined in ServerConfigs.java (line 115: UNSTABLE_API_VERSIONS_ENABLE_CONFIG).

Unless there's a specific reason why StreamsGroupHeartbeatRequestTest doesn't need this configuration while other API version tests do, this removal should be reconsidered or accompanied by a broader cleanup across all test files.

Suggested change
new ClusterConfigProperty(key = "group.streams.initial.rebalance.delay.ms", value = "0")
new ClusterConfigProperty(key = "group.streams.initial.rebalance.delay.ms", value = "0"),
new ClusterConfigProperty(key = "unstable.api.versions.enable", value = "true")

Copilot uses AI. Check for mistakes.
)
Expand Down Expand Up @@ -111,31 +110,6 @@ class StreamsGroupHeartbeatRequestTest(cluster: ClusterInstance) extends GroupCo
assertEquals(expectedResponse, streamsGroupHeartbeatResponse)
}

@ClusterTest(
serverProperties = Array(
new ClusterConfigProperty(key = GroupCoordinatorConfig.GROUP_COORDINATOR_REBALANCE_PROTOCOLS_CONFIG, value = "classic,consumer,streams"),
)
)
def testStreamsGroupHeartbeatIsInaccessibleWhenUnstableLatestVersionNotEnabled(): Unit = {
val topology = new StreamsGroupHeartbeatRequestData.Topology()
.setEpoch(1)
.setSubtopologies(List().asJava)

val streamsGroupHeartbeatResponse = streamsGroupHeartbeat(
groupId = "test-group",
memberId = "test-member",
rebalanceTimeoutMs = 1000,
activeTasks = List.empty,
standbyTasks = List.empty,
warmupTasks = List.empty,
topology = topology,
expectedError = Errors.NOT_COORDINATOR
Copy link
Member

@chia7712 chia7712 Dec 13, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This error appears to be caused by the non-existence of the offset topic. Perhaps we can keep the test, but rename it to reflect this specific scenario

Copy link
Member

@lucasbru lucasbru Dec 15, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could do that as well I guess

Copy link
Contributor Author

@lucliu1108 lucliu1108 Dec 19, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks both! I changed the test name to reflect the scenario.

)

val expectedResponse = new StreamsGroupHeartbeatResponseData().setErrorCode(Errors.NOT_COORDINATOR.code())
assertEquals(expectedResponse, streamsGroupHeartbeatResponse)
}

@ClusterTest
Copy link

Copilot AI Dec 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The removed test testStreamsGroupHeartbeatIsInaccessibleWhenUnstableLatestVersionNotEnabled was expecting Errors.NOT_COORDINATOR as the error code when the unstable API version is disabled. However, the other similar tests in this file that check for API inaccessibility (e.g., testStreamsGroupHeartbeatIsInaccessibleWhenDisabledByFeatureConfig and testStreamsGroupHeartbeatIsInaccessibleWhenDisabledByStaticGroupCoordinatorProtocolConfig) expect Errors.UNSUPPORTED_VERSION.

The mismatch in expected error codes suggests that either:

  1. The removed test had an incorrect expected error code and was not properly testing the intended behavior, or
  2. There was a distinct scenario being tested that is no longer covered after this removal

Please verify that removing this test doesn't eliminate coverage for a specific edge case or that the test was indeed incorrect and redundant.

Copilot uses AI. Check for mistakes.
def testStreamsGroupHeartbeatIsAccessibleWhenNewGroupCoordinatorIsEnabledTopicNotExistFirst(): Unit = {
val admin = cluster.admin()
Expand Down