You are viewing a plain text version of this content. The canonical link for it is here.
Posted to jira@kafka.apache.org by "m1a2st (via GitHub)" <gi...@apache.org> on 2024/04/25 16:29:59 UTC
[PR] MINOR: Clean up TestUtils.scala [kafka]
m1a2st opened a new pull request, #15808:
URL: https://github.com/apache/kafka/pull/15808
Because TestUtils.scala has some unused method, and some methods only use in one class, so I delete unused methods and move methods to individual class which only one class used
--
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
Re: [PR] MINOR: Clean up TestUtils.scala [kafka]
Posted by "m1a2st (via GitHub)" <gi...@apache.org>.
m1a2st commented on PR #15808:
URL: https://github.com/apache/kafka/pull/15808#issuecomment-2077728033
@chia7712 please review this PR, thank you
--
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
Re: [PR] MINOR: Clean up TestUtils.scala [kafka]
Posted by "m1a2st (via GitHub)" <gi...@apache.org>.
m1a2st commented on code in PR #15808:
URL: https://github.com/apache/kafka/pull/15808#discussion_r1582841761
##########
core/src/test/scala/unit/kafka/utils/TestUtils.scala:
##########
@@ -1124,24 +933,18 @@ object TestUtils extends Logging {
}, msg = msg, pause = 0L, waitTimeMs = waitTimeMs)
}
- /**
- * Wait for the presence of an optional value.
- *
- * @param func The function defining the optional value
- * @param msg Error message in the case that the value never appears
- * @param waitTimeMs Maximum time to wait
- * @return The unwrapped value returned by the function
- */
- def awaitValue[T](func: () => Option[T], msg: => String, waitTimeMs: Long = JTestUtils.DEFAULT_MAX_WAIT_MS): T = {
- var value: Option[T] = None
- waitUntilTrue(() => {
- value = func()
- value.isDefined
- }, msg, waitTimeMs)
- value.get
+ def subscribeAndWaitForRecords(topic: String,
Review Comment:
`assertBadConfigContainingMessage` this method are used by two classes, so I didn't move it.
The `kafka.KafkaConfigTest` and `server.KafkaConfigTest` are using it
--
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
Re: [PR] MINOR: Clean up TestUtils.scala [kafka]
Posted by "chia7712 (via GitHub)" <gi...@apache.org>.
chia7712 commented on code in PR #15808:
URL: https://github.com/apache/kafka/pull/15808#discussion_r1582280178
##########
core/src/test/scala/unit/kafka/utils/TestUtils.scala:
##########
@@ -1124,24 +933,18 @@ object TestUtils extends Logging {
}, msg = msg, pause = 0L, waitTimeMs = waitTimeMs)
}
- /**
- * Wait for the presence of an optional value.
- *
- * @param func The function defining the optional value
- * @param msg Error message in the case that the value never appears
- * @param waitTimeMs Maximum time to wait
- * @return The unwrapped value returned by the function
- */
- def awaitValue[T](func: () => Option[T], msg: => String, waitTimeMs: Long = JTestUtils.DEFAULT_MAX_WAIT_MS): T = {
- var value: Option[T] = None
- waitUntilTrue(() => {
- value = func()
- value.isDefined
- }, msg, waitTimeMs)
- value.get
+ def subscribeAndWaitForRecords(topic: String,
Review Comment:
why adding this new method? it is unused.
--
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
Re: [PR] MINOR: Clean up TestUtils.scala [kafka]
Posted by "chia7712 (via GitHub)" <gi...@apache.org>.
chia7712 merged PR #15808:
URL: https://github.com/apache/kafka/pull/15808
--
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
Re: [PR] MINOR: Clean up TestUtils.scala [kafka]
Posted by "m1a2st (via GitHub)" <gi...@apache.org>.
m1a2st commented on code in PR #15808:
URL: https://github.com/apache/kafka/pull/15808#discussion_r1584248006
##########
core/src/test/scala/unit/kafka/log/remote/RemoteIndexCacheTest.scala:
##########
@@ -321,16 +322,16 @@ class RemoteIndexCacheTest {
// Give the thread cleaner thread some time to throw an exception
Thread.sleep(100)
// Verify that Cleaner thread is still running even when exception is thrown in doWork()
- var threads = TestUtils.numThreadsRunning(REMOTE_LOG_INDEX_CACHE_CLEANER_THREAD, isDaemon = true)
+ var threads = getRunningCleanerThread
assertEquals(1, threads.size,
- s"Found unexpected ${threads.size} threads=${threads.map(t => t.getName).mkString(", ")}")
+ s"Found unexpected ${threads.size} threads=${threads.stream().map(t => t.getName).collect(Collectors.joining(", "))}")
Review Comment:
fix it, and run `./gradlew clean build -x test -PscalaVersion=2.12` is build successful
--
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
Re: [PR] MINOR: Clean up TestUtils.scala [kafka]
Posted by "chia7712 (via GitHub)" <gi...@apache.org>.
chia7712 commented on code in PR #15808:
URL: https://github.com/apache/kafka/pull/15808#discussion_r1584251393
##########
core/src/test/scala/unit/kafka/log/remote/RemoteIndexCacheTest.scala:
##########
@@ -321,16 +322,16 @@ class RemoteIndexCacheTest {
// Give the thread cleaner thread some time to throw an exception
Thread.sleep(100)
// Verify that Cleaner thread is still running even when exception is thrown in doWork()
- var threads = TestUtils.numThreadsRunning(REMOTE_LOG_INDEX_CACHE_CLEANER_THREAD, isDaemon = true)
+ var threads = getRunningCleanerThread
assertEquals(1, threads.size,
- s"Found unexpected ${threads.size} threads=${threads.map(t => t.getName).mkString(", ")}")
+ s"Found unexpected ${threads.size} threads=${threads.stream().map(t => t.getName).collect(Collectors.joining(", "))}")
Review Comment:
nice job!
--
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
Re: [PR] MINOR: Clean up TestUtils.scala [kafka]
Posted by "m1a2st (via GitHub)" <gi...@apache.org>.
m1a2st commented on code in PR #15808:
URL: https://github.com/apache/kafka/pull/15808#discussion_r1584199554
##########
core/src/test/scala/unit/kafka/log/remote/RemoteIndexCacheTest.scala:
##########
@@ -1086,4 +1087,10 @@ class RemoteIndexCacheTest {
case e @ (_ : NoSuchFileException | _ : UncheckedIOException) => Optional.empty()
}
}
+
+ private def numThreadsRunning(threadNamePrefix: String, isDaemon: Boolean): mutable.Set[Thread] = {
Review Comment:
I change the function name, and modify using java set
```java
private def getRunningCleanerThread: java.util.Set[Thread] = {
Thread.getAllStackTraces.keySet()
.stream()
.filter(t => t.isAlive && t.getName.startsWith(REMOTE_LOG_INDEX_CACHE_CLEANER_THREAD))
.collect(Collectors.toSet())
}
```
--
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
Re: [PR] MINOR: Clean up TestUtils.scala [kafka]
Posted by "m1a2st (via GitHub)" <gi...@apache.org>.
m1a2st commented on code in PR #15808:
URL: https://github.com/apache/kafka/pull/15808#discussion_r1582842857
##########
core/src/test/scala/unit/kafka/utils/TestUtils.scala:
##########
@@ -1124,24 +933,18 @@ object TestUtils extends Logging {
}, msg = msg, pause = 0L, waitTimeMs = waitTimeMs)
}
- /**
- * Wait for the presence of an optional value.
- *
- * @param func The function defining the optional value
- * @param msg Error message in the case that the value never appears
- * @param waitTimeMs Maximum time to wait
- * @return The unwrapped value returned by the function
- */
- def awaitValue[T](func: () => Option[T], msg: => String, waitTimeMs: Long = JTestUtils.DEFAULT_MAX_WAIT_MS): T = {
- var value: Option[T] = None
- waitUntilTrue(() => {
- value = func()
- value.isDefined
- }, msg, waitTimeMs)
- value.get
+ def subscribeAndWaitForRecords(topic: String,
Review Comment:
Other methods refactored by your comment.
--
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
Re: [PR] MINOR: Clean up TestUtils.scala [kafka]
Posted by "m1a2st (via GitHub)" <gi...@apache.org>.
m1a2st commented on PR #15808:
URL: https://github.com/apache/kafka/pull/15808#issuecomment-2078587040
@chia7712 thanls for your comment, I have already change these code by your comment
--
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
Re: [PR] MINOR: Clean up TestUtils.scala [kafka]
Posted by "chia7712 (via GitHub)" <gi...@apache.org>.
chia7712 commented on PR #15808:
URL: https://github.com/apache/kafka/pull/15808#issuecomment-2078361586
@m1a2st Could you please fix the conflicts? thanks!
--
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
Re: [PR] MINOR: Clean up TestUtils.scala [kafka]
Posted by "chia7712 (via GitHub)" <gi...@apache.org>.
chia7712 commented on code in PR #15808:
URL: https://github.com/apache/kafka/pull/15808#discussion_r1584095279
##########
core/src/test/scala/unit/kafka/log/remote/RemoteIndexCacheTest.scala:
##########
@@ -1086,4 +1087,10 @@ class RemoteIndexCacheTest {
case e @ (_ : NoSuchFileException | _ : UncheckedIOException) => Optional.empty()
}
}
+
+ private def numThreadsRunning(threadNamePrefix: String, isDaemon: Boolean): mutable.Set[Thread] = {
Review Comment:
We can fix the build error by rewriting this function via java lambda. For example:
```scala
private def numThreadsRunning(threadNamePrefix: String, isDaemon: Boolean): java.util.Set[Thread] = {
Thread.getAllStackTraces.keySet.stream().filter { t =>
isDaemon && t.isAlive && t.getName.startsWith(threadNamePrefix)
}.collect(Collectors.toSet)
}
```
##########
core/src/test/scala/unit/kafka/log/remote/RemoteIndexCacheTest.scala:
##########
@@ -23,25 +23,26 @@ import org.apache.kafka.common.{TopicIdPartition, TopicPartition, Uuid}
import org.apache.kafka.server.log.remote.storage.RemoteStorageManager.IndexType
import org.apache.kafka.server.log.remote.storage.{RemoteLogSegmentId, RemoteLogSegmentMetadata, RemoteResourceNotFoundException, RemoteStorageManager}
import org.apache.kafka.server.util.MockTime
-import org.apache.kafka.storage.internals.log.RemoteIndexCache.{DIR_NAME, Entry, REMOTE_LOG_INDEX_CACHE_CLEANER_THREAD, remoteDeletedSuffixIndexFileName, remoteOffsetIndexFile, remoteOffsetIndexFileName, remoteTimeIndexFile, remoteTimeIndexFileName, remoteTransactionIndexFile, remoteTransactionIndexFileName}
-import org.apache.kafka.storage.internals.log.{AbortedTxn, CorruptIndexException, LogFileUtils, OffsetIndex, OffsetPosition, RemoteIndexCache, TimeIndex, TransactionIndex}
+import org.apache.kafka.storage.internals.log.RemoteIndexCache._
+import org.apache.kafka.storage.internals.log._
import org.apache.kafka.test.{TestUtils => JTestUtils}
import org.junit.jupiter.api.Assertions._
import org.junit.jupiter.api.{AfterEach, BeforeEach, Test}
import org.junit.jupiter.params.ParameterizedTest
import org.junit.jupiter.params.provider.EnumSource
import org.mockito.ArgumentMatchers
import org.mockito.ArgumentMatchers.any
-import org.mockito.invocation.InvocationOnMock
import org.mockito.Mockito._
+import org.mockito.invocation.InvocationOnMock
import org.slf4j.{Logger, LoggerFactory}
-import java.io.{File, FileInputStream, IOException, PrintWriter, UncheckedIOException}
+import java.io._
Review Comment:
We should avoid using `_` since it could cause naming conflicts.
--
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
Re: [PR] MINOR: Clean up TestUtils.scala [kafka]
Posted by "chia7712 (via GitHub)" <gi...@apache.org>.
chia7712 commented on PR #15808:
URL: https://github.com/apache/kafka/pull/15808#issuecomment-2080226483
@m1a2st Could you please fix conflicts?
--
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
Re: [PR] MINOR: Clean up TestUtils.scala [kafka]
Posted by "m1a2st (via GitHub)" <gi...@apache.org>.
m1a2st commented on PR #15808:
URL: https://github.com/apache/kafka/pull/15808#issuecomment-2081287054
I have been remove these unused import
--
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
Re: [PR] MINOR: Clean up TestUtils.scala [kafka]
Posted by "chia7712 (via GitHub)" <gi...@apache.org>.
chia7712 commented on code in PR #15808:
URL: https://github.com/apache/kafka/pull/15808#discussion_r1580291046
##########
core/src/test/scala/unit/kafka/utils/TestUtils.scala:
##########
@@ -1594,10 +1313,6 @@ object TestUtils extends Logging {
}
}
- def createIsrChangeListener(): MockAlterPartitionListener = {
Review Comment:
`MockAlterPartitionListener` is used by `AbstractPartitionTest` only, so could you move it to `AbstractPartitionTest`?
##########
core/src/test/scala/unit/kafka/admin/DeleteTopicTest.scala:
##########
@@ -548,4 +549,31 @@ class DeleteTopicTest extends QuorumTestHarness {
TestUtils.waitUntilTrue(() => brokers.exists(_.asInstanceOf[KafkaServer].kafkaController.isActive), "No controller is elected")
TestUtils.verifyTopicDeletion(zkClient, topic, 2, brokers)
}
+
+ private def increasePartitions[B <: KafkaBroker](admin: Admin,
+ topic: String,
+ totalPartitionCount: Int,
+ brokersToValidate: Seq[B]
+ ): Unit = {
+
+ try {
+ val newPartitionSet: Map[String, NewPartitions] = Map.apply(topic -> NewPartitions.increaseTo(totalPartitionCount))
+ admin.createPartitions(newPartitionSet.asJava)
+ } catch {
+ case e: ExecutionException =>
+ throw e
+ }
+
+ if (brokersToValidate.nonEmpty) {
+ // wait until we've propagated all partitions metadata to all brokers
+ val allPartitionsMetadata = waitForAllPartitionsMetadata(brokersToValidate, topic, totalPartitionCount)
+
+ (0 until totalPartitionCount - 1).map { i =>
+ i -> allPartitionsMetadata.get(new TopicPartition(topic, i)).map(_.leader()).getOrElse(
+ throw new IllegalStateException(s"Cannot get the partition leader for topic: $topic, partition: $i in server metadata cache"))
Review Comment:
this method return nothing, so we don't need to collect the metadata. However, it needs to check the existence of topic partition
##########
core/src/test/scala/unit/kafka/log/LogSegmentTest.scala:
##########
@@ -613,4 +613,19 @@ class LogSegmentTest {
)
}
+ private def checkEquals[T](s1: java.util.Iterator[T], s2: java.util.Iterator[T]): Unit = {
+ while (s1.hasNext && s2.hasNext)
+ assertEquals(s1.next, s2.next)
+ assertFalse(s1.hasNext, "Iterators have uneven length--first has more")
+ assertFalse(s2.hasNext, "Iterators have uneven length--second has more")
+ }
+
+ private def writeNonsenseToFile(fileName: File, position: Long, size: Int): Unit = {
+ val file = new RandomAccessFile(fileName, "rw")
+ file.seek(position)
+ for (_ <- 0 until size)
+ file.writeByte(random.nextInt(255))
+ file.close()
Review Comment:
could you please add try-finally?
##########
core/src/test/scala/integration/kafka/api/SaslPlainSslEndToEndAuthorizationTest.scala:
##########
@@ -149,6 +150,23 @@ class SaslPlainSslEndToEndAuthorizationTest extends SaslEndToEndAuthorizationTes
*/
@Test
def testAcls(): Unit = {
- TestUtils.verifySecureZkAcls(zkClient, 1)
+ verifySecureZkAcls(zkClient, 1)
+ }
+
+ /**
+ * Verifies that all secure paths in ZK are created with the expected ACL.
+ */
+ private def verifySecureZkAcls(zkClient: KafkaZkClient, usersWithAccess: Int): Unit = {
Review Comment:
ditto
##########
core/src/test/scala/unit/kafka/admin/DeleteTopicTest.scala:
##########
@@ -548,4 +549,31 @@ class DeleteTopicTest extends QuorumTestHarness {
TestUtils.waitUntilTrue(() => brokers.exists(_.asInstanceOf[KafkaServer].kafkaController.isActive), "No controller is elected")
TestUtils.verifyTopicDeletion(zkClient, topic, 2, brokers)
}
+
+ private def increasePartitions[B <: KafkaBroker](admin: Admin,
+ topic: String,
+ totalPartitionCount: Int,
+ brokersToValidate: Seq[B]
+ ): Unit = {
+
+ try {
+ val newPartitionSet: Map[String, NewPartitions] = Map.apply(topic -> NewPartitions.increaseTo(totalPartitionCount))
+ admin.createPartitions(newPartitionSet.asJava)
+ } catch {
Review Comment:
We do nothing for this catch, so it should be fine to remove it.
##########
core/src/test/scala/integration/kafka/api/SaslPlainPlaintextConsumerTest.scala:
##########
@@ -52,6 +56,20 @@ class SaslPlainPlaintextConsumerTest extends BaseConsumerTest with SaslSetup {
*/
@Test
def testZkAclsDisabled(): Unit = {
- TestUtils.verifyUnsecureZkAcls(zkClient)
+ verifyUnsecureZkAcls(zkClient)
+ }
+
+ /**
+ * Verifies that secure paths in ZK have no access control. This is
+ * the case when zookeeper.set.acl=false and no ACLs have been configured.
+ */
+ private def verifyUnsecureZkAcls(zkClient: KafkaZkClient): Unit = {
+ secureZkPaths(zkClient).foreach(path => {
Review Comment:
We can merge this method into test case `testZkAclsDisabled`
--
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
Re: [PR] MINOR: Clean up TestUtils.scala [kafka]
Posted by "chia7712 (via GitHub)" <gi...@apache.org>.
chia7712 commented on PR #15808:
URL: https://github.com/apache/kafka/pull/15808#issuecomment-2084476066
@m1a2st please make sure the build gets work with scala 2.12. You can use following command: `./gradlew clean build -x test -PscalaVersion=2.12`
--
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
Re: [PR] MINOR: Clean up TestUtils.scala [kafka]
Posted by "chia7712 (via GitHub)" <gi...@apache.org>.
chia7712 commented on PR #15808:
URL: https://github.com/apache/kafka/pull/15808#issuecomment-2081116425
```
[2024-04-27T01:29:26.736Z] > Task :core:compileTestScala
[2024-04-27T01:29:26.736Z] [Error] /home/jenkins/workspace/Kafka_kafka-pr_PR-15808/core/src/test/scala/unit/kafka/server/ReplicaManagerTest.scala:20:33: Unused import
[2024-04-27T01:29:26.736Z] [Error] /home/jenkins/workspace/Kafka_kafka-pr_PR-15808/core/src/test/scala/unit/kafka/server/ReplicaManagerTest.scala:20:40: Unused import
[2024-04-27T01:29:26.736Z] [Error] /home/jenkins/workspace/Kafka_kafka-pr_PR-15808/core/src/test/scala/unit/kafka/server/ReplicaManagerTest.scala:25:25: Unused import
[2024-04-27T01:29:26.736Z] [Error] /home/jenkins/workspace/Kafka_kafka-pr_PR-15808/core/src/test/scala/unit/kafka/server/ReplicaManagerTest.scala:31:17: Unused import
[2024-04-27T01:29:26.736Z] [Error] /home/jenkins/workspace/Kafka_kafka-pr_PR-15808/core/src/test/scala/unit/kafka/server/ReplicaManagerTest.scala:34:40: Unused import
[2024-04-27T01:29:26.736Z] [Error] /home/jenkins/workspace/Kafka_kafka-pr_PR-15808/core/src/test/scala/unit/kafka/server/ReplicaManagerTest.scala:34:56: Unused import
[2024-04-27T01:29:26.736Z] [Error] /home/jenkins/workspace/Kafka_kafka-pr_PR-15808/core/src/test/scala/unit/kafka/server/ReplicaManagerTest.scala:53:49: Unused import
[2024-04-27T01:29:26.736Z] [Error] /home/jenkins/workspace/Kafka_kafka-pr_PR-15808/core/src/test/scala/unit/kafka/server/ReplicaManagerTest.scala:58:46: Unused import
[2024-04-27T01:29:26.736Z] [Error] /home/jenkins/workspace/Kafka_kafka-pr_PR-15808/core/src/test/scala/unit/kafka/server/ReplicaManagerTest.scala:58:62: Unused import
[2024-04-27T01:29:26.736Z] [Error] /home/jenkins/workspace/Kafka_kafka-pr_PR-15808/core/src/test/scala/unit/kafka/server/ReplicaManagerTest.scala:58:86: Unused import
[2024-04-27T01:29:26.736Z] [Error] /home/jenkins/workspace/Kafka_kafka-pr_PR-15808/core/src/test/scala/unit/kafka/server/ReplicaManagerTest.scala:58:109: Unused import
[2024-04-27T01:29:26.736Z] [Error] /home/jenkins/workspace/Kafka_kafka-pr_PR-15808/core/src/test/scala/unit/kafka/server/ReplicaManagerTest.scala:59:33: Unused import
[2024-04-27T01:29:26.736Z] [Error] /home/jenkins/workspace/Kafka_kafka-pr_PR-15808/core/src/test/scala/unit/kafka/server/ReplicaManagerTest.scala:60:30: Unused import
[2024-04-27T01:29:26.736Z] [Error] /home/jenkins/workspace/Kafka_kafka-pr_PR-15808/core/src/test/scala/unit/kafka/server/ReplicaManagerTest.scala:63:40: Unused import
[2024-04-27T01:29:26.736Z] [Error] /home/jenkins/workspace/Kafka_kafka-pr_PR-15808/core/src/test/scala/unit/kafka/server/ReplicaManagerTest.scala:63:60: Unused import
[2024-04-27T01:29:26.736Z] [Error] /home/jenkins/workspace/Kafka_kafka-pr_PR-15808/core/src/test/scala/unit/kafka/server/ReplicaManagerTest.scala:66:43: Unused import
[2024-04-27T01:29:26.736Z] [Error] /home/jenkins/workspace/Kafka_kafka-pr_PR-15808/core/src/test/scala/unit/kafka/server/ReplicaManagerTest.scala:68:54: Unused import
[2024-04-27T01:29:28.026Z] [Error] /home/jenkins/workspace/Kafka_kafka-pr_PR-15808/core/src/test/scala/unit/kafka/utils/TestUtils.scala:61:33: Unused import
[2024-04-27T01:29:28.026Z] [Error] /home/jenkins/workspace/Kafka_kafka-pr_PR-15808/core/src/test/scala/unit/kafka/utils/TestUtils.scala:61:55: Unused import
[2024-04-27T01:29:28.026Z] [Error] /home/jenkins/workspace/Kafka_kafka-pr_PR-15808/core/src/test/scala/unit/kafka/utils/TestUtils.scala:63:54: Unused import
[2024-04-27T01:29:28.026Z] [Error] /home/jenkins/workspace/Kafka_kafka-pr_PR-15808/core/src/test/scala/unit/kafka/utils/TestUtils.scala:63:74: Unused import
[2024-04-27T01:29:28.026Z] [Error] /home/jenkins/workspace/Kafka_kafka-pr_PR-15808/core/src/test/scala/unit/kafka/utils/TestUtils.scala:63:92: Unused import
[2024-04-27T01:29:28.026Z] [Error] /home/jenkins/workspace/Kafka_kafka-pr_PR-15808/core/src/test/scala/unit/kafka/utils/TestUtils.scala:63:106: Unused import
[2024-04-27T01:29:29.239Z] 23 errors found
```
please fix the build error :)
--
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
Re: [PR] MINOR: Clean up TestUtils.scala [kafka]
Posted by "m1a2st (via GitHub)" <gi...@apache.org>.
m1a2st commented on code in PR #15808:
URL: https://github.com/apache/kafka/pull/15808#discussion_r1582841761
##########
core/src/test/scala/unit/kafka/utils/TestUtils.scala:
##########
@@ -1124,24 +933,18 @@ object TestUtils extends Logging {
}, msg = msg, pause = 0L, waitTimeMs = waitTimeMs)
}
- /**
- * Wait for the presence of an optional value.
- *
- * @param func The function defining the optional value
- * @param msg Error message in the case that the value never appears
- * @param waitTimeMs Maximum time to wait
- * @return The unwrapped value returned by the function
- */
- def awaitValue[T](func: () => Option[T], msg: => String, waitTimeMs: Long = JTestUtils.DEFAULT_MAX_WAIT_MS): T = {
- var value: Option[T] = None
- waitUntilTrue(() => {
- value = func()
- value.isDefined
- }, msg, waitTimeMs)
- value.get
+ def subscribeAndWaitForRecords(topic: String,
Review Comment:
`assertBadConfigContainingMessage` this method are used by two class, so I didn't move it.
The `kafka.KafkaConfigTest` and `server.KafkaConfigTest` are using it
--
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
Re: [PR] MINOR: Clean up TestUtils.scala [kafka]
Posted by "chia7712 (via GitHub)" <gi...@apache.org>.
chia7712 commented on code in PR #15808:
URL: https://github.com/apache/kafka/pull/15808#discussion_r1584212773
##########
core/src/test/scala/unit/kafka/log/remote/RemoteIndexCacheTest.scala:
##########
@@ -321,16 +322,16 @@ class RemoteIndexCacheTest {
// Give the thread cleaner thread some time to throw an exception
Thread.sleep(100)
// Verify that Cleaner thread is still running even when exception is thrown in doWork()
- var threads = TestUtils.numThreadsRunning(REMOTE_LOG_INDEX_CACHE_CLEANER_THREAD, isDaemon = true)
+ var threads = getRunningCleanerThread
assertEquals(1, threads.size,
- s"Found unexpected ${threads.size} threads=${threads.map(t => t.getName).mkString(", ")}")
+ s"Found unexpected ${threads.size} threads=${threads.stream().map(t => t.getName).collect(Collectors.joining(", "))}")
Review Comment:
`threads.stream().map[String](t => t.getName).collect(Collectors.joining(", "))`
You can define the `type` explicitly to fix the type mismatch in scala 2.12
--
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
Re: [PR] MINOR: Clean up TestUtils.scala [kafka]
Posted by "m1a2st (via GitHub)" <gi...@apache.org>.
m1a2st commented on PR #15808:
URL: https://github.com/apache/kafka/pull/15808#issuecomment-2078376955
@chia7712 I resolve the conflicts
--
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