mumrah commented on a change in pull request #11179:
URL: https://github.com/apache/kafka/pull/11179#discussion_r685414248
##########
File path: core/src/test/scala/unit/kafka/raft/RaftManagerTest.scala
##########
@@ -28,27 +28,41 @@ import org.apache.kafka.common.metrics.Metrics
import org.apache.kafka.common.utils.Time
import org.apache.kafka.raft.KafkaRaftClient
import org.apache.kafka.raft.RaftConfig
+import org.apache.kafka.test.TestUtils
import org.junit.jupiter.api.Assertions._
import org.junit.jupiter.api.Test
import org.mockito.Mockito._
+import java.io.File
+
class RaftManagerTest {
- private def instantiateRaftManagerWithConfigs(processRoles: String,
nodeId:String) = {
- def configWithProcessRolesAndNodeId(processRoles: String, nodeId: String):
KafkaConfig = {
+ private def instantiateRaftManagerWithConfigs(topicPartition:
TopicPartition, processRoles: String, nodeId: String) = {
+ def configWithProcessRolesAndNodeId(processRoles: String, nodeId: String,
logDir: File): KafkaConfig = {
val props = new Properties
+ props.setProperty(KafkaConfig.MetadataLogDirProp, logDir.getPath)
props.setProperty(KafkaConfig.ProcessRolesProp, processRoles)
props.setProperty(KafkaConfig.NodeIdProp, nodeId)
props.setProperty(KafkaConfig.ListenersProp,
"PLAINTEXT://localhost:9093")
props.setProperty(KafkaConfig.ControllerListenerNamesProp, "PLAINTEXT")
- props.setProperty(RaftConfig.QUORUM_VOTERS_CONFIG,
nodeId.concat("@localhost:9093"))
- if (processRoles.contains("broker"))
+ if (processRoles.contains("broker")) {
props.setProperty(KafkaConfig.InterBrokerListenerNameProp, "PLAINTEXT")
props.setProperty(KafkaConfig.AdvertisedListenersProp,
"PLAINTEXT://localhost:9092")
+ if (!processRoles.contains("controller")) {
+ val voterId = (nodeId.toInt + 1)
+ props.setProperty(KafkaConfig.QuorumVotersProp,
s"${voterId}@localhost:9093")
+ }
+ }
+
+ if (processRoles.contains("controller")) {
+ props.setProperty(KafkaConfig.QuorumVotersProp,
s"${nodeId}@localhost:9093")
+ }
+
new KafkaConfig(props)
}
- val config = configWithProcessRolesAndNodeId(processRoles, nodeId)
+ val logDir = TestUtils.tempDirectory()
Review comment:
Yea probably a good idea. Looks like other tests using tempDirectory are
cleaning up with `o.a.k.common.utils.Utils.delete`
##########
File path: core/src/test/scala/unit/kafka/raft/RaftManagerTest.scala
##########
@@ -28,27 +28,41 @@ import org.apache.kafka.common.metrics.Metrics
import org.apache.kafka.common.utils.Time
import org.apache.kafka.raft.KafkaRaftClient
import org.apache.kafka.raft.RaftConfig
+import org.apache.kafka.test.TestUtils
import org.junit.jupiter.api.Assertions._
import org.junit.jupiter.api.Test
import org.mockito.Mockito._
+import java.io.File
+
class RaftManagerTest {
- private def instantiateRaftManagerWithConfigs(processRoles: String,
nodeId:String) = {
- def configWithProcessRolesAndNodeId(processRoles: String, nodeId: String):
KafkaConfig = {
+ private def instantiateRaftManagerWithConfigs(topicPartition:
TopicPartition, processRoles: String, nodeId: String) = {
+ def configWithProcessRolesAndNodeId(processRoles: String, nodeId: String,
logDir: File): KafkaConfig = {
val props = new Properties
+ props.setProperty(KafkaConfig.MetadataLogDirProp, logDir.getPath)
props.setProperty(KafkaConfig.ProcessRolesProp, processRoles)
props.setProperty(KafkaConfig.NodeIdProp, nodeId)
props.setProperty(KafkaConfig.ListenersProp,
"PLAINTEXT://localhost:9093")
props.setProperty(KafkaConfig.ControllerListenerNamesProp, "PLAINTEXT")
- props.setProperty(RaftConfig.QUORUM_VOTERS_CONFIG,
nodeId.concat("@localhost:9093"))
- if (processRoles.contains("broker"))
+ if (processRoles.contains("broker")) {
props.setProperty(KafkaConfig.InterBrokerListenerNameProp, "PLAINTEXT")
props.setProperty(KafkaConfig.AdvertisedListenersProp,
"PLAINTEXT://localhost:9092")
+ if (!processRoles.contains("controller")) {
+ val voterId = (nodeId.toInt + 1)
+ props.setProperty(KafkaConfig.QuorumVotersProp,
s"${voterId}@localhost:9093")
+ }
+ }
+
+ if (processRoles.contains("controller")) {
+ props.setProperty(KafkaConfig.QuorumVotersProp,
s"${nodeId}@localhost:9093")
+ }
+
new KafkaConfig(props)
}
- val config = configWithProcessRolesAndNodeId(processRoles, nodeId)
+ val logDir = TestUtils.tempDirectory()
Review comment:
Yea probably a good idea. Looks like other tests using tempDirectory are
cleaning up with `o.a.k.common.utils.Utils#delete`
--
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
