mumrah commented on a change in pull request #10927: URL: https://github.com/apache/kafka/pull/10927#discussion_r659005600
########## File path: core/src/test/scala/unit/kafka/server/metadata/MockConfigRepositoryTest.scala ########## @@ -0,0 +1,78 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package kafka.server.metadata + +import java.util.Properties + +import org.junit.jupiter.api.Assertions.assertEquals +import org.junit.jupiter.api.Test + +class MockConfigRepositoryTest { Review comment: So this is a test of the mock class itself? ########## File path: core/src/test/scala/unit/kafka/log/LogManagerTest.scala ########## @@ -564,7 +563,7 @@ class LogManagerTest { @Test def testBrokerConfigChangeDeliveredToAllLogs(): Unit = { logManager.shutdown() - val spyConfigRepository = spy(new CachedConfigRepository) + val spyConfigRepository = spy(new MockConfigRepository) Review comment: Do we need to "spy" on these instances? Seems like we could put whatever test-only behavior we want into the MockConfigRepository class itself. Or maybe we need to do this for some Mockito reason like "all arguments must be mock/spy-ed" ########## File path: core/src/test/scala/kafka/server/metadata/MockConfigRepository.scala ########## @@ -0,0 +1,57 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package kafka.server.metadata; + +import java.util +import java.util.Properties + +import org.apache.kafka.common.config.ConfigResource +import org.apache.kafka.common.config.ConfigResource.Type.TOPIC + +object MockConfigRepository { + def forTopic(topic: String, key: String, value: String): MockConfigRepository = { + val properties = new Properties() + properties.put(key, value) + forTopic(topic, properties) + } + + def forTopic(topic:String, properties: Properties): MockConfigRepository = { + val repository = new MockConfigRepository() + repository.configs.put(new ConfigResource(TOPIC, topic), properties) + repository + } +} + +class MockConfigRepository extends ConfigRepository { + val configs = new util.HashMap[ConfigResource, Properties]() + + override def config(configResource: ConfigResource): Properties = configs.synchronized { + configs.getOrDefault(configResource, new Properties()) + } + + def setConfig(configResource: ConfigResource, key: String, value: String): Unit = configs.synchronized { + val properties = configs.getOrDefault(configResource, new Properties()) + val newProperties = new Properties(properties) + newProperties.put(key, value) + configs.put(configResource, newProperties) + } + + def setTopicConfig(topicName: String, key: String, value: String): Unit = configs.synchronized { + setConfig(new ConfigResource(TOPIC, topicName), key, value) + } +} Review comment: nit: missing newline -- 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
