C0urante commented on code in PR #12010:
URL: https://github.com/apache/kafka/pull/12010#discussion_r853549366
##########
clients/src/test/java/org/apache/kafka/clients/consumer/ConsumerConfigTest.java:
##########
@@ -108,4 +112,42 @@ public void testDefaultPartitionAssignor() {
assertEquals(Arrays.asList(RangeAssignor.class,
CooperativeStickyAssignor.class),
new
ConsumerConfig(properties).getList(ConsumerConfig.PARTITION_ASSIGNMENT_STRATEGY_CONFIG));
}
+
+ @Test
+ public void testInvalidKeyDeserializer() {
Review Comment:
I was thinking of some way of simulating people using that constructor in
their consumer with a non-null serializer, but a null value for the property
for that serializer in the config map. Something like this:
```java
Map<String, Object> props = new HashMap<>();
// populate props with basic required properties like bootstrap.servers
props.put("key.deserializer", null);
props.put("value.deserializer", null);
Consumer<String, String> consumer = new KafkaConsumer<>(props, new
StringDeserializer(), new StringDeserializer());
```
It does look like we provide pretty good coverage for that case in the
`testAppendDeserializerToConfig` though, so we could also hold off for now.
There is technically a gap in coverage with the case where the value for the
property is null _and_ the user passes in a null `Deserializer` instance to the
consumer, but up to you if you feel it's worth addressing or not.
--
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
