mjsax commented on a change in pull request #9720:
URL: https://github.com/apache/kafka/pull/9720#discussion_r562200891
##########
File path:
streams/src/test/java/org/apache/kafka/streams/integration/EosBetaUpgradeIntegrationTest.java
##########
@@ -978,7 +971,7 @@ private void waitForRunning(final
List<KeyValue<KafkaStreams.State, KafkaStreams
waitForCondition(
() -> !observed.isEmpty() && observed.get(observed.size() -
1).value.equals(State.RUNNING),
MAX_WAIT_TIME_MS,
- () -> "Client did not startup on time. Observers transitions: " +
observed
+ () -> "Client did not have the expected state transition on time.
Observers transitions: " + observed
Review comment:
Why this change? We do wait for `RUNNING`?
##########
File path:
streams/src/test/java/org/apache/kafka/streams/integration/EosBetaUpgradeIntegrationTest.java
##########
@@ -993,6 +986,17 @@ private void waitForStateTransition(final
List<KeyValue<KafkaStreams.State, Kafk
);
}
+ private void waitForStateTransitionContains(final
List<KeyValue<KafkaStreams.State, KafkaStreams.State>> observed,
+ final
List<KeyValue<KafkaStreams.State, KafkaStreams.State>> expected)
+ throws Exception {
+
+ waitForCondition(
+ () -> observed.containsAll(expected),
+ MAX_WAIT_TIME_MS,
+ () -> "Client did not have the expected state transition on time.
Observers transitions: " + observed
Review comment:
Can we add the expected transitions, too? Easier to debug if the test
fails.
##########
File path:
streams/src/test/java/org/apache/kafka/streams/integration/StandbyTaskEOSIntegrationTest.java
##########
@@ -174,8 +177,8 @@ private KafkaStreams buildStreamWithDirtyStateDir(final
String stateDirPath,
}
@Test
- @Deprecated
public void shouldWipeOutStandbyStateDirectoryIfCheckpointIsMissing()
throws Exception {
+ final long time = System.currentTimeMillis();
Review comment:
The PR you liked seems to be unrelated to this test.
Still wondering if we should extract this change to a dedicated PR and
cherry-pick to older branches? -- Or do we have a good explanation why older
branches would not be affected?
##########
File path:
streams/src/test/java/org/apache/kafka/streams/integration/EosBetaUpgradeIntegrationTest.java
##########
@@ -993,6 +986,17 @@ private void waitForStateTransition(final
List<KeyValue<KafkaStreams.State, Kafk
);
}
+ private void waitForStateTransitionContains(final
List<KeyValue<KafkaStreams.State, KafkaStreams.State>> observed,
+ final
List<KeyValue<KafkaStreams.State, KafkaStreams.State>> expected)
Review comment:
nit: fix indention
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
[email protected]
