Skip to content

KAFKA-20949: Remove hamcrest from org.apache.kafka.streams.query package - #23177

Merged
chia7712 merged 5 commits into
apache:trunkfrom
mimaison:kafka-20949
Aug 31, 2026
Merged

KAFKA-20949: Remove hamcrest from org.apache.kafka.streams.query package#23177
chia7712 merged 5 commits into
apache:trunkfrom
mimaison:kafka-20949

Conversation

@mimaison

@mimaison mimaison commented Aug 17, 2026

Copy link
Copy Markdown
Member

Migrate usages of hamcrest to junit

Reviewers: Chia-Ping Tsai chia7712@gmail.com

@github-actions github-actions Bot added streams tests Test fixes (including flaky tests) labels Aug 17, 2026

@chia7712 chia7712 left a comment

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.

@mimaison thanks for this patch. A couple of comments left

public void shouldNotEqualNull() {
final PositionBound bound1 = PositionBound.at(Position.emptyPosition());
assertNotEquals(bound1, null);
assertNotNull(bound1);

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 test is used to verify equals(null), so it seems we should keep it as it was.

assertEquals(Set.of("topic", "topic1"), merged.getTopics());
assertEquals(mkMap(mkEntry(0, 5L)), merged.getPartitionPositions("topic"));
assertEquals(mkMap(
mkEntry(0, 5L),

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.

Please remove redundant indent

assertEquals(Set.of("topic", "topic1", "topic2"), position.getTopics());
assertEquals(mkMap(mkEntry(0, 6L)), position.getPartitionPositions("topic"));
assertEquals(mkMap(
mkEntry(0, 5L),

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.

ditto

mkEntry(7, 0L)
)));
assertEquals(mkMap(
mkEntry(0, 5L),

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.

ditto

assertEquals(Set.of("topic", "topic1", "topic2"), merged.getTopics());
assertEquals(mkMap(mkEntry(0, 7L)), merged.getPartitionPositions("topic"));
assertEquals(mkMap(
mkEntry(0, 5L),

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.

ditto

assertEquals(Set.of("topic", "topic1"), copy.getTopics());
assertEquals(mkMap(mkEntry(0, 5L)), copy.getPartitionPositions("topic"));
assertEquals(mkMap(
mkEntry(0, 5L),

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.

ditto

@github-actions github-actions Bot added the small Small PRs label Aug 26, 2026
@mimaison

Copy link
Copy Markdown
Member Author

Thanks for the review, I pushed an update

@chia7712

Copy link
Copy Markdown
Member
> Task :streams:checkstyleTest
[ant:checkstyle] [ERROR] /home/chia7712/project/kafka/streams/src/test/java/org/apache/kafka/streams/query/PositionBoundTest.java:28:15: Unused import - org.junit.jupiter.api.Assertions.assertNotNull. [UnusedImports]

@mimaison please push a fix for the build failure

@mimaison

Copy link
Copy Markdown
Member Author

My bad, updated

public void shouldNotMatchNull() {
final Position position = Position.emptyPosition();
assertNotEquals(position, null);
assertNotNull(position);

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 was used to verify the behavior of position.equals(null)

public void shouldNotEqualNull() {
final PositionBound bound1 = PositionBound.at(Position.emptyPosition());
assertNotEquals(bound1, null);
assertNotEquals(null, bound1);

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 should be assertNotEquals(bound1, null);

public void shouldNotMatchNull() {
final Position position = Position.emptyPosition();
assertNotEquals(position, null);
assertNotEquals(null, position);

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 should be assertNotEquals(position, null);

@chia7712 chia7712 left a comment

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.

LGTM

@chia7712
chia7712 merged commit a0f9558 into apache:trunk Aug 31, 2026
22 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

small Small PRs streams tests Test fixes (including flaky tests)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants