apoorvmittal10 commented on code in PR #20144:
URL: https://github.com/apache/kafka/pull/20144#discussion_r2285718764


##########
clients/src/test/java/org/apache/kafka/common/telemetry/internals/ClientTelemetryUtilsTest.java:
##########
@@ -123,10 +124,36 @@ public void testValidateIntervalMsInvalid(int 
pushIntervalMs) {
 
     @Test
     public void testPreferredCompressionType() {
-        assertEquals(CompressionType.NONE, 
ClientTelemetryUtils.preferredCompressionType(Collections.emptyList()));
-        assertEquals(CompressionType.NONE, 
ClientTelemetryUtils.preferredCompressionType(null));
-        assertEquals(CompressionType.NONE, 
ClientTelemetryUtils.preferredCompressionType(Arrays.asList(CompressionType.NONE,
 CompressionType.GZIP)));
-        assertEquals(CompressionType.GZIP, 
ClientTelemetryUtils.preferredCompressionType(Arrays.asList(CompressionType.GZIP,
 CompressionType.NONE)));
+        // Test with no unsupported types
+        assertEquals(CompressionType.NONE, 
ClientTelemetryUtils.preferredCompressionType(Collections.emptyList(), 
Collections.emptySet()));

Review Comment:
   Same elsewhere:
   ```suggestion
           assertEquals(CompressionType.NONE, 
ClientTelemetryUtils.preferredCompressionType(List.of(), Set.of()));
   ```



##########
clients/src/test/java/org/apache/kafka/common/telemetry/internals/ClientTelemetryUtilsTest.java:
##########
@@ -123,10 +124,36 @@ public void testValidateIntervalMsInvalid(int 
pushIntervalMs) {
 
     @Test
     public void testPreferredCompressionType() {
-        assertEquals(CompressionType.NONE, 
ClientTelemetryUtils.preferredCompressionType(Collections.emptyList()));
-        assertEquals(CompressionType.NONE, 
ClientTelemetryUtils.preferredCompressionType(null));
-        assertEquals(CompressionType.NONE, 
ClientTelemetryUtils.preferredCompressionType(Arrays.asList(CompressionType.NONE,
 CompressionType.GZIP)));
-        assertEquals(CompressionType.GZIP, 
ClientTelemetryUtils.preferredCompressionType(Arrays.asList(CompressionType.GZIP,
 CompressionType.NONE)));
+        // Test with no unsupported types
+        assertEquals(CompressionType.NONE, 
ClientTelemetryUtils.preferredCompressionType(Collections.emptyList(), 
Collections.emptySet()));
+        assertEquals(CompressionType.NONE, 
ClientTelemetryUtils.preferredCompressionType(Arrays.asList(CompressionType.NONE,
 CompressionType.GZIP), Collections.emptySet()));
+        assertEquals(CompressionType.GZIP, 
ClientTelemetryUtils.preferredCompressionType(Arrays.asList(CompressionType.GZIP,
 CompressionType.NONE), Collections.emptySet()));
+
+        // Test with unsupported types filtering
+        assertEquals(CompressionType.LZ4, 
ClientTelemetryUtils.preferredCompressionType(Arrays.asList(CompressionType.GZIP,
 CompressionType.LZ4), Set.of(CompressionType.GZIP)));
+        assertEquals(CompressionType.NONE, 
ClientTelemetryUtils.preferredCompressionType(Arrays.asList(CompressionType.GZIP,
 CompressionType.NONE), Set.of(CompressionType.GZIP)));
+        assertEquals(CompressionType.SNAPPY, 
ClientTelemetryUtils.preferredCompressionType(Arrays.asList(CompressionType.GZIP,
 CompressionType.LZ4, CompressionType.SNAPPY), Set.of(CompressionType.GZIP, 
CompressionType.LZ4)));
+
+        // Test when all types are unsupported
+        assertEquals(CompressionType.NONE, 
ClientTelemetryUtils.preferredCompressionType(Arrays.asList(CompressionType.GZIP),
 Set.of(CompressionType.GZIP)));
+        assertEquals(CompressionType.NONE, 
ClientTelemetryUtils.preferredCompressionType(Arrays.asList(CompressionType.GZIP,
 CompressionType.LZ4), Set.of(CompressionType.GZIP, CompressionType.LZ4)));
+
+        // Test priority order with unsupported types (first available wins)
+        assertEquals(CompressionType.ZSTD, 
ClientTelemetryUtils.preferredCompressionType(Arrays.asList(CompressionType.ZSTD,
 CompressionType.SNAPPY), Set.of()));
+        assertEquals(CompressionType.SNAPPY, 
ClientTelemetryUtils.preferredCompressionType(Arrays.asList(CompressionType.ZSTD,
 CompressionType.SNAPPY), Set.of(CompressionType.ZSTD)));
+        assertEquals(CompressionType.GZIP, 
ClientTelemetryUtils.preferredCompressionType(Arrays.asList(CompressionType.LZ4,
 CompressionType.GZIP, CompressionType.SNAPPY), Set.of(CompressionType.LZ4)));
+
+        // Test NullPointerException for null acceptedCompressionTypes 
parameter
+        assertThrows(NullPointerException.class, () ->
+            ClientTelemetryUtils.preferredCompressionType(null, 
Collections.emptySet()));
+        assertThrows(NullPointerException.class, () -> 
+            ClientTelemetryUtils.preferredCompressionType(null, 
Set.of(CompressionType.GZIP)));
+
+        // Test NullPointerException for null unsupportedCompressionTypes 
parameter
+        assertThrows(NullPointerException.class, () ->
+            
ClientTelemetryUtils.preferredCompressionType(Arrays.asList(CompressionType.GZIP,
 CompressionType.NONE), null));
+        assertThrows(NullPointerException.class, () -> 
+            
ClientTelemetryUtils.preferredCompressionType(Arrays.asList(CompressionType.LZ4,
 CompressionType.SNAPPY), null));

Review Comment:
   This is not adding any value, already tested in previous line.



##########
clients/src/test/java/org/apache/kafka/common/telemetry/internals/ClientTelemetryUtilsTest.java:
##########
@@ -123,10 +124,36 @@ public void testValidateIntervalMsInvalid(int 
pushIntervalMs) {
 
     @Test
     public void testPreferredCompressionType() {
-        assertEquals(CompressionType.NONE, 
ClientTelemetryUtils.preferredCompressionType(Collections.emptyList()));
-        assertEquals(CompressionType.NONE, 
ClientTelemetryUtils.preferredCompressionType(null));
-        assertEquals(CompressionType.NONE, 
ClientTelemetryUtils.preferredCompressionType(Arrays.asList(CompressionType.NONE,
 CompressionType.GZIP)));
-        assertEquals(CompressionType.GZIP, 
ClientTelemetryUtils.preferredCompressionType(Arrays.asList(CompressionType.GZIP,
 CompressionType.NONE)));
+        // Test with no unsupported types
+        assertEquals(CompressionType.NONE, 
ClientTelemetryUtils.preferredCompressionType(Collections.emptyList(), 
Collections.emptySet()));
+        assertEquals(CompressionType.NONE, 
ClientTelemetryUtils.preferredCompressionType(Arrays.asList(CompressionType.NONE,
 CompressionType.GZIP), Collections.emptySet()));
+        assertEquals(CompressionType.GZIP, 
ClientTelemetryUtils.preferredCompressionType(Arrays.asList(CompressionType.GZIP,
 CompressionType.NONE), Collections.emptySet()));
+
+        // Test with unsupported types filtering

Review Comment:
   Can you add a test when the list has no common match i.e. 
`ClientTelemetryUtils.preferredCompressionType(List.of(CompressionType.GZIP, 
CompressionType.LZ4), Set.of(CompressionType.SNAPPY)`



-- 
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: jira-unsubscr...@kafka.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Reply via email to