KAFKA-20512 : Fix flakiness - same output topic is being used for all parameterized tests #22112
KAFKA-20512 : Fix flakiness - same output topic is being used for all parameterized tests #22112mjsax merged 2 commits intoapache:trunkfrom
Conversation
github-actions
Bot
added
triage
|
|
||
| @BeforeEach | ||
| public void before(final TestInfo testInfo) { | ||
| public void before(final TestInfo testInfo) throws InterruptedException { |
There was a problem hiding this comment.
| public void before(final TestInfo testInfo) throws InterruptedException { | |
| public void before(final TestInfo testInfo) throws Exception { |
There was a problem hiding this comment.
Nobody will catch an exception, so using generic throws Exception is best to avoid future edits in case other exception types might get throws in the future; because nobody catches it, there is no value to declared the exact exception type.
mjsax
left a comment
mjsax
left a comment
There was a problem hiding this comment.
Thanks for the PR. Made a pass.
|
|
||
| @BeforeEach | ||
| public void before(final TestInfo testInfo) { | ||
| public void before(final TestInfo testInfo) throws InterruptedException { |
There was a problem hiding this comment.
Nobody will catch an exception, so using generic throws Exception is best to avoid future edits in case other exception types might get throws in the future; because nobody catches it, there is no value to declared the exact exception type.
| startApplicationAndWaitUntilRunning(streams); | ||
|
|
||
| // Wait until restoring tasks have been started | ||
| waitForActiveRestoringTask(streams, 0, 30000); |
There was a problem hiding this comment.
Why de we need this addition step?
There was a problem hiding this comment.
After "Step 4: Restart with Restoration", added a little wait time, but deleted it now.
|
|
||
| assertFalse(results.isEmpty(), "Should have received output records"); | ||
| final Set<String> unmatchedKeys = results.stream() | ||
| .filter(kv -> kv.value != null && kv.value.endsWith("right=null")) |
There was a problem hiding this comment.
Should we revers this filter, and only drop if !"probe".equals(kv.key) ?
There was a problem hiding this comment.
Added 1 more assertion. Now we verify record shapes and key based recs.
@mjsax thanks for the review. Pushed a few changes based on the comments. |
… parameterized tests (#22112) - Same output topic is used for all parameterizations. Isolating them with creating topic prefixing with app id - wait for restoring task is called so that assert works for all of those 10 unmatched recs Reviewers: Matthias J. Sax <matthias@confluent.io>
|
Thanks for the fix. Merged to |
2 participants
with creating topic prefixing with app id
those 10 unmatched recs
Reviewers: Matthias J. Sax matthias@confluent.io