Re: [PR] MINOR: Rewrite unchecked operations in Mock API [kafka]

[via GitHub]

chia7712 commented on code in PR #19071:
URL: https://github.com/apache/kafka/pull/19071#discussion_r1986194037


##########
streams/src/test/java/org/apache/kafka/streams/kstream/internals/KTableImplTest.java:
##########
@@ -391,24 +386,20 @@ public void testStateStore() {
     public void 
shouldNotEnableSendingOldValuesIfNotMaterializedAlreadyAndNotForcedToMaterialize()
 {
         final StreamsBuilder builder = new StreamsBuilder();
 
-        final KTableImpl<String, String, String> table =
-            (KTableImpl<String, String, String>) builder.table("topic1", 
consumed);
+        final var kTable = assertInstanceOf(KTableImpl.class, 
builder.table("topic1", consumed));
+        kTable.enableSendingOldValues(false);
 
-        table.enableSendingOldValues(false);
-
-        assertThat(table.sendingOldValueEnabled(), is(false));
+        assertFalse(kTable.sendingOldValueEnabled());

Review Comment:
   ditto



##########
streams/src/test/java/org/apache/kafka/streams/kstream/internals/KTableImplTest.java:
##########
@@ -587,19 +577,16 @@ public void 
shouldThrowNullPointerOnTransformValuesWithKeyWhenTransformerSupplie
         assertThrows(NullPointerException.class, () -> 
table.transformValues(null));
     }
 
-    @SuppressWarnings("unchecked")
     @Test
     public void 
shouldThrowNullPointerOnTransformValuesWithKeyWhenMaterializedIsNull() {
-        final ValueTransformerWithKeySupplier<String, String, ?> 
valueTransformerSupplier =
-            mock(ValueTransformerWithKeySupplier.class);
+        final ValueTransformerWithKeySupplier<String, String, ?> 
valueTransformerSupplier = mock();
         assertThrows(NullPointerException.class, () -> 
table.transformValues(valueTransformerSupplier, (Materialized<String, Object, 
KeyValueStore<Bytes, byte[]>>) null));
     }
 
-    @SuppressWarnings("unchecked")
+
     @Test
     public void 
shouldThrowNullPointerOnTransformValuesWithKeyWhenStoreNamesNull() {
-        final ValueTransformerWithKeySupplier<String, String, ?> 
valueTransformerSupplier =
-            mock(ValueTransformerWithKeySupplier.class);
+        final ValueTransformerWithKeySupplier<String, String, ?> 
valueTransformerSupplier = mock();
         assertThrows(NullPointerException.class, () -> 
table.transformValues(valueTransformerSupplier, (String[]) null));

Review Comment:
   maybe we can pass `mock()` to `transformValues` directly?
   
   ```java
           assertThrows(NullPointerException.class, () -> 
table.transformValues(mock(), (String[]) null));
   ```



##########
streams/src/test/java/org/apache/kafka/streams/kstream/internals/KTableImplTest.java:
##########
@@ -587,19 +577,16 @@ public void 
shouldThrowNullPointerOnTransformValuesWithKeyWhenTransformerSupplie
         assertThrows(NullPointerException.class, () -> 
table.transformValues(null));
     }
 
-    @SuppressWarnings("unchecked")
     @Test
     public void 
shouldThrowNullPointerOnTransformValuesWithKeyWhenMaterializedIsNull() {
-        final ValueTransformerWithKeySupplier<String, String, ?> 
valueTransformerSupplier =
-            mock(ValueTransformerWithKeySupplier.class);
+        final ValueTransformerWithKeySupplier<String, String, ?> 
valueTransformerSupplier = mock();
         assertThrows(NullPointerException.class, () -> 
table.transformValues(valueTransformerSupplier, (Materialized<String, Object, 
KeyValueStore<Bytes, byte[]>>) null));

Review Comment:
   ditto



##########
streams/src/test/java/org/apache/kafka/streams/kstream/internals/KTableImplTest.java:
##########
@@ -391,24 +386,20 @@ public void testStateStore() {
     public void 
shouldNotEnableSendingOldValuesIfNotMaterializedAlreadyAndNotForcedToMaterialize()
 {
         final StreamsBuilder builder = new StreamsBuilder();
 
-        final KTableImpl<String, String, String> table =
-            (KTableImpl<String, String, String>) builder.table("topic1", 
consumed);
+        final var kTable = assertInstanceOf(KTableImpl.class, 
builder.table("topic1", consumed));
+        kTable.enableSendingOldValues(false);
 
-        table.enableSendingOldValues(false);
-
-        assertThat(table.sendingOldValueEnabled(), is(false));
+        assertFalse(kTable.sendingOldValueEnabled());
     }
 
     @Test
     public void 
shouldEnableSendingOldValuesIfNotMaterializedAlreadyButForcedToMaterialize() {
         final StreamsBuilder builder = new StreamsBuilder();
 
-        final KTableImpl<String, String, String> table =
-            (KTableImpl<String, String, String>) builder.table("topic1", 
consumed);
-
-        table.enableSendingOldValues(true);
+        final var kTable = assertInstanceOf(KTableImpl.class, 
builder.table("topic1", consumed));
+        kTable.enableSendingOldValues(true);
 
-        assertThat(table.sendingOldValueEnabled(), is(true));
+        assertTrue(kTable.sendingOldValueEnabled());

Review Comment:
   this change seems to be unnecessary? Also, we don't need to rename the 
`table` to `kTable`, right?



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