KAFKA-17587 Refactor test infrastructure (apache#18602)

This patch reorganizes our test infrastructure into three Gradle modules:

":test-common:test-common-internal-api" is now a minimal dependency which exposes interfaces and annotations only. It has one project dependency on server-common to expose commonly used data classes (MetadataVersion, Feature, etc). Since this pulls in server-common, this module is Java 17+. It cannot be used by ":clients" or other Java 11 modules.

":test-common:test-common-util" includes the auto-quarantined JUnit extension. The @flaky annotation has been moved here. Since this module has no project dependencies, we can add it to the Java 11 list so that ":clients" and others can utilize the @flaky annotation

":test-common:test-common-runtime" now includes all of the test infrastructure code (TestKitNodes, etc). This module carries heavy dependencies (core, etc) and so it should not normally be included as a compile-time dependency.

In addition to this reorganization, this patch leverages JUnit SPI service discovery so that modules can utilize the integration test framework without depending on ":core". This will allow us to start moving integration tests out of core and into the appropriate sub-module. This is done by adding ":test-common:test-common-runtime" as a testRuntimeOnly dependency rather than as a testImplementation dependency. A trivial example was added to QuorumControllerTest to illustrate this.

Reviewers: Ismael Juma <[email protected]>, Chia-Ping Tsai <[email protected]>

Author: mumrah

.github/scripts/junit.py

@@ -91,7 +91,10 @@ class TestSuite:
 def clean_test_name(test_name: str) -> str:
     cleaned = test_name.strip("\"").rstrip("()")
     m = method_matcher.match(cleaned)
-    return m.group(1)
+    if m is None:
+        raise ValueError(f"Could not parse test name '{test_name}'. Expected a valid Java method name.")
+    else:
+        return m.group(1)
 
 
 class TestCatalogExporter:

build.gradle

@@ -69,7 +69,7 @@ ext {
   gradleVersion = versions.gradle
   minClientJavaVersion = 11
   minNonClientJavaVersion = 17
-  modulesNeedingJava11 = [":clients", ":generator", ":streams", ":streams:test-utils", ":streams-scala", ":test-common:test-common-runtime"]
+  modulesNeedingJava11 = [":clients", ":generator", ":streams", ":streams:test-utils", ":streams-scala", ":test-common:test-common-util"]
 
   buildVersionFileName = "kafka-version.properties"
 
@@ -160,10 +160,11 @@ ext {
   runtimeTestLibs = [
     libs.slf4jLog4j2,
     libs.junitPlatformLanucher,
-    project(":test-common:test-common-runtime")
+    libs.jacksonDatabindYaml,
+    project(":test-common:test-common-util")
   ]
 
-  log4jRuntimeLibs = [
+  log4jReleaseLibs = [
     libs.slf4jLog4j2,
     libs.log4j1Bridge2Api,
     libs.jacksonDatabindYaml
@@ -1118,7 +1119,7 @@ project(':core') {
   }
 
   dependencies {
-    releaseOnly log4jRuntimeLibs
+    releaseOnly log4jReleaseLibs
     // `core` is often used in users' tests, define the following dependencies as `api` for backwards compatibility
     // even though the `core` module doesn't expose any public API
     api project(':clients')
@@ -1161,8 +1162,9 @@ project(':core') {
     testImplementation project(':server-common').sourceSets.test.output
     testImplementation project(':storage:storage-api').sourceSets.test.output
     testImplementation project(':server').sourceSets.test.output
-    testImplementation project(':test-common')
-    testImplementation project(':test-common:test-common-api')
+    testImplementation project(':test-common:test-common-runtime')
+    testImplementation project(':test-common:test-common-internal-api')
+    testImplementation project(':test-common:test-common-util')
     testImplementation libs.bcpkix
     testImplementation libs.mockitoCore
     testImplementation(libs.apacheda) {
@@ -1598,21 +1600,17 @@ project(':group-coordinator') {
   srcJar.dependsOn 'processMessages'
 }
 
-project(':test-common') {
-  // Test framework stuff. Implementations that support test-common-api
+
+project(':test-common:test-common-internal-api') {
+  // Interfaces, config classes, and other test APIs. Java 17 only
   base {
-    archivesName = "kafka-test-common"
+    archivesName = "kafka-test-common-internal-api"
   }
 
   dependencies {
-    implementation project(':core')
-    implementation project(':metadata')
-    implementation project(':server')
-    implementation project(':raft')
-    implementation project(':storage')
-    implementation project(':server-common')
-    implementation libs.jacksonDatabindYaml
-    implementation libs.slf4jApi
+    implementation project(':server-common') // Only project dependency allowed
+
+    implementation libs.junitJupiterApi
 
     testImplementation libs.junitJupiter
     testImplementation libs.mockitoCore
@@ -1622,41 +1620,30 @@ project(':test-common') {
   }
 
   checkstyle {
-    configProperties = checkstyleConfigProperties("import-control-test-common.xml")
+    configProperties = checkstyleConfigProperties("import-control-test-common-internal-api.xml")
   }
 
   javadoc {
     enabled = false
   }
 }
 
-project(':test-common:test-common-api') {
-  // Interfaces, config classes, and other test APIs
+project(':test-common:test-common-util') {
+  // Runtime-only JUnit extensions for entire project. Java 11 only
   base {
-    archivesName = "kafka-test-common-api"
+    archivesName = "kafka-test-common-util"
   }
 
   dependencies {
-    implementation project(':clients')
-    implementation project(':core')
-    implementation project(':group-coordinator')
-    implementation project(':metadata')
-    implementation project(':raft')
-    implementation project(':server')
-    implementation project(':server-common')
-    implementation project(':storage')
-    implementation project(':test-common')
+    implementation libs.junitPlatformLanucher
     implementation libs.junitJupiterApi
-
-    testImplementation libs.junitJupiter
-    testImplementation libs.mockitoCore
+    implementation libs.junitJupiter
+    implementation libs.slf4jApi
     testImplementation testLog4j2Libs
-
-    testRuntimeOnly runtimeTestLibs
   }
 
   checkstyle {
-    configProperties = checkstyleConfigProperties("import-control-test-common-api.xml")
+    configProperties = checkstyleConfigProperties("import-control-test-common-util.xml")
   }
 
   javadoc {
@@ -1665,21 +1652,36 @@ project(':test-common:test-common-api') {
 }
 
 project(':test-common:test-common-runtime') {
-  // Runtime-only test code including JUnit extentions
+  // Runtime-only JUnit extensions for integration tests. Java 17 only
   base {
     archivesName = "kafka-test-common-runtime"
   }
 
   dependencies {
+    implementation project(':test-common:test-common-internal-api')
+    implementation project(':clients')
+    implementation project(':core')
+    implementation project(':group-coordinator')
+    implementation project(':metadata')
+    implementation project(':raft')
+    implementation project(':server')
+    implementation project(':server-common')
+    implementation project(':storage')
+
     implementation libs.junitPlatformLanucher
-    implementation libs.junitJupiterApi
     implementation libs.junitJupiter
+    implementation libs.jacksonDatabindYaml
     implementation libs.slf4jApi
+
+    testImplementation libs.junitJupiter
+    testImplementation libs.mockitoCore
     testImplementation testLog4j2Libs
+
+    testRuntimeOnly runtimeTestLibs
   }
 
   checkstyle {
-    configProperties = checkstyleConfigProperties("import-control-test-common-api.xml")
+    configProperties = checkstyleConfigProperties("import-control-test-common-runtime.xml")
   }
 
   javadoc {
@@ -1707,8 +1709,8 @@ project(':transaction-coordinator') {
     testImplementation libs.junitJupiter
     testImplementation libs.mockitoCore
     testImplementation project(':clients').sourceSets.test.output
-    testImplementation project(':test-common')
-    testImplementation project(':test-common:test-common-api')
+    testImplementation project(':test-common:test-common-runtime')
+    testImplementation project(':test-common:test-common-internal-api')
 
     testRuntimeOnly runtimeTestLibs
 
@@ -1929,6 +1931,7 @@ project(':clients') {
     compileOnly libs.jose4j          // for SASL/OAUTHBEARER JWT validation; only used by broker
 
 
+    testImplementation project(':test-common:test-common-util')
     testImplementation libs.bcpkix
     testImplementation libs.jacksonJakartarsJsonProvider
     testImplementation libs.jose4j
@@ -1943,7 +1946,6 @@ project(':clients') {
     testRuntimeOnly libs.jacksonDatabind
     testRuntimeOnly libs.jacksonJDK8Datatypes
     testRuntimeOnly runtimeTestLibs
-    testRuntimeOnly log4jRuntimeLibs
 
     generator project(':generator')
   }
@@ -2330,7 +2332,8 @@ project(':storage') {
     testImplementation project(':clients').sourceSets.test.output
     testImplementation project(':core')
     testImplementation project(':core').sourceSets.test.output
-    testImplementation project(':test-common:test-common-api')
+    testImplementation project(':test-common:test-common-internal-api')
+    testImplementation project(':test-common:test-common-runtime')
     testImplementation project(':server')
     testImplementation project(':server-common')
     testImplementation project(':server-common').sourceSets.test.output
@@ -2487,7 +2490,7 @@ project(':tools') {
   }
 
   dependencies {
-    releaseOnly log4jRuntimeLibs
+    releaseOnly log4jReleaseLibs
 
     implementation project(':clients')
     implementation project(':metadata')
@@ -2519,7 +2522,8 @@ project(':tools') {
     testImplementation project(':server').sourceSets.test.output
     testImplementation project(':core')
     testImplementation project(':core').sourceSets.test.output
-    testImplementation project(':test-common:test-common-api')
+    testImplementation project(':test-common:test-common-internal-api')
+    testImplementation project(':test-common:test-common-runtime')
     testImplementation project(':server-common')
     testImplementation project(':server-common').sourceSets.test.output
     testImplementation project(':connect:api')
@@ -2530,7 +2534,6 @@ project(':tools') {
     testImplementation project(':streams')
     testImplementation project(':streams').sourceSets.test.output
     testImplementation project(':streams:integration-tests').sourceSets.test.output
-    testImplementation project(':test-common')
     testImplementation libs.junitJupiter
     testImplementation libs.mockitoCore
     testImplementation libs.mockitoJunitJupiter // supports MockitoExtension
@@ -2711,7 +2714,6 @@ project(':streams') {
 
     testRuntimeOnly project(':streams:test-utils')
     testRuntimeOnly runtimeTestLibs
-    testRuntimeOnly log4jRuntimeLibs
 
     generator project(':generator')
   }
@@ -2902,7 +2904,7 @@ project(':streams:integration-tests') {
     testImplementation project(':storage')
     testImplementation project(':streams').sourceSets.test.output
     testImplementation project(':streams:streams-scala')
-    testImplementation project(':test-common')
+    testImplementation project(':test-common:test-common-runtime')
     testImplementation project(':tools')
     testImplementation project(':transaction-coordinator')
     testImplementation libs.bcpkix
@@ -3578,14 +3580,15 @@ project(':connect:runtime') {
     testImplementation project(':server')
     testImplementation project(':metadata')
     testImplementation project(':server-common')
-    testImplementation project(':test-common')
+    testImplementation project(':test-common:test-common-internal-api')
+    testImplementation project(':test-common:test-common-util')
+    testImplementation project(':test-common:test-common-runtime')
     testImplementation project(':server-common')
     testImplementation project(':server')
     testImplementation project(':group-coordinator')
     testImplementation project(':storage')
     testImplementation project(':connect:test-plugins')
     testImplementation project(':server-common').sourceSets.test.output
-    testImplementation project(':test-common:test-common-api')
 
     testImplementation libs.jacksonDatabindYaml
     testImplementation libs.junitJupiter
@@ -3699,7 +3702,7 @@ project(':connect:file') {
     testImplementation project(':connect:runtime')
     testImplementation project(':connect:runtime').sourceSets.test.output
     testImplementation project(':core')
-    testImplementation project(':test-common')
+    testImplementation project(':test-common:test-common-runtime')
     testImplementation project(':server-common').sourceSets.test.output
 
     testRuntimeOnly runtimeTestLibs
@@ -3803,7 +3806,7 @@ project(':connect:mirror') {
     testImplementation project(':clients').sourceSets.test.output
     testImplementation project(':connect:runtime').sourceSets.test.output
     testImplementation project(':core')
-    testImplementation project(':test-common')
+    testImplementation project(':test-common:test-common-runtime')
     testImplementation project(':server')
     testImplementation project(':server-common').sourceSets.test.output
 

checkstyle/import-control-coordinator-common.xml

@@ -37,6 +37,7 @@
     <allow pkg="org.apache.kafka.common.security" />
     <allow pkg="org.apache.kafka.common.serialization" />
     <allow pkg="org.apache.kafka.common.utils" />
+    <allow pkg="org.apache.kafka.common.test.api" />
 
     <subpackage name="coordinator">
         <subpackage name="common">

checkstyle/import-control-core.xml

@@ -36,6 +36,7 @@
   <allow pkg="kafka.utils" />
   <allow pkg="kafka.serializer" />
   <allow pkg="org.apache.kafka.common" />
+  <allow pkg="org.apache.kafka.common.test.api" />
   <allow pkg="org.mockito" class="AssignmentsManagerTest"/>
   <allow pkg="org.apache.kafka.server"/>
   <allow pkg="org.opentest4j" class="RemoteLogManagerTest"/>

checkstyle/import-control-group-coordinator.xml

@@ -45,6 +45,7 @@
     <allow pkg="org.apache.kafka.common.utils" />
     <allow pkg="org.apache.kafka.common.errors" exact-match="true" />
     <allow pkg="org.apache.kafka.common.memory" />
+    <allow pkg="org.apache.kafka.common.test.api" />
 
     <subpackage name="coordinator">
         <subpackage name="group">

checkstyle/import-control-metadata.xml

@@ -44,6 +44,7 @@
     <allow pkg="org.apache.kafka.common.utils" />
     <allow pkg="org.apache.kafka.common.errors" exact-match="true" />
     <allow pkg="org.apache.kafka.common.memory" />
+    <allow pkg="org.apache.kafka.common.test.api" />
 
     <!-- persistent collection factories/non-library-specific wrappers -->
     <allow pkg="org.apache.kafka.server.immutable" exact-match="true" />

checkstyle/import-control-server-common.xml

@@ -44,6 +44,7 @@
     <allow pkg="org.apache.kafka.common.utils" />
     <allow pkg="org.apache.kafka.common.errors" exact-match="true" />
     <allow pkg="org.apache.kafka.common.memory" />
+    <allow pkg="org.apache.kafka.common.test.api" />
 
     <!-- persistent collection factories/non-library-specific wrappers -->
     <allow pkg="org.apache.kafka.server.immutable" exact-match="true" />

checkstyle/import-control-server.xml

@@ -47,6 +47,7 @@
   <allow pkg="org.apache.kafka.common.memory" />
   <allow pkg="org.apache.kafka.common.network" />
   <allow pkg="org.apache.kafka.server.config"/>
+  <allow pkg="org.apache.kafka.common.test.api" />
 
 
   <!-- protocol, records and request/response utilities -->

checkstyle/import-control-share-coordinator.xml

@@ -40,6 +40,7 @@
     <allow pkg="org.apache.kafka.common.security" />
     <allow pkg="org.apache.kafka.common.serialization" />
     <allow pkg="org.apache.kafka.common.utils" />
+    <allow pkg="org.apache.kafka.common.test.api" />
 
     <subpackage name="coordinator">
         <subpackage name="share">

checkstyle/import-control-storage.xml

@@ -45,6 +45,7 @@
     <allow pkg="org.apache.kafka.common.errors" exact-match="true" />
     <allow pkg="org.apache.kafka.common.memory" />
     <allow pkg="org.apache.kafka.common.test" />
+    <allow pkg="org.apache.kafka.common.test.api" />
 
 
     <subpackage name="server">

checkstyle/import-control-test-common-internal-api.xml

@@ -0,0 +1,43 @@
+<!DOCTYPE import-control PUBLIC
+        "-//Puppy Crawl//DTD Import Control 1.1//EN"
+        "http://www.puppycrawl.com/dtds/import_control_1_1.dtd">
+<!--
+ Licensed to the Apache Software Foundation (ASF) under one or more
+ contributor license agreements.  See the NOTICE file distributed with
+ this work for additional information regarding copyright ownership.
+ The ASF licenses this file to You under the Apache License, Version 2.0
+ (the "License"); you may not use this file except in compliance with
+ the License.  You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+ Unless required by applicable law or agreed to in writing, software
+ distributed under the License is distributed on an "AS IS" BASIS,
+ WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ See the License for the specific language governing permissions and
+ limitations under the License.
+-->
+
+<import-control pkg="org.apache.kafka">
+    <!-- no one depends on the server -->
+    <disallow pkg="kafka" />
+
+    <!-- anyone can use public classes -->
+    <allow pkg="org.apache.kafka.common" exact-match="true" />
+    <allow pkg="org.apache.kafka.common.security" />
+    <allow pkg="org.apache.kafka.common.serialization" />
+    <allow pkg="org.apache.kafka.common.utils" />
+    <allow pkg="org.apache.kafka.common.errors" exact-match="true" />
+    <allow pkg="org.apache.kafka.common.memory" />
+    <allow pkg="org.apache.kafka.common.network" />
+    <allow pkg="org.apache.kafka.common.test" />
+
+    <!-- things from server-common -->
+    <allow pkg="org.apache.kafka.server.common" />
+
+    <allow pkg="java" />
+    <allow pkg="javax.security" />
+    <allow pkg="org.junit" />
+    <allow pkg="org.slf4j" />
+
+</import-control>