chia7712 commented on code in PR #22302:
URL: https://github.com/apache/kafka/pull/22302#discussion_r3281352410
##########
server/src/test/java/org/apache/kafka/server/config/AbstractKafkaConfigTest.java:
##########
@@ -52,4 +65,51 @@ public void testPopulateSynonymsOnMapWithoutBrokerId() {
expectedOutput.put(KRaftConfigs.NODE_ID_CONFIG, "4");
assertEquals(expectedOutput,
AbstractKafkaConfig.populateSynonyms(input));
}
+
+ @Test
+ public void
testExtractGroupConfigMapExcludesInternalConfigWithUnconfiguredBrokerSynonym() {
+ Map<String, Object> config = extractGroupConfigMap(Map.of(), true);
+
+ assertFalse(config.containsKey(TEST_INTERNAL_GROUP_CONFIG));
+ }
+
+ @Test
+ public void
testExtractGroupConfigMapIncludesInternalConfigWithConfiguredBrokerSynonym() {
+ Map<String, Object> config =
extractGroupConfigMap(Map.of(TEST_INTERNAL_GROUP_CONFIG_BROKER_SYNONYM,
"override-value"), true);
+
+ assertTrue(config.containsKey(TEST_INTERNAL_GROUP_CONFIG));
+ assertEquals("override-value", config.get(TEST_INTERNAL_GROUP_CONFIG));
+ }
+
+ @Test
+ public void testExtractGroupConfigMapIncludesNonInternalConfig() {
+ Map<String, Object> config = extractGroupConfigMap(Map.of(), false);
+
+ assertTrue(config.containsKey(TEST_INTERNAL_GROUP_CONFIG));
+ assertEquals("default-value", config.get(TEST_INTERNAL_GROUP_CONFIG));
+ }
+
+ private static Map<String, Object> extractGroupConfigMap(Map<String,
Object> brokerProps, boolean isInternal) {
+ try (MockedStatic<GroupConfig> mocked = mockStatic(GroupConfig.class,
Mockito.CALLS_REAL_METHODS)) {
+
+ // mock group config
+
mocked.when(GroupConfig::configNames).thenReturn(Set.of(TEST_INTERNAL_GROUP_CONFIG));
+ mocked.when(() ->
GroupConfig.brokerSynonym(TEST_INTERNAL_GROUP_CONFIG)).thenReturn(Optional.of(TEST_INTERNAL_GROUP_CONFIG_BROKER_SYNONYM));
+ mocked.when(() ->
GroupConfig.isInternal(TEST_INTERNAL_GROUP_CONFIG)).thenReturn(isInternal);
+
+ // mock broker synonym config
+ ConfigDef configDef = new
ConfigDef().define(TEST_INTERNAL_GROUP_CONFIG_BROKER_SYNONYM,
ConfigDef.Type.STRING,
+ "default-value", ConfigDef.Importance.LOW, "test broker
synonym");
+
+ AbstractKafkaConfig kafkaConfig = new
AbstractKafkaConfig(configDef, new HashMap<>(brokerProps), Map.of(), false) {
+ @Override
+ public void addReconfigurable(Reconfigurable reconfigurable) {
}
Review Comment:
@majialoong It seems the current abstraction is a bit suboptimal and forces
unnecessary dummy implementation in tests. would you mind providing empty
implementation for these two methods to clean up the codebase
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
