You are viewing a plain text version of this content. The canonical link for it is here.
Posted to jira@kafka.apache.org by "mimaison (via GitHub)" <gi...@apache.org> on 2026/08/17 14:12:28 UTC
[PR] KAFKA-20949: Remove hamcrest from org.apache.kafka.streams.query package [kafka]
mimaison opened a new pull request, #23177:
URL: https://github.com/apache/kafka/pull/23177
Migrate usages of hamcrest to junit
--
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] KAFKA-20949: Remove hamcrest from org.apache.kafka.streams.query package [kafka]
Posted by "mimaison (via GitHub)" <gi...@apache.org>.
mimaison commented on PR #23177:
URL: https://github.com/apache/kafka/pull/23177#issuecomment-5428283369
Thanks for the review, I pushed an update
--
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] KAFKA-20949: Remove hamcrest from org.apache.kafka.streams.query package [kafka]
Posted by "chia7712 (via GitHub)" <gi...@apache.org>.
chia7712 commented on code in PR #23177:
URL: https://github.com/apache/kafka/pull/23177#discussion_r3872234783
##########
streams/src/test/java/org/apache/kafka/streams/query/PositionTest.java:
##########
@@ -213,7 +211,7 @@ public void shouldNotMatchOnUnEqual() {
@Test
public void shouldNotMatchNull() {
final Position position = Position.emptyPosition();
- assertNotEquals(position, null);
+ assertNotNull(position);
Review Comment:
It was used to verify the behavior of `position.equals(null)`
--
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] KAFKA-20949: Remove hamcrest from org.apache.kafka.streams.query package [kafka]
Posted by "chia7712 (via GitHub)" <gi...@apache.org>.
chia7712 commented on PR #23177:
URL: https://github.com/apache/kafka/pull/23177#issuecomment-5430386583
```
> 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
--
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] KAFKA-20949: Remove hamcrest from org.apache.kafka.streams.query package [kafka]
Posted by "chia7712 (via GitHub)" <gi...@apache.org>.
chia7712 commented on code in PR #23177:
URL: https://github.com/apache/kafka/pull/23177#discussion_r3846629253
##########
streams/src/test/java/org/apache/kafka/streams/query/PositionBoundTest.java:
##########
@@ -78,7 +75,7 @@ public void shouldEqualSelf() {
@Test
public void shouldNotEqualNull() {
final PositionBound bound1 = PositionBound.at(Position.emptyPosition());
- assertNotEquals(bound1, null);
+ assertNotNull(bound1);
Review Comment:
This test is used to verify `equals(null)`, so it seems we should keep it as it was.
##########
streams/src/test/java/org/apache/kafka/streams/query/PositionTest.java:
##########
@@ -59,24 +57,24 @@ public void shouldCreateFromMap() {
);
final Position position = Position.fromMap(map);
- assertThat(position.getTopics(), equalTo(Set.of("topic", "topic1")));
- assertThat(position.getPartitionPositions("topic"), equalTo(mkMap(mkEntry(0, 5L))));
+ assertEquals(Set.of("topic", "topic1"), position.getTopics());
+ assertEquals(mkMap(mkEntry(0, 5L)), position.getPartitionPositions("topic"));
// Should be a copy of the constructor map
map.get("topic1").put(99, 99L);
// so the position is still the original one
- assertThat(position.getPartitionPositions("topic1"), equalTo(mkMap(
- mkEntry(0, 5L),
- mkEntry(7, 0L)
- )));
+ assertEquals(mkMap(
+ mkEntry(0, 5L),
Review Comment:
ditto
##########
streams/src/test/java/org/apache/kafka/streams/query/PositionTest.java:
##########
@@ -165,12 +163,12 @@ public void shouldMergeNull() {
final Position merged = position.merge(null);
- assertThat(merged.getTopics(), equalTo(Set.of("topic", "topic1")));
- assertThat(merged.getPartitionPositions("topic"), equalTo(mkMap(mkEntry(0, 5L))));
- assertThat(merged.getPartitionPositions("topic1"), equalTo(mkMap(
- mkEntry(0, 5L),
- mkEntry(7, 0L)
- )));
+ assertEquals(Set.of("topic", "topic1"), merged.getTopics());
+ assertEquals(mkMap(mkEntry(0, 5L)), merged.getPartitionPositions("topic"));
+ assertEquals(mkMap(
+ mkEntry(0, 5L),
Review Comment:
Please remove redundant indent
##########
streams/src/test/java/org/apache/kafka/streams/query/PositionTest.java:
##########
@@ -135,22 +133,22 @@ public void shouldCopy() {
position.withComponent("topic2", 2, 4L);
// copy has not changed
- assertThat(copy.getTopics(), equalTo(Set.of("topic", "topic1")));
- assertThat(copy.getPartitionPositions("topic"), equalTo(mkMap(mkEntry(0, 5L))));
- assertThat(copy.getPartitionPositions("topic1"), equalTo(mkMap(
- mkEntry(0, 5L),
- mkEntry(7, 0L)
- )));
+ assertEquals(Set.of("topic", "topic1"), copy.getTopics());
+ assertEquals(mkMap(mkEntry(0, 5L)), copy.getPartitionPositions("topic"));
+ assertEquals(mkMap(
+ mkEntry(0, 5L),
+ mkEntry(7, 0L)
+ ), copy.getPartitionPositions("topic1"));
// original has changed
- assertThat(position.getTopics(), equalTo(Set.of("topic", "topic1", "topic2")));
- assertThat(position.getPartitionPositions("topic"), equalTo(mkMap(mkEntry(0, 6L))));
- assertThat(position.getPartitionPositions("topic1"), equalTo(mkMap(
- mkEntry(0, 5L),
- mkEntry(7, 0L),
- mkEntry(8, 1L)
- )));
- assertThat(position.getPartitionPositions("topic2"), equalTo(mkMap(mkEntry(2, 4L))));
+ assertEquals(Set.of("topic", "topic1", "topic2"), position.getTopics());
+ assertEquals(mkMap(mkEntry(0, 6L)), position.getPartitionPositions("topic"));
+ assertEquals(mkMap(
+ mkEntry(0, 5L),
Review Comment:
ditto
##########
streams/src/test/java/org/apache/kafka/streams/query/PositionTest.java:
##########
@@ -97,24 +95,24 @@ public void shouldMerge() {
final Position merged = position.merge(position1);
- assertThat(merged.getTopics(), equalTo(Set.of("topic", "topic1", "topic2")));
- assertThat(merged.getPartitionPositions("topic"), equalTo(mkMap(mkEntry(0, 7L))));
- assertThat(merged.getPartitionPositions("topic1"), equalTo(mkMap(
- mkEntry(0, 5L),
- mkEntry(7, 0L),
- mkEntry(8, 1L)
- )));
- assertThat(merged.getPartitionPositions("topic2"), equalTo(mkMap(mkEntry(9, 5L))));
+ assertEquals(Set.of("topic", "topic1", "topic2"), merged.getTopics());
+ assertEquals(mkMap(mkEntry(0, 7L)), merged.getPartitionPositions("topic"));
+ assertEquals(mkMap(
+ mkEntry(0, 5L),
Review Comment:
ditto
##########
streams/src/test/java/org/apache/kafka/streams/query/PositionTest.java:
##########
@@ -135,22 +133,22 @@ public void shouldCopy() {
position.withComponent("topic2", 2, 4L);
// copy has not changed
- assertThat(copy.getTopics(), equalTo(Set.of("topic", "topic1")));
- assertThat(copy.getPartitionPositions("topic"), equalTo(mkMap(mkEntry(0, 5L))));
- assertThat(copy.getPartitionPositions("topic1"), equalTo(mkMap(
- mkEntry(0, 5L),
- mkEntry(7, 0L)
- )));
+ assertEquals(Set.of("topic", "topic1"), copy.getTopics());
+ assertEquals(mkMap(mkEntry(0, 5L)), copy.getPartitionPositions("topic"));
+ assertEquals(mkMap(
+ mkEntry(0, 5L),
Review Comment:
ditto
--
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] KAFKA-20949: Remove hamcrest from org.apache.kafka.streams.query package [kafka]
Posted by "mimaison (via GitHub)" <gi...@apache.org>.
mimaison commented on PR #23177:
URL: https://github.com/apache/kafka/pull/23177#issuecomment-5436078779
My bad, updated
--
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] KAFKA-20949: Remove hamcrest from org.apache.kafka.streams.query package [kafka]
Posted by "chia7712 (via GitHub)" <gi...@apache.org>.
chia7712 merged PR #23177:
URL: https://github.com/apache/kafka/pull/23177
--
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] KAFKA-20949: Remove hamcrest from org.apache.kafka.streams.query package [kafka]
Posted by "chia7712 (via GitHub)" <gi...@apache.org>.
chia7712 commented on code in PR #23177:
URL: https://github.com/apache/kafka/pull/23177#discussion_r3881728922
##########
streams/src/test/java/org/apache/kafka/streams/query/PositionBoundTest.java:
##########
@@ -78,7 +75,7 @@ public void shouldEqualSelf() {
@Test
public void shouldNotEqualNull() {
final PositionBound bound1 = PositionBound.at(Position.emptyPosition());
- assertNotEquals(bound1, null);
+ assertNotEquals(null, bound1);
Review Comment:
It should be `assertNotEquals(bound1, null);`
##########
streams/src/test/java/org/apache/kafka/streams/query/PositionTest.java:
##########
@@ -213,7 +210,7 @@ public void shouldNotMatchOnUnEqual() {
@Test
public void shouldNotMatchNull() {
final Position position = Position.emptyPosition();
- assertNotEquals(position, null);
+ assertNotEquals(null, position);
Review Comment:
It should be `assertNotEquals(position, null);`
--
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