mirror of https://github.com/apache/kafka.git
MINOR: Rewrite unchecked operations in Mock API (#19071)
We encountered unchecked or unsafe operations in `GroupMetadataManagerTest.java`, `KTableImplTest.java`, and `ConfigCommandIntegrationTest.java`. * Rewrite getArgument of invocation in InvocationOnMock API because the implementation of InvocationOnMock discards type anyway in in `GroupMetadataManagerTest.java`. * Remove unchecked annotations for using mock API without variable assignment in `KTableImplTest.java`. <img width="1422" alt="Screenshot 2025-03-02 at 8 50 55 AM" src="https://github.com/user-attachments/assets/10ff1799-ebaa-499c-9acd-ca3b30484e6d" /> * Follow-up: https://github.com/mockito/mockito/issues/1609 Update on March 2. * Fix unchecked cast for KTableImpl in `KTableImplTest.java`. <img width="1259" alt="Screenshot 2025-03-02 at 5 17 47 PM" src="https://github.com/user-attachments/assets/a5ffa3d7-4897-43ee-9b5f-26337e2560c5" /> Update on March 10. * Use anyMap instead any for unchecked map type issues. <img width="1691" alt="Screenshot 2025-03-10 at 9 36 38 AM" src="https://github.com/user-attachments/assets/9aabc595-e7ba-4e04-81f6-f238d42af5a6" /> Pass all testing. <img width="946" alt="Screenshot 2025-03-10 at 10 10 56 AM" src="https://github.com/user-attachments/assets/793f67ea-09dc-44af-9d6c-de15531e9e72" /> Reviewers: TengYao Chi <kitingiao@gmail.com>, Ken Huang <s7133700@gmail.com>, Chia-Ping Tsai <chia7712@gmail.com>
This commit is contained in:
parent
a6a0ea56d8
commit
ec3c319c35
|
@ -17017,7 +17017,7 @@ public class GroupMetadataManagerTest {
|
||||||
acls.put(fooTopicName, AuthorizationResult.ALLOWED);
|
acls.put(fooTopicName, AuthorizationResult.ALLOWED);
|
||||||
acls.put(barTopicName, AuthorizationResult.DENIED);
|
acls.put(barTopicName, AuthorizationResult.DENIED);
|
||||||
when(authorizer.authorize(any(), any())).thenAnswer(invocation -> {
|
when(authorizer.authorize(any(), any())).thenAnswer(invocation -> {
|
||||||
List<Action> actions = invocation.getArgument(1, List.class);
|
List<Action> actions = invocation.getArgument(1);
|
||||||
return actions.stream()
|
return actions.stream()
|
||||||
.map(action -> acls.getOrDefault(action.resourcePattern().name(), AuthorizationResult.DENIED))
|
.map(action -> acls.getOrDefault(action.resourcePattern().name(), AuthorizationResult.DENIED))
|
||||||
.collect(Collectors.toList());
|
.collect(Collectors.toList());
|
||||||
|
|
|
@ -68,6 +68,7 @@ import static java.util.Arrays.asList;
|
||||||
import static org.hamcrest.MatcherAssert.assertThat;
|
import static org.hamcrest.MatcherAssert.assertThat;
|
||||||
import static org.hamcrest.Matchers.is;
|
import static org.hamcrest.Matchers.is;
|
||||||
import static org.junit.jupiter.api.Assertions.assertEquals;
|
import static org.junit.jupiter.api.Assertions.assertEquals;
|
||||||
|
import static org.junit.jupiter.api.Assertions.assertInstanceOf;
|
||||||
import static org.junit.jupiter.api.Assertions.assertNotNull;
|
import static org.junit.jupiter.api.Assertions.assertNotNull;
|
||||||
import static org.junit.jupiter.api.Assertions.assertNull;
|
import static org.junit.jupiter.api.Assertions.assertNull;
|
||||||
import static org.junit.jupiter.api.Assertions.assertThrows;
|
import static org.junit.jupiter.api.Assertions.assertThrows;
|
||||||
|
@ -352,12 +353,10 @@ public class KTableImplTest {
|
||||||
final String topic1 = "topic1";
|
final String topic1 = "topic1";
|
||||||
final String topic2 = "topic2";
|
final String topic2 = "topic2";
|
||||||
|
|
||||||
final KTableImpl<String, String, String> table1 =
|
final var table1 = builder.table(topic1, consumed);
|
||||||
(KTableImpl<String, String, String>) builder.table(topic1, consumed);
|
|
||||||
builder.table(topic2, consumed);
|
builder.table(topic2, consumed);
|
||||||
|
|
||||||
final KTableImpl<String, String, Integer> table1Mapped =
|
final var table1Mapped = table1.mapValues(s -> Integer.valueOf(s));
|
||||||
(KTableImpl<String, String, Integer>) table1.mapValues(s -> Integer.valueOf(s));
|
|
||||||
table1Mapped.filter((key, value) -> (value % 2) == 0);
|
table1Mapped.filter((key, value) -> (value % 2) == 0);
|
||||||
|
|
||||||
try (final TopologyTestDriver driver = new TopologyTestDriver(builder.build(), props)) {
|
try (final TopologyTestDriver driver = new TopologyTestDriver(builder.build(), props)) {
|
||||||
|
@ -371,15 +370,11 @@ public class KTableImplTest {
|
||||||
final String topic1 = "topic1";
|
final String topic1 = "topic1";
|
||||||
final String topic2 = "topic2";
|
final String topic2 = "topic2";
|
||||||
|
|
||||||
final KTableImpl<String, String, String> table1 =
|
final var table1 = builder.table(topic1, consumed);
|
||||||
(KTableImpl<String, String, String>) builder.table(topic1, consumed);
|
final var table2 = builder.table(topic2, consumed);
|
||||||
final KTableImpl<String, String, String> table2 =
|
|
||||||
(KTableImpl<String, String, String>) builder.table(topic2, consumed);
|
|
||||||
|
|
||||||
final KTableImpl<String, String, Integer> table1Mapped =
|
final var table1Mapped = table1.mapValues(s -> Integer.valueOf(s));
|
||||||
(KTableImpl<String, String, Integer>) table1.mapValues(s -> Integer.valueOf(s));
|
final var table1MappedFiltered = table1Mapped.filter((key, value) -> (value % 2) == 0);
|
||||||
final KTableImpl<String, Integer, Integer> table1MappedFiltered =
|
|
||||||
(KTableImpl<String, Integer, Integer>) table1Mapped.filter((key, value) -> (value % 2) == 0);
|
|
||||||
table2.join(table1MappedFiltered, (v1, v2) -> v1 + v2);
|
table2.join(table1MappedFiltered, (v1, v2) -> v1 + v2);
|
||||||
|
|
||||||
try (final TopologyTestDriver driver = new TopologyTestDriver(builder.build(), props)) {
|
try (final TopologyTestDriver driver = new TopologyTestDriver(builder.build(), props)) {
|
||||||
|
@ -391,9 +386,7 @@ public class KTableImplTest {
|
||||||
public void shouldNotEnableSendingOldValuesIfNotMaterializedAlreadyAndNotForcedToMaterialize() {
|
public void shouldNotEnableSendingOldValuesIfNotMaterializedAlreadyAndNotForcedToMaterialize() {
|
||||||
final StreamsBuilder builder = new StreamsBuilder();
|
final StreamsBuilder builder = new StreamsBuilder();
|
||||||
|
|
||||||
final KTableImpl<String, String, String> table =
|
final var table = assertInstanceOf(KTableImpl.class, builder.table("topic1", consumed));
|
||||||
(KTableImpl<String, String, String>) builder.table("topic1", consumed);
|
|
||||||
|
|
||||||
table.enableSendingOldValues(false);
|
table.enableSendingOldValues(false);
|
||||||
|
|
||||||
assertThat(table.sendingOldValueEnabled(), is(false));
|
assertThat(table.sendingOldValueEnabled(), is(false));
|
||||||
|
@ -403,9 +396,7 @@ public class KTableImplTest {
|
||||||
public void shouldEnableSendingOldValuesIfNotMaterializedAlreadyButForcedToMaterialize() {
|
public void shouldEnableSendingOldValuesIfNotMaterializedAlreadyButForcedToMaterialize() {
|
||||||
final StreamsBuilder builder = new StreamsBuilder();
|
final StreamsBuilder builder = new StreamsBuilder();
|
||||||
|
|
||||||
final KTableImpl<String, String, String> table =
|
final var table = assertInstanceOf(KTableImpl.class, builder.table("topic1", consumed));
|
||||||
(KTableImpl<String, String, String>) builder.table("topic1", consumed);
|
|
||||||
|
|
||||||
table.enableSendingOldValues(true);
|
table.enableSendingOldValues(true);
|
||||||
|
|
||||||
assertThat(table.sendingOldValueEnabled(), is(true));
|
assertThat(table.sendingOldValueEnabled(), is(true));
|
||||||
|
@ -429,8 +420,7 @@ public class KTableImplTest {
|
||||||
final String topic1 = "topic1";
|
final String topic1 = "topic1";
|
||||||
final String storeName1 = "storeName1";
|
final String storeName1 = "storeName1";
|
||||||
|
|
||||||
final KTableImpl<String, String, String> table1 =
|
final var table1 = builder.table(
|
||||||
(KTableImpl<String, String, String>) builder.table(
|
|
||||||
topic1,
|
topic1,
|
||||||
consumed,
|
consumed,
|
||||||
Materialized.<String, String, KeyValueStore<Bytes, byte[]>>as(storeName1)
|
Materialized.<String, String, KeyValueStore<Bytes, byte[]>>as(storeName1)
|
||||||
|
@ -587,19 +577,14 @@ public class KTableImplTest {
|
||||||
assertThrows(NullPointerException.class, () -> table.transformValues(null));
|
assertThrows(NullPointerException.class, () -> table.transformValues(null));
|
||||||
}
|
}
|
||||||
|
|
||||||
@SuppressWarnings("unchecked")
|
|
||||||
@Test
|
@Test
|
||||||
public void shouldThrowNullPointerOnTransformValuesWithKeyWhenMaterializedIsNull() {
|
public void shouldThrowNullPointerOnTransformValuesWithKeyWhenMaterializedIsNull() {
|
||||||
final ValueTransformerWithKeySupplier<String, String, ?> valueTransformerSupplier =
|
assertThrows(NullPointerException.class, () -> table.transformValues(mock(), (Materialized<String, Object, KeyValueStore<Bytes, byte[]>>) null));
|
||||||
mock(ValueTransformerWithKeySupplier.class);
|
|
||||||
assertThrows(NullPointerException.class, () -> table.transformValues(valueTransformerSupplier, (Materialized<String, Object, KeyValueStore<Bytes, byte[]>>) null));
|
|
||||||
}
|
}
|
||||||
|
|
||||||
@SuppressWarnings("unchecked")
|
|
||||||
@Test
|
@Test
|
||||||
public void shouldThrowNullPointerOnTransformValuesWithKeyWhenStoreNamesNull() {
|
public void shouldThrowNullPointerOnTransformValuesWithKeyWhenStoreNamesNull() {
|
||||||
final ValueTransformerWithKeySupplier<String, String, ?> valueTransformerSupplier =
|
assertThrows(NullPointerException.class, () -> table.transformValues(mock(), (String[]) null));
|
||||||
mock(ValueTransformerWithKeySupplier.class);
|
|
||||||
assertThrows(NullPointerException.class, () -> table.transformValues(valueTransformerSupplier, (String[]) null));
|
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
|
@ -69,6 +69,7 @@ import static org.junit.jupiter.api.Assertions.assertNotNull;
|
||||||
import static org.junit.jupiter.api.Assertions.assertThrows;
|
import static org.junit.jupiter.api.Assertions.assertThrows;
|
||||||
import static org.junit.jupiter.api.Assertions.assertTrue;
|
import static org.junit.jupiter.api.Assertions.assertTrue;
|
||||||
import static org.mockito.ArgumentMatchers.any;
|
import static org.mockito.ArgumentMatchers.any;
|
||||||
|
import static org.mockito.ArgumentMatchers.anyMap;
|
||||||
|
|
||||||
public class ConfigCommandIntegrationTest {
|
public class ConfigCommandIntegrationTest {
|
||||||
private final String defaultBrokerId = "0";
|
private final String defaultBrokerId = "0";
|
||||||
|
@ -413,7 +414,7 @@ public class ConfigCommandIntegrationTest {
|
||||||
AlterConfigsResult mockResult = AdminClientTestUtils.alterConfigsResult(
|
AlterConfigsResult mockResult = AdminClientTestUtils.alterConfigsResult(
|
||||||
new ConfigResource(ConfigResource.Type.BROKER, ""), new UnsupportedVersionException("simulated error"));
|
new ConfigResource(ConfigResource.Type.BROKER, ""), new UnsupportedVersionException("simulated error"));
|
||||||
Mockito.doReturn(mockResult).when(spyAdmin)
|
Mockito.doReturn(mockResult).when(spyAdmin)
|
||||||
.incrementalAlterConfigs(any(java.util.Map.class), any(AlterConfigsOptions.class));
|
.incrementalAlterConfigs(anyMap(), any(AlterConfigsOptions.class));
|
||||||
assertEquals(
|
assertEquals(
|
||||||
"The INCREMENTAL_ALTER_CONFIGS API is not supported by the cluster. The API is supported starting from version 2.3.0. You may want to use an older version of this tool to interact with your cluster, or upgrade your brokers to version 2.3.0 or newer to avoid this error.",
|
"The INCREMENTAL_ALTER_CONFIGS API is not supported by the cluster. The API is supported starting from version 2.3.0. You may want to use an older version of this tool to interact with your cluster, or upgrade your brokers to version 2.3.0 or newer to avoid this error.",
|
||||||
assertThrows(UnsupportedVersionException.class, () -> {
|
assertThrows(UnsupportedVersionException.class, () -> {
|
||||||
|
@ -426,7 +427,7 @@ public class ConfigCommandIntegrationTest {
|
||||||
"--entity-default"))));
|
"--entity-default"))));
|
||||||
}).getMessage()
|
}).getMessage()
|
||||||
);
|
);
|
||||||
Mockito.verify(spyAdmin).incrementalAlterConfigs(any(java.util.Map.class), any(AlterConfigsOptions.class));
|
Mockito.verify(spyAdmin).incrementalAlterConfigs(anyMap(), any(AlterConfigsOptions.class));
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
Loading…
Reference in New Issue