lianetm commented on code in PR #18062:
URL: https://github.com/apache/kafka/pull/18062#discussion_r1872242765


##########
clients/src/test/java/org/apache/kafka/clients/consumer/internals/ConsumerHeartbeatRequestManagerTest.java:
##########
@@ -602,30 +605,21 @@ public void testHeartbeatResponseOnErrorHandling(final 
Errors error, final boole
         }
     }
 
-    @Test
-    public void testUnsupportedVersion() {
-        mockErrorResponse(Errors.UNSUPPORTED_VERSION, null);
+    /**
+     * This validates the UnsupportedApiVersion the client generates while 
building a HB if:
+     * 1. HB API is not supported.
+     * 2. Required HB API version is not available.
+     */
+    @ParameterizedTest
+    @ValueSource(strings = {CONSUMER_PROTOCOL_NOT_SUPPORTED_MSG, 
REGEX_RESOLUTION_NOT_SUPPORTED_MSG})
+    public void testUnsupportedVersion(String errorMsg) {
+        mockResponseWithException(new UnsupportedVersionException(errorMsg));
         ArgumentCaptor<ErrorEvent> errorEventArgumentCaptor = 
ArgumentCaptor.forClass(ErrorEvent.class);
         verify(backgroundEventHandler).add(errorEventArgumentCaptor.capture());
         ErrorEvent errorEvent = errorEventArgumentCaptor.getValue();
-
-        // UnsupportedApiVersion in HB response without any custom message. 
It's considered as new protocol not supported.
-        String hbNotSupportedMsg = "The cluster does not support the new 
consumer group protocol. Set group" +
-            ".protocol=classic on the consumer configs to revert to the 
classic protocol until the cluster is upgraded.";
         assertInstanceOf(Errors.UNSUPPORTED_VERSION.exception().getClass(), 
errorEvent.error());
-        assertEquals(hbNotSupportedMsg, errorEvent.error().getMessage());
+        assertEquals(errorMsg, errorEvent.error().getMessage());
         clearInvocations(backgroundEventHandler);
-
-        // UnsupportedApiVersion in HB response with custom message. Specific 
to required version not present, should

Review Comment:
   it's different things, that commit you shared is handling the error received 
in a HB response from the broker, but this test is not covering that case. This 
test is about the UnsupportedVersions generated on the client side before even 
sending the request to the broker (if the client detects that the HB API is not 
supported, or a required version is not supported). We need to fix it because I 
changed the msg generated on the client but did not update the test (and a bug 
in the build setting let that failure go unnoticed). Makes sense?



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