KAFKA-20779: KIP-1265 review follow-ups from #21337

api-checker/README.md

@@ -25,11 +25,27 @@ below cover building, testing, and publishing the plugins themselves.
 ## Test
 
 ```bash
-./gradlew :api-checker:core:test :api-checker:gradle-plugins:test
+./gradlew :api-checker:core:test :api-checker:gradle-plugins:test :api-checker:maven-plugin:test
 ```
 
-`:gradle-plugins:test` includes a Gradle TestKit end-to-end test that applies the
-`org.apache.kafka.internal-api-checker` plugin to a synthetic consumer project.
+- `:core:test` — unit tests for the scanner, validators, and reporter, plus the shared
+  `testFixtures` (`AsmClassFactory`, `TempJarBuilder`) they and the plugin tests use.
+- `:gradle-plugins:test` — includes a Gradle TestKit end-to-end test that applies the
+  `org.apache.kafka.internal-api-checker` plugin to a synthetic consumer project.
+- `:maven-plugin:test` — hosts `PluginXmlParityTest`, which locks the generated
+  `plugin.xml` Mojo descriptor against the fields on `KafkaInternalApiCheckerMojo` so
+  adding a parameter without exposing it (or vice versa) fails CI.
+
+### CI integration
+
+The root `check` task depends on `:api-checker:core:check`,
+`:api-checker:gradle-plugins:check`, and `:api-checker:maven-plugin:check` (wired in the
+top-level `build.gradle`). Since `.github/workflows/build.yml` runs `./gradlew check` on
+every PR, the composite build's checkstyle and unit tests — including
+`PluginXmlParityTest` — are exercised in the standard PR workflow.
+
+The producer-side checker itself is separately exercised via `docsJar`, which every
+module's `check` finalises against real annotated code and a real javadoc jar.
 
 ## Publish
 
@@ -84,7 +100,9 @@ api-checker/
 │   └── src/{main,test}/java/.../gradle/           # Plugin/Task/Extension × 2
 └── maven-plugin/
     ├── build.gradle          # Maven deps; templates plugin.xml at processResources
-    └── src/main/
-        ├── java/.../maven/KafkaInternalApiCheckerMojo.java
-        └── resources/META-INF/maven/plugin.xml
+    └── src/
+        ├── main/
+        │   ├── java/.../maven/KafkaInternalApiCheckerMojo.java
+        │   └── resources/META-INF/maven/plugin.xml
+        └── test/java/.../maven/PluginXmlParityTest.java   # locks plugin.xml ↔ Mojo fields
 ```

api-checker/build.gradle

@@ -80,6 +80,9 @@ subprojects {
 
     test {
         useJUnitPlatform()
+        testLogging {
+            events "passed", "skipped", "failed"
+        }
     }
 
     checkstyle {

api-checker/core/src/main/java/org/apache/kafka/apicheck/CascadeValidator.java

@@ -444,8 +444,14 @@ private static void checkBinaryReference(String binaryName, String violationType
     private static PublicApiViolation asSuppression(PublicApiViolation original, String reason) {
         boolean noReason = reason.isEmpty();
         String prettyReason = noReason ? PublicApiViolation.NO_REASON_MARKER : reason;
+        // Class-level cascade findings (e.g. INVALID_SUPERTYPE) carry a null memberName; only
+        // append "#member" when we actually have one, so class-scope suppressions render as
+        // "Suppressed INVALID_SUPERTYPE in org.apache.kafka…Owner" rather than "…Owner#null".
+        String location = original.getMemberName() != null
+                ? original.getClassName() + "#" + original.getMemberName()
+                : original.getClassName();
         String description = "Suppressed " + original.getViolationType() + " in "
-                + original.getClassName() + "#" + original.getMemberName()
+                + location
                 + " — " + original.getDescription()
                 + " — reason: " + prettyReason;
         return new PublicApiViolation(original.getClassName(), "SUPPRESSED",

api-checker/core/src/main/java/org/apache/kafka/apicheck/PublicApiViolation.java

@@ -16,6 +16,8 @@
  */
 package org.apache.kafka.apicheck;
 
+import java.util.Objects;
+
 /**
  * Represents a violation of the public API rules.
  */
@@ -86,7 +88,7 @@ public boolean equals(Object o) {
         if (!className.equals(that.className)) return false;
         if (!violationType.equals(that.violationType)) return false;
         if (!description.equals(that.description)) return false;
-        return memberName != null ? memberName.equals(that.memberName) : that.memberName == null;
+        return Objects.equals(memberName, that.memberName);
     }
 
     /**

api-checker/maven-plugin/src/main/java/org/apache/kafka/maven/KafkaInternalApiCheckerMojo.java

@@ -49,6 +49,10 @@
       threadSafe = true)
 public class KafkaInternalApiCheckerMojo extends AbstractMojo {
 
+    /** Public no-arg constructor invoked by Maven when instantiating the Mojo. */
+    public KafkaInternalApiCheckerMojo() {
+    }
+
     /**
      * The Maven project.
      */
@@ -199,27 +203,61 @@ private List<File> getKafkaJarsFromDependencies() {
         return kafkaJars;
     }
 
-    // Getters and setters for testing
+    // Setters used by tests to construct the Mojo without going through the Maven container.
+
+    /**
+     * Sets the Maven project the Mojo runs against (test-only injection).
+     *
+     * @param project the Maven project to scan
+     */
     public void setProject(MavenProject project) {
         this.project = project;
     }
 
+    /**
+     * Enables or disables the checker for one invocation.
+     *
+     * @param enabled {@code true} to run the checker, {@code false} to skip
+     */
     public void setEnabled(boolean enabled) {
         this.enabled = enabled;
     }
 
+    /**
+     * Sets whether reported violations fail the Maven build.
+     *
+     * @param failOnViolation {@code true} to fail the build on any violation
+     */
     public void setFailOnViolation(boolean failOnViolation) {
         this.failOnViolation = failOnViolation;
     }
 
+    /**
+     * Sets whether the checker fails when no {@code org.apache.kafka:*} dependency is present.
+     *
+     * @param failOnNoKafkaDependency {@code true} to fail (rather than warn) when the project has
+     *                                no Kafka artifact on its classpath
+     */
     public void setFailOnNoKafkaDependency(boolean failOnNoKafkaDependency) {
         this.failOnNoKafkaDependency = failOnNoKafkaDependency;
     }
 
+    /**
+     * Overrides the classes/jars scanned. Each entry may be a class directory, an individual
+     * {@code .class} file, or a {@code .jar} archive.
+     *
+     * @param classesDirectories roots to scan; if {@code null} or empty, defaults to the project's
+     *                           main compiled output
+     */
     public void setClassesDirectories(List<File> classesDirectories) {
         this.classesDirectories = classesDirectories;
     }
 
+    /**
+     * Sets the file path where the text report is written.
+     *
+     * @param reportFile destination file for the human-readable report
+     */
     public void setReportFile(File reportFile) {
         this.reportFile = reportFile;
     }

build.gradle

@@ -4403,3 +4403,12 @@ def verifyLatestStableMetadataVersionTask = tasks.register('verifyLatestStableMe
 }
 
 check.dependsOn verifyLatestStableMetadataVersionTask
+
+// api-checker is a composite-included build (settings.gradle → pluginManagement { includeBuild
+// 'api-checker' }). Its own `check` tasks — checkstyle, unit tests, and the PluginXmlParityTest —
+// aren't reached by the root `check` aggregate by default, so hook them in explicitly. This is
+// what `./gradlew check` invokes in .github/workflows/build.yml, so wiring them here also picks
+// them up on every PR CI run.
+check.dependsOn gradle.includedBuild('api-checker').task(':core:check')
+check.dependsOn gradle.includedBuild('api-checker').task(':gradle-plugins:check')
+check.dependsOn gradle.includedBuild('api-checker').task(':maven-plugin:check')