junrao commented on code in PR #16893:
URL: https://github.com/apache/kafka/pull/16893#discussion_r1718781094


##########
core/src/test/scala/unit/kafka/server/ListOffsetsRequestTest.scala:
##########
@@ -35,11 +36,18 @@ class ListOffsetsRequestTest extends BaseRequestTest {
 
   val topic = "topic"
   val partition = new TopicPartition(topic, 0)
+  var enableUnstableVersions = true
+
+  @BeforeEach
+  override def setUp(testInfo: TestInfo): Unit = {
+    enableUnstableVersions = 
!testInfo.getDisplayName.contains("testResponseIncludesLeaderEpochWithUnstableAPIs")

Review Comment:
   If the test name contains "UnstableAPI", we set `enableUnstableVersions` to 
false. Setting it to true will be more intuitive.



##########
core/src/test/scala/unit/kafka/server/ListOffsetsRequestTest.scala:
##########
@@ -202,7 +210,17 @@ class ListOffsetsRequestTest extends BaseRequestTest {
 
   @ParameterizedTest
   @ValueSource(strings = Array("zk", "kraft"))
-  def testResponseIncludesLeaderEpoch(quorum: String): Unit = {
+  def testResponseIncludesLeaderEpochWithUnstableAPIs(quorum: String): Unit = {
+    testResponseIncludesLeaderEpoch()
+  }
+
+  @ParameterizedTest
+  @ValueSource(strings = Array("zk", "kraft"))
+  def testResponseIncludesLeaderEpochWithStableAPIs(quorum: String): Unit = {
+    testResponseIncludesLeaderEpoch()
+  }
+  
+  def testResponseIncludesLeaderEpoch(): Unit = {

Review Comment:
   I am not sure if this truly captures the problem. The test seems to verify 
that the broker can accept the latest version of ListOffset when 
enableUnstableVersions is set to false. This means that we could never 
introduce an unstable version of ListOffset in the future. When introducing a 
new version of ListOffset, it's ok to make it unstable, as long as the stable 
MV doesn't use it for inter broker communication.



-- 
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]

Reply via email to