cadonna commented on code in PR #12441:
URL: https://github.com/apache/kafka/pull/12441#discussion_r932429697
##########
build.gradle:
##########
@@ -466,6 +466,10 @@ subprojects {
if (shouldUseJUnit5) {
useJUnitPlatform {
includeTags "integration"
+ // KAFKA-14109
+ // Both engines are needed to run JUnit 4 tests alongside JUnit 5
tests.
+ // junit-vintage (JUnit 4) can be removed once the JUnit 4 migration
is complete.
+ includeEngines "junit-vintage", "junit-jupiter"
}
Review Comment:
I experimented a bit and found the following solution.
```suggestion
if (project.name.equals('streams')) {
useJUnitPlatform {
includeTags "integration"
includeTags 'org.apache.kafka.test.IntegrationTest'
includeEngines "junit-vintage", "junit-jupiter"
}
} else {
useJUnitPlatform {
includeTags "integration"
}
}
```
In this way, we can limit the change to Streams.
The caveat is that we need to replace `@Category({IntegrationTest.class})`
with `@Tag{"integration"}` in the integration tests that have already been
migrated to JUnit 5.
##########
build.gradle:
##########
@@ -1832,12 +1840,17 @@ project(':streams') {
// testCompileOnly prevents streams from exporting a dependency on
test-utils, which would cause a dependency cycle
testCompileOnly project(':streams:test-utils')
+ // KAFKA-14109
+ // The below compileOnly dependency is needed for JUnit 4 tests.
+ // It can be safely removed once all of streams has moved to JUnit 5.
+ testCompileOnly libs.junit4
+
Review Comment:
As we said below this is not needed if you move `libs.junitVintageEngine`
back to `testImplementation`.
##########
build.gradle:
##########
@@ -466,6 +466,10 @@ subprojects {
if (shouldUseJUnit5) {
useJUnitPlatform {
includeTags "integration"
+ // KAFKA-14109
Review Comment:
I second Ismael's comment.
##########
build.gradle:
##########
@@ -503,6 +507,10 @@ subprojects {
if (shouldUseJUnit5) {
useJUnitPlatform {
excludeTags "integration"
+ // KAFKA-14109
+ // Both engines are needed to run JUnit 4 tests alongside JUnit 5
tests.
+ // junit-vintage (JUnit 4) can be removed once the JUnit 4 migration
is complete.
+ includeEngines "junit-vintage", "junit-jupiter"
}
Review Comment:
Here we can do similar as above but excluding the tags.
```suggestion
if (project.name.equals('streams')) {
useJUnitPlatform {
excludeTags "integration"
excludeTags 'org.apache.kafka.test.IntegrationTest'
includeEngines "junit-vintage", "junit-jupiter"
}
} else {
useJUnitPlatform {
excludeTags "integration"
}
}
```
##########
build.gradle:
##########
@@ -1832,12 +1840,17 @@ project(':streams') {
// testCompileOnly prevents streams from exporting a dependency on
test-utils, which would cause a dependency cycle
testCompileOnly project(':streams:test-utils')
+ // KAFKA-14109
+ // The below compileOnly dependency is needed for JUnit 4 tests.
+ // It can be safely removed once all of streams has moved to JUnit 5.
+ testCompileOnly libs.junit4
+
testImplementation project(':clients').sourceSets.test.output
testImplementation project(':core')
testImplementation project(':core').sourceSets.test.output
testImplementation libs.log4j
- testImplementation libs.junitJupiterApi
- testImplementation libs.junitVintageEngine
+ testImplementation libs.junitJupiter
+ testImplementation libs.junitJupiterParams // needed for parameterized
tests
Review Comment:
I would prefer to add this when we need it during the migration of the
parametrized tests. But I am also fine if it stays.
The inline comment is not needed.
--
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
