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 2020/10/13 18:22:03 UTC

[GitHub] [kafka] vvcephei opened a new pull request #9426: MINOR: Fix flaky shouldRejectNonExistentStoreName

vvcephei opened a new pull request #9426:
URL: https://github.com/apache/kafka/pull/9426


   Fix flaky test by making sure Streams is
   running before making assertions about IQ.
   
   ### Committer Checklist (excluded from commit message)
   - [ ] Verify design and implementation 
   - [ ] Verify test coverage and CI build status
   - [ ] Verify documentation (including upgrade notes)
   


----------------------------------------------------------------
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:
users@infra.apache.org

[GitHub] [kafka] guozhangwang commented on pull request #9426: MINOR: Fix flaky shouldRejectNonExistentStoreName

Posted by GitBox <gi...@apache.org>.
guozhangwang commented on pull request #9426:
URL: https://github.com/apache/kafka/pull/9426#issuecomment-710289562


   LGTM！


----------------------------------------------------------------
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:
users@infra.apache.org

[GitHub] [kafka] vvcephei commented on pull request #9426: MINOR: Fix flaky shouldRejectNonExistentStoreName

Posted by GitBox <gi...@apache.org>.
vvcephei commented on pull request #9426:
URL: https://github.com/apache/kafka/pull/9426#issuecomment-718806219


   Thanks for the reviews, all!
   
   @dongjinleekr : Sorry, I should have made the fix clear. It's just the one line where I changed `getStartedStreams` to `getRunningStreams`. The race condition was that sometimes Streams didn't start running before the verification.
   
   The other changes to use unique topic names for the test is just general hygine.


----------------------------------------------------------------
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:
users@infra.apache.org

[GitHub] [kafka] vvcephei commented on pull request #9426: MINOR: Fix flaky shouldRejectNonExistentStoreName

Posted by GitBox <gi...@apache.org>.
vvcephei commented on pull request #9426:
URL: https://github.com/apache/kafka/pull/9426#issuecomment-718900823


   The only build failure was:
   `Build / JDK 11 / org.apache.kafka.streams.integration.StreamTableJoinTopologyOptimizationIntegrationTest.shouldDoStreamTableJoinWithDifferentNumberOfPartitions[Optimization = none]`
   
   Which seems to have been caused by the OS deleting some files from the `tmp` directory during the run.


----------------------------------------------------------------
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:
users@infra.apache.org

[GitHub] [kafka] vvcephei commented on pull request #9426: MINOR: Fix flaky shouldRejectNonExistentStoreName

Posted by GitBox <gi...@apache.org>.
vvcephei commented on pull request #9426:
URL: https://github.com/apache/kafka/pull/9426#issuecomment-707925313


   Hey @guozhangwang , I've seen this test fail on a PR branch since we merged the last change. Can you take a quick look at this?


----------------------------------------------------------------
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:
users@infra.apache.org

[GitHub] [kafka] chia7712 commented on a change in pull request #9426: MINOR: Fix flaky shouldRejectNonExistentStoreName

Posted by GitBox <gi...@apache.org>.
chia7712 commented on a change in pull request #9426:
URL: https://github.com/apache/kafka/pull/9426#discussion_r509966875



##########
File path: streams/src/test/java/org/apache/kafka/streams/integration/QueryableStateIntegrationTest.java
##########
@@ -470,6 +475,8 @@ public void shouldRejectNonExistentStoreName() {
                 exception.getMessage(),
                 is("Cannot get state store no-table because no such store is registered in the topology.")
             );
+        } finally {
+            CLUSTER.deleteAllTopicsAndWait(0L);

Review comment:
       How about moving this cleanup to ```@after```?




----------------------------------------------------------------
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:
users@infra.apache.org

[GitHub] [kafka] vvcephei commented on a change in pull request #9426: MINOR: Fix flaky shouldRejectNonExistentStoreName

Posted by GitBox <gi...@apache.org>.
vvcephei commented on a change in pull request #9426:
URL: https://github.com/apache/kafka/pull/9426#discussion_r514316662



##########
File path: streams/src/test/java/org/apache/kafka/streams/integration/QueryableStateIntegrationTest.java
##########
@@ -470,6 +475,8 @@ public void shouldRejectNonExistentStoreName() {
                 exception.getMessage(),
                 is("Cannot get state store no-table because no such store is registered in the topology.")
             );
+        } finally {
+            CLUSTER.deleteAllTopicsAndWait(0L);

Review comment:
       Sure, I can do that.
   
   Thanks for the review!




----------------------------------------------------------------
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:
users@infra.apache.org

[GitHub] [kafka] vvcephei merged pull request #9426: MINOR: Fix flaky shouldRejectNonExistentStoreName

Posted by GitBox <gi...@apache.org>.
vvcephei merged pull request #9426:
URL: https://github.com/apache/kafka/pull/9426


   


----------------------------------------------------------------
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:
users@infra.apache.org