You are viewing a plain text version of this content. The canonical link for it is here.
Posted to jira@kafka.apache.org by GitBox <gi...@apache.org> on 2022/12/09 06:55:46 UTC
[GitHub] [kafka] vamossagar12 opened a new pull request, #12971: KAFKA-14454: Making unique StreamsConfig for tests
vamossagar12 opened a new pull request, #12971:
URL: https://github.com/apache/kafka/pull/12971
Newly added test KTableKTableForeignKeyInnerJoinCustomPartitionerIntegrationTest#shouldThrowIllegalArgumentExceptionWhenCustomPartionerReturnsMultiplePartitions as part of KIP-837 passes when run individually but fails when is part of IT class and hence is marked as Ignored.
That seemed to have been because of the way StreamsConfig was being initialised so any new test would have used the same names. Because of which the second test never got to the desired state. With this PR, every test gets a unique app name which seems to have fixed the issue. Also, a couple of cosmetic changes
--
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
[GitHub] [kafka] ableegoldman commented on a diff in pull request #12971: KAFKA-14454: Making unique StreamsConfig for tests
Posted by GitBox <gi...@apache.org>.
ableegoldman commented on code in PR #12971:
URL: https://github.com/apache/kafka/pull/12971#discussion_r1044153926
##########
streams/src/test/java/org/apache/kafka/streams/integration/KTableKTableForeignKeyInnerJoinCustomPartitionerIntegrationTest.java:
##########
@@ -196,19 +202,16 @@ public void shouldThrowIllegalArgumentExceptionWhenCustomPartionerReturnsMultipl
for (final KafkaStreams stream: kafkaStreamsList) {
stream.setUncaughtExceptionHandler(e -> {
- assertEquals("The partitions returned by StreamPartitioner#partitions method when used for FK join should be a singleton set", e.getCause().getMessage());
+ Assertions.assertEquals("The partitions returned by StreamPartitioner#partitions method when used for FK join should be a singleton set", e.getCause().getMessage());
return StreamsUncaughtExceptionHandler.StreamThreadExceptionResponse.SHUTDOWN_CLIENT;
});
}
startApplicationAndWaitUntilRunning(kafkaStreamsList, ofSeconds(120));
- // Sleeping to let the processing happen inducing the failure
- Thread.sleep(60000);
+ // the streams applications should in failed state due to the IllegalStateException.
+ waitForApplicationState(Arrays.asList(streams, streamsTwo, streamsThree), KafkaStreams.State.ERROR, ofSeconds(30));
Review Comment:
nit: no idea if we actually used the full 60s that we used to sleep, but let's continue to give it 60s
##########
streams/src/test/java/org/apache/kafka/streams/integration/KTableKTableForeignKeyInnerJoinCustomPartitionerIntegrationTest.java:
##########
@@ -196,19 +202,16 @@ public void shouldThrowIllegalArgumentExceptionWhenCustomPartionerReturnsMultipl
for (final KafkaStreams stream: kafkaStreamsList) {
stream.setUncaughtExceptionHandler(e -> {
- assertEquals("The partitions returned by StreamPartitioner#partitions method when used for FK join should be a singleton set", e.getCause().getMessage());
+ Assertions.assertEquals("The partitions returned by StreamPartitioner#partitions method when used for FK join should be a singleton set", e.getCause().getMessage());
return StreamsUncaughtExceptionHandler.StreamThreadExceptionResponse.SHUTDOWN_CLIENT;
});
}
startApplicationAndWaitUntilRunning(kafkaStreamsList, ofSeconds(120));
- // Sleeping to let the processing happen inducing the failure
- Thread.sleep(60000);
+ // the streams applications should in failed state due to the IllegalStateException.
Review Comment:
```suggestion
// the streams applications should have shut down into `ERROR` due to the IllegalStateException
```
--
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
[GitHub] [kafka] ableegoldman commented on pull request #12971: KAFKA-14454: Making unique StreamsConfig for tests
Posted by GitBox <gi...@apache.org>.
ableegoldman commented on PR #12971:
URL: https://github.com/apache/kafka/pull/12971#issuecomment-1344973861
Merged to trunk and cherrypicked to 3.4
--
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
[GitHub] [kafka] vamossagar12 commented on pull request #12971: KAFKA-14454: Making unique StreamsConfig for tests
Posted by GitBox <gi...@apache.org>.
vamossagar12 commented on PR #12971:
URL: https://github.com/apache/kafka/pull/12971#issuecomment-1343979588
> Looks like Commit failed after one of the suggestions. Would push a fix for that in sometime.
Pushed a fix and tests seemed to have passed locally.
--
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
[GitHub] [kafka] ableegoldman commented on a diff in pull request #12971: KAFKA-14454: Making unique StreamsConfig for tests
Posted by GitBox <gi...@apache.org>.
ableegoldman commented on code in PR #12971:
URL: https://github.com/apache/kafka/pull/12971#discussion_r1044152824
##########
streams/src/test/java/org/apache/kafka/streams/integration/KTableKTableForeignKeyInnerJoinCustomPartitionerIntegrationTest.java:
##########
@@ -196,19 +202,16 @@ public void shouldThrowIllegalArgumentExceptionWhenCustomPartionerReturnsMultipl
for (final KafkaStreams stream: kafkaStreamsList) {
stream.setUncaughtExceptionHandler(e -> {
- assertEquals("The partitions returned by StreamPartitioner#partitions method when used for FK join should be a singleton set", e.getCause().getMessage());
+ Assertions.assertEquals("The partitions returned by StreamPartitioner#partitions method when used for FK join should be a singleton set", e.getCause().getMessage());
Review Comment:
We've been moving to `assertThat` instead of `assertEquals` and co:
```suggestion
assertThat(e.getCause().getMessage(), equalTo("The partitions returned by StreamPartitioner#partitions method when used for FK join should be a singleton set"));
```
--
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
[GitHub] [kafka] sagarrao12 commented on pull request #12971: KAFKA-14454: Making unique StreamsConfig for tests
Posted by GitBox <gi...@apache.org>.
sagarrao12 commented on PR #12971:
URL: https://github.com/apache/kafka/pull/12971#issuecomment-1344187051
@ableegoldman , the failed tests seem unrelated to this change.
--
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
[GitHub] [kafka] vamossagar12 commented on pull request #12971: KAFKA-14454: Making unique StreamsConfig for tests
Posted by GitBox <gi...@apache.org>.
vamossagar12 commented on PR #12971:
URL: https://github.com/apache/kafka/pull/12971#issuecomment-1343960862
Looks like Commit failed after one of the suggestions. Would push a fix for that in sometime.
--
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
[GitHub] [kafka] ableegoldman merged pull request #12971: KAFKA-14454: Making unique StreamsConfig for tests
Posted by GitBox <gi...@apache.org>.
ableegoldman merged PR #12971:
URL: https://github.com/apache/kafka/pull/12971
--
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