dajac commented on a change in pull request #10351:
URL: https://github.com/apache/kafka/pull/10351#discussion_r597859840



##########
File path: 
clients/src/test/java/org/apache/kafka/common/requests/RequestResponseTest.java
##########
@@ -930,6 +930,28 @@ public void 
testCreateTopicRequestV3FailsIfNoPartitionsOrReplicas() {
         assertTrue(exception.getMessage().contains("[foo, bar]"));
     }
 
+    @Test
+    public void testDeleteTopicsRequestNumTopics() {
+        for (short version : DELETE_TOPICS.allVersions()) {
+            DeleteTopicsRequest request = createDeleteTopicsRequest(version);
+            DeleteTopicsRequest serializedRequest = 
DeleteTopicsRequest.parse(request.serialize(), version);
+            // createDeleteTopicsRequest sets 2 topics
+            assertEquals(2, request.numberOfTopics());
+            assertEquals(2, serializedRequest.numberOfTopics());

Review comment:
       Ah, right. I completely missed that part in the builder. We usually put 
unit tests for requests in their own classes (e.g. DeleteTopicsRequestTest) 
except the ones really verifying the serialisation which are here. I wonder if 
this one wouldn't be better in `DeleteTopicsRequestTest`. What do you think?
   
   Slightly related, do we have a unit test verifying the normalization logic 
in the builder?




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

For queries about this service, please contact Infrastructure at:
[email protected]


Reply via email to