dajac commented on code in PR #14642:
URL: https://github.com/apache/kafka/pull/14642#discussion_r1377336442
##########
clients/src/test/java/org/apache/kafka/clients/consumer/ConsumerConfigTest.java:
##########
@@ -158,4 +163,54 @@ public void testCaseInsensitiveSecurityProtocol() {
final ConsumerConfig consumerConfig = new ConsumerConfig(configs);
assertEquals(saslSslLowerCase,
consumerConfig.originals().get(CommonClientConfigs.SECURITY_PROTOCOL_CONFIG));
}
+
+ @Test
+ public void testDefaultConsumerGroupConfig() {
+ final Map<String, Object> configs = new HashMap<>();
+ configs.put(ConsumerConfig.KEY_DESERIALIZER_CLASS_CONFIG,
keyDeserializerClass);
+ configs.put(ConsumerConfig.VALUE_DESERIALIZER_CLASS_CONFIG,
valueDeserializerClass);
+ final ConsumerConfig consumerConfig = new ConsumerConfig(configs);
+ assertEquals("generic",
consumerConfig.getString(ConsumerConfig.GROUP_PROTOCOL_CONFIG));
+
assertNull(consumerConfig.getString(ConsumerConfig.REMOTE_ASSIGNOR_CONFIG));
+ }
+
+ @Test
+ public void testRemoteAssignorConfig() {
+ String remoteAssignorName =
"org.apache.kafka.clients.group.someAssignor";
+ String protocol = "consumer";
+ final Map<String, Object> configs = new HashMap<>();
+ configs.put(ConsumerConfig.KEY_DESERIALIZER_CLASS_CONFIG,
keyDeserializerClass);
+ configs.put(ConsumerConfig.VALUE_DESERIALIZER_CLASS_CONFIG,
valueDeserializerClass);
+ configs.put(ConsumerConfig.REMOTE_ASSIGNOR_CONFIG, remoteAssignorName);
+ configs.put(ConsumerConfig.GROUP_PROTOCOL_CONFIG, protocol);
+ final ConsumerConfig consumerConfig = new ConsumerConfig(configs);
+ assertEquals(protocol,
consumerConfig.getString(ConsumerConfig.GROUP_PROTOCOL_CONFIG));
+ assertEquals(remoteAssignorName,
consumerConfig.getString(ConsumerConfig.REMOTE_ASSIGNOR_CONFIG));
+ }
+
+ @ParameterizedTest
+ @MethodSource("protocolNameSupplier")
+ public void testProtocolConfigValidation(String protocol, boolean isValid)
{
+ final Map<String, Object> configs = new HashMap<>();
+ configs.put(ConsumerConfig.KEY_DESERIALIZER_CLASS_CONFIG,
keyDeserializerClass);
+ configs.put(ConsumerConfig.VALUE_DESERIALIZER_CLASS_CONFIG,
valueDeserializerClass);
+ configs.put(ConsumerConfig.GROUP_PROTOCOL_CONFIG, protocol);
+ try {
+ ConsumerConfig config = new ConsumerConfig(configs);
+ assertEquals(protocol,
config.getString(ConsumerConfig.GROUP_PROTOCOL_CONFIG));
+ } catch (ConfigException e) {
+ if (isValid)
+ fail("Should not have thrown an exception");
Review Comment:
I am not a fan of this because you don't know why it fails if it ever fails.
At minimum, we should include the exception in the `fail` message.
Otherwise, we could do it as follow:
```
if (isValid) {
ConsumerConfig config = new ConsumerConfig(configs);
assertEquals(protocol,
config.getString(ConsumerConfig.GROUP_PROTOCOL_CONFIG));
} else {
assertThrows(ConfigException.class, () -> new ConsumerConfig(configs));
}
```
--
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
