divijvaidya commented on code in PR #12285:
URL: https://github.com/apache/kafka/pull/12285#discussion_r898989140
##########
streams/src/test/java/org/apache/kafka/streams/integration/RestoreIntegrationTest.java:
##########
@@ -87,37 +85,34 @@
import static
org.apache.kafka.streams.integration.utils.IntegrationTestUtils.waitForStandbyCompletion;
import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.core.IsEqual.equalTo;
-import static org.junit.Assert.assertTrue;
+import static org.junit.jupiter.api.Assertions.assertTrue;
-@Category({IntegrationTest.class})
+@Timeout(600)
+@Tag("integration")
Review Comment:
I understand that this change is necessary for JUnit5 migration but in it's
current state if we merge this PR, these tests will not execute. Here's why:
In `build.gradle`, these tests are not marked for execution with JUnit5 and
hence will enter the `else` block in
```
if (shouldUseJUnit5) {
useJUnitPlatform {
includeTags "integration"
}
} else {
useJUnit {
includeCategories 'org.apache.kafka.test.IntegrationTest'
}
}
```
where they will try to find the previous
`@Category({IntegrationTest.class})` but will not find it and hence, these
tests will not execute.
My suggestion would be to keep `@Category({IntegrationTest.class})` in these
tests and make all other changes.
Once we have all tests migrated to Junit5, create a PR where we remove the
Junit5 eligibility from `build.gradle` and at the same time, we replace this
annotation with `@Tag("integration")`
--
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-unsubscr...@kafka.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
