Skip to content

MINOR: Fix the handling source topic deletion integration test #20155

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: trunk
Choose a base branch
from

Conversation

RaidenE1
Copy link
Contributor

Fix an integration test error:

In the old version, the HandlingSouceTopicDeleteIntegrationTest expects state change to ERROR in both old and new protocols, but in the new protocol, we should expects MissingSourceTopicException.

@github-actions github-actions bot added the triage PRs from the community label Jul 11, 2025
@RaidenE1 RaidenE1 changed the title fix the handling source topic deletion integration test MINOR: Fix the handling source topic deletion integration test Jul 11, 2025
@github-actions github-actions bot added streams tests Test fixes (including flaky tests) small Small PRs labels Jul 11, 2025
Copy link
Collaborator

@Yunyung Yunyung left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR @RaidenE1. Left some comments.

Comment on lines +138 to 140
TimeUnit.SECONDS.sleep(5);
assertThat(calledUncaughtExceptionHandler1.get(), is(true));
assertThat(calledUncaughtExceptionHandler2.get(), is(true));
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about doing this instead of always sleeping for a fixed time

Suggested change
TimeUnit.SECONDS.sleep(5);
assertThat(calledUncaughtExceptionHandler1.get(), is(true));
assertThat(calledUncaughtExceptionHandler2.get(), is(true));
TestUtils.waitForCondition(
() -> calledUncaughtExceptionHandler1.get() && calledUncaughtExceptionHandler2.get(),
TIMEOUT,
() -> "......Error Message......"
);

@@ -42,10 +42,13 @@
import java.util.Locale;
import java.util.Properties;
import java.util.concurrent.atomic.AtomicBoolean;
import java.util.concurrent.CountDownLatch;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why import this?


import static org.apache.kafka.streams.utils.TestUtils.safeUniqueTestName;
import static org.hamcrest.CoreMatchers.is;
import static org.hamcrest.MatcherAssert.assertThat;
import static org.junit.jupiter.api.Assertions.assertTrue;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ditto

@github-actions github-actions bot removed the triage PRs from the community label Jul 12, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ci-approved small Small PRs streams tests Test fixes (including flaky tests)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants