Skip to content

MINOR: Cleanup TestUtils.scala#22091

Open
m1a2st wants to merge 8 commits intoapache:trunkfrom
m1a2st:MINOR-cleanup-testutils-scala
Open

MINOR: Cleanup TestUtils.scala#22091
m1a2st wants to merge 8 commits intoapache:trunkfrom
m1a2st:MINOR-cleanup-testutils-scala

Conversation

@m1a2st
Copy link
Copy Markdown
Collaborator

@m1a2st m1a2st commented Apr 18, 2026

Some methods are only used in a single class; they should be moved into
that class. Java tests should not use TestUtils.scala

Reviewers: Chia-Ping Tsai chia7712@gmail.com

@github-actions github-actions Bot added triage PRs from the community core Kafka Broker tools tests Test fixes (including flaky tests) labels Apr 18, 2026
TestUtils.waitForCondition(
() -> {
try {
TopicDescription topicDescription = client.describeTopics(List.of(topic))
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we use listTopics instead of handling UnknownTopicOrPartitionException?

@github-actions github-actions Bot removed the triage PRs from the community label Apr 19, 2026
// the last (active) segment has just one message

def distinctValuesBySegment = log.logSegments.asScala.map(s => s.log.records.asScala.map(record => TestUtils.readString(record.value)).toSet.size).toSeq
def distinctValuesBySegment = log.logSegments.asScala.map(s => s.log.records.asScala.map(record => LogTestUtils.readString(record.value)).toSet.size).toSeq
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

def distinctValuesBySegment = log.logSegments.asScala.map(s => s.log.records.asScala.map(record => StandardCharsets.UTF_8.decode(record.value()).toString).toSet.size).toSeq

for (segment <- log.logSegments.asScala; batch <- segment.log.batches.asScala; record <- batch.asScala) {
assertTrue(record.hasMagic(batch.magic))
val value = TestUtils.readString(record.value).toLong
val value = LogTestUtils.readString(record.value).toLong
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

 val value = StandardCharsets.UTF_8.decode(record.value()).toString.toLong

for (logEntry <- records.records.asScala) {
val offset = logEntry.offset
val value = TestUtils.readString(logEntry.value).toLong
val value = LogTestUtils.readString(logEntry.value).toLong
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

val value = StandardCharsets.UTF_8.decode(logEntry.value()).toString.toLong

* @param buffer The buffer to translate
* @param encoding The encoding to use in translating bytes to characters
*/
def readString(buffer: ByteBuffer, encoding: String = Charset.defaultCharset.toString): String = {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks a bit verbose. Please see my other comments for suggestions on how to remove it.

addedConfigs.put("min.insync.replicas", "2");
if (file) {
File f = kafka.utils.TestUtils.tempPropertiesFile(CollectionConverters.asScala(addedConfigs));
File f = tempPropertiesFile(addedConfigs);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It could be replaced by ToolsTestUtils#tempPropertiesFile

@m1a2st m1a2st force-pushed the MINOR-cleanup-testutils-scala branch from 6a0dfd2 to 8a0997b Compare April 23, 2026 01:26
# Conflicts:
#	tools/src/test/java/org/apache/kafka/tools/LeaderElectionCommandTest.java
@m1a2st m1a2st force-pushed the MINOR-cleanup-testutils-scala branch from 8a0997b to c2ccb7e Compare April 23, 2026 01:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

core Kafka Broker tests Test fixes (including flaky tests) tools

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants