Skip to content

MINOR: Remove unused methods in MockApiFixedKeyProcessor - #23287

Merged
m1a2st merged 3 commits into
apache:trunkfrom
hysuuuu:KAFKA-20992
Aug 28, 2026
Merged

MINOR: Remove unused methods in MockApiFixedKeyProcessor#23287
m1a2st merged 3 commits into
apache:trunkfrom
hysuuuu:KAFKA-20992

Conversation

@hysuuuu

@hysuuuu hysuuuu commented Aug 26, 2026

Copy link
Copy Markdown
Contributor

Remove unused methods in MockApiFixedKeyProcessor.

Testing ./gradlew streams:test --tests StreamsBuilderTest --tests KStreamImplTest

Reviewers: Ken Huang s7133700@gmail.com, Chia-Ping Tsai
chia7712@gmail.com

@github-actions github-actions Bot added triage PRs from the community streams tests Test fixes (including flaky tests) small Small PRs labels Aug 26, 2026

@m1a2st m1a2st left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks for the patch, some minor comments


public void checkAndClearProcessResult(final KeyValueTimestamp<?, ?>... expected) {
assertThat("the number of outputs:" + processed, processed.size(), is(expected.length));
assertEquals(expected.length, processed.size(), () -> "the number of outputs:" + processed);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
assertEquals(expected.length, processed.size(), () -> "the number of outputs:" + processed);
assertEquals(expected.length, processed.size(), "the number of outputs:" + processed);


public void checkEmptyAndClearProcessResult() {
assertThat("the number of outputs:", processed.size(), is(0));
assertEquals(0, processed.size(), "the number of outputs:");

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
assertEquals(0, processed.size(), "the number of outputs:");
assertEquals(0, processed.size(), "the number of outputs:" + processed);

public void checkAndClearPunctuateResult(final PunctuationType type, final long... expected) {
final ArrayList<Long> punctuated = type == PunctuationType.STREAM_TIME ? punctuatedStreamTime : punctuatedSystemTime;
assertThat("the number of outputs:", punctuated.size(), is(expected.length));
assertEquals(expected.length, punctuated.size(), "the number of outputs:");

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

ditto


public void checkAndClearProcessedRecords(final Record<?, ?>... expected) {
assertThat("the number of outputs:" + processed, processed.size(), is(expected.length));
assertEquals(expected.length, processed.size(), () -> "the number of outputs:" + processed);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
assertEquals(expected.length, processed.size(), () -> "the number of outputs:" + processed);
assertEquals(expected.length, processed.size(), "the number of outputs:" + processed);

@hysuuuu

hysuuuu commented Aug 27, 2026

Copy link
Copy Markdown
Contributor Author

@m1a2st Updated. PTAL, thanks!

@github-actions github-actions Bot removed the triage PRs from the community label Aug 27, 2026
@@ -93,23 +92,23 @@ public void process(final FixedKeyRecord<KIn, VIn> record) {
}

public void checkAndClearProcessResult(final KeyValueTimestamp<?, ?>... expected) {

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.

I checked the usages and neither method is called anywhere, so we could just remove them, right?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yes you're right, those methods are not used. I've removed them. Thanks.

@m1a2st m1a2st left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks @hysuuuu for the update, could you resolve the conflict

@hysuuuu

hysuuuu commented Aug 28, 2026

Copy link
Copy Markdown
Contributor Author

@m1a2st Of course. I've resolved the conflict. PTAL, thanks!

@hysuuuu hysuuuu changed the title KAFKA-20992: Remove hamcrest from org.apache.kafka.test package MINOR: Remove unused methods in MockApiFixedKeyProcessor Aug 28, 2026

@m1a2st m1a2st left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks, LGTM

@m1a2st
m1a2st merged commit 14019b9 into apache:trunk Aug 28, 2026
23 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ci-approved small Small PRs streams tests Test fixes (including flaky tests)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants