You are viewing a plain text version of this content. The canonical link for it is here.
Posted to jira@kafka.apache.org by GitBox <gi...@apache.org> on 2021/06/24 21:41:50 UTC
[GitHub] [kafka] cmccabe opened a new pull request #10927: MINOR: add MockConfigRepository
cmccabe opened a new pull request #10927:
URL: https://github.com/apache/kafka/pull/10927
Use MockConfigRepository rather than CachedConfigRepository in unit
tests. This is useful for an upcoming change that will remove
CachedConfigRepository.
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] cmccabe commented on a change in pull request #10927: MINOR: add MockConfigRepository
Posted by GitBox <gi...@apache.org>.
cmccabe commented on a change in pull request #10927:
URL: https://github.com/apache/kafka/pull/10927#discussion_r659042611
##########
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:
we want to verify that `topicConfig` was called, and this seemed like the easiest way.
--
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-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] cmccabe merged pull request #10927: MINOR: add MockConfigRepository
Posted by GitBox <gi...@apache.org>.
cmccabe merged pull request #10927:
URL: https://github.com/apache/kafka/pull/10927
--
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-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] cmccabe commented on a change in pull request #10927: MINOR: add MockConfigRepository
Posted by GitBox <gi...@apache.org>.
cmccabe commented on a change in pull request #10927:
URL: https://github.com/apache/kafka/pull/10927#discussion_r659043629
##########
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:
Yeah...
--
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-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] mumrah commented on a change in pull request #10927: MINOR: add MockConfigRepository
Posted by GitBox <gi...@apache.org>.
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-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org