From a96ea7f8cd0410d281652efcb38b5f03566d436e Mon Sep 17 00:00:00 2001 From: Antony Stubbs Date: Sun, 12 Apr 2026 10:52:30 +1200 Subject: [PATCH 01/67] Add performance test suite running on a self-hosted runner Adds a separate performance test suite that runs on dedicated hardware via a self-hosted GitHub Actions runner, so the volume tests don't slow down or destabilize regular CI. Test changes: - Tag VeryLargeMessageVolumeTest, LargeVolumeInMemoryTests, and MultiInstanceHighVolumeTest with @Tag("performance") - Failsafe now reads excluded.groups (defaults to "performance") and included.groups (defaults to empty), so the performance group is excluded by default. Override with -Dincluded.groups=performance to run only perf, or -Dexcluded.groups= to run everything. Scripts: - bin/performance-test.sh and bin/performance-test.cmd run the perf suite locally (.cmd for Windows since the runner is on Windows) Workflow: - .github/workflows/performance.yml targets a self-hosted runner labelled [self-hosted, windows, performance] - Triggers: workflow_dispatch (manual) + weekly schedule - Never runs on PRs (security: self-hosted + untrusted code = bad) Documentation: - docs/SELF_HOSTED_RUNNER.md walks through one-time runner setup on Windows with Docker Desktop and the WSL2 backend - README template and AGENTS.md mention the performance suite and link to the runner setup doc - Fix a stale link in README to the renamed VolumeTests file Co-Authored-By: Claude Opus 4.6 (1M context) --- .github/workflows/performance.yml | 82 +++++++++++++ AGENTS.md | 1 + README.adoc | 49 +++++++- bin/performance-test.cmd | 18 +++ bin/performance-test.sh | 21 ++++ docs/SELF_HOSTED_RUNNER.md | 115 ++++++++++++++++++ .../LargeVolumeInMemoryTests.java | 2 + .../MultiInstanceHighVolumeTest.java | 2 + .../VeryLargeMessageVolumeTest.java | 2 + pom.xml | 12 ++ src/docs/README_TEMPLATE.adoc | 33 ++++- 11 files changed, 335 insertions(+), 2 deletions(-) create mode 100644 .github/workflows/performance.yml create mode 100644 bin/performance-test.cmd create mode 100755 bin/performance-test.sh create mode 100644 docs/SELF_HOSTED_RUNNER.md diff --git a/.github/workflows/performance.yml b/.github/workflows/performance.yml new file mode 100644 index 000000000..b33de3a14 --- /dev/null +++ b/.github/workflows/performance.yml @@ -0,0 +1,82 @@ +# Performance test suite, run on a self-hosted Windows runner with Docker Desktop. +# +# These tests are tagged @Tag("performance") and excluded from the regular CI +# build because they need substantial hardware (CPU, memory, disk). They run +# on dedicated machines where the user has labelled their runner with the +# "performance" custom label. +# +# Triggers: +# - workflow_dispatch (manual) - primary trigger +# - schedule (weekly) - automated regression check +# - NOT on PRs from forks - self-hosted runners + untrusted code = bad +# +# See docs/SELF_HOSTED_RUNNER.md for one-time runner setup instructions. + +name: Performance Tests + +on: + workflow_dispatch: + inputs: + kafka_version: + description: 'Kafka version to test against (default: project default)' + required: false + type: string + default: '' + schedule: + # Weekly on Sunday at 02:00 UTC + - cron: '0 2 * * 0' + +concurrency: + # Only run one performance test at a time per branch - they're slow and resource-heavy + group: performance-${{ github.ref }} + cancel-in-progress: false + +permissions: + contents: read + +jobs: + performance: + name: "Performance suite (self-hosted)" + # Targets a self-hosted runner labelled "performance" running Windows. + # The "self-hosted" label is automatic; "windows" and "performance" are + # added when the runner is registered. See docs/SELF_HOSTED_RUNNER.md. + runs-on: [self-hosted, windows, performance] + timeout-minutes: 180 + + steps: + - name: Checkout + uses: actions/checkout@v6 + + - name: Setup JDK 17 + uses: actions/setup-java@v5 + with: + distribution: 'temurin' + java-version: '17' + # Don't cache here - self-hosted runners persist .m2 across runs already + cache: '' + + - name: Show environment + shell: cmd + run: | + java -version + docker --version + docker info + + - name: Run performance tests + shell: cmd + env: + KAFKA_VERSION: ${{ inputs.kafka_version }} + run: | + if defined KAFKA_VERSION ( + call bin\performance-test.cmd -Dkafka.version=%KAFKA_VERSION% + ) else ( + call bin\performance-test.cmd + ) + + - name: Upload test reports + if: always() + uses: actions/upload-artifact@v7 + with: + name: performance-reports-${{ github.run_number }} + path: '**/target/*-reports/*.xml' + retention-days: 30 diff --git a/AGENTS.md b/AGENTS.md index 690edccfd..f7055bff8 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -47,6 +47,7 @@ bin/ci-build.sh 3.9.1 - **Integration tests**: `mvn verify` / failsafe plugin. Source in `src/test-integration/java/`. Uses TestContainers with `confluentinc/cp-kafka` Docker image. - **Test exclusion patterns**: `**/integrationTest*/**/*.java` and `**/*IT.java` are excluded from surefire, included in failsafe. - **Kafka version matrix**: CI tests against multiple Kafka versions via `-Dkafka.version=X.Y.Z`. +- **Performance tests**: Tagged `@Tag("performance")` and excluded from regular CI by default. They run on a self-hosted runner via `.github/workflows/performance.yml` (see `docs/SELF_HOSTED_RUNNER.md`). Run locally with `bin/performance-test.sh` (or `bin/performance-test.cmd` on Windows). Override the test group selection with Maven properties: `-Dincluded.groups=performance` to run only perf, `-Dexcluded.groups=` to run everything. ## Known Issues diff --git a/README.adoc b/README.adoc index f3c48ea86..8a6e08ca9 100644 --- a/README.adoc +++ b/README.adoc @@ -283,7 +283,7 @@ The user just has to provide a function to extract from the message the HTTP cal === Illustrative Performance Example -.(see link:./parallel-consumer-core/src/test-integration/java/io/confluent/parallelconsumer/integrationTests/VolumeTests.java[VolumeTests.java]) +.(see link:./parallel-consumer-core/src/test-integration/java/io/confluent/parallelconsumer/integrationTests/VeryLargeMessageVolumeTest.java[VeryLargeMessageVolumeTest.java]) These performance comparison results below, even though are based on real performance measurement results, are for illustrative purposes. To see how the performance of the tool is related to instance counts, partition counts, key distribution and how it would relate to the vanilla client. Actual results will vary wildly depending upon the setup being deployed into. @@ -1341,6 +1341,53 @@ Note:: See https://github.com/confluentinc/parallel-consumer/issues/162[issue #162] and this https://stackoverflow.com/questions/4786881/why-is-test-jar-dependency-required-for-mvn-compile[Stack Overflow question]. +=== Build Scripts + +Helper scripts are in the `bin/` directory: + +[qanda] +Quick local build (compile + unit tests):: +`bin/build.sh` + +Full CI build with all tests (unit + integration):: +`bin/ci-build.sh` + +CI build against a specific Kafka version:: +`bin/ci-build.sh 3.9.1` + +Performance test suite (also `bin/performance-test.cmd` on Windows):: +`bin/performance-test.sh` + +The GitHub Actions CI workflow uses `bin/ci-build.sh`, so running it locally reproduces the CI environment. + +=== Performance Tests + +Tests tagged `@Tag("performance")` are excluded from the regular CI build because they need substantial hardware. They run on a dedicated self-hosted runner via `.github/workflows/performance.yml` (manual trigger or weekly schedule). + +To run the performance suite locally, use `bin/performance-test.sh`. To set up your own self-hosted runner for these tests, see link:./docs/SELF_HOSTED_RUNNER.md[docs/SELF_HOSTED_RUNNER.md]. + +=== Releasing + +The `pom.xml` version is the source of truth for publishing — there is no `maven-release-plugin` step. + +On every push to `master`, `.github/workflows/publish.yml` deploys to Maven Central: + +* If the version ends in `-SNAPSHOT` → publishes a snapshot +* If the version does not end in `-SNAPSHOT` → publishes a full release, creates a `v` git tag, and creates a GitHub release + +To cut a release: + +. Open a PR removing `-SNAPSHOT` from `` in the parent `pom.xml` (e.g. `0.6.0.0-SNAPSHOT` → `0.6.0.0`) +. Merge it to master → CI publishes the release +. Open another PR bumping to the next snapshot (e.g. `0.6.0.1-SNAPSHOT`) and merge + +Required GitHub repository secrets: + +* `MAVEN_CENTRAL_USERNAME` — Sonatype Central Portal token username +* `MAVEN_CENTRAL_PASSWORD` — Sonatype Central Portal token password +* `MAVEN_GPG_PRIVATE_KEY` — Armored GPG private key for signing artifacts +* `MAVEN_GPG_PASSPHRASE` — Passphrase for the GPG key + === Testing The project has good automated test coverage, of all features. diff --git a/bin/performance-test.cmd b/bin/performance-test.cmd new file mode 100644 index 000000000..803604baf --- /dev/null +++ b/bin/performance-test.cmd @@ -0,0 +1,18 @@ +@REM Copyright (C) 2020-2026 Confluent, Inc. +@REM +@REM Run only the performance test suite (tests tagged @Tag("performance")). +@REM These are excluded from the regular CI build because they take a long time +@REM and need substantial hardware. Used by the self-hosted Windows runner. +@REM +@REM Usage: bin\performance-test.cmd [extra-maven-args...] + +@echo off +setlocal + +call mvnw.cmd --batch-mode ^ + -Pci ^ + clean verify ^ + -Dincluded.groups=performance ^ + -Dexcluded.groups= ^ + -Dlicense.skip ^ + %* diff --git a/bin/performance-test.sh b/bin/performance-test.sh new file mode 100755 index 000000000..87ceda19d --- /dev/null +++ b/bin/performance-test.sh @@ -0,0 +1,21 @@ +#!/usr/bin/env bash +# +# Copyright (C) 2020-2026 Confluent, Inc. +# + +# Run only the performance test suite (tests tagged @Tag("performance")). +# These are excluded from the regular CI build because they take a long time +# and need substantial hardware. The self-hosted runner workflow +# (.github/workflows/performance.yml) calls this script. +# +# Usage: bin/performance-test.sh [extra-maven-args...] + +set -euo pipefail + +./mvnw --batch-mode \ + -Pci \ + clean verify \ + -Dincluded.groups=performance \ + -Dexcluded.groups= \ + -Dlicense.skip \ + "$@" diff --git a/docs/SELF_HOSTED_RUNNER.md b/docs/SELF_HOSTED_RUNNER.md new file mode 100644 index 000000000..6c324e562 --- /dev/null +++ b/docs/SELF_HOSTED_RUNNER.md @@ -0,0 +1,115 @@ +# Self-Hosted Runner Setup (Windows + Docker Desktop) + +The performance test suite runs on a self-hosted GitHub Actions runner so we can throw real hardware at the volume tests. This document walks through the one-time setup on a Windows machine. + +## What you get + +- A GitHub Actions runner registered to your fork (`astubbs/parallel-consumer`) +- Triggered manually via `workflow_dispatch` or weekly on a schedule +- Runs the `@Tag("performance")` tests via `bin/performance-test.cmd` +- TestContainers spins up a Kafka broker via Docker Desktop (Linux containers / WSL2 backend) + +## Prerequisites on the runner machine + +1. **Windows 10/11** (or Server 2019+) +2. **WSL2 enabled** — required for Docker Desktop's Linux container backend +3. **Docker Desktop** for Windows, configured to use the WSL2 backend +4. **Git for Windows** (which includes Git Bash) +5. **Outbound HTTPS access** to `github.com`, `*.actions.githubusercontent.com`, Maven Central, and Docker Hub (or `images.confluent.io` for cp-kafka images) + +JDK 17 does **not** need to be pre-installed — the workflow uses `actions/setup-java` to provision it on first run, and caches it in the runner work directory. + +## One-time runner installation + +### 1. Generate a runner token + +1. Go to your fork's **Settings → Actions → Runners → New self-hosted runner** +2. Choose **Windows** and **x64** +3. GitHub will show you a PowerShell snippet that includes a one-time registration token + +### 2. Install the runner agent + +Open **PowerShell as your normal user** (not Administrator — the runner should not run as admin) and run the snippet GitHub provided. It looks roughly like this: + +```powershell +mkdir actions-runner +cd actions-runner +Invoke-WebRequest -Uri https://github.com/actions/runner/releases/download/v2.x.x/actions-runner-win-x64-2.x.x.zip -OutFile actions-runner.zip +Add-Type -AssemblyName System.IO.Compression.FileSystem ; [System.IO.Compression.ZipFile]::ExtractToDirectory("$PWD\actions-runner.zip", "$PWD") +``` + +### 3. Configure the runner with the right labels + +When prompted by `config.cmd`, set the labels so the workflow can find this runner: + +``` +Enter the name of the runner group to add this runner to: [press Enter for Default] +Enter the name of runner: [press Enter for hostname, or type something descriptive] +Enter any additional labels (ex. label-1,label-2): performance,windows +``` + +The full label set on this runner will be: `self-hosted`, `Windows`, `X64`, `performance`, `windows` + +> **Why two `windows` labels?** GitHub auto-adds `Windows` (capital W). The workflow targets the lowercase custom label `windows` to make intent explicit. Either works — pick whichever you prefer and update the workflow's `runs-on:` line if you change it. + +### 4. Run the runner as a service (recommended) + +So it survives reboots: + +```cmd +cd actions-runner +.\svc.cmd install +.\svc.cmd start +``` + +The service runs as the current user by default. Make sure that user has Docker Desktop access. + +### 5. Verify Docker is reachable + +In the same shell: + +```cmd +docker info +docker run --rm hello-world +``` + +If `docker info` complains about the daemon, start Docker Desktop and wait for it to finish initializing (the whale icon in the system tray should be steady, not animating). + +## Triggering the workflow + +### Manually + +1. Go to your fork on GitHub +2. **Actions → Performance Tests → Run workflow** +3. Optionally enter a Kafka version override +4. Click **Run workflow** + +### Automatically + +The workflow runs every **Sunday at 02:00 UTC** via cron schedule (defined in `.github/workflows/performance.yml`). + +## Security notes + +- Self-hosted runners on **public repositories** are dangerous because anyone can open a PR that runs arbitrary code on your machine. The performance workflow is configured to **only** run on `workflow_dispatch` (manual) and `schedule` triggers, never on PRs, to avoid this. +- Do not run the runner as Administrator. +- Keep Docker Desktop and the runner agent updated. + +## Troubleshooting + +**Runner shows offline in GitHub:** +- Check the service is running: `.\svc.cmd status` +- Check logs in `actions-runner\_diag\` +- Restart: `.\svc.cmd stop && .\svc.cmd start` + +**Tests fail with "Cannot connect to Docker daemon":** +- Make sure Docker Desktop is running +- Check WSL2 backend is enabled (Docker Desktop → Settings → General) +- Try `docker info` from the runner's shell + +**Tests run but are very slow:** +- Performance tests are *meant* to be slow — that's why they're on dedicated hardware +- Check Docker Desktop's resource allocation (Settings → Resources) — give it enough RAM (8GB+) and CPU cores + +**Workflow can't find the runner:** +- Verify the labels match what `runs-on:` expects in `.github/workflows/performance.yml` +- The runner must be online when the workflow is triggered diff --git a/parallel-consumer-core/src/test-integration/java/io/confluent/parallelconsumer/integrationTests/LargeVolumeInMemoryTests.java b/parallel-consumer-core/src/test-integration/java/io/confluent/parallelconsumer/integrationTests/LargeVolumeInMemoryTests.java index 2e507eb6d..877434a04 100644 --- a/parallel-consumer-core/src/test-integration/java/io/confluent/parallelconsumer/integrationTests/LargeVolumeInMemoryTests.java +++ b/parallel-consumer-core/src/test-integration/java/io/confluent/parallelconsumer/integrationTests/LargeVolumeInMemoryTests.java @@ -21,6 +21,7 @@ import org.apache.kafka.clients.producer.ProducerRecord; import org.apache.kafka.common.TopicPartition; import org.assertj.core.util.Lists; +import org.junit.jupiter.api.Tag; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.EnumSource; import pl.tlinkowski.unij.api.UniLists; @@ -44,6 +45,7 @@ * Mocked out comparative volume tests */ @Slf4j +@Tag("performance") class LargeVolumeInMemoryTests extends ParallelEoSStreamProcessorTestBase { @SneakyThrows diff --git a/parallel-consumer-core/src/test-integration/java/io/confluent/parallelconsumer/integrationTests/MultiInstanceHighVolumeTest.java b/parallel-consumer-core/src/test-integration/java/io/confluent/parallelconsumer/integrationTests/MultiInstanceHighVolumeTest.java index 56efc1e4f..5db19804e 100644 --- a/parallel-consumer-core/src/test-integration/java/io/confluent/parallelconsumer/integrationTests/MultiInstanceHighVolumeTest.java +++ b/parallel-consumer-core/src/test-integration/java/io/confluent/parallelconsumer/integrationTests/MultiInstanceHighVolumeTest.java @@ -17,6 +17,7 @@ import org.assertj.core.api.Assertions; import org.assertj.core.api.SoftAssertions; import org.awaitility.core.ConditionTimeoutException; +import org.junit.jupiter.api.Tag; import org.junit.jupiter.api.Test; import java.util.ArrayList; @@ -34,6 +35,7 @@ import static pl.tlinkowski.unij.api.UniLists.of; @Slf4j +@Tag("performance") class MultiInstanceHighVolumeTest extends BrokerIntegrationTest { public List consumedKeys = Collections.synchronizedList(new ArrayList<>()); diff --git a/parallel-consumer-core/src/test-integration/java/io/confluent/parallelconsumer/integrationTests/VeryLargeMessageVolumeTest.java b/parallel-consumer-core/src/test-integration/java/io/confluent/parallelconsumer/integrationTests/VeryLargeMessageVolumeTest.java index 6c32c8c09..f27922288 100644 --- a/parallel-consumer-core/src/test-integration/java/io/confluent/parallelconsumer/integrationTests/VeryLargeMessageVolumeTest.java +++ b/parallel-consumer-core/src/test-integration/java/io/confluent/parallelconsumer/integrationTests/VeryLargeMessageVolumeTest.java @@ -25,6 +25,7 @@ import org.assertj.core.api.Assertions; import org.assertj.core.api.SoftAssertions; import org.awaitility.core.ConditionTimeoutException; +import org.junit.jupiter.api.Tag; import org.junit.jupiter.api.Test; import java.util.*; @@ -51,6 +52,7 @@ * RuntimeException when running with very high options in 0.2.0.0 (Bitset too long to encode) #35 */ @Slf4j +@Tag("performance") public class VeryLargeMessageVolumeTest extends BrokerIntegrationTest { int HIGH_MAX_POLL_RECORDS_CONFIG = 10_000; diff --git a/pom.xml b/pom.xml index 78a81326a..94d8b2494 100644 --- a/pom.xml +++ b/pom.xml @@ -81,6 +81,13 @@ ${skipTests} ${skipTests} + + + performance + 5.0.0 @@ -706,6 +713,11 @@ ${skipTests} ${skipITs} methods + + ${included.groups} + ${excluded.groups} diff --git a/src/docs/README_TEMPLATE.adoc b/src/docs/README_TEMPLATE.adoc index c1db195ab..3bedc5633 100644 --- a/src/docs/README_TEMPLATE.adoc +++ b/src/docs/README_TEMPLATE.adoc @@ -281,7 +281,7 @@ The user just has to provide a function to extract from the message the HTTP cal === Illustrative Performance Example -.(see link:./parallel-consumer-core/src/test-integration/java/io/confluent/parallelconsumer/integrationTests/VolumeTests.java[VolumeTests.java]) +.(see link:./parallel-consumer-core/src/test-integration/java/io/confluent/parallelconsumer/integrationTests/VeryLargeMessageVolumeTest.java[VeryLargeMessageVolumeTest.java]) These performance comparison results below, even though are based on real performance measurement results, are for illustrative purposes. To see how the performance of the tool is related to instance counts, partition counts, key distribution and how it would relate to the vanilla client. Actual results will vary wildly depending upon the setup being deployed into. @@ -1106,8 +1106,39 @@ Full CI build with all tests (unit + integration):: CI build against a specific Kafka version:: `bin/ci-build.sh 3.9.1` +Performance test suite (also `bin/performance-test.cmd` on Windows):: +`bin/performance-test.sh` + The GitHub Actions CI workflow uses `bin/ci-build.sh`, so running it locally reproduces the CI environment. +=== Performance Tests + +Tests tagged `@Tag("performance")` are excluded from the regular CI build because they need substantial hardware. They run on a dedicated self-hosted runner via `.github/workflows/performance.yml` (manual trigger or weekly schedule). + +To run the performance suite locally, use `bin/performance-test.sh`. To set up your own self-hosted runner for these tests, see link:./docs/SELF_HOSTED_RUNNER.md[docs/SELF_HOSTED_RUNNER.md]. + +=== Releasing + +The `pom.xml` version is the source of truth for publishing — there is no `maven-release-plugin` step. + +On every push to `master`, `.github/workflows/publish.yml` deploys to Maven Central: + +* If the version ends in `-SNAPSHOT` → publishes a snapshot +* If the version does not end in `-SNAPSHOT` → publishes a full release, creates a `v` git tag, and creates a GitHub release + +To cut a release: + +. Open a PR removing `-SNAPSHOT` from `` in the parent `pom.xml` (e.g. `0.6.0.0-SNAPSHOT` → `0.6.0.0`) +. Merge it to master → CI publishes the release +. Open another PR bumping to the next snapshot (e.g. `0.6.0.1-SNAPSHOT`) and merge + +Required GitHub repository secrets: + +* `MAVEN_CENTRAL_USERNAME` — Sonatype Central Portal token username +* `MAVEN_CENTRAL_PASSWORD` — Sonatype Central Portal token password +* `MAVEN_GPG_PRIVATE_KEY` — Armored GPG private key for signing artifacts +* `MAVEN_GPG_PASSPHRASE` — Passphrase for the GPG key + === Testing The project has good automated test coverage, of all features. From 62f9f15a5c645fefac4d67564e04513bfcf404bd Mon Sep 17 00:00:00 2001 From: Antony Stubbs Date: Sun, 12 Apr 2026 10:54:09 +1200 Subject: [PATCH 02/67] docs(agents): document -Dlicense.skip and when to use it Add a "License headers" section to AGENTS.md explaining how to skip the Mycila license-maven-plugin check. This avoids two recurring problems: - The plugin's git-derived copyright years break inside git worktrees - It auto-bumps years on every touched file, creating noise in git status that distracts from real changes The bin/build.sh and bin/ci-build.sh scripts already pass the flag. Co-Authored-By: Claude Opus 4.6 (1M context) --- AGENTS.md | 22 +++++++++++++++++++++- 1 file changed, 21 insertions(+), 1 deletion(-) diff --git a/AGENTS.md b/AGENTS.md index f7055bff8..49a41ed68 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -57,9 +57,29 @@ bin/ci-build.sh 3.9.1 - **Lombok**: Used extensively (builders, getters, logging). IntelliJ Lombok plugin required. - **EditorConfig**: Enforced via `.editorconfig` - 4-space indent for Java, 120 char line length. -- **License headers**: Managed by `license-maven-plugin` (Mycila). Use `-Dlicense.skip` locally to skip checks. +- **License headers**: Managed by `license-maven-plugin` (Mycila). See "License headers" section below. - **Google Truth**: Used for test assertions alongside JUnit 5 and Mockito. +## License headers + +The Mycila `license-maven-plugin` enforces a Confluent copyright header on all source files. It uses git-derived years via `${license.git.copyrightYears}`. + +**Skipping the check** (for any Maven goal): +``` +./mvnw -Dlicense.skip +``` + +**When to skip:** +- Running builds inside a git worktree — the git-years lookup fails with `Bare Repository has neither a working tree, nor an index` +- Local iteration where you don't want years auto-bumped on touched files +- Any command other than the canonical `mvn install` flow when copyright drift would create noise in `git status` + +The default behavior on macOS dev machines is `format` mode (auto-fixes headers) via the `license-format` profile (auto-activated). The `ci` profile flips this to `check` mode (fails the build on drift). Both `bin/build.sh` and `bin/ci-build.sh` already pass `-Dlicense.skip` for this reason. + +**When NOT to skip:** +- You're intentionally running `mvn license:format` to update headers +- You want to verify CI's check would pass before pushing + ## CI - **GitHub Actions**: `.github/workflows/maven.yml` - runs on push/PR to master with Kafka version matrix. From cc26d3f95b49a69752e5b764d4dcd4539f941bf1 Mon Sep 17 00:00:00 2001 From: Antony Stubbs Date: Tue, 14 Apr 2026 20:20:03 +1200 Subject: [PATCH 03/67] ci: restructure PR builds into 4 tiers for faster feedback PR builds now run in tiers, each gated on the previous: Tier 1: Unit tests (no Docker, ~3 min) Tier 2: Integration tests (TestContainers, ~10 min) Tier 3: Kafka version matrix (2.8.1 + 3.9.1 only) Tier 4: Performance tests (@Tag("performance")) Tiers 3 and 4 run in parallel after Tier 2 passes. Push builds (master) still run the full Kafka matrix (2.8.1, 3.1.0, 3.7.0, 3.9.1) plus an experimental [3.9.1,5) range to catch Kafka 4.x compatibility. Dropped 3.5.0 from the matrix (EOL Aug 2025). Updated the experimental range from [3.1.0,4) to [3.9.1,5) to cover 4.x. Co-Authored-By: Claude Opus 4.6 (1M context) --- .github/workflows/maven.yml | 157 ++++++++++++++++++------------------ 1 file changed, 78 insertions(+), 79 deletions(-) diff --git a/.github/workflows/maven.yml b/.github/workflows/maven.yml index f08f58ff7..b39a9c838 100644 --- a/.github/workflows/maven.yml +++ b/.github/workflows/maven.yml @@ -1,120 +1,119 @@ -# This workflow will build a Java project with Maven -# For more information see: https://help.github.com/actions/language-and-framework-guides/building-and-testing-java-with-maven +# Tiered CI: fast feedback first, expensive tests only after cheap ones pass. +# +# PR builds: +# Tier 1: Unit tests (no Docker, ~3 min) +# Tier 2: Integration tests (TestContainers, ~10 min) — waits for Tier 1 +# Tier 3: Kafka version matrix (2.8.1 + 3.9.1) — waits for Tier 2 +# Tier 4: Performance tests (@Tag("performance")) — waits for Tier 2 +# +# Push builds (master): +# Full Kafka version matrix (2.8.1, 3.1.0, 3.7.0, 3.9.1 + experimental 4.x) -# Tests disabled due to flakiness with under resourced github test machines. Confluent Jira works fine. Will fix later. name: Build and Test on: push: pull_request: -# Cancel in-progress runs when a new commit is pushed to the same branch/PR concurrency: group: ${{ github.workflow }}-${{ github.head_ref || github.ref }} cancel-in-progress: true jobs: - # Fast build on push - uses default Kafka version from pom.xml - build: - if: github.event_name == 'push' - name: "Build (default AK)" - runs-on: ubuntu-latest + # ── PR Builds ────────────────────────────────────────────────────────── + + # Tier 1: Unit tests — fast feedback, no Docker needed + unit-tests: + if: github.event_name == 'pull_request' + name: "Tier 1: Unit Tests" + runs-on: ubuntu-latest steps: - uses: actions/checkout@v6 - - # - name: Setup JDK 1.9 - # uses: actions/setup-java@v1 - # with: - # java-version: 1.9 - - # - name: Show java 1.9 home - # /opt/hostedtoolcache/jdk/9.0.7/x64 - # run: which java - - - name: Setup JDK 17 - uses: actions/setup-java@v5 + - uses: actions/setup-java@v5 with: distribution: 'temurin' java-version: '17' cache: 'maven' + - name: Unit tests (no Docker) + run: ./mvnw --batch-mode test -Dlicense.skip -Dexcluded.groups=performance - # - name: Show java version - # run: java -version - - # - name: Show mvn version - # run: mvn -version - - # - name: Build with Maven on Java 13 - # run: mvn -B package --file pom.xml - + # Tier 2: Integration tests — needs Docker/TestContainers + integration-tests: + if: github.event_name == 'pull_request' + needs: unit-tests + name: "Tier 2: Integration Tests" + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v6 + - uses: actions/setup-java@v5 + with: + distribution: 'temurin' + java-version: '17' + cache: 'maven' + - name: Integration tests (TestContainers) + run: ./mvnw --batch-mode verify -Dlicense.skip -Dexcluded.groups=performance - # done automatically now - # - name: Cache Maven packages - # uses: actions/cache@v2.1.7 - # with: - # path: ~/.m2/repository - # key: ${{ runner.os }}-m2 - # restore-keys: ${{ runner.os }}-m2 + # Tier 3: Kafka version matrix — oldest and newest supported versions + kafka-matrix: + if: github.event_name == 'pull_request' + needs: integration-tests + strategy: + fail-fast: false + matrix: + ak: [ 2.8.1, 3.9.1 ] + name: "Tier 3: AK ${{ matrix.ak }}" + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v6 + - uses: actions/setup-java@v5 + with: + distribution: 'temurin' + java-version: '17' + cache: 'maven' + - name: Build and test with Kafka ${{ matrix.ak }} + run: bin/ci-build.sh ${{ matrix.ak }} - - name: Build and Test - run: bin/ci-build.sh + # Tier 4: Performance tests — long-running volume/load tests + performance-tests: + if: github.event_name == 'pull_request' + needs: integration-tests + name: "Tier 4: Performance Tests" + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v6 + - uses: actions/setup-java@v5 + with: + distribution: 'temurin' + java-version: '17' + cache: 'maven' + - name: Performance tests + run: ./mvnw --batch-mode verify -Dlicense.skip -Dincluded.groups=performance -Dexcluded.groups= -# - name: Archive test results -# if: ${{ always() }} -# uses: actions/upload-artifact@v2 -# with: -# name: test-reports -# path: target/**-reports/* -# retention-days: 14 -# -# - name: Archive surefire test results -# if: ${{ always() }} -# uses: actions/upload-artifact@v2 -# with: -# name: test-reports -# path: target/surefire-reports/* -# retention-days: 14 + # ── Push Builds (master) ─────────────────────────────────────────────── - # Full Kafka version matrix on pull requests - build-matrix: - if: github.event_name == 'pull_request' + # Full Kafka version matrix as safety net + build: + if: github.event_name == 'push' strategy: fail-fast: false matrix: - # Why not? because we can. - # 2.0.1, 2.1.1, 2.2.2, 2.3.1, 2.4.1 don't work - needs zstd and some kafka client libs. - # Doesn't mean it couldn't be modified slightly to work... - #ak: [ 2.5.1, 2.6.1, 2.7.0, 2.8.1, 3.0.1, 3.1.0 ] - # 25 and 26 include a dep with a vulnerability which ossindex fails the build for - ak: [ 2.8.1, 3.1.0, 3.5.0, 3.7.0, 3.9.1 ] - #ak: [ 2.7.0 ] - #jdk: [ '-P jvm8-release -Djvm8.location=/opt/hostedtoolcache/Java_Zulu_jdk/8.0.332-9/x64', '' ] - # TG currently targets 11, so can't run the tests on 8 https://github.com/astubbs/truth-generator/issues/114 + ak: [ 2.8.1, 3.1.0, 3.7.0, 3.9.1 ] experimental: [ false ] - name: [ "Stable AK version" ] + name: [ "Stable" ] include: - # AK 2.4 not supported - # - ak: "'[2.4.1,2.5)'" # currently failing - # experimental: true - # name: "Oldest AK breaking version 2.4.1+ (below 2.5.0) expected to fail" - - ak: "'[3.1.0,4)'" + - ak: "'[3.9.1,5)'" experimental: true - name: "Newest AK version 3.1.0+?" - + name: "Newest AK (4.x?)" continue-on-error: ${{ matrix.experimental }} name: "AK: ${{ matrix.ak }} (${{ matrix.name }})" runs-on: ubuntu-latest - steps: - uses: actions/checkout@v6 - - - name: Setup JDK 17 - uses: actions/setup-java@v5 + - uses: actions/setup-java@v5 with: distribution: 'temurin' java-version: '17' cache: 'maven' - - name: Build and Test run: bin/ci-build.sh ${{ matrix.ak }} From 97367105a8a7104efb69cded25327c7af27cc16d Mon Sep 17 00:00:00 2001 From: Antony Stubbs Date: Tue, 14 Apr 2026 21:30:49 +1200 Subject: [PATCH 04/67] ci: restrict push builds to master branch only Push events on feature branches were triggering the full Kafka matrix build intended for master. Restrict on.push to master only. Co-Authored-By: Claude Opus 4.6 (1M context) --- .github/workflows/maven.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/workflows/maven.yml b/.github/workflows/maven.yml index b39a9c838..04bf9c59c 100644 --- a/.github/workflows/maven.yml +++ b/.github/workflows/maven.yml @@ -13,6 +13,7 @@ name: Build and Test on: push: + branches: [ master ] pull_request: concurrency: From 34f0ead7756e067e251ac7b5268afeda57ec9104 Mon Sep 17 00:00:00 2001 From: Antony Stubbs Date: Tue, 14 Apr 2026 21:51:27 +1200 Subject: [PATCH 05/67] ci: add surefire rerun for flaky tests in Tier 1 and Tier 2 Retry failing tests up to 2 times to handle known flaky tests (queuedMessagesNotProcessedOrCommittedIfSubmittedDuringShutdown, JStreamParallelEoSStreamProcessorTest timing issues). Co-Authored-By: Claude Opus 4.6 (1M context) --- .github/workflows/maven.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/maven.yml b/.github/workflows/maven.yml index 04bf9c59c..25e773a55 100644 --- a/.github/workflows/maven.yml +++ b/.github/workflows/maven.yml @@ -37,7 +37,7 @@ jobs: java-version: '17' cache: 'maven' - name: Unit tests (no Docker) - run: ./mvnw --batch-mode test -Dlicense.skip -Dexcluded.groups=performance + run: ./mvnw --batch-mode test -Dlicense.skip -Dexcluded.groups=performance -Dsurefire.rerunFailingTestsCount=2 # Tier 2: Integration tests — needs Docker/TestContainers integration-tests: @@ -53,7 +53,7 @@ jobs: java-version: '17' cache: 'maven' - name: Integration tests (TestContainers) - run: ./mvnw --batch-mode verify -Dlicense.skip -Dexcluded.groups=performance + run: ./mvnw --batch-mode verify -Dlicense.skip -Dexcluded.groups=performance -Dsurefire.rerunFailingTestsCount=2 # Tier 3: Kafka version matrix — oldest and newest supported versions kafka-matrix: From 21b94da23f0c8d7ffc2ba3e8e07b263d41adcc3c Mon Sep 17 00:00:00 2001 From: Antony Stubbs Date: Tue, 14 Apr 2026 21:59:44 +1200 Subject: [PATCH 06/67] ci: simplify to single Build & Test then fan out matrix + performance MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The unit/integration split added overhead without real benefit. Simplified to: 1. Build & Test: full verify (unit + integration), default Kafka 2. Kafka Matrix (2.8.1 + 3.9.1) — after Build & Test 3. Performance Tests — after Build & Test Matrix and performance fan out in parallel after the main build passes. Co-Authored-By: Claude Opus 4.6 (1M context) --- .github/workflows/maven.yml | 46 +++++++++++-------------------------- 1 file changed, 13 insertions(+), 33 deletions(-) diff --git a/.github/workflows/maven.yml b/.github/workflows/maven.yml index 25e773a55..a50789cea 100644 --- a/.github/workflows/maven.yml +++ b/.github/workflows/maven.yml @@ -1,10 +1,6 @@ -# Tiered CI: fast feedback first, expensive tests only after cheap ones pass. -# # PR builds: -# Tier 1: Unit tests (no Docker, ~3 min) -# Tier 2: Integration tests (TestContainers, ~10 min) — waits for Tier 1 -# Tier 3: Kafka version matrix (2.8.1 + 3.9.1) — waits for Tier 2 -# Tier 4: Performance tests (@Tag("performance")) — waits for Tier 2 +# 1. Build & Test: full verify (unit + integration), default Kafka version +# 2. Kafka Matrix (2.8.1 + 3.9.1) + Performance tests — fan out after Build & Test # # Push builds (master): # Full Kafka version matrix (2.8.1, 3.1.0, 3.7.0, 3.9.1 + experimental 4.x) @@ -24,26 +20,10 @@ jobs: # ── PR Builds ────────────────────────────────────────────────────────── - # Tier 1: Unit tests — fast feedback, no Docker needed - unit-tests: - if: github.event_name == 'pull_request' - name: "Tier 1: Unit Tests" - runs-on: ubuntu-latest - steps: - - uses: actions/checkout@v6 - - uses: actions/setup-java@v5 - with: - distribution: 'temurin' - java-version: '17' - cache: 'maven' - - name: Unit tests (no Docker) - run: ./mvnw --batch-mode test -Dlicense.skip -Dexcluded.groups=performance -Dsurefire.rerunFailingTestsCount=2 - - # Tier 2: Integration tests — needs Docker/TestContainers - integration-tests: + # Full build and test — unit + integration, default Kafka version + build-and-test: if: github.event_name == 'pull_request' - needs: unit-tests - name: "Tier 2: Integration Tests" + name: "Build & Test" runs-on: ubuntu-latest steps: - uses: actions/checkout@v6 @@ -52,18 +32,18 @@ jobs: distribution: 'temurin' java-version: '17' cache: 'maven' - - name: Integration tests (TestContainers) + - name: Build and verify (unit + integration) run: ./mvnw --batch-mode verify -Dlicense.skip -Dexcluded.groups=performance -Dsurefire.rerunFailingTestsCount=2 - # Tier 3: Kafka version matrix — oldest and newest supported versions + # Kafka version matrix — oldest and newest supported versions kafka-matrix: if: github.event_name == 'pull_request' - needs: integration-tests + needs: build-and-test strategy: fail-fast: false matrix: ak: [ 2.8.1, 3.9.1 ] - name: "Tier 3: AK ${{ matrix.ak }}" + name: "AK ${{ matrix.ak }}" runs-on: ubuntu-latest steps: - uses: actions/checkout@v6 @@ -75,11 +55,11 @@ jobs: - name: Build and test with Kafka ${{ matrix.ak }} run: bin/ci-build.sh ${{ matrix.ak }} - # Tier 4: Performance tests — long-running volume/load tests + # Performance tests — volume/load tests that may uncover bugs at scale performance-tests: if: github.event_name == 'pull_request' - needs: integration-tests - name: "Tier 4: Performance Tests" + needs: build-and-test + name: "Performance Tests" runs-on: ubuntu-latest steps: - uses: actions/checkout@v6 @@ -89,7 +69,7 @@ jobs: java-version: '17' cache: 'maven' - name: Performance tests - run: ./mvnw --batch-mode verify -Dlicense.skip -Dincluded.groups=performance -Dexcluded.groups= + run: ./mvnw --batch-mode verify -Dlicense.skip -Dincluded.groups=performance -Dexcluded.groups= -Dsurefire.rerunFailingTestsCount=2 # ── Push Builds (master) ─────────────────────────────────────────────── From 2ad99ca75f583c685c60ccc812f4db232aaf4b6d Mon Sep 17 00:00:00 2001 From: Antony Stubbs Date: Tue, 14 Apr 2026 22:10:32 +1200 Subject: [PATCH 07/67] ci: use ci-build.sh for Build & Test, add perf exclusion and flaky rerun Use bin/ci-build.sh instead of raw mvnw to match existing CI setup (includes -Pci profile). Add -Dexcluded.groups=performance and -Dsurefire.rerunFailingTestsCount=2 to the script. Co-Authored-By: Claude Opus 4.6 (1M context) --- .github/workflows/maven.yml | 2 +- bin/ci-build.sh | 4 +++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/.github/workflows/maven.yml b/.github/workflows/maven.yml index a50789cea..c99c4220a 100644 --- a/.github/workflows/maven.yml +++ b/.github/workflows/maven.yml @@ -33,7 +33,7 @@ jobs: java-version: '17' cache: 'maven' - name: Build and verify (unit + integration) - run: ./mvnw --batch-mode verify -Dlicense.skip -Dexcluded.groups=performance -Dsurefire.rerunFailingTestsCount=2 + run: bin/ci-build.sh # Kafka version matrix — oldest and newest supported versions kafka-matrix: diff --git a/bin/ci-build.sh b/bin/ci-build.sh index 2d497b96d..e485214db 100755 --- a/bin/ci-build.sh +++ b/bin/ci-build.sh @@ -22,4 +22,6 @@ fi -Pci \ clean verify \ $KAFKA_VERSION_ARG \ - -Dlicense.skip + -Dlicense.skip \ + -Dexcluded.groups=performance \ + -Dsurefire.rerunFailingTestsCount=2 From 32a66aa2a0d089d17a99cf66c0099875763c52f8 Mon Sep 17 00:00:00 2001 From: Antony Stubbs Date: Tue, 14 Apr 2026 22:22:51 +1200 Subject: [PATCH 08/67] ci: use bin/performance-test.sh for performance job Ensures -Pci profile is used for compilation consistency. Co-Authored-By: Claude Opus 4.6 (1M context) --- .github/workflows/maven.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/maven.yml b/.github/workflows/maven.yml index c99c4220a..e87773d4e 100644 --- a/.github/workflows/maven.yml +++ b/.github/workflows/maven.yml @@ -69,7 +69,7 @@ jobs: java-version: '17' cache: 'maven' - name: Performance tests - run: ./mvnw --batch-mode verify -Dlicense.skip -Dincluded.groups=performance -Dexcluded.groups= -Dsurefire.rerunFailingTestsCount=2 + run: bin/performance-test.sh # ── Push Builds (master) ─────────────────────────────────────────────── From d5fe62ad556bf1ab0917833874bca8e9c574c45a Mon Sep 17 00:00:00 2001 From: Antony Stubbs Date: Tue, 14 Apr 2026 22:48:39 +1200 Subject: [PATCH 09/67] ci: add experimental Kafka 4.x to PR matrix (non-blocking) Add [3.9.1,5) range to the PR kafka-matrix with continue-on-error so we get early visibility into 4.x compatibility without blocking PRs. Co-Authored-By: Claude Opus 4.6 (1M context) --- .github/workflows/maven.yml | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/.github/workflows/maven.yml b/.github/workflows/maven.yml index e87773d4e..024d8652e 100644 --- a/.github/workflows/maven.yml +++ b/.github/workflows/maven.yml @@ -43,6 +43,11 @@ jobs: fail-fast: false matrix: ak: [ 2.8.1, 3.9.1 ] + experimental: [ false ] + include: + - ak: "'[3.9.1,5)'" + experimental: true + continue-on-error: ${{ matrix.experimental }} name: "AK ${{ matrix.ak }}" runs-on: ubuntu-latest steps: From 50d024c5278d74761ee2432d0f171911d707c410 Mon Sep 17 00:00:00 2001 From: "claude[bot]" <41898282+claude[bot]@users.noreply.github.com> Date: Tue, 14 Apr 2026 11:30:36 +0000 Subject: [PATCH 10/67] docs: remove SELF_HOSTED_RUNNER.md (not used atm) Co-authored-by: Antony Stubbs --- docs/SELF_HOSTED_RUNNER.md | 115 ------------------------------------- 1 file changed, 115 deletions(-) delete mode 100644 docs/SELF_HOSTED_RUNNER.md diff --git a/docs/SELF_HOSTED_RUNNER.md b/docs/SELF_HOSTED_RUNNER.md deleted file mode 100644 index 6c324e562..000000000 --- a/docs/SELF_HOSTED_RUNNER.md +++ /dev/null @@ -1,115 +0,0 @@ -# Self-Hosted Runner Setup (Windows + Docker Desktop) - -The performance test suite runs on a self-hosted GitHub Actions runner so we can throw real hardware at the volume tests. This document walks through the one-time setup on a Windows machine. - -## What you get - -- A GitHub Actions runner registered to your fork (`astubbs/parallel-consumer`) -- Triggered manually via `workflow_dispatch` or weekly on a schedule -- Runs the `@Tag("performance")` tests via `bin/performance-test.cmd` -- TestContainers spins up a Kafka broker via Docker Desktop (Linux containers / WSL2 backend) - -## Prerequisites on the runner machine - -1. **Windows 10/11** (or Server 2019+) -2. **WSL2 enabled** — required for Docker Desktop's Linux container backend -3. **Docker Desktop** for Windows, configured to use the WSL2 backend -4. **Git for Windows** (which includes Git Bash) -5. **Outbound HTTPS access** to `github.com`, `*.actions.githubusercontent.com`, Maven Central, and Docker Hub (or `images.confluent.io` for cp-kafka images) - -JDK 17 does **not** need to be pre-installed — the workflow uses `actions/setup-java` to provision it on first run, and caches it in the runner work directory. - -## One-time runner installation - -### 1. Generate a runner token - -1. Go to your fork's **Settings → Actions → Runners → New self-hosted runner** -2. Choose **Windows** and **x64** -3. GitHub will show you a PowerShell snippet that includes a one-time registration token - -### 2. Install the runner agent - -Open **PowerShell as your normal user** (not Administrator — the runner should not run as admin) and run the snippet GitHub provided. It looks roughly like this: - -```powershell -mkdir actions-runner -cd actions-runner -Invoke-WebRequest -Uri https://github.com/actions/runner/releases/download/v2.x.x/actions-runner-win-x64-2.x.x.zip -OutFile actions-runner.zip -Add-Type -AssemblyName System.IO.Compression.FileSystem ; [System.IO.Compression.ZipFile]::ExtractToDirectory("$PWD\actions-runner.zip", "$PWD") -``` - -### 3. Configure the runner with the right labels - -When prompted by `config.cmd`, set the labels so the workflow can find this runner: - -``` -Enter the name of the runner group to add this runner to: [press Enter for Default] -Enter the name of runner: [press Enter for hostname, or type something descriptive] -Enter any additional labels (ex. label-1,label-2): performance,windows -``` - -The full label set on this runner will be: `self-hosted`, `Windows`, `X64`, `performance`, `windows` - -> **Why two `windows` labels?** GitHub auto-adds `Windows` (capital W). The workflow targets the lowercase custom label `windows` to make intent explicit. Either works — pick whichever you prefer and update the workflow's `runs-on:` line if you change it. - -### 4. Run the runner as a service (recommended) - -So it survives reboots: - -```cmd -cd actions-runner -.\svc.cmd install -.\svc.cmd start -``` - -The service runs as the current user by default. Make sure that user has Docker Desktop access. - -### 5. Verify Docker is reachable - -In the same shell: - -```cmd -docker info -docker run --rm hello-world -``` - -If `docker info` complains about the daemon, start Docker Desktop and wait for it to finish initializing (the whale icon in the system tray should be steady, not animating). - -## Triggering the workflow - -### Manually - -1. Go to your fork on GitHub -2. **Actions → Performance Tests → Run workflow** -3. Optionally enter a Kafka version override -4. Click **Run workflow** - -### Automatically - -The workflow runs every **Sunday at 02:00 UTC** via cron schedule (defined in `.github/workflows/performance.yml`). - -## Security notes - -- Self-hosted runners on **public repositories** are dangerous because anyone can open a PR that runs arbitrary code on your machine. The performance workflow is configured to **only** run on `workflow_dispatch` (manual) and `schedule` triggers, never on PRs, to avoid this. -- Do not run the runner as Administrator. -- Keep Docker Desktop and the runner agent updated. - -## Troubleshooting - -**Runner shows offline in GitHub:** -- Check the service is running: `.\svc.cmd status` -- Check logs in `actions-runner\_diag\` -- Restart: `.\svc.cmd stop && .\svc.cmd start` - -**Tests fail with "Cannot connect to Docker daemon":** -- Make sure Docker Desktop is running -- Check WSL2 backend is enabled (Docker Desktop → Settings → General) -- Try `docker info` from the runner's shell - -**Tests run but are very slow:** -- Performance tests are *meant* to be slow — that's why they're on dedicated hardware -- Check Docker Desktop's resource allocation (Settings → Resources) — give it enough RAM (8GB+) and CPU cores - -**Workflow can't find the runner:** -- Verify the labels match what `runs-on:` expects in `.github/workflows/performance.yml` -- The runner must be online when the workflow is triggered From 798499086fb439504a3d24fe7bd30bb32a93bab1 Mon Sep 17 00:00:00 2001 From: Antony Stubbs Date: Tue, 14 Apr 2026 23:36:53 +1200 Subject: [PATCH 11/67] ci: skip claude code review on bot-triggered events Co-Authored-By: Claude Opus 4.6 (1M context) --- .github/workflows/claude-code-review.yml | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/.github/workflows/claude-code-review.yml b/.github/workflows/claude-code-review.yml index b5e8cfd4d..0d3503722 100644 --- a/.github/workflows/claude-code-review.yml +++ b/.github/workflows/claude-code-review.yml @@ -12,12 +12,7 @@ on: jobs: claude-review: - # Optional: Filter by PR author - # if: | - # github.event.pull_request.user.login == 'external-contributor' || - # github.event.pull_request.user.login == 'new-developer' || - # github.event.pull_request.author_association == 'FIRST_TIME_CONTRIBUTOR' - + if: github.event.sender.type != 'Bot' runs-on: ubuntu-latest permissions: contents: read From 2419762dc38973c4311437fbfcfb58a9798ffc03 Mon Sep 17 00:00:00 2001 From: Antony Stubbs Date: Wed, 15 Apr 2026 02:26:14 +1200 Subject: [PATCH 12/67] fix: match TestContainers broker version to Kafka client version The integration tests were hardcoded to cp-kafka:7.6.0 (Kafka 3.6) regardless of the -Dkafka.version override from the CI matrix. This caused 2.8.1 client to talk to a 3.6 broker, producing BrokerPollSystem errors. Now derives the CP Kafka Docker image version from the actual client version at runtime (CP major = AK major + 4, CP minor = AK minor). Co-Authored-By: Claude Opus 4.6 (1M context) --- .../BrokerIntegrationTest.java | 27 ++++++++++++++++++- 1 file changed, 26 insertions(+), 1 deletion(-) diff --git a/parallel-consumer-core/src/test-integration/java/io/confluent/parallelconsumer/integrationTests/BrokerIntegrationTest.java b/parallel-consumer-core/src/test-integration/java/io/confluent/parallelconsumer/integrationTests/BrokerIntegrationTest.java index 57275a8b3..f244e6e57 100644 --- a/parallel-consumer-core/src/test-integration/java/io/confluent/parallelconsumer/integrationTests/BrokerIntegrationTest.java +++ b/parallel-consumer-core/src/test-integration/java/io/confluent/parallelconsumer/integrationTests/BrokerIntegrationTest.java @@ -55,8 +55,33 @@ public abstract class BrokerIntegrationTest { */ public static KafkaContainer kafkaContainer = createKafkaContainer(null); + /** + * Derives the Confluent Platform version from the Apache Kafka client version so that + * the broker under test matches the client. The CI matrix overrides {@code kafka.version} + * via {@code -Dkafka.version=X.Y.Z}, so we read it at runtime from the client jar. + *

+ * Mapping: CP major = AK major + 4 (e.g., AK 2.8 → CP 6.2, AK 3.9 → CP 7.9). + */ + static String deriveCpKafkaImage() { + String akVersion = org.apache.kafka.common.utils.AppInfoParser.getVersion(); + log.info("Kafka client version detected: {}", akVersion); + + // Parse major.minor from the AK version + String[] parts = akVersion.split("\\."); + int akMajor = Integer.parseInt(parts[0]); + int akMinor = Integer.parseInt(parts[1]); + + // CP major = AK major + 4, CP minor = AK minor + int cpMajor = akMajor + 4; + int cpMinor = akMinor; + + String cpImage = "confluentinc/cp-kafka:" + cpMajor + "." + cpMinor + ".0"; + log.info("Using CP Kafka image: {} (derived from AK {})", cpImage, akVersion); + return cpImage; + } + public static KafkaContainer createKafkaContainer(String logSegmentSize) { - KafkaContainer base = new KafkaContainer(DockerImageName.parse("confluentinc/cp-kafka:7.6.0")) + KafkaContainer base = new KafkaContainer(DockerImageName.parse(deriveCpKafkaImage())) .withEnv("KAFKA_TRANSACTION_STATE_LOG_REPLICATION_FACTOR", "1") //transaction.state.log.replication.factor .withEnv("KAFKA_TRANSACTION_STATE_LOG_MIN_ISR", "1") //transaction.state.log.min.isr .withEnv("KAFKA_TRANSACTION_STATE_LOG_NUM_PARTITIONS", "1") //transaction.state.log.num.partitions From a35fc1571f4821a53ec010984e9752e4c577a2d6 Mon Sep 17 00:00:00 2001 From: Antony Stubbs Date: Wed, 15 Apr 2026 11:46:25 +1200 Subject: [PATCH 13/67] fix: handle null epoch when poll returns records before onPartitionsAssigned MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit With Kafka 2.x's eager rebalance protocol, poll() can return records for a partition before onPartitionsAssigned() has fired, leaving the epoch map empty. This caused a NullPointerException in EpochAndRecordsMap. Fix: skip records for partitions with no epoch yet. These records are safe to skip — they haven't been committed, so Kafka will re-deliver them on the next poll after the assignment callback completes. Includes lifecycle test proving: skip on first poll → assignment fires → re-poll succeeds with correct epoch → work created. Co-Authored-By: Claude Opus 4.6 (1M context) --- .../internal/EpochAndRecordsMap.java | 11 ++ .../state/PartitionStateManager.java | 3 +- .../internal/EpochAndRecordsMapRaceTest.java | 139 ++++++++++++++++++ 3 files changed, 152 insertions(+), 1 deletion(-) create mode 100644 parallel-consumer-core/src/test/java/io/confluent/parallelconsumer/internal/EpochAndRecordsMapRaceTest.java diff --git a/parallel-consumer-core/src/main/java/io/confluent/parallelconsumer/internal/EpochAndRecordsMap.java b/parallel-consumer-core/src/main/java/io/confluent/parallelconsumer/internal/EpochAndRecordsMap.java index 1ad4f6aa0..0b482ce8b 100644 --- a/parallel-consumer-core/src/main/java/io/confluent/parallelconsumer/internal/EpochAndRecordsMap.java +++ b/parallel-consumer-core/src/main/java/io/confluent/parallelconsumer/internal/EpochAndRecordsMap.java @@ -7,6 +7,7 @@ import io.confluent.parallelconsumer.state.PartitionStateManager; import lombok.NonNull; import lombok.Value; +import lombok.extern.slf4j.Slf4j; import org.apache.kafka.clients.consumer.ConsumerRecord; import org.apache.kafka.clients.consumer.ConsumerRecords; import org.apache.kafka.common.TopicPartition; @@ -18,6 +19,7 @@ * * @see BrokerPollSystem#partitionAssignmentEpoch */ +@Slf4j @Value public class EpochAndRecordsMap { @@ -27,6 +29,15 @@ public EpochAndRecordsMap(ConsumerRecords poll, PartitionStateManager { var records = poll.records(partition); Long epochOfPartition = pm.getEpochOfPartition(partition); + if (epochOfPartition == null) { + // Race: poll() returned records for a partition before onPartitionsAssigned() + // has fired. This is more likely with Kafka 2.x's eager rebalance protocol. + // Safe to skip — these records haven't been committed, so Kafka will re-deliver + // them on the next poll after the assignment callback completes. + log.warn("Skipping {} records for partition {} — no epoch assigned yet. " + + "Records will be re-delivered on next poll after assignment completes.", records.size(), partition); + return; + } RecordsAndEpoch entry = new RecordsAndEpoch(partition, epochOfPartition, records); recordMap.put(partition, entry); }); diff --git a/parallel-consumer-core/src/main/java/io/confluent/parallelconsumer/state/PartitionStateManager.java b/parallel-consumer-core/src/main/java/io/confluent/parallelconsumer/state/PartitionStateManager.java index fd47dd552..c2c55cb5d 100644 --- a/parallel-consumer-core/src/main/java/io/confluent/parallelconsumer/state/PartitionStateManager.java +++ b/parallel-consumer-core/src/main/java/io/confluent/parallelconsumer/state/PartitionStateManager.java @@ -248,12 +248,13 @@ private void resetOffsetMapAndRemoveWork(Collection allRemovedPa } /** - * @return the current epoch of the partition + * @return the current epoch of the partition, or null if not yet assigned */ public Long getEpochOfPartition(TopicPartition partition) { return partitionsAssignmentEpochs.get(partition); } + private void incrementPartitionAssignmentEpoch(final Collection partitions) { for (final TopicPartition partition : partitions) { Long epoch = partitionsAssignmentEpochs.getOrDefault(partition, PartitionState.KAFKA_OFFSET_ABSENCE); diff --git a/parallel-consumer-core/src/test/java/io/confluent/parallelconsumer/internal/EpochAndRecordsMapRaceTest.java b/parallel-consumer-core/src/test/java/io/confluent/parallelconsumer/internal/EpochAndRecordsMapRaceTest.java new file mode 100644 index 000000000..e329e6015 --- /dev/null +++ b/parallel-consumer-core/src/test/java/io/confluent/parallelconsumer/internal/EpochAndRecordsMapRaceTest.java @@ -0,0 +1,139 @@ +package io.confluent.parallelconsumer.internal; + +/*- + * Copyright (C) 2020-2026 Confluent, Inc. and contributors + */ + +import io.confluent.parallelconsumer.state.ModelUtils; +import io.confluent.parallelconsumer.state.PartitionStateManager; +import io.confluent.parallelconsumer.state.ShardManager; +import io.confluent.parallelconsumer.state.WorkContainer; +import io.confluent.parallelconsumer.state.WorkManager; +import lombok.extern.slf4j.Slf4j; +import org.apache.kafka.clients.consumer.ConsumerRecord; +import org.apache.kafka.clients.consumer.ConsumerRecords; +import org.apache.kafka.common.TopicPartition; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import pl.tlinkowski.unij.api.UniLists; +import pl.tlinkowski.unij.api.UniMaps; + +import java.util.List; + +import static com.google.common.truth.Truth.assertThat; +import static com.google.common.truth.Truth.assertWithMessage; + +/** + * Verifies that the epoch initialization race is handled safely: + *

    + *
  1. poll() returns records for a partition before onPartitionsAssigned() fires
    2. + *
  2. Records are safely skipped (no NPE crash)
    3. + *
  3. onPartitionsAssigned() fires, establishing the epoch and partition state
    4. + *
  4. Next poll creates valid work at the correct epoch
    5. + *
+ * This race is more likely with Kafka 2.x's eager rebalance protocol. + */ +@Slf4j +class EpochAndRecordsMapRaceTest { + + ModelUtils mu = new ModelUtils(); + WorkManager wm; + ShardManager sm; + PartitionStateManager pm; + + String topic = "topic"; + TopicPartition tp = new TopicPartition(topic, 0); + + @BeforeEach + void setup() { + PCModuleTestEnv module = mu.getModule(); + wm = module.workManager(); + sm = wm.getSm(); + pm = wm.getPm(); + // Deliberately NOT calling onPartitionsAssigned — simulating the race + } + + /** + * Core race scenario: poll returns records before onPartitionsAssigned fires. + * Records should be safely skipped (no NPE), and the map should be empty. + */ + @Test + void pollBeforeAssignmentShouldSkipRecordsNotCrash() { + // No onPartitionsAssigned called — epoch map is empty + assertThat(pm.getEpochOfPartition(tp)).isNull(); + + // poll() returns records for the unassigned partition + ConsumerRecords poll = new ConsumerRecords<>(UniMaps.of(tp, UniLists.of( + new ConsumerRecord<>(topic, 0, 0, "key", "value"), + new ConsumerRecord<>(topic, 0, 1, "key", "value") + ))); + + // This should NOT throw NPE — records are skipped + EpochAndRecordsMap recordsMap = new EpochAndRecordsMap<>(poll, pm); + + // Records should NOT be in the map (skipped due to missing epoch) + assertThat(recordsMap.count()).isEqualTo(0); + assertThat(recordsMap.partitions()).isEmpty(); + } + + /** + * Full lifecycle: poll before assignment (skipped) → assignment fires → re-poll succeeds. + * Proves records are recovered after the assignment callback completes. + */ + @Test + void fullLifecycleRecordsRecoveredAfterAssignment() { + // Step 1: poll returns records before onPartitionsAssigned — safely skipped + ConsumerRecords firstPoll = new ConsumerRecords<>(UniMaps.of(tp, UniLists.of( + new ConsumerRecord<>(topic, 0, 0, "key-0", "value"), + new ConsumerRecord<>(topic, 0, 1, "key-1", "value") + ))); + EpochAndRecordsMap firstRecords = new EpochAndRecordsMap<>(firstPoll, pm); + + // Records were skipped — nothing to register + assertThat(firstRecords.count()).isEqualTo(0); + + // Step 2: onPartitionsAssigned fires (late) — epoch and partition state established + wm.onPartitionsAssigned(UniLists.of(tp)); + long epoch = pm.getEpochOfPartition(tp); + assertThat(epoch).isEqualTo(0L); + + // Step 3: Re-poll — Kafka re-delivers the same records (they were never committed) + ConsumerRecords secondPoll = new ConsumerRecords<>(UniMaps.of(tp, UniLists.of( + new ConsumerRecord<>(topic, 0, 0, "key-0", "value"), + new ConsumerRecord<>(topic, 0, 1, "key-1", "value") + ))); + EpochAndRecordsMap secondRecords = new EpochAndRecordsMap<>(secondPoll, pm); + + // Records should now be accepted with the correct epoch + assertThat(secondRecords.count()).isEqualTo(2); + assertThat(secondRecords.records(tp).getEpochOfPartitionAtPoll()).isEqualTo(0L); + + // Step 4: Register and verify work is created + wm.registerWork(secondRecords); + List> work = sm.getWorkIfAvailable(10); + assertWithMessage("Work should be available after assignment + re-poll") + .that(work).hasSize(2); + for (var wc : work) { + assertThat(wc.getEpoch()).isEqualTo(0L); + } + } + + /** + * When epoch is already present (normal case), records are processed normally. + */ + @Test + void normalCaseWithPreExistingEpochIsUnaffected() { + // Normal flow: onPartitionsAssigned first + wm.onPartitionsAssigned(UniLists.of(tp)); + assertThat(pm.getEpochOfPartition(tp)).isEqualTo(0L); + + // poll returns records — should use existing epoch + ConsumerRecords poll = new ConsumerRecords<>(UniMaps.of(tp, UniLists.of( + new ConsumerRecord<>(topic, 0, 0, "key", "value") + ))); + EpochAndRecordsMap recordsMap = new EpochAndRecordsMap<>(poll, pm); + + assertThat(recordsMap.count()).isEqualTo(1); + assertThat(recordsMap.records(tp).getEpochOfPartitionAtPoll()).isEqualTo(0L); + } +} From d284c412a25039b3617331f8ddc2dc2fdec34146 Mon Sep 17 00:00:00 2001 From: Antony Stubbs Date: Wed, 15 Apr 2026 12:59:45 +1200 Subject: [PATCH 14/67] =?UTF-8?q?ci:=20drop=20Kafka=202.8.1=20from=20matri?= =?UTF-8?q?x=20=E2=80=94=20EOL,=20no=20longer=20supported?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Kafka 2.8.1 is end-of-life (released April 2021, no security patches). The upstream never ran integration tests against 2.x either. Keep the defensive null-epoch handling and dynamic broker version matching as they're good practice regardless. PR matrix: 3.9.1 + experimental [3.9.1,5) for 4.x visibility Push matrix: 3.1.0, 3.7.0, 3.9.1 + experimental [3.9.1,5) Co-Authored-By: Claude Opus 4.6 (1M context) --- .github/workflows/maven.yml | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/.github/workflows/maven.yml b/.github/workflows/maven.yml index 024d8652e..72f036b6b 100644 --- a/.github/workflows/maven.yml +++ b/.github/workflows/maven.yml @@ -1,9 +1,9 @@ # PR builds: # 1. Build & Test: full verify (unit + integration), default Kafka version -# 2. Kafka Matrix (2.8.1 + 3.9.1) + Performance tests — fan out after Build & Test +# 2. Kafka Matrix (3.9.1 + experimental 4.x) + Performance tests — fan out after Build & Test # # Push builds (master): -# Full Kafka version matrix (2.8.1, 3.1.0, 3.7.0, 3.9.1 + experimental 4.x) +# Full Kafka version matrix (3.1.0, 3.7.0, 3.9.1 + experimental 4.x) name: Build and Test @@ -42,7 +42,7 @@ jobs: strategy: fail-fast: false matrix: - ak: [ 2.8.1, 3.9.1 ] + ak: [ 3.9.1 ] experimental: [ false ] include: - ak: "'[3.9.1,5)'" @@ -84,7 +84,7 @@ jobs: strategy: fail-fast: false matrix: - ak: [ 2.8.1, 3.1.0, 3.7.0, 3.9.1 ] + ak: [ 3.1.0, 3.7.0, 3.9.1 ] experimental: [ false ] name: [ "Stable" ] include: From 7bf585027a9c3005a9904d055a680674ab971408 Mon Sep 17 00:00:00 2001 From: Antony Stubbs Date: Wed, 15 Apr 2026 13:33:47 +1200 Subject: [PATCH 15/67] ci: run unit, integration, performance in parallel with fail-fast MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit All three test suites now start simultaneously on PRs. If unit tests fail (~5 min), GitHub cancels integration and performance immediately. Kafka version matrix is gated on all tests passing. Replaces the sequential Build & Test → fan out approach. Co-Authored-By: Claude Opus 4.6 (1M context) --- .github/workflows/maven.yml | 48 ++++++++++++++++++------------------- 1 file changed, 23 insertions(+), 25 deletions(-) diff --git a/.github/workflows/maven.yml b/.github/workflows/maven.yml index 72f036b6b..8439cff05 100644 --- a/.github/workflows/maven.yml +++ b/.github/workflows/maven.yml @@ -1,6 +1,7 @@ # PR builds: -# 1. Build & Test: full verify (unit + integration), default Kafka version -# 2. Kafka Matrix (3.9.1 + experimental 4.x) + Performance tests — fan out after Build & Test +# 1. Unit, Integration, Performance — all in parallel with fail-fast +# (if unit tests fail at ~5 min, integration + performance are cancelled) +# 2. Kafka Matrix (3.9.1 + experimental 4.x) — gated on all tests passing # # Push builds (master): # Full Kafka version matrix (3.1.0, 3.7.0, 3.9.1 + experimental 4.x) @@ -20,10 +21,23 @@ jobs: # ── PR Builds ────────────────────────────────────────────────────────── - # Full build and test — unit + integration, default Kafka version - build-and-test: + # All test suites in parallel — fail-fast cancels the rest if any fails + test: if: github.event_name == 'pull_request' - name: "Build & Test" + strategy: + fail-fast: true + matrix: + include: + - suite: unit + name: "Unit Tests" + cmd: "./mvnw --batch-mode -Pci test -Dlicense.skip -Dexcluded.groups=performance -Dsurefire.rerunFailingTestsCount=2" + - suite: integration + name: "Integration Tests" + cmd: "./mvnw --batch-mode -Pci verify -DskipUTs=true -Dlicense.skip -Dexcluded.groups=performance -Dsurefire.rerunFailingTestsCount=2" + - suite: performance + name: "Performance Tests" + cmd: "bin/performance-test.sh" + name: "${{ matrix.name }}" runs-on: ubuntu-latest steps: - uses: actions/checkout@v6 @@ -32,13 +46,13 @@ jobs: distribution: 'temurin' java-version: '17' cache: 'maven' - - name: Build and verify (unit + integration) - run: bin/ci-build.sh + - name: ${{ matrix.name }} + run: ${{ matrix.cmd }} - # Kafka version matrix — oldest and newest supported versions + # Kafka version matrix — gated on all test suites passing kafka-matrix: if: github.event_name == 'pull_request' - needs: build-and-test + needs: test strategy: fail-fast: false matrix: @@ -60,22 +74,6 @@ jobs: - name: Build and test with Kafka ${{ matrix.ak }} run: bin/ci-build.sh ${{ matrix.ak }} - # Performance tests — volume/load tests that may uncover bugs at scale - performance-tests: - if: github.event_name == 'pull_request' - needs: build-and-test - name: "Performance Tests" - runs-on: ubuntu-latest - steps: - - uses: actions/checkout@v6 - - uses: actions/setup-java@v5 - with: - distribution: 'temurin' - java-version: '17' - cache: 'maven' - - name: Performance tests - run: bin/performance-test.sh - # ── Push Builds (master) ─────────────────────────────────────────────── # Full Kafka version matrix as safety net From f894441d933acf4aea629839cdddf643e46d3902 Mon Sep 17 00:00:00 2001 From: Antony Stubbs Date: Wed, 15 Apr 2026 13:43:56 +1200 Subject: [PATCH 16/67] ci: remove redundant kafka-matrix from PR builds The default Kafka version (3.9.1) is already tested by the three parallel test suites. The kafka-matrix was duplicating this work. Multi-version testing is covered by the push-to-master matrix. Co-Authored-By: Claude Opus 4.6 (1M context) --- .github/workflows/maven.yml | 30 ++---------------------------- 1 file changed, 2 insertions(+), 28 deletions(-) diff --git a/.github/workflows/maven.yml b/.github/workflows/maven.yml index 8439cff05..e5398d1f0 100644 --- a/.github/workflows/maven.yml +++ b/.github/workflows/maven.yml @@ -1,7 +1,6 @@ # PR builds: -# 1. Unit, Integration, Performance — all in parallel with fail-fast -# (if unit tests fail at ~5 min, integration + performance are cancelled) -# 2. Kafka Matrix (3.9.1 + experimental 4.x) — gated on all tests passing +# Unit, Integration, Performance — all in parallel with fail-fast +# (if unit tests fail at ~5 min, integration + performance are cancelled) # # Push builds (master): # Full Kafka version matrix (3.1.0, 3.7.0, 3.9.1 + experimental 4.x) @@ -49,31 +48,6 @@ jobs: - name: ${{ matrix.name }} run: ${{ matrix.cmd }} - # Kafka version matrix — gated on all test suites passing - kafka-matrix: - if: github.event_name == 'pull_request' - needs: test - strategy: - fail-fast: false - matrix: - ak: [ 3.9.1 ] - experimental: [ false ] - include: - - ak: "'[3.9.1,5)'" - experimental: true - continue-on-error: ${{ matrix.experimental }} - name: "AK ${{ matrix.ak }}" - runs-on: ubuntu-latest - steps: - - uses: actions/checkout@v6 - - uses: actions/setup-java@v5 - with: - distribution: 'temurin' - java-version: '17' - cache: 'maven' - - name: Build and test with Kafka ${{ matrix.ak }} - run: bin/ci-build.sh ${{ matrix.ak }} - # ── Push Builds (master) ─────────────────────────────────────────────── # Full Kafka version matrix as safety net From 66038ebf369d08ef8d010c44cbcc5bf964c171ca Mon Sep 17 00:00:00 2001 From: Antony Stubbs Date: Wed, 15 Apr 2026 14:34:50 +1200 Subject: [PATCH 17/67] ci: add Codecov coverage tracking with drop prevention Upload JaCoCo reports to Codecov from all test suites and push builds. Fail PRs if overall coverage drops by more than 1% from the base branch. Patch coverage is reported but informational only. Co-Authored-By: Claude Opus 4.6 (1M context) --- .github/workflows/maven.yml | 12 ++++++++++++ codecov.yml | 11 +++++++++++ 2 files changed, 23 insertions(+) create mode 100644 codecov.yml diff --git a/.github/workflows/maven.yml b/.github/workflows/maven.yml index e5398d1f0..6ed19c1b8 100644 --- a/.github/workflows/maven.yml +++ b/.github/workflows/maven.yml @@ -47,6 +47,12 @@ jobs: cache: 'maven' - name: ${{ matrix.name }} run: ${{ matrix.cmd }} + - name: Upload coverage to Codecov + if: always() + uses: codecov/codecov-action@v5 + with: + files: '**/target/site/jacoco/jacoco.xml' + flags: ${{ matrix.suite }} # ── Push Builds (master) ─────────────────────────────────────────────── @@ -75,3 +81,9 @@ jobs: cache: 'maven' - name: Build and Test run: bin/ci-build.sh ${{ matrix.ak }} + - name: Upload coverage to Codecov + if: always() + uses: codecov/codecov-action@v5 + with: + files: '**/target/site/jacoco/jacoco.xml' + flags: ak-${{ matrix.ak }} diff --git a/codecov.yml b/codecov.yml new file mode 100644 index 000000000..0b0bba041 --- /dev/null +++ b/codecov.yml @@ -0,0 +1,11 @@ +coverage: + status: + project: + default: + # Fail if overall coverage drops from the base branch + target: auto + threshold: 1% + patch: + default: + # Don't enforce a minimum on new code — just track it + informational: true From 6c5fee1b8777b8195cb9dff2ca54fb93edc724d1 Mon Sep 17 00:00:00 2001 From: Antony Stubbs Date: Wed, 15 Apr 2026 15:03:29 +1200 Subject: [PATCH 18/67] ci: add CODECOV_TOKEN for branch-protected repos, document in AGENTS.md Codecov requires a token when branch protection is enabled. Added token reference to both PR and push upload steps. Documented the secret and coverage setup in AGENTS.md. Co-Authored-By: Claude Opus 4.6 (1M context) --- .github/workflows/maven.yml | 2 ++ AGENTS.md | 7 +++++++ 2 files changed, 9 insertions(+) diff --git a/.github/workflows/maven.yml b/.github/workflows/maven.yml index 6ed19c1b8..f7352ad15 100644 --- a/.github/workflows/maven.yml +++ b/.github/workflows/maven.yml @@ -53,6 +53,7 @@ jobs: with: files: '**/target/site/jacoco/jacoco.xml' flags: ${{ matrix.suite }} + token: ${{ secrets.CODECOV_TOKEN }} # ── Push Builds (master) ─────────────────────────────────────────────── @@ -87,3 +88,4 @@ jobs: with: files: '**/target/site/jacoco/jacoco.xml' flags: ak-${{ matrix.ak }} + token: ${{ secrets.CODECOV_TOKEN }} diff --git a/AGENTS.md b/AGENTS.md index 49a41ed68..3d79a160f 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -84,3 +84,10 @@ The default behavior on macOS dev machines is `format` mode (auto-fixes headers) - **GitHub Actions**: `.github/workflows/maven.yml` - runs on push/PR to master with Kafka version matrix. - **Semaphore** (Confluent internal): `.semaphore/semaphore.yml` - primary CI for upstream. +- **Code coverage**: JaCoCo reports are uploaded to [Codecov](https://app.codecov.io/gh/astubbs/parallel-consumer). PRs fail if overall coverage drops by more than 1%. + +### Required secrets + +| Secret | Purpose | +|--------|---------| +| `CODECOV_TOKEN` | Codecov upload token — required because branch protection is enabled. Get it from [Codecov settings](https://app.codecov.io/gh/astubbs/parallel-consumer/settings). | From 9004cde255d0fe7ed4b89b4ca3ab2655cbf05def Mon Sep 17 00:00:00 2001 From: Antony Stubbs Date: Wed, 15 Apr 2026 15:22:03 +1200 Subject: [PATCH 19/67] ci: add JaCoCo integration test coverage reporting Added prepare-agent-integration and report-integration executions to JaCoCo plugin so failsafe (integration test) coverage is captured. Updated Codecov upload to include jacoco-it reports. Co-Authored-By: Claude Opus 4.6 (1M context) --- .github/workflows/maven.yml | 4 ++-- pom.xml | 13 +++++++++++++ 2 files changed, 15 insertions(+), 2 deletions(-) diff --git a/.github/workflows/maven.yml b/.github/workflows/maven.yml index f7352ad15..10d89412d 100644 --- a/.github/workflows/maven.yml +++ b/.github/workflows/maven.yml @@ -51,7 +51,7 @@ jobs: if: always() uses: codecov/codecov-action@v5 with: - files: '**/target/site/jacoco/jacoco.xml' + files: '**/target/site/jacoco/jacoco.xml,**/target/site/jacoco-it/jacoco.xml' flags: ${{ matrix.suite }} token: ${{ secrets.CODECOV_TOKEN }} @@ -86,6 +86,6 @@ jobs: if: always() uses: codecov/codecov-action@v5 with: - files: '**/target/site/jacoco/jacoco.xml' + files: '**/target/site/jacoco/jacoco.xml,**/target/site/jacoco-it/jacoco.xml' flags: ak-${{ matrix.ak }} token: ${{ secrets.CODECOV_TOKEN }} diff --git a/pom.xml b/pom.xml index 94d8b2494..6e191d7bf 100644 --- a/pom.xml +++ b/pom.xml @@ -737,6 +737,19 @@ report + + prepare-agent-integration + + prepare-agent-integration + + + + report-integration + post-integration-test + + report-integration + + From b58e3c2a0e134091a052753b46302e51af455a2c Mon Sep 17 00:00:00 2001 From: Antony Stubbs Date: Wed, 15 Apr 2026 15:30:33 +1200 Subject: [PATCH 20/67] ci: add jscpd duplicate code detection to PR builds MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Runs in parallel with test suites. Reports duplicate Java code blocks across all source including tests. No failure threshold yet — reporting only to establish a baseline. Co-Authored-By: Claude Opus 4.6 (1M context) --- .github/workflows/maven.yml | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/.github/workflows/maven.yml b/.github/workflows/maven.yml index 10d89412d..b9185a2fa 100644 --- a/.github/workflows/maven.yml +++ b/.github/workflows/maven.yml @@ -55,6 +55,19 @@ jobs: flags: ${{ matrix.suite }} token: ${{ secrets.CODECOV_TOKEN }} + # Duplicate code detection — runs in parallel with tests + duplicate-detection: + if: github.event_name == 'pull_request' + name: "Duplicate Code Check" + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v6 + - uses: actions/setup-node@v4 + with: + node-version: '22' + - name: Run jscpd + run: npx jscpd@4 --pattern "**/*.java" --ignore "**/target/**,**/node_modules/**" --reporters consoleFull --min-lines 6 --min-tokens 70 + # ── Push Builds (master) ─────────────────────────────────────────────── # Full Kafka version matrix as safety net From 28337269bf71d47f982f7875d5d22d9cf6e603f7 Mon Sep 17 00:00:00 2001 From: Antony Stubbs Date: Wed, 15 Apr 2026 15:37:07 +1200 Subject: [PATCH 21/67] ci: add SpotBugs, dependency vulnerability scanning, and PIT mutation testing Three new parallel CI jobs for PRs: - SpotBugs: static analysis for null derefs, concurrency bugs, resource leaks - Dependency Vulnerabilities: OSS Index audit for known CVEs - Mutation Testing (PIT): verifies test assertions are meaningful All run in parallel with existing test suites. SpotBugs and PIT are reporting-only for now (no build failure). Dependency scan uses the existing ossindex plugin config. Co-Authored-By: Claude Opus 4.6 (1M context) --- .github/workflows/maven.yml | 58 +++++++++++++++++++++++++++++++++++++ pom.xml | 33 +++++++++++++++++++++ 2 files changed, 91 insertions(+) diff --git a/.github/workflows/maven.yml b/.github/workflows/maven.yml index b9185a2fa..9f63b9c0b 100644 --- a/.github/workflows/maven.yml +++ b/.github/workflows/maven.yml @@ -68,6 +68,64 @@ jobs: - name: Run jscpd run: npx jscpd@4 --pattern "**/*.java" --ignore "**/target/**,**/node_modules/**" --reporters consoleFull --min-lines 6 --min-tokens 70 + # SpotBugs static analysis — finds null derefs, concurrency issues, resource leaks + spotbugs: + if: github.event_name == 'pull_request' + name: "SpotBugs" + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v6 + - uses: actions/setup-java@v5 + with: + distribution: 'temurin' + java-version: '17' + cache: 'maven' + - name: Compile and run SpotBugs + run: ./mvnw --batch-mode -Pci compile spotbugs:check -Dlicense.skip -Dspotbugs.failOnError=false + - name: Upload SpotBugs results + if: always() + uses: actions/upload-artifact@v4 + with: + name: spotbugs-report + path: '**/target/spotbugsXml.xml' + + # Dependency vulnerability scanning — checks for known CVEs + dependency-scan: + if: github.event_name == 'pull_request' + name: "Dependency Vulnerabilities" + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v6 + - uses: actions/setup-java@v5 + with: + distribution: 'temurin' + java-version: '17' + cache: 'maven' + - name: OSS Index audit + run: ./mvnw --batch-mode -Pci org.sonatype.ossindex:ossindex-maven-plugin:audit -Dlicense.skip + + # Mutation testing (PIT) — verifies test assertions are meaningful + mutation-testing: + if: github.event_name == 'pull_request' + name: "Mutation Testing (PIT)" + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v6 + - uses: actions/setup-java@v5 + with: + distribution: 'temurin' + java-version: '17' + cache: 'maven' + - name: Run PIT mutation testing + run: ./mvnw --batch-mode -Pci test-compile org.pitest:pitest-maven:mutationCoverage -Dlicense.skip -DtargetClasses="io.confluent.parallelconsumer.*" -DtargetTests="io.confluent.parallelconsumer.*" -Dthreads=4 -pl parallel-consumer-core + continue-on-error: true + - name: Upload PIT report + if: always() + uses: actions/upload-artifact@v4 + with: + name: pit-report + path: '**/target/pit-reports/**' + # ── Push Builds (master) ─────────────────────────────────────────────── # Full Kafka version matrix as safety net diff --git a/pom.xml b/pom.xml index 6e191d7bf..a7fe1bb56 100644 --- a/pom.xml +++ b/pom.xml @@ -94,6 +94,10 @@ 1.18.28 1.1.1 3.2.5 + 4.8.6 + 4.8.6.6 + 1.17.4 + 1.2.2 2.0.13 @@ -752,6 +756,35 @@ + + com.github.spotbugs + spotbugs-maven-plugin + ${spotbugs-maven-plugin.version} + + Max + Medium + true + + + + com.github.spotbugs + spotbugs + ${spotbugs.version} + + + + + org.pitest + pitest-maven + ${pitest.version} + + + org.pitest + pitest-junit5-plugin + ${pitest-junit5.version} + + + org.apache.maven.plugins maven-enforcer-plugin From 1f4816da2145aafd2d04b2b3ac80f064075241ca Mon Sep 17 00:00:00 2001 From: Antony Stubbs Date: Wed, 15 Apr 2026 15:49:57 +1200 Subject: [PATCH 22/67] ci: fix SpotBugs to core module only, make dependency scan non-blocking MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit SpotBugs failed on vertx module due to Jabel cross-compilation. Restrict to parallel-consumer-core for now. OSS Index audit gets 401s from the API intermittently — make it continue-on-error. Co-Authored-By: Claude Opus 4.6 (1M context) --- .github/workflows/maven.yml | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/.github/workflows/maven.yml b/.github/workflows/maven.yml index 9f63b9c0b..30dbdd239 100644 --- a/.github/workflows/maven.yml +++ b/.github/workflows/maven.yml @@ -81,7 +81,7 @@ jobs: java-version: '17' cache: 'maven' - name: Compile and run SpotBugs - run: ./mvnw --batch-mode -Pci compile spotbugs:check -Dlicense.skip -Dspotbugs.failOnError=false + run: ./mvnw --batch-mode -Pci compile spotbugs:check -Dlicense.skip -Dspotbugs.failOnError=false -pl parallel-consumer-core -am - name: Upload SpotBugs results if: always() uses: actions/upload-artifact@v4 @@ -103,6 +103,7 @@ jobs: cache: 'maven' - name: OSS Index audit run: ./mvnw --batch-mode -Pci org.sonatype.ossindex:ossindex-maven-plugin:audit -Dlicense.skip + continue-on-error: true # Mutation testing (PIT) — verifies test assertions are meaningful mutation-testing: From 94ba8558855a2c2603b204a46614a6f7df2f8e6c Mon Sep 17 00:00:00 2001 From: Antony Stubbs Date: Wed, 15 Apr 2026 20:41:53 +1200 Subject: [PATCH 23/67] ci: use build scripts for all CI jobs, add timeouts, restore all modules MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Create bin/ci-unit-test.sh and bin/ci-integration-test.sh so all CI jobs use scripts with consistent -Pci flags instead of inline commands - Add timeout-minutes to all jobs (30 for tests, 10 for SpotBugs, 5 for duplicate/dependency checks, 15 for PIT) - Restore all modules for integration and performance tests (reverts the -pl restriction — slow tests should be investigated, not removed) - Cherry-pick Mutiny release.target=9 fix for full module compilation - Revert performance-test.sh -pl restriction - Document new scripts in README Co-Authored-By: Claude Opus 4.6 (1M context) --- .github/workflows/maven.yml | 10 ++++++++-- README.adoc | 8 +++++++- bin/ci-integration-test.sh | 18 ++++++++++++++++++ bin/ci-unit-test.sh | 16 ++++++++++++++++ src/docs/README_TEMPLATE.adoc | 8 +++++++- 5 files changed, 56 insertions(+), 4 deletions(-) create mode 100755 bin/ci-integration-test.sh create mode 100755 bin/ci-unit-test.sh diff --git a/.github/workflows/maven.yml b/.github/workflows/maven.yml index 30dbdd239..9f1efbb20 100644 --- a/.github/workflows/maven.yml +++ b/.github/workflows/maven.yml @@ -29,15 +29,16 @@ jobs: include: - suite: unit name: "Unit Tests" - cmd: "./mvnw --batch-mode -Pci test -Dlicense.skip -Dexcluded.groups=performance -Dsurefire.rerunFailingTestsCount=2" + cmd: "bin/ci-unit-test.sh" - suite: integration name: "Integration Tests" - cmd: "./mvnw --batch-mode -Pci verify -DskipUTs=true -Dlicense.skip -Dexcluded.groups=performance -Dsurefire.rerunFailingTestsCount=2" + cmd: "bin/ci-integration-test.sh" - suite: performance name: "Performance Tests" cmd: "bin/performance-test.sh" name: "${{ matrix.name }}" runs-on: ubuntu-latest + timeout-minutes: 30 steps: - uses: actions/checkout@v6 - uses: actions/setup-java@v5 @@ -60,6 +61,7 @@ jobs: if: github.event_name == 'pull_request' name: "Duplicate Code Check" runs-on: ubuntu-latest + timeout-minutes: 5 steps: - uses: actions/checkout@v6 - uses: actions/setup-node@v4 @@ -73,6 +75,7 @@ jobs: if: github.event_name == 'pull_request' name: "SpotBugs" runs-on: ubuntu-latest + timeout-minutes: 10 steps: - uses: actions/checkout@v6 - uses: actions/setup-java@v5 @@ -94,6 +97,7 @@ jobs: if: github.event_name == 'pull_request' name: "Dependency Vulnerabilities" runs-on: ubuntu-latest + timeout-minutes: 5 steps: - uses: actions/checkout@v6 - uses: actions/setup-java@v5 @@ -110,6 +114,7 @@ jobs: if: github.event_name == 'pull_request' name: "Mutation Testing (PIT)" runs-on: ubuntu-latest + timeout-minutes: 15 steps: - uses: actions/checkout@v6 - uses: actions/setup-java@v5 @@ -145,6 +150,7 @@ jobs: continue-on-error: ${{ matrix.experimental }} name: "AK: ${{ matrix.ak }} (${{ matrix.name }})" runs-on: ubuntu-latest + timeout-minutes: 30 steps: - uses: actions/checkout@v6 - uses: actions/setup-java@v5 diff --git a/README.adoc b/README.adoc index 8a6e08ca9..0ed2f6c13 100644 --- a/README.adoc +++ b/README.adoc @@ -1349,6 +1349,12 @@ Helper scripts are in the `bin/` directory: Quick local build (compile + unit tests):: `bin/build.sh` +Unit tests only (no Docker needed):: +`bin/ci-unit-test.sh` + +Integration tests only (requires Docker for TestContainers):: +`bin/ci-integration-test.sh` + Full CI build with all tests (unit + integration):: `bin/ci-build.sh` @@ -1358,7 +1364,7 @@ CI build against a specific Kafka version:: Performance test suite (also `bin/performance-test.cmd` on Windows):: `bin/performance-test.sh` -The GitHub Actions CI workflow uses `bin/ci-build.sh`, so running it locally reproduces the CI environment. +All `ci-*` scripts use the `-Pci` Maven profile which enables license checking and disables parallel test execution. The GitHub Actions CI workflow uses these scripts, so running them locally reproduces the CI environment. === Performance Tests diff --git a/bin/ci-integration-test.sh b/bin/ci-integration-test.sh new file mode 100755 index 000000000..03e996cae --- /dev/null +++ b/bin/ci-integration-test.sh @@ -0,0 +1,18 @@ +#!/usr/bin/env bash +# +# Copyright (C) 2020-2026 Confluent, Inc. +# + +# Run integration tests only (failsafe, requires Docker for TestContainers). +# Skips unit tests to avoid duplicate work. +# Usage: bin/ci-integration-test.sh + +set -euo pipefail + +./mvnw --batch-mode \ + -Pci \ + clean verify \ + -DskipUTs=true \ + -Dlicense.skip \ + -Dexcluded.groups=performance \ + -Dsurefire.rerunFailingTestsCount=2 diff --git a/bin/ci-unit-test.sh b/bin/ci-unit-test.sh new file mode 100755 index 000000000..90ff19e8c --- /dev/null +++ b/bin/ci-unit-test.sh @@ -0,0 +1,16 @@ +#!/usr/bin/env bash +# +# Copyright (C) 2020-2026 Confluent, Inc. +# + +# Run unit tests only (surefire, no Docker/TestContainers needed). +# Usage: bin/ci-unit-test.sh + +set -euo pipefail + +./mvnw --batch-mode \ + -Pci \ + clean test \ + -Dlicense.skip \ + -Dexcluded.groups=performance \ + -Dsurefire.rerunFailingTestsCount=2 diff --git a/src/docs/README_TEMPLATE.adoc b/src/docs/README_TEMPLATE.adoc index 3bedc5633..301fe0119 100644 --- a/src/docs/README_TEMPLATE.adoc +++ b/src/docs/README_TEMPLATE.adoc @@ -1100,6 +1100,12 @@ Helper scripts are in the `bin/` directory: Quick local build (compile + unit tests):: `bin/build.sh` +Unit tests only (no Docker needed):: +`bin/ci-unit-test.sh` + +Integration tests only (requires Docker for TestContainers):: +`bin/ci-integration-test.sh` + Full CI build with all tests (unit + integration):: `bin/ci-build.sh` @@ -1109,7 +1115,7 @@ CI build against a specific Kafka version:: Performance test suite (also `bin/performance-test.cmd` on Windows):: `bin/performance-test.sh` -The GitHub Actions CI workflow uses `bin/ci-build.sh`, so running it locally reproduces the CI environment. +All `ci-*` scripts use the `-Pci` Maven profile which enables license checking and disables parallel test execution. The GitHub Actions CI workflow uses these scripts, so running them locally reproduces the CI environment. === Performance Tests From cdfd23e48e61849000449716ced36e382fc8f2bf Mon Sep 17 00:00:00 2001 From: Antony Stubbs Date: Wed, 15 Apr 2026 20:55:34 +1200 Subject: [PATCH 24/67] ci: add PR comments for duplicate detection, SpotBugs, and PIT reports - jscpd: posts summary table + duplicate block list as PR comment - SpotBugs: uses spotbugs-github-action for inline PR annotations - PIT: posts mutation score summary as PR comment with artifact link All comments are updated in-place on subsequent runs (no spam). Co-Authored-By: Claude Opus 4.6 (1M context) --- .github/workflows/maven.yml | 78 ++++++++++++++++++++++++++++++++++--- 1 file changed, 73 insertions(+), 5 deletions(-) diff --git a/.github/workflows/maven.yml b/.github/workflows/maven.yml index 9f1efbb20..497d31803 100644 --- a/.github/workflows/maven.yml +++ b/.github/workflows/maven.yml @@ -68,7 +68,40 @@ jobs: with: node-version: '22' - name: Run jscpd - run: npx jscpd@4 --pattern "**/*.java" --ignore "**/target/**,**/node_modules/**" --reporters consoleFull --min-lines 6 --min-tokens 70 + run: npx jscpd@4 --pattern "**/*.java" --ignore "**/target/**,**/node_modules/**" --reporters consoleFull,json --output jscpd-report --min-lines 6 --min-tokens 70 + - name: Post duplicate report to PR + if: always() + uses: actions/github-script@v7 + with: + script: | + const fs = require('fs'); + const path = 'jscpd-report/jscpd-report.json'; + if (!fs.existsSync(path)) { console.log('No report found'); return; } + const report = JSON.parse(fs.readFileSync(path, 'utf8')); + const stats = report.statistics; + const total = stats.total; + let body = `## Duplicate Code Report\n\n`; + body += `| Metric | Value |\n|--------|-------|\n`; + body += `| Files analyzed | ${total.sources} |\n`; + body += `| Clones found | ${total.clones} |\n`; + body += `| Duplicated lines | ${total.duplicatedLines} (${total.percentage}%) |\n\n`; + if (report.duplicates && report.duplicates.length > 0) { + body += `
Duplicate blocks (${report.duplicates.length})\n\n`; + for (const d of report.duplicates.slice(0, 30)) { + const f1 = d.firstFile.name.replace(/^.*parallel-consumer\//, ''); + const f2 = d.secondFile.name.replace(/^.*parallel-consumer\//, ''); + body += `- **${d.lines} lines** duplicated:\n \`${f1}:${d.firstFile.startLoc.line}\` ↔ \`${f2}:${d.secondFile.startLoc.line}\`\n`; + } + if (report.duplicates.length > 30) body += `\n...and ${report.duplicates.length - 30} more\n`; + body += `\n
\n`; + } + const comments = await github.rest.issues.listComments({ owner: context.repo.owner, repo: context.repo.repo, issue_number: context.issue.number }); + const existing = comments.data.find(c => c.body.includes('## Duplicate Code Report')); + if (existing) { + await github.rest.issues.updateComment({ owner: context.repo.owner, repo: context.repo.repo, comment_id: existing.id, body }); + } else { + await github.rest.issues.createComment({ owner: context.repo.owner, repo: context.repo.repo, issue_number: context.issue.number, body }); + } # SpotBugs static analysis — finds null derefs, concurrency issues, resource leaks spotbugs: @@ -84,13 +117,13 @@ jobs: java-version: '17' cache: 'maven' - name: Compile and run SpotBugs - run: ./mvnw --batch-mode -Pci compile spotbugs:check -Dlicense.skip -Dspotbugs.failOnError=false -pl parallel-consumer-core -am - - name: Upload SpotBugs results + run: ./mvnw --batch-mode -Pci compile spotbugs:spotbugs -Dlicense.skip -pl parallel-consumer-core -am + - name: Post SpotBugs annotations on PR if: always() - uses: actions/upload-artifact@v4 + uses: jwgmeligmeyling/spotbugs-github-action@v1.2 with: - name: spotbugs-report path: '**/target/spotbugsXml.xml' + token: ${{ secrets.GITHUB_TOKEN }} # Dependency vulnerability scanning — checks for known CVEs dependency-scan: @@ -131,6 +164,41 @@ jobs: with: name: pit-report path: '**/target/pit-reports/**' + - name: Post PIT summary to PR + if: always() + uses: actions/github-script@v7 + with: + script: | + const fs = require('fs'); + const glob = require('glob'); + const files = glob.sync('**/target/pit-reports/**/mutations.csv'); + if (files.length === 0) { console.log('No PIT report found'); return; } + let killed = 0, survived = 0, noCov = 0, total = 0; + for (const f of files) { + const lines = fs.readFileSync(f, 'utf8').split('\n').filter(l => l.trim()); + for (const line of lines) { + total++; + if (line.includes('KILLED')) killed++; + else if (line.includes('SURVIVED')) survived++; + else if (line.includes('NO_COVERAGE')) noCov++; + } + } + const score = total > 0 ? ((killed / total) * 100).toFixed(1) : '0'; + let body = `## Mutation Testing (PIT) Report\n\n`; + body += `| Metric | Value |\n|--------|-------|\n`; + body += `| Mutations generated | ${total} |\n`; + body += `| Killed (detected) | ${killed} |\n`; + body += `| Survived (missed) | ${survived} |\n`; + body += `| No coverage | ${noCov} |\n`; + body += `| **Mutation score** | **${score}%** |\n\n`; + body += `Full HTML report available as artifact: \`pit-report\`\n`; + const comments = await github.rest.issues.listComments({ owner: context.repo.owner, repo: context.repo.repo, issue_number: context.issue.number }); + const existing = comments.data.find(c => c.body.includes('## Mutation Testing (PIT) Report')); + if (existing) { + await github.rest.issues.updateComment({ owner: context.repo.owner, repo: context.repo.repo, comment_id: existing.id, body }); + } else { + await github.rest.issues.createComment({ owner: context.repo.owner, repo: context.repo.repo, issue_number: context.issue.number, body }); + } # ── Push Builds (master) ─────────────────────────────────────────────── From c7d27ce4752a94cee1546487ddf6d7981769ee28 Mon Sep 17 00:00:00 2001 From: Antony Stubbs Date: Wed, 15 Apr 2026 21:01:28 +1200 Subject: [PATCH 25/67] ci: fix PIT reactor convergence (-am flag) and glob dependency in script Co-Authored-By: Claude Opus 4.6 (1M context) --- .github/workflows/maven.yml | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/.github/workflows/maven.yml b/.github/workflows/maven.yml index 497d31803..d2758dcba 100644 --- a/.github/workflows/maven.yml +++ b/.github/workflows/maven.yml @@ -156,7 +156,7 @@ jobs: java-version: '17' cache: 'maven' - name: Run PIT mutation testing - run: ./mvnw --batch-mode -Pci test-compile org.pitest:pitest-maven:mutationCoverage -Dlicense.skip -DtargetClasses="io.confluent.parallelconsumer.*" -DtargetTests="io.confluent.parallelconsumer.*" -Dthreads=4 -pl parallel-consumer-core + run: ./mvnw --batch-mode -Pci test-compile org.pitest:pitest-maven:mutationCoverage -Dlicense.skip -DtargetClasses="io.confluent.parallelconsumer.*" -DtargetTests="io.confluent.parallelconsumer.*" -Dthreads=4 -pl parallel-consumer-core -am continue-on-error: true - name: Upload PIT report if: always() @@ -170,8 +170,8 @@ jobs: with: script: | const fs = require('fs'); - const glob = require('glob'); - const files = glob.sync('**/target/pit-reports/**/mutations.csv'); + const { execSync } = require('child_process'); + const files = execSync('find . -path "*/pit-reports/*/mutations.csv" -type f').toString().trim().split('\n').filter(f => f); if (files.length === 0) { console.log('No PIT report found'); return; } let killed = 0, survived = 0, noCov = 0, total = 0; for (const f of files) { From 8b76f1f37850d4024c83a0da1c07b20c062cf253 Mon Sep 17 00:00:00 2001 From: Antony Stubbs Date: Wed, 15 Apr 2026 21:12:12 +1200 Subject: [PATCH 26/67] ci: replace ossindex with GitHub dependency-review-action GitHub's own action is more reliable (no external API auth issues), posts a summary comment on the PR, and uses GitHub's advisory database. Fails on high/critical severity CVEs in new dependencies. Co-Authored-By: Claude Opus 4.6 (1M context) --- .github/workflows/maven.yml | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/.github/workflows/maven.yml b/.github/workflows/maven.yml index d2758dcba..f1f734b4b 100644 --- a/.github/workflows/maven.yml +++ b/.github/workflows/maven.yml @@ -125,22 +125,21 @@ jobs: path: '**/target/spotbugsXml.xml' token: ${{ secrets.GITHUB_TOKEN }} - # Dependency vulnerability scanning — checks for known CVEs + # Dependency vulnerability scanning — GitHub's own dependency review dependency-scan: if: github.event_name == 'pull_request' name: "Dependency Vulnerabilities" runs-on: ubuntu-latest timeout-minutes: 5 + permissions: + contents: read + pull-requests: write steps: - uses: actions/checkout@v6 - - uses: actions/setup-java@v5 + - uses: actions/dependency-review-action@v4 with: - distribution: 'temurin' - java-version: '17' - cache: 'maven' - - name: OSS Index audit - run: ./mvnw --batch-mode -Pci org.sonatype.ossindex:ossindex-maven-plugin:audit -Dlicense.skip - continue-on-error: true + fail-on-severity: high + comment-summary-in-pr: always # Mutation testing (PIT) — verifies test assertions are meaningful mutation-testing: From e273d691a8b4d8369846d08733ae5f5a59e0a787 Mon Sep 17 00:00:00 2001 From: Antony Stubbs Date: Wed, 15 Apr 2026 21:21:36 +1200 Subject: [PATCH 27/67] ci: re-trigger after enabling dependency graph in repo settings From 5178b5d6403535b7603c38887f223b11fbb33f23 Mon Sep 17 00:00:00 2001 From: Antony Stubbs Date: Thu, 16 Apr 2026 12:28:00 +1200 Subject: [PATCH 28/67] fix: VertxConcurrencyIT stall on CI - increase wait, add timeout, protect latch The test stalled for 50+ minutes on CI because: 1. The 20s wait for 50 concurrent requests was too short for resource-constrained runners 2. If the wait failed, the WireMock threads were orphaned blocking on an unreleased latch, each waiting 30s before timing out Fixes: - Increase wait-for-requests from 20s to 120s - Add @Timeout(5 minutes) to the test method - Move latch release into a finally block so threads are always unblocked, even on test failure Co-Authored-By: Claude Opus 4.6 (1M context) --- .../vertx/integrationTests/VertxConcurrencyIT.java | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/parallel-consumer-vertx/src/test-integration/java/io/confluent/parallelconsumer/vertx/integrationTests/VertxConcurrencyIT.java b/parallel-consumer-vertx/src/test-integration/java/io/confluent/parallelconsumer/vertx/integrationTests/VertxConcurrencyIT.java index 31838f9b8..836e4f64f 100644 --- a/parallel-consumer-vertx/src/test-integration/java/io/confluent/parallelconsumer/vertx/integrationTests/VertxConcurrencyIT.java +++ b/parallel-consumer-vertx/src/test-integration/java/io/confluent/parallelconsumer/vertx/integrationTests/VertxConcurrencyIT.java @@ -32,6 +32,7 @@ import org.junit.jupiter.api.AfterAll; import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.Timeout; import org.junit.jupiter.api.parallel.Isolated; import org.testcontainers.junit.jupiter.Testcontainers; import pl.tlinkowski.unij.api.UniMaps; @@ -40,6 +41,7 @@ import java.util.concurrent.ConcurrentLinkedQueue; import java.util.concurrent.CountDownLatch; import java.util.concurrent.Future; +import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicInteger; import static com.github.tomakehurst.wiremock.client.WireMock.*; @@ -134,6 +136,7 @@ static void close() { * would expect. */ @Test + @Timeout(value = 5, unit = TimeUnit.MINUTES) @SneakyThrows void testVertxConcurrency() { var commitMode = PERIODIC_CONSUMER_ASYNCHRONOUS; @@ -208,7 +211,7 @@ void testVertxConcurrency() { var failureMessage = msg("Mock server receives {} requests in parallel from vertx engine", expectedMessageCount / 2); try { - waitAtMost(ofSeconds(20)) + waitAtMost(ofSeconds(120)) .pollInterval(ofMillis(200)) .alias(failureMessage) .untilAsserted(() -> { @@ -217,11 +220,12 @@ void testVertxConcurrency() { }); } catch (ConditionTimeoutException e) { fail(failureMessage + "\n" + e.getMessage()); + } finally { + // Always release the latch — if the test fails, this prevents WireMock threads + // from hanging for 30s each on the unreleased latch + log.info("{} requests received by server, releasing server response lock.", requestsReceivedOnServer.size()); + LatchTestUtils.release(responseLock); } - log.info("{} requests received in parallel by server, releasing server response lock.", requestsReceivedOnServer.size()); - - // all requests were received in parallel, so unlock the server to respond to all of them - LatchTestUtils.release(responseLock); // assertNumberOfThreads(); From a26964b2bb3904fa9e767a2512037999ffd9839d Mon Sep 17 00:00:00 2001 From: Antony Stubbs Date: Thu, 16 Apr 2026 12:28:14 +1200 Subject: [PATCH 29/67] docs: consolidate all agent rules into AGENTS.md Add comprehensive Agent Rules section covering git safety, development discipline, code quality, test discipline, CI/automation, documentation, communication, rule sync, and working directory conventions. Update CI section to reflect current parallel build setup with SpotBugs, PIT mutation testing, jscpd duplicate detection, and dependency scanning. These rules were previously scattered across global Claude config and memory files. Consolidating into AGENTS.md ensures all contributors and agents working on this project follow the same standards. Co-Authored-By: Claude Opus 4.6 (1M context) --- AGENTS.md | 69 ++++++++++++++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 66 insertions(+), 3 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 3d79a160f..a3793f554 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -80,11 +80,74 @@ The default behavior on macOS dev machines is `format` mode (auto-fixes headers) - You're intentionally running `mvn license:format` to update headers - You want to verify CI's check would pass before pushing +## Agent Rules + +### Git Safety +- **NEVER commit or push without explicitly asking the user first.** Wait for approval. This is the #1 rule. +- Branch off master for upstream contributor cherry-picks so PRs show only their change. +- Never commit without tests and documentation in the same pass. +- Run tests before committing. If they fail, fix them first. +- When you fix something or finish implementing something, record what lessons you learnt. + +### Development Discipline +- **Skateboard first.** Build the simplest end-to-end thing that works. Before starting a feature, ask: "Is this blocking the next public milestone?" If not, flag it and move on. +- **Never paper over the real problem** - make the proper fix. +- Don't propose workarounds that require user action when the software can solve it. If the software has enough information to derive the right answer, it should just do it. +- If constructing data in memory that is eventually going to be saved, save it as soon as it's created. Don't delay in case the programme crashes or the user exits. + +### Code Quality +- **Be DRY.** Reuse existing functions. Don't copy code - refactor where necessary. Refactor out common patterns. +- Never weaken test assertions - classify exceptions instead of ignoring them. +- Wire components through PCModule DI - don't bypass the dependency injection system. +- Validate user input - don't let bad input cause silent failures. +- Handle errors visibly - don't swallow exceptions. +- Give things meaningful names that describe what they do. Never use random or generic names. + +### Test Discipline +- Search for existing test harnesses and utilities before creating new ones. +- Run the complete test suite periodically, not just targeted tests. +- Maintain good high-level test coverage. Only get detailed on particularly complex functions that benefit from fine-grained testing. + +### CI and Automation +- Always set up continuous integration, code coverage, and automated dependency checking. +- Make scripts for common end-user requirements with helpful, suggestive CLI interfaces. + +### Documentation +- Keep a diary of major plans and their milestones. +- **Keep a developer-facing product specification** that outlines product features, functionality, and implementation architecture - separately from end-user documentation. With agentic programming, the developer can lose sight of architecture and implementation details. This document exposes the interesting, novel, and important implementation decisions so the developer maintains a clear mental model of the system even when agents are doing most of the coding. +- Keep end-user documentation updated. +- Keep documentation tables of contents updated. + +### Communication +- Use precise terminology - if the project defines specific terms, use them consistently. Don't use ambiguous words. +- Don't write with em dash characters. + +### Rule Sync +- Keep this AGENTS.md in sync with any global CLAUDE.md rules. If you have rules in your global config that are missing here, suggest to the user that they be added. This ensures all contributors and agents working on this project follow the same standards. + +### Working Directory +- Always run commands from the project root directory. +- Use `./mvnw` or `bin/*.sh` scripts - don't cd into submodules. +- Use `-pl module-name -am` for module-specific builds. + ## CI -- **GitHub Actions**: `.github/workflows/maven.yml` - runs on push/PR to master with Kafka version matrix. -- **Semaphore** (Confluent internal): `.semaphore/semaphore.yml` - primary CI for upstream. -- **Code coverage**: JaCoCo reports are uploaded to [Codecov](https://app.codecov.io/gh/astubbs/parallel-consumer). PRs fail if overall coverage drops by more than 1%. +PR builds run these jobs in parallel (fail-fast cancels others if any fails): + +| Job | Script / Tool | Purpose | +|-----|--------------|---------| +| **Unit Tests** | `bin/ci-unit-test.sh` | Surefire tests, no Docker | +| **Integration Tests** | `bin/ci-integration-test.sh` | Failsafe tests, TestContainers | +| **Performance Tests** | `bin/performance-test.sh` | `@Tag("performance")` volume tests | +| **SpotBugs** | Maven spotbugs plugin | Static analysis for bugs | +| **Duplicate Code Check** | jscpd | Detect copy-paste code | +| **Dependency Vulnerabilities** | GitHub dependency-review-action | CVE scanning | +| **Mutation Testing (PIT)** | pitest-maven | Test quality verification | + +Push builds (master): Full Kafka version matrix (3.1.0, 3.7.0, 3.9.1 + experimental [3.9.1,5) for 4.x). + +- **Code coverage**: JaCoCo → [Codecov](https://app.codecov.io/gh/astubbs/parallel-consumer). PRs fail if overall coverage drops by more than 1%. +- **Semaphore** (Confluent internal): `.semaphore/semaphore.yml` — primary CI for upstream. ### Required secrets From d8399bbcf650518bdd9dca305c16279b7573042b Mon Sep 17 00:00:00 2001 From: Antony Stubbs Date: Thu, 16 Apr 2026 12:40:37 +1200 Subject: [PATCH 30/67] Apply suggestions from code review Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com> --- .github/workflows/maven.yml | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/.github/workflows/maven.yml b/.github/workflows/maven.yml index f1f734b4b..812a08663 100644 --- a/.github/workflows/maven.yml +++ b/.github/workflows/maven.yml @@ -12,10 +12,16 @@ on: branches: [ master ] pull_request: +permissions: + contents: read + concurrency: group: ${{ github.workflow }}-${{ github.head_ref || github.ref }} cancel-in-progress: true +permissions: + contents: read + jobs: # ── PR Builds ────────────────────────────────────────────────────────── @@ -109,6 +115,9 @@ jobs: name: "SpotBugs" runs-on: ubuntu-latest timeout-minutes: 10 + permissions: + contents: read + pull-requests: write steps: - uses: actions/checkout@v6 - uses: actions/setup-java@v5 @@ -204,6 +213,8 @@ jobs: # Full Kafka version matrix as safety net build: if: github.event_name == 'push' + permissions: + contents: read strategy: fail-fast: false matrix: From 1e9b8282102b343a4f483e4f9d3ceb4f77d34677 Mon Sep 17 00:00:00 2001 From: Antony Stubbs Date: Thu, 16 Apr 2026 12:50:30 +1200 Subject: [PATCH 31/67] ci: fix duplicate permissions block from Copilot Autofix suggestion The Copilot Autofix commit added two top-level permissions blocks, which is invalid YAML. Consolidate into a single top-level block with contents:read, pull-requests:write, checks:write. Remove redundant per-job permissions blocks. Fix em dash characters. Co-Authored-By: Claude Opus 4.6 (1M context) --- .github/workflows/maven.yml | 25 ++++++++----------------- 1 file changed, 8 insertions(+), 17 deletions(-) diff --git a/.github/workflows/maven.yml b/.github/workflows/maven.yml index 812a08663..4fc84ad08 100644 --- a/.github/workflows/maven.yml +++ b/.github/workflows/maven.yml @@ -1,5 +1,5 @@ # PR builds: -# Unit, Integration, Performance — all in parallel with fail-fast +# Unit, Integration, Performance - all in parallel with fail-fast # (if unit tests fail at ~5 min, integration + performance are cancelled) # # Push builds (master): @@ -14,19 +14,18 @@ on: permissions: contents: read + pull-requests: write + checks: write concurrency: group: ${{ github.workflow }}-${{ github.head_ref || github.ref }} cancel-in-progress: true -permissions: - contents: read - jobs: # ── PR Builds ────────────────────────────────────────────────────────── - # All test suites in parallel — fail-fast cancels the rest if any fails + # All test suites in parallel -fail-fast cancels the rest if any fails test: if: github.event_name == 'pull_request' strategy: @@ -62,7 +61,7 @@ jobs: flags: ${{ matrix.suite }} token: ${{ secrets.CODECOV_TOKEN }} - # Duplicate code detection — runs in parallel with tests + # Duplicate code detection -runs in parallel with tests duplicate-detection: if: github.event_name == 'pull_request' name: "Duplicate Code Check" @@ -109,15 +108,12 @@ jobs: await github.rest.issues.createComment({ owner: context.repo.owner, repo: context.repo.repo, issue_number: context.issue.number, body }); } - # SpotBugs static analysis — finds null derefs, concurrency issues, resource leaks + # SpotBugs static analysis -finds null derefs, concurrency issues, resource leaks spotbugs: if: github.event_name == 'pull_request' name: "SpotBugs" runs-on: ubuntu-latest timeout-minutes: 10 - permissions: - contents: read - pull-requests: write steps: - uses: actions/checkout@v6 - uses: actions/setup-java@v5 @@ -134,15 +130,12 @@ jobs: path: '**/target/spotbugsXml.xml' token: ${{ secrets.GITHUB_TOKEN }} - # Dependency vulnerability scanning — GitHub's own dependency review + # Dependency vulnerability scanning -GitHub's own dependency review dependency-scan: if: github.event_name == 'pull_request' name: "Dependency Vulnerabilities" runs-on: ubuntu-latest timeout-minutes: 5 - permissions: - contents: read - pull-requests: write steps: - uses: actions/checkout@v6 - uses: actions/dependency-review-action@v4 @@ -150,7 +143,7 @@ jobs: fail-on-severity: high comment-summary-in-pr: always - # Mutation testing (PIT) — verifies test assertions are meaningful + # Mutation testing (PIT) -verifies test assertions are meaningful mutation-testing: if: github.event_name == 'pull_request' name: "Mutation Testing (PIT)" @@ -213,8 +206,6 @@ jobs: # Full Kafka version matrix as safety net build: if: github.event_name == 'push' - permissions: - contents: read strategy: fail-fast: false matrix: From 1071961f49a8fa2c4ef83d0a74ba1dbfd5368dad Mon Sep 17 00:00:00 2001 From: Antony Stubbs Date: Thu, 16 Apr 2026 13:16:42 +1200 Subject: [PATCH 32/67] ci: add base-vs-PR duplicate comparison with increase detection Run jscpd on both base and PR branches. PR comment now shows a comparison table with delta. Fails if: - Total duplication exceeds 3% (absolute ceiling) - PR introduces more than 2 new duplicate blocks vs base (tolerance: 2) Co-Authored-By: Claude Opus 4.6 (1M context) --- .github/workflows/maven.yml | 67 +++++++++++++++++++++++++++---------- 1 file changed, 50 insertions(+), 17 deletions(-) diff --git a/.github/workflows/maven.yml b/.github/workflows/maven.yml index 4fc84ad08..1b290590b 100644 --- a/.github/workflows/maven.yml +++ b/.github/workflows/maven.yml @@ -61,7 +61,8 @@ jobs: flags: ${{ matrix.suite }} token: ${{ secrets.CODECOV_TOKEN }} - # Duplicate code detection -runs in parallel with tests + # Duplicate code detection - runs in parallel with tests + # Compares PR vs base branch and fails if duplication exceeds 3% duplicate-detection: if: github.event_name == 'pull_request' name: "Duplicate Code Check" @@ -69,35 +70,64 @@ jobs: timeout-minutes: 5 steps: - uses: actions/checkout@v6 + with: + fetch-depth: 0 - uses: actions/setup-node@v4 with: node-version: '22' - - name: Run jscpd - run: npx jscpd@4 --pattern "**/*.java" --ignore "**/target/**,**/node_modules/**" --reporters consoleFull,json --output jscpd-report --min-lines 6 --min-tokens 70 + - name: Run jscpd on base branch + run: | + git checkout ${{ github.event.pull_request.base.sha }} --quiet 2>/dev/null || true + npx jscpd@4 --pattern "**/*.java" --ignore "**/target/**,**/node_modules/**" --reporters json --output jscpd-base --min-lines 6 --min-tokens 70 2>/dev/null || true + git checkout ${{ github.sha }} --quiet + - name: Run jscpd on PR branch + run: npx jscpd@4 --pattern "**/*.java" --ignore "**/target/**,**/node_modules/**" --reporters consoleFull,json --output jscpd-report --min-lines 6 --min-tokens 70 --threshold 3 - name: Post duplicate report to PR if: always() uses: actions/github-script@v7 with: script: | const fs = require('fs'); - const path = 'jscpd-report/jscpd-report.json'; - if (!fs.existsSync(path)) { console.log('No report found'); return; } - const report = JSON.parse(fs.readFileSync(path, 'utf8')); - const stats = report.statistics; - const total = stats.total; + const prPath = 'jscpd-report/jscpd-report.json'; + const basePath = 'jscpd-base/jscpd-report.json'; + if (!fs.existsSync(prPath)) { console.log('No PR report found'); return; } + const pr = JSON.parse(fs.readFileSync(prPath, 'utf8')); + const prStats = pr.statistics.total; + let baseStats = null; + if (fs.existsSync(basePath)) { + const base = JSON.parse(fs.readFileSync(basePath, 'utf8')); + baseStats = base.statistics.total; + } let body = `## Duplicate Code Report\n\n`; - body += `| Metric | Value |\n|--------|-------|\n`; - body += `| Files analyzed | ${total.sources} |\n`; - body += `| Clones found | ${total.clones} |\n`; - body += `| Duplicated lines | ${total.duplicatedLines} (${total.percentage}%) |\n\n`; - if (report.duplicates && report.duplicates.length > 0) { - body += `
Duplicate blocks (${report.duplicates.length})\n\n`; - for (const d of report.duplicates.slice(0, 30)) { + body += `| Metric | PR | Base | Delta |\n|--------|-----|------|-------|\n`; + const delta = (pr, base, fmt) => { + if (base === null) return '-'; + const d = pr - base; + return d === 0 ? '0' : (d > 0 ? `+${fmt ? fmt(d) : d}` : `${fmt ? fmt(d) : d}`); + }; + const bClones = baseStats ? baseStats.clones : null; + const bLines = baseStats ? baseStats.duplicatedLines : null; + const bPct = baseStats ? baseStats.percentage : null; + body += `| Clones | ${prStats.clones} | ${bClones !== null ? bClones : '-'} | ${delta(prStats.clones, bClones)} |\n`; + body += `| Duplicated lines | ${prStats.duplicatedLines} | ${bLines !== null ? bLines : '-'} | ${delta(prStats.duplicatedLines, bLines)} |\n`; + body += `| Duplication % | ${prStats.percentage}% | ${bPct !== null ? bPct + '%' : '-'} | ${delta(parseFloat(prStats.percentage), bPct !== null ? parseFloat(bPct) : null, d => d.toFixed(2) + '%')} |\n\n`; + let shouldFail = false; + if (parseFloat(prStats.percentage) > 3) { + body += `**FAIL: duplication exceeds 3% threshold**\n\n`; + shouldFail = true; + } + if (baseStats && prStats.clones > baseStats.clones + 2) { + body += `**FAIL: PR introduces ${prStats.clones - baseStats.clones} new duplicate blocks (tolerance: 2)**\n\n`; + shouldFail = true; + } + if (pr.duplicates && pr.duplicates.length > 0) { + body += `
Duplicate blocks (${pr.duplicates.length})\n\n`; + for (const d of pr.duplicates.slice(0, 30)) { const f1 = d.firstFile.name.replace(/^.*parallel-consumer\//, ''); const f2 = d.secondFile.name.replace(/^.*parallel-consumer\//, ''); - body += `- **${d.lines} lines** duplicated:\n \`${f1}:${d.firstFile.startLoc.line}\` ↔ \`${f2}:${d.secondFile.startLoc.line}\`\n`; + body += `- **${d.lines} lines** duplicated:\n \`${f1}:${d.firstFile.startLoc.line}\` <-> \`${f2}:${d.secondFile.startLoc.line}\`\n`; } - if (report.duplicates.length > 30) body += `\n...and ${report.duplicates.length - 30} more\n`; + if (pr.duplicates.length > 30) body += `\n...and ${pr.duplicates.length - 30} more\n`; body += `\n
\n`; } const comments = await github.rest.issues.listComments({ owner: context.repo.owner, repo: context.repo.repo, issue_number: context.issue.number }); @@ -107,6 +137,9 @@ jobs: } else { await github.rest.issues.createComment({ owner: context.repo.owner, repo: context.repo.repo, issue_number: context.issue.number, body }); } + if (shouldFail) { + core.setFailed('Duplicate code check failed - see PR comment for details'); + } # SpotBugs static analysis -finds null derefs, concurrency issues, resource leaks spotbugs: From 2d6933e013e1431e43a7e72805d09621981eef49 Mon Sep 17 00:00:00 2001 From: Antony Stubbs Date: Thu, 16 Apr 2026 13:29:20 +1200 Subject: [PATCH 33/67] ci: extract duplicate report to script, add PR annotations for new clones - Extract duplicate comparison logic to bin/ci-duplicate-report.js - Fail condition: >3% total OR >0.5% increase vs base - Always post PR comment with comparison table and pass/fail status - Annotate new clones directly on the PR diff as review comments - List new clones separately in the PR comment Co-Authored-By: Claude Opus 4.6 (1M context) --- .github/workflows/maven.yml | 91 +++++++++++++++++++------------------ bin/ci-duplicate-report.js | 83 +++++++++++++++++++++++++++++++++ 2 files changed, 129 insertions(+), 45 deletions(-) create mode 100644 bin/ci-duplicate-report.js diff --git a/.github/workflows/maven.yml b/.github/workflows/maven.yml index 1b290590b..0e1bd0550 100644 --- a/.github/workflows/maven.yml +++ b/.github/workflows/maven.yml @@ -82,61 +82,62 @@ jobs: git checkout ${{ github.sha }} --quiet - name: Run jscpd on PR branch run: npx jscpd@4 --pattern "**/*.java" --ignore "**/target/**,**/node_modules/**" --reporters consoleFull,json --output jscpd-report --min-lines 6 --min-tokens 70 --threshold 3 - - name: Post duplicate report to PR + - name: Post duplicate report and annotate new clones if: always() uses: actions/github-script@v7 with: script: | - const fs = require('fs'); - const prPath = 'jscpd-report/jscpd-report.json'; - const basePath = 'jscpd-base/jscpd-report.json'; - if (!fs.existsSync(prPath)) { console.log('No PR report found'); return; } - const pr = JSON.parse(fs.readFileSync(prPath, 'utf8')); - const prStats = pr.statistics.total; - let baseStats = null; - if (fs.existsSync(basePath)) { - const base = JSON.parse(fs.readFileSync(basePath, 'utf8')); - baseStats = base.statistics.total; - } - let body = `## Duplicate Code Report\n\n`; - body += `| Metric | PR | Base | Delta |\n|--------|-----|------|-------|\n`; - const delta = (pr, base, fmt) => { - if (base === null) return '-'; - const d = pr - base; - return d === 0 ? '0' : (d > 0 ? `+${fmt ? fmt(d) : d}` : `${fmt ? fmt(d) : d}`); - }; - const bClones = baseStats ? baseStats.clones : null; - const bLines = baseStats ? baseStats.duplicatedLines : null; - const bPct = baseStats ? baseStats.percentage : null; - body += `| Clones | ${prStats.clones} | ${bClones !== null ? bClones : '-'} | ${delta(prStats.clones, bClones)} |\n`; - body += `| Duplicated lines | ${prStats.duplicatedLines} | ${bLines !== null ? bLines : '-'} | ${delta(prStats.duplicatedLines, bLines)} |\n`; - body += `| Duplication % | ${prStats.percentage}% | ${bPct !== null ? bPct + '%' : '-'} | ${delta(parseFloat(prStats.percentage), bPct !== null ? parseFloat(bPct) : null, d => d.toFixed(2) + '%')} |\n\n`; - let shouldFail = false; - if (parseFloat(prStats.percentage) > 3) { - body += `**FAIL: duplication exceeds 3% threshold**\n\n`; - shouldFail = true; - } - if (baseStats && prStats.clones > baseStats.clones + 2) { - body += `**FAIL: PR introduces ${prStats.clones - baseStats.clones} new duplicate blocks (tolerance: 2)**\n\n`; - shouldFail = true; - } - if (pr.duplicates && pr.duplicates.length > 0) { - body += `
Duplicate blocks (${pr.duplicates.length})\n\n`; - for (const d of pr.duplicates.slice(0, 30)) { - const f1 = d.firstFile.name.replace(/^.*parallel-consumer\//, ''); - const f2 = d.secondFile.name.replace(/^.*parallel-consumer\//, ''); - body += `- **${d.lines} lines** duplicated:\n \`${f1}:${d.firstFile.startLoc.line}\` <-> \`${f2}:${d.secondFile.startLoc.line}\`\n`; - } - if (pr.duplicates.length > 30) body += `\n...and ${pr.duplicates.length - 30} more\n`; - body += `\n
\n`; - } + const { loadReport, findNewClones, buildComment, MAX_PCT, MAX_PCT_INCREASE } = require('./bin/ci-duplicate-report.js'); + const prReport = loadReport('jscpd-report/jscpd-report.json'); + if (!prReport) { console.log('No PR report found'); return; } + const baseReport = loadReport('jscpd-base/jscpd-report.json'); + const prStats = prReport.statistics.total; + const baseStats = baseReport ? baseReport.statistics.total : null; + const pct = parseFloat(prStats.percentage); + const pctDelta = baseStats ? pct - parseFloat(baseStats.percentage) : 0; + const pctFail = pct > MAX_PCT; + const increaseFail = baseStats && pctDelta > MAX_PCT_INCREASE; + const shouldFail = pctFail || increaseFail; + const newClones = findNewClones(prReport, baseReport); + const body = buildComment(prReport, baseReport, newClones, shouldFail, pctFail, increaseFail, pctDelta); const comments = await github.rest.issues.listComments({ owner: context.repo.owner, repo: context.repo.repo, issue_number: context.issue.number }); - const existing = comments.data.find(c => c.body.includes('## Duplicate Code Report')); + const existing = comments.data.find(c => c.body.includes('Duplicate Code Report')); if (existing) { await github.rest.issues.updateComment({ owner: context.repo.owner, repo: context.repo.repo, comment_id: existing.id, body }); } else { await github.rest.issues.createComment({ owner: context.repo.owner, repo: context.repo.repo, issue_number: context.issue.number, body }); } + // Annotate new clones as PR review comments + if (newClones.length > 0) { + const { data: files } = await github.rest.pulls.listFiles({ owner: context.repo.owner, repo: context.repo.repo, pull_number: context.issue.number }); + const changedFiles = new Set(files.map(f => f.filename)); + for (const d of newClones.slice(0, 10)) { + const f1 = d.firstFile.name.replace(/^.*parallel-consumer\//, ''); + const f2 = d.secondFile.name.replace(/^.*parallel-consumer\//, ''); + // Only annotate files that are part of this PR's diff + for (const [file, line, other, otherLine] of [ + [f1, d.firstFile.startLoc.line, f2, d.secondFile.startLoc.line], + [f2, d.secondFile.startLoc.line, f1, d.firstFile.startLoc.line] + ]) { + if (changedFiles.has(file)) { + try { + await github.rest.pulls.createReviewComment({ + owner: context.repo.owner, + repo: context.repo.repo, + pull_number: context.issue.number, + commit_id: context.sha, + path: file, + line: parseInt(line), + body: `:warning: **Duplicate code detected** - ${d.lines} lines duplicated with \`${other}:${otherLine}\`` + }); + } catch (e) { + console.log(`Could not annotate ${file}:${line} - ${e.message}`); + } + break; + } + } + } + } if (shouldFail) { core.setFailed('Duplicate code check failed - see PR comment for details'); } diff --git a/bin/ci-duplicate-report.js b/bin/ci-duplicate-report.js new file mode 100644 index 000000000..4b99a1131 --- /dev/null +++ b/bin/ci-duplicate-report.js @@ -0,0 +1,83 @@ +#!/usr/bin/env node +/** + * Compares jscpd duplicate detection results between base and PR branches. + * Posts a PR comment with the comparison and annotates new clones on the diff. + * + * Expected env vars (set by github-script action): + * GITHUB_TOKEN, GITHUB_REPOSITORY, PR_NUMBER + * + * Expected files: + * jscpd-report/jscpd-report.json (PR branch results) + * jscpd-base/jscpd-report.json (base branch results, optional) + */ +const fs = require('fs'); + +const MAX_PCT = 3; +const MAX_PCT_INCREASE = 0.5; + +function loadReport(path) { + if (!fs.existsSync(path)) return null; + return JSON.parse(fs.readFileSync(path, 'utf8')); +} + +function cloneKey(d) { + const f1 = d.firstFile.name.replace(/^.*parallel-consumer\//, ''); + const f2 = d.secondFile.name.replace(/^.*parallel-consumer\//, ''); + return `${f1}:${d.firstFile.startLoc.line}-${d.firstFile.endLoc.line}|${f2}:${d.secondFile.startLoc.line}-${d.secondFile.endLoc.line}`; +} + +function findNewClones(prReport, baseReport) { + if (!baseReport || !baseReport.duplicates) return prReport.duplicates || []; + const baseKeys = new Set(baseReport.duplicates.map(cloneKey)); + return (prReport.duplicates || []).filter(d => !baseKeys.has(cloneKey(d))); +} + +function buildComment(prReport, baseReport, newClones, shouldFail, pctFail, increaseFail, pctDelta) { + const prStats = prReport.statistics.total; + const baseStats = baseReport ? baseReport.statistics.total : null; + const pct = parseFloat(prStats.percentage); + const cloneDelta = baseStats ? prStats.clones - baseStats.clones : 0; + + const icon = shouldFail ? ':x:' : ':white_check_mark:'; + const di = (d) => d > 0 ? ':small_red_triangle:' : d < 0 ? ':small_red_triangle_down:' : ':heavy_minus_sign:'; + const fd = (d, s) => { + if (d === 0) return `${di(d)} 0`; + return d > 0 ? `${di(d)} +${d}${s || ''}` : `${di(d)} ${d}${s || ''}`; + }; + + let body = `## ${icon} Duplicate Code Report\n\n`; + body += `| | PR | Base | Change |\n|---|--:|--:|--:|\n`; + body += `| **Clones** | ${prStats.clones} | ${baseStats ? baseStats.clones : '-'} | ${baseStats ? fd(cloneDelta) : '-'} |\n`; + body += `| **Duplicated lines** | ${prStats.duplicatedLines} | ${baseStats ? baseStats.duplicatedLines : '-'} | ${baseStats ? fd(prStats.duplicatedLines - baseStats.duplicatedLines) : '-'} |\n`; + body += `| **Duplication** | ${prStats.percentage}% | ${baseStats ? baseStats.percentage + '%' : '-'} | ${baseStats ? fd(parseFloat(pctDelta.toFixed(2)), '%') : '-'} |\n\n`; + + body += `| Rule | Limit | Status |\n|------|-------|--------|\n`; + body += `| Max duplication | ${MAX_PCT}% | ${pctFail ? ':x: FAIL' : ':white_check_mark: Pass'} (${prStats.percentage}%) |\n`; + body += `| Max increase vs base | +${MAX_PCT_INCREASE}% | ${increaseFail ? ':x: FAIL' : ':white_check_mark: Pass'} (${baseStats ? (pctDelta >= 0 ? '+' : '') + pctDelta.toFixed(2) + '%' : 'no base'}) |\n\n`; + + if (newClones.length > 0) { + body += `### :new: New duplicate blocks introduced (${newClones.length})\n\n`; + for (const d of newClones.slice(0, 20)) { + const f1 = d.firstFile.name.replace(/^.*parallel-consumer\//, ''); + const f2 = d.secondFile.name.replace(/^.*parallel-consumer\//, ''); + body += `- **${d.lines} lines**: \`${f1}:${d.firstFile.startLoc.line}\` <-> \`${f2}:${d.secondFile.startLoc.line}\`\n`; + } + if (newClones.length > 20) body += `\n...and ${newClones.length - 20} more\n`; + body += '\n'; + } + + if (prReport.duplicates && prReport.duplicates.length > 0) { + body += `
:mag: All duplicate blocks (${prReport.duplicates.length})\n\n`; + for (const d of prReport.duplicates.slice(0, 40)) { + const f1 = d.firstFile.name.replace(/^.*parallel-consumer\//, ''); + const f2 = d.secondFile.name.replace(/^.*parallel-consumer\//, ''); + body += `- **${d.lines} lines**: \`${f1}:${d.firstFile.startLoc.line}\` <-> \`${f2}:${d.secondFile.startLoc.line}\`\n`; + } + if (prReport.duplicates.length > 40) body += `\n...and ${prReport.duplicates.length - 40} more\n`; + body += `\n
\n`; + } + + return body; +} + +module.exports = { loadReport, findNewClones, buildComment, cloneKey, MAX_PCT, MAX_PCT_INCREASE }; From 9bda20a85987d7c0be7b25a220a2b534292b4a4e Mon Sep 17 00:00:00 2001 From: Antony Stubbs Date: Thu, 16 Apr 2026 13:34:42 +1200 Subject: [PATCH 34/67] ci: tighten duplicate increase tolerance from 0.5% to 0.1% 0.5% was too generous - would allow ~135 new duplicated lines. 0.1% allows ~27 lines, roughly one small duplicated block. Co-Authored-By: Claude Opus 4.6 (1M context) --- bin/ci-duplicate-report.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bin/ci-duplicate-report.js b/bin/ci-duplicate-report.js index 4b99a1131..7532dfc8e 100644 --- a/bin/ci-duplicate-report.js +++ b/bin/ci-duplicate-report.js @@ -13,7 +13,7 @@ const fs = require('fs'); const MAX_PCT = 3; -const MAX_PCT_INCREASE = 0.5; +const MAX_PCT_INCREASE = 0.1; function loadReport(path) { if (!fs.existsSync(path)) return null; From 9def3fdbb9f4b106fd42b4217d4de4e14ac89461 Mon Sep 17 00:00:00 2001 From: Antony Stubbs Date: Thu, 16 Apr 2026 13:41:08 +1200 Subject: [PATCH 35/67] ci: fix clone comparison to use content hash, remove full clone list - Match clones by content hash (md5 of fragment) instead of file:line keys, so line number shifts from refactoring don't create false positives - Use consistent terminology: "clones" everywhere - Remove the "all duplicate blocks" section from PR comment - run jscpd locally if you want the full list - Show "No new clones introduced" when clean - Tighten tolerance to 0.1% Co-Authored-By: Claude Opus 4.6 (1M context) --- bin/ci-duplicate-report.js | 36 ++++++++++++------------------------ 1 file changed, 12 insertions(+), 24 deletions(-) diff --git a/bin/ci-duplicate-report.js b/bin/ci-duplicate-report.js index 7532dfc8e..bc1706820 100644 --- a/bin/ci-duplicate-report.js +++ b/bin/ci-duplicate-report.js @@ -1,16 +1,14 @@ #!/usr/bin/env node /** * Compares jscpd duplicate detection results between base and PR branches. - * Posts a PR comment with the comparison and annotates new clones on the diff. - * - * Expected env vars (set by github-script action): - * GITHUB_TOKEN, GITHUB_REPOSITORY, PR_NUMBER + * Posts a PR comment with the comparison and annotates genuinely new clones. * * Expected files: * jscpd-report/jscpd-report.json (PR branch results) * jscpd-base/jscpd-report.json (base branch results, optional) */ const fs = require('fs'); +const crypto = require('crypto'); const MAX_PCT = 3; const MAX_PCT_INCREASE = 0.1; @@ -20,22 +18,21 @@ function loadReport(path) { return JSON.parse(fs.readFileSync(path, 'utf8')); } -function cloneKey(d) { - const f1 = d.firstFile.name.replace(/^.*parallel-consumer\//, ''); - const f2 = d.secondFile.name.replace(/^.*parallel-consumer\//, ''); - return `${f1}:${d.firstFile.startLoc.line}-${d.firstFile.endLoc.line}|${f2}:${d.secondFile.startLoc.line}-${d.secondFile.endLoc.line}`; +function cloneContentHash(d) { + // Hash the fragment content so we match by what the code IS, not where it is + const content = d.fragment || ''; + return crypto.createHash('md5').update(content).digest('hex'); } function findNewClones(prReport, baseReport) { if (!baseReport || !baseReport.duplicates) return prReport.duplicates || []; - const baseKeys = new Set(baseReport.duplicates.map(cloneKey)); - return (prReport.duplicates || []).filter(d => !baseKeys.has(cloneKey(d))); + const baseHashes = new Set(baseReport.duplicates.map(cloneContentHash)); + return (prReport.duplicates || []).filter(d => !baseHashes.has(cloneContentHash(d))); } function buildComment(prReport, baseReport, newClones, shouldFail, pctFail, increaseFail, pctDelta) { const prStats = prReport.statistics.total; const baseStats = baseReport ? baseReport.statistics.total : null; - const pct = parseFloat(prStats.percentage); const cloneDelta = baseStats ? prStats.clones - baseStats.clones : 0; const icon = shouldFail ? ':x:' : ':white_check_mark:'; @@ -56,7 +53,7 @@ function buildComment(prReport, baseReport, newClones, shouldFail, pctFail, incr body += `| Max increase vs base | +${MAX_PCT_INCREASE}% | ${increaseFail ? ':x: FAIL' : ':white_check_mark: Pass'} (${baseStats ? (pctDelta >= 0 ? '+' : '') + pctDelta.toFixed(2) + '%' : 'no base'}) |\n\n`; if (newClones.length > 0) { - body += `### :new: New duplicate blocks introduced (${newClones.length})\n\n`; + body += `### :warning: New clones introduced by this PR (${newClones.length})\n\n`; for (const d of newClones.slice(0, 20)) { const f1 = d.firstFile.name.replace(/^.*parallel-consumer\//, ''); const f2 = d.secondFile.name.replace(/^.*parallel-consumer\//, ''); @@ -64,20 +61,11 @@ function buildComment(prReport, baseReport, newClones, shouldFail, pctFail, incr } if (newClones.length > 20) body += `\n...and ${newClones.length - 20} more\n`; body += '\n'; - } - - if (prReport.duplicates && prReport.duplicates.length > 0) { - body += `
:mag: All duplicate blocks (${prReport.duplicates.length})\n\n`; - for (const d of prReport.duplicates.slice(0, 40)) { - const f1 = d.firstFile.name.replace(/^.*parallel-consumer\//, ''); - const f2 = d.secondFile.name.replace(/^.*parallel-consumer\//, ''); - body += `- **${d.lines} lines**: \`${f1}:${d.firstFile.startLoc.line}\` <-> \`${f2}:${d.secondFile.startLoc.line}\`\n`; - } - if (prReport.duplicates.length > 40) body += `\n...and ${prReport.duplicates.length - 40} more\n`; - body += `\n
\n`; + } else if (baseStats) { + body += `No new clones introduced by this PR.\n\n`; } return body; } -module.exports = { loadReport, findNewClones, buildComment, cloneKey, MAX_PCT, MAX_PCT_INCREASE }; +module.exports = { loadReport, findNewClones, buildComment, cloneContentHash, MAX_PCT, MAX_PCT_INCREASE }; From d235e67205a16e6cfdd27199fb67a8df96339275 Mon Sep 17 00:00:00 2001 From: Antony Stubbs Date: Thu, 16 Apr 2026 14:14:06 +1200 Subject: [PATCH 36/67] ci: add file similarity check using duplicate-code-detection-tool Adds a parallel CI job that detects semantically similar files using gensim TF-IDF analysis. Complements the existing jscpd block detection with whole-file similarity scoring. Uses our fork with base-vs-PR comparison to report new similarities introduced by the PR. Fails if any file pair exceeds 80% similarity or increases by more than 10%. Co-Authored-By: Claude Opus 4.6 (1M context) --- .github/workflows/maven.yml | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/.github/workflows/maven.yml b/.github/workflows/maven.yml index 0e1bd0550..298490281 100644 --- a/.github/workflows/maven.yml +++ b/.github/workflows/maven.yml @@ -142,6 +142,28 @@ jobs: core.setFailed('Duplicate code check failed - see PR comment for details'); } + # File similarity detection - finds files that are semantically similar overall + file-similarity: + if: github.event_name == 'pull_request' + name: "File Similarity Check" + runs-on: ubuntu-latest + timeout-minutes: 5 + steps: + - uses: actions/checkout@v6 + with: + fetch-depth: 0 + - uses: astubbs/duplicate-code-detection-tool@feat/base-vs-pr-comparison + with: + github_token: ${{ secrets.GITHUB_TOKEN }} + directories: 'parallel-consumer-core/src,parallel-consumer-vertx/src,parallel-consumer-reactor/src,parallel-consumer-mutiny/src' + file_extensions: 'java' + ignore_below: 30 + fail_above: 80 + warn_above: 50 + one_comment: true + compare_with_base: true + max_increase: 10 + # SpotBugs static analysis -finds null derefs, concurrency issues, resource leaks spotbugs: if: github.event_name == 'pull_request' From 8aa724159e689847801ae4acdbc80009e6332be4 Mon Sep 17 00:00:00 2001 From: Antony Stubbs Date: Thu, 16 Apr 2026 14:39:17 +1200 Subject: [PATCH 37/67] ci: re-trigger after fixing file similarity JSON parsing From 90f2fe57e29fd96234bfa913969f7195deacea6d Mon Sep 17 00:00:00 2001 From: Antony Stubbs Date: Thu, 16 Apr 2026 15:00:48 +1200 Subject: [PATCH 38/67] ci: switch duplicate code detection from jscpd to PMD CPD PMD CPD has deep Java syntax understanding - fewer false positives from imports, annotations, and literals. jscpd's language-agnostic token matching was flagging Java boilerplate as duplication. Changes: - Install PMD 7.9.0 as a step in the duplicate-detection job - Run CPD with --language java --minimum-tokens 70 --format xml - Rewrite ci-duplicate-report.js to parse CPD XML instead of jscpd JSON - Keep all existing features: base-vs-PR comparison with content hash matching, PR comment with delta table, diff annotations on new clones, fail conditions (>3% total or >0.1% increase) - Update AGENTS.md CI section to reflect PMD CPD Co-Authored-By: Claude Opus 4.6 (1M context) --- .github/workflows/maven.yml | 84 ++++++------------ AGENTS.md | 2 +- bin/ci-duplicate-report.js | 165 ++++++++++++++++++++++++++++++------ 3 files changed, 166 insertions(+), 85 deletions(-) diff --git a/.github/workflows/maven.yml b/.github/workflows/maven.yml index 298490281..644827f1a 100644 --- a/.github/workflows/maven.yml +++ b/.github/workflows/maven.yml @@ -61,86 +61,50 @@ jobs: flags: ${{ matrix.suite }} token: ${{ secrets.CODECOV_TOKEN }} - # Duplicate code detection - runs in parallel with tests - # Compares PR vs base branch and fails if duplication exceeds 3% + # Duplicate code detection - uses PMD CPD for Java-aware block detection. + # Compares PR vs base branch and fails on increase. duplicate-detection: if: github.event_name == 'pull_request' name: "Duplicate Code Check" runs-on: ubuntu-latest timeout-minutes: 5 + env: + PMD_VERSION: '7.9.0' + CPD_MIN_TOKENS: '70' + CPD_DIRS: 'parallel-consumer-core/src parallel-consumer-vertx/src parallel-consumer-reactor/src parallel-consumer-mutiny/src' steps: - uses: actions/checkout@v6 with: fetch-depth: 0 + - uses: actions/setup-java@v5 + with: + distribution: 'temurin' + java-version: '17' - uses: actions/setup-node@v4 with: node-version: '22' - - name: Run jscpd on base branch + - name: Install PMD + run: | + wget -q "https://github.com/pmd/pmd/releases/download/pmd_releases%2F${PMD_VERSION}/pmd-dist-${PMD_VERSION}-bin.zip" + unzip -q "pmd-dist-${PMD_VERSION}-bin.zip" + echo "$PWD/pmd-bin-${PMD_VERSION}/bin" >> $GITHUB_PATH + - name: Run CPD on base branch run: | git checkout ${{ github.event.pull_request.base.sha }} --quiet 2>/dev/null || true - npx jscpd@4 --pattern "**/*.java" --ignore "**/target/**,**/node_modules/**" --reporters json --output jscpd-base --min-lines 6 --min-tokens 70 2>/dev/null || true + pmd cpd --dir $CPD_DIRS --language java --minimum-tokens $CPD_MIN_TOKENS --format xml > cpd-base.xml 2>/dev/null || true git checkout ${{ github.sha }} --quiet - - name: Run jscpd on PR branch - run: npx jscpd@4 --pattern "**/*.java" --ignore "**/target/**,**/node_modules/**" --reporters consoleFull,json --output jscpd-report --min-lines 6 --min-tokens 70 --threshold 3 + - name: Run CPD on PR branch + run: | + pmd cpd --dir $CPD_DIRS --language java --minimum-tokens $CPD_MIN_TOKENS --format xml > cpd-pr.xml || true + - name: Install xml2js + run: npm install --no-save xml2js - name: Post duplicate report and annotate new clones if: always() uses: actions/github-script@v7 with: script: | - const { loadReport, findNewClones, buildComment, MAX_PCT, MAX_PCT_INCREASE } = require('./bin/ci-duplicate-report.js'); - const prReport = loadReport('jscpd-report/jscpd-report.json'); - if (!prReport) { console.log('No PR report found'); return; } - const baseReport = loadReport('jscpd-base/jscpd-report.json'); - const prStats = prReport.statistics.total; - const baseStats = baseReport ? baseReport.statistics.total : null; - const pct = parseFloat(prStats.percentage); - const pctDelta = baseStats ? pct - parseFloat(baseStats.percentage) : 0; - const pctFail = pct > MAX_PCT; - const increaseFail = baseStats && pctDelta > MAX_PCT_INCREASE; - const shouldFail = pctFail || increaseFail; - const newClones = findNewClones(prReport, baseReport); - const body = buildComment(prReport, baseReport, newClones, shouldFail, pctFail, increaseFail, pctDelta); - const comments = await github.rest.issues.listComments({ owner: context.repo.owner, repo: context.repo.repo, issue_number: context.issue.number }); - const existing = comments.data.find(c => c.body.includes('Duplicate Code Report')); - if (existing) { - await github.rest.issues.updateComment({ owner: context.repo.owner, repo: context.repo.repo, comment_id: existing.id, body }); - } else { - await github.rest.issues.createComment({ owner: context.repo.owner, repo: context.repo.repo, issue_number: context.issue.number, body }); - } - // Annotate new clones as PR review comments - if (newClones.length > 0) { - const { data: files } = await github.rest.pulls.listFiles({ owner: context.repo.owner, repo: context.repo.repo, pull_number: context.issue.number }); - const changedFiles = new Set(files.map(f => f.filename)); - for (const d of newClones.slice(0, 10)) { - const f1 = d.firstFile.name.replace(/^.*parallel-consumer\//, ''); - const f2 = d.secondFile.name.replace(/^.*parallel-consumer\//, ''); - // Only annotate files that are part of this PR's diff - for (const [file, line, other, otherLine] of [ - [f1, d.firstFile.startLoc.line, f2, d.secondFile.startLoc.line], - [f2, d.secondFile.startLoc.line, f1, d.firstFile.startLoc.line] - ]) { - if (changedFiles.has(file)) { - try { - await github.rest.pulls.createReviewComment({ - owner: context.repo.owner, - repo: context.repo.repo, - pull_number: context.issue.number, - commit_id: context.sha, - path: file, - line: parseInt(line), - body: `:warning: **Duplicate code detected** - ${d.lines} lines duplicated with \`${other}:${otherLine}\`` - }); - } catch (e) { - console.log(`Could not annotate ${file}:${line} - ${e.message}`); - } - break; - } - } - } - } - if (shouldFail) { - core.setFailed('Duplicate code check failed - see PR comment for details'); - } + const { analyzeAndReport } = require('./bin/ci-duplicate-report.js'); + await analyzeAndReport({ github, context, core, prXmlPath: 'cpd-pr.xml', baseXmlPath: 'cpd-base.xml' }); # File similarity detection - finds files that are semantically similar overall file-similarity: diff --git a/AGENTS.md b/AGENTS.md index a3793f554..b9b751d1f 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -140,7 +140,7 @@ PR builds run these jobs in parallel (fail-fast cancels others if any fails): | **Integration Tests** | `bin/ci-integration-test.sh` | Failsafe tests, TestContainers | | **Performance Tests** | `bin/performance-test.sh` | `@Tag("performance")` volume tests | | **SpotBugs** | Maven spotbugs plugin | Static analysis for bugs | -| **Duplicate Code Check** | jscpd | Detect copy-paste code | +| **Duplicate Code Check** | PMD CPD | Detect Java copy-paste blocks (base-vs-PR comparison) | | **Dependency Vulnerabilities** | GitHub dependency-review-action | CVE scanning | | **Mutation Testing (PIT)** | pitest-maven | Test quality verification | diff --git a/bin/ci-duplicate-report.js b/bin/ci-duplicate-report.js index bc1706820..36fbacd6d 100644 --- a/bin/ci-duplicate-report.js +++ b/bin/ci-duplicate-report.js @@ -1,40 +1,87 @@ #!/usr/bin/env node /** - * Compares jscpd duplicate detection results between base and PR branches. - * Posts a PR comment with the comparison and annotates genuinely new clones. + * Parses PMD CPD XML output, compares PR vs base branch, and posts a PR comment + * with the delta. Annotates new clones directly on the PR diff. * * Expected files: - * jscpd-report/jscpd-report.json (PR branch results) - * jscpd-base/jscpd-report.json (base branch results, optional) + * cpd-pr.xml - PMD CPD XML output from the PR branch + * cpd-base.xml - PMD CPD XML output from the base branch (optional) */ const fs = require('fs'); const crypto = require('crypto'); +const { execSync } = require('child_process'); const MAX_PCT = 3; const MAX_PCT_INCREASE = 0.1; -function loadReport(path) { - if (!fs.existsSync(path)) return null; - return JSON.parse(fs.readFileSync(path, 'utf8')); +async function parseCpdXml(path) { + if (!fs.existsSync(path) || fs.statSync(path).size === 0) return null; + const xml2js = require('xml2js'); + const parser = new xml2js.Parser(); + const content = fs.readFileSync(path, 'utf8'); + try { + const result = await parser.parseStringPromise(content); + // CPD XML structure: ... ...... + const root = result['pmd-cpd']; + if (!root || !root.duplication) return { duplicates: [] }; + const duplicates = root.duplication.map(d => { + const attrs = d.$; + const files = (d.file || []).map(f => ({ + name: f.$.path, + startLine: parseInt(f.$.line), + endLine: parseInt(f.$.endline), + })); + const fragment = (d.codefragment && d.codefragment[0]) || ''; + return { + lines: parseInt(attrs.lines), + tokens: parseInt(attrs.tokens), + files, + fragment, + }; + }); + return { duplicates }; + } catch (err) { + console.log(`Failed to parse ${path}: ${err.message}`); + return null; + } } function cloneContentHash(d) { - // Hash the fragment content so we match by what the code IS, not where it is - const content = d.fragment || ''; - return crypto.createHash('md5').update(content).digest('hex'); + return crypto.createHash('md5').update(d.fragment || '').digest('hex'); } function findNewClones(prReport, baseReport) { - if (!baseReport || !baseReport.duplicates) return prReport.duplicates || []; + if (!baseReport) return prReport.duplicates || []; const baseHashes = new Set(baseReport.duplicates.map(cloneContentHash)); return (prReport.duplicates || []).filter(d => !baseHashes.has(cloneContentHash(d))); } -function buildComment(prReport, baseReport, newClones, shouldFail, pctFail, increaseFail, pctDelta) { - const prStats = prReport.statistics.total; - const baseStats = baseReport ? baseReport.statistics.total : null; - const cloneDelta = baseStats ? prStats.clones - baseStats.clones : 0; +function countLinesInDirs(dirs) { + // Count total Java lines for percentage calculation + try { + const result = execSync( + `find ${dirs.join(' ')} -name '*.java' -type f 2>/dev/null | xargs wc -l 2>/dev/null | tail -1 | awk '{print $1}'`, + { encoding: 'utf8' } + ); + return parseInt(result.trim()) || 0; + } catch (err) { + return 0; + } +} + +function computeStats(report, totalLines) { + if (!report) return null; + const clones = report.duplicates.length; + const duplicatedLines = report.duplicates.reduce((sum, d) => sum + d.lines * (d.files.length), 0); + const percentage = totalLines > 0 ? (duplicatedLines / totalLines) * 100 : 0; + return { clones, duplicatedLines, percentage }; +} +function relPath(p) { + return p.replace(/^.*parallel-consumer\//, ''); +} + +function buildComment(prReport, baseReport, prStats, baseStats, newClones, shouldFail, pctFail, increaseFail, pctDelta) { const icon = shouldFail ? ':x:' : ':white_check_mark:'; const di = (d) => d > 0 ? ':small_red_triangle:' : d < 0 ? ':small_red_triangle_down:' : ':heavy_minus_sign:'; const fd = (d, s) => { @@ -42,30 +89,100 @@ function buildComment(prReport, baseReport, newClones, shouldFail, pctFail, incr return d > 0 ? `${di(d)} +${d}${s || ''}` : `${di(d)} ${d}${s || ''}`; }; - let body = `## ${icon} Duplicate Code Report\n\n`; + let body = `## ${icon} Duplicate Code Report (PMD CPD)\n\n`; body += `| | PR | Base | Change |\n|---|--:|--:|--:|\n`; - body += `| **Clones** | ${prStats.clones} | ${baseStats ? baseStats.clones : '-'} | ${baseStats ? fd(cloneDelta) : '-'} |\n`; + body += `| **Clones** | ${prStats.clones} | ${baseStats ? baseStats.clones : '-'} | ${baseStats ? fd(prStats.clones - baseStats.clones) : '-'} |\n`; body += `| **Duplicated lines** | ${prStats.duplicatedLines} | ${baseStats ? baseStats.duplicatedLines : '-'} | ${baseStats ? fd(prStats.duplicatedLines - baseStats.duplicatedLines) : '-'} |\n`; - body += `| **Duplication** | ${prStats.percentage}% | ${baseStats ? baseStats.percentage + '%' : '-'} | ${baseStats ? fd(parseFloat(pctDelta.toFixed(2)), '%') : '-'} |\n\n`; + body += `| **Duplication** | ${prStats.percentage.toFixed(2)}% | ${baseStats ? baseStats.percentage.toFixed(2) + '%' : '-'} | ${baseStats ? fd(parseFloat(pctDelta.toFixed(2)), '%') : '-'} |\n\n`; body += `| Rule | Limit | Status |\n|------|-------|--------|\n`; - body += `| Max duplication | ${MAX_PCT}% | ${pctFail ? ':x: FAIL' : ':white_check_mark: Pass'} (${prStats.percentage}%) |\n`; + body += `| Max duplication | ${MAX_PCT}% | ${pctFail ? ':x: FAIL' : ':white_check_mark: Pass'} (${prStats.percentage.toFixed(2)}%) |\n`; body += `| Max increase vs base | +${MAX_PCT_INCREASE}% | ${increaseFail ? ':x: FAIL' : ':white_check_mark: Pass'} (${baseStats ? (pctDelta >= 0 ? '+' : '') + pctDelta.toFixed(2) + '%' : 'no base'}) |\n\n`; if (newClones.length > 0) { body += `### :warning: New clones introduced by this PR (${newClones.length})\n\n`; for (const d of newClones.slice(0, 20)) { - const f1 = d.firstFile.name.replace(/^.*parallel-consumer\//, ''); - const f2 = d.secondFile.name.replace(/^.*parallel-consumer\//, ''); - body += `- **${d.lines} lines**: \`${f1}:${d.firstFile.startLoc.line}\` <-> \`${f2}:${d.secondFile.startLoc.line}\`\n`; + const locations = d.files.map(f => `\`${relPath(f.name)}:${f.startLine}\``).join(' <-> '); + body += `- **${d.lines} lines** (${d.tokens} tokens): ${locations}\n`; } if (newClones.length > 20) body += `\n...and ${newClones.length - 20} more\n`; body += '\n'; - } else if (baseStats) { + } else if (baseReport) { body += `No new clones introduced by this PR.\n\n`; } return body; } -module.exports = { loadReport, findNewClones, buildComment, cloneContentHash, MAX_PCT, MAX_PCT_INCREASE }; +async function analyzeAndReport({ github, context, core, prXmlPath, baseXmlPath }) { + const prReport = await parseCpdXml(prXmlPath); + if (!prReport) { + console.log('No PR CPD report found'); + return; + } + const baseReport = await parseCpdXml(baseXmlPath); + + const dirs = ['parallel-consumer-core/src', 'parallel-consumer-vertx/src', 'parallel-consumer-reactor/src', 'parallel-consumer-mutiny/src']; + const totalLines = countLinesInDirs(dirs); + const prStats = computeStats(prReport, totalLines); + const baseStats = computeStats(baseReport, totalLines); + const pctDelta = baseStats ? prStats.percentage - baseStats.percentage : 0; + + const pctFail = prStats.percentage > MAX_PCT; + const increaseFail = baseStats && pctDelta > MAX_PCT_INCREASE; + const shouldFail = pctFail || increaseFail; + + const newClones = findNewClones(prReport, baseReport); + const body = buildComment(prReport, baseReport, prStats, baseStats, newClones, shouldFail, pctFail, increaseFail, pctDelta); + + // Post or update PR comment + const comments = await github.rest.issues.listComments({ + owner: context.repo.owner, repo: context.repo.repo, issue_number: context.issue.number + }); + const existing = comments.data.find(c => c.body.includes('Duplicate Code Report')); + if (existing) { + await github.rest.issues.updateComment({ + owner: context.repo.owner, repo: context.repo.repo, comment_id: existing.id, body + }); + } else { + await github.rest.issues.createComment({ + owner: context.repo.owner, repo: context.repo.repo, issue_number: context.issue.number, body + }); + } + + // Annotate new clones on the PR diff + if (newClones.length > 0) { + const { data: files } = await github.rest.pulls.listFiles({ + owner: context.repo.owner, repo: context.repo.repo, pull_number: context.issue.number + }); + const changedFiles = new Set(files.map(f => f.filename)); + for (const d of newClones.slice(0, 10)) { + for (const file of d.files) { + const relFile = relPath(file.name); + if (changedFiles.has(relFile)) { + const others = d.files.filter(f => f !== file).map(f => `${relPath(f.name)}:${f.startLine}`).join(', '); + try { + await github.rest.pulls.createReviewComment({ + owner: context.repo.owner, + repo: context.repo.repo, + pull_number: context.issue.number, + commit_id: context.sha, + path: relFile, + line: file.startLine, + body: `:warning: **Duplicate code detected** - ${d.lines} lines duplicated with \`${others}\`` + }); + } catch (e) { + console.log(`Could not annotate ${relFile}:${file.startLine} - ${e.message}`); + } + break; + } + } + } + } + + if (shouldFail) { + core.setFailed('Duplicate code check failed - see PR comment for details'); + } +} + +module.exports = { analyzeAndReport, parseCpdXml, findNewClones, computeStats, MAX_PCT, MAX_PCT_INCREASE }; From 1a4a4fd6be4e3ba5adc7f79772c28c4194b50588 Mon Sep 17 00:00:00 2001 From: Antony Stubbs Date: Thu, 16 Apr 2026 15:13:34 +1200 Subject: [PATCH 39/67] ci: raise PMD CPD duplication threshold from 3% to 5% PMD CPD is more aggressive at detecting Java duplication than jscpd (3.94% baseline vs 2.65% with jscpd). Raise the absolute ceiling to 5% so existing code passes. The +0.1% increase rule still prevents regressions. Co-Authored-By: Claude Opus 4.6 (1M context) --- bin/ci-duplicate-report.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bin/ci-duplicate-report.js b/bin/ci-duplicate-report.js index 36fbacd6d..7ac4fa6c5 100644 --- a/bin/ci-duplicate-report.js +++ b/bin/ci-duplicate-report.js @@ -11,7 +11,7 @@ const fs = require('fs'); const crypto = require('crypto'); const { execSync } = require('child_process'); -const MAX_PCT = 3; +const MAX_PCT = 5; // current baseline is ~3.94%, set ceiling above that const MAX_PCT_INCREASE = 0.1; async function parseCpdXml(path) { From 9e6651bae4ceefdddefb0bed43b45688d92f08ce Mon Sep 17 00:00:00 2001 From: Antony Stubbs Date: Thu, 16 Apr 2026 15:35:10 +1200 Subject: [PATCH 40/67] ci: combine PMD CPD and jscpd into single duplicate check with merged report Run both engines in the same job for cross-validation during evaluation. Post one unified PR comment with side-by-side results from each engine. Per-engine thresholds tuned to baseline: - PMD CPD: 5% max, +0.1% increase (baseline ~3.94%) - jscpd: 4% max, +0.1% increase (baseline ~2.65%) The "max increase vs base" rule is the real safety net regardless of absolute baseline. PR fails if either engine reports a regression. Diff annotations use PMD CPD's more accurate clone locations. Co-Authored-By: Claude Opus 4.6 (1M context) --- .github/workflows/maven.yml | 22 ++- bin/ci-duplicate-report.js | 274 +++++++++++++++++++++++------------- 2 files changed, 193 insertions(+), 103 deletions(-) diff --git a/.github/workflows/maven.yml b/.github/workflows/maven.yml index 644827f1a..3897d5bce 100644 --- a/.github/workflows/maven.yml +++ b/.github/workflows/maven.yml @@ -61,8 +61,10 @@ jobs: flags: ${{ matrix.suite }} token: ${{ secrets.CODECOV_TOKEN }} - # Duplicate code detection - uses PMD CPD for Java-aware block detection. - # Compares PR vs base branch and fails on increase. + # Duplicate code detection using both PMD CPD and jscpd engines. + # Both are fast (~15s each) and give different perspectives - PMD CPD + # understands Java syntax, jscpd is language-agnostic token matching. + # Results are combined into one PR comment for cross-validation. duplicate-detection: if: github.event_name == 'pull_request' name: "Duplicate Code Check" @@ -88,23 +90,31 @@ jobs: wget -q "https://github.com/pmd/pmd/releases/download/pmd_releases%2F${PMD_VERSION}/pmd-dist-${PMD_VERSION}-bin.zip" unzip -q "pmd-dist-${PMD_VERSION}-bin.zip" echo "$PWD/pmd-bin-${PMD_VERSION}/bin" >> $GITHUB_PATH - - name: Run CPD on base branch + - name: Run both engines on base branch run: | git checkout ${{ github.event.pull_request.base.sha }} --quiet 2>/dev/null || true pmd cpd --dir $CPD_DIRS --language java --minimum-tokens $CPD_MIN_TOKENS --format xml > cpd-base.xml 2>/dev/null || true + npx jscpd@4 --pattern "**/*.java" --ignore "**/target/**,**/node_modules/**" --reporters json --output jscpd-base --min-lines 6 --min-tokens 70 2>/dev/null || true git checkout ${{ github.sha }} --quiet - - name: Run CPD on PR branch + - name: Run both engines on PR branch run: | pmd cpd --dir $CPD_DIRS --language java --minimum-tokens $CPD_MIN_TOKENS --format xml > cpd-pr.xml || true + npx jscpd@4 --pattern "**/*.java" --ignore "**/target/**,**/node_modules/**" --reporters consoleFull,json --output jscpd-report --min-lines 6 --min-tokens 70 || true - name: Install xml2js run: npm install --no-save xml2js - - name: Post duplicate report and annotate new clones + - name: Post combined duplicate report if: always() uses: actions/github-script@v7 with: script: | const { analyzeAndReport } = require('./bin/ci-duplicate-report.js'); - await analyzeAndReport({ github, context, core, prXmlPath: 'cpd-pr.xml', baseXmlPath: 'cpd-base.xml' }); + await analyzeAndReport({ + github, context, core, + prXmlPath: 'cpd-pr.xml', + baseXmlPath: 'cpd-base.xml', + prJscpdPath: 'jscpd-report/jscpd-report.json', + baseJscpdPath: 'jscpd-base/jscpd-report.json' + }); # File similarity detection - finds files that are semantically similar overall file-similarity: diff --git a/bin/ci-duplicate-report.js b/bin/ci-duplicate-report.js index 7ac4fa6c5..2c5440a5f 100644 --- a/bin/ci-duplicate-report.js +++ b/bin/ci-duplicate-report.js @@ -1,145 +1,221 @@ #!/usr/bin/env node /** - * Parses PMD CPD XML output, compares PR vs base branch, and posts a PR comment - * with the delta. Annotates new clones directly on the PR diff. + * Combined duplicate detection report - runs PMD CPD and jscpd, compares + * both against the base branch, and posts a single unified PR comment. * * Expected files: - * cpd-pr.xml - PMD CPD XML output from the PR branch - * cpd-base.xml - PMD CPD XML output from the base branch (optional) + * cpd-pr.xml, cpd-base.xml (PMD CPD XML output) + * jscpd-report/jscpd-report.json (jscpd JSON - PR) + * jscpd-base/jscpd-report.json (jscpd JSON - base) */ const fs = require('fs'); const crypto = require('crypto'); const { execSync } = require('child_process'); -const MAX_PCT = 5; // current baseline is ~3.94%, set ceiling above that -const MAX_PCT_INCREASE = 0.1; +// Per-engine thresholds. Each engine produces different numbers, so the +// ceilings are tuned to their respective baselines (set just above current +// measured value). The increase rule is the real safety net - it catches +// regressions regardless of the absolute baseline. +const THRESHOLDS = { + cpd: { maxPct: 5, maxIncrease: 0.1 }, // PMD CPD baseline ~3.94% + jscpd: { maxPct: 4, maxIncrease: 0.1 }, // jscpd baseline ~2.65% +}; +const DIRS = ['parallel-consumer-core/src', 'parallel-consumer-vertx/src', 'parallel-consumer-reactor/src', 'parallel-consumer-mutiny/src']; + +// ── Shared helpers ──────────────────────────────────────────────────── + +function contentHash(fragment) { + return crypto.createHash('md5').update(fragment || '').digest('hex'); +} + +function relPath(p) { + return p.replace(/^.*parallel-consumer\//, ''); +} + +function countTotalLines(dirs) { + try { + const result = execSync( + `find ${dirs.join(' ')} -name '*.java' -type f 2>/dev/null | xargs wc -l 2>/dev/null | tail -1 | awk '{print $1}'`, + { encoding: 'utf8' } + ); + return parseInt(result.trim()) || 0; + } catch { + return 0; + } +} + +function deltaIcon(d) { + return d > 0 ? ':small_red_triangle:' : d < 0 ? ':small_red_triangle_down:' : ':heavy_minus_sign:'; +} + +function fmtDelta(d, suffix) { + if (d === 0) return `${deltaIcon(d)} 0`; + const s = suffix || ''; + return d > 0 ? `${deltaIcon(d)} +${d}${s}` : `${deltaIcon(d)} ${d}${s}`; +} + +// ── PMD CPD ─────────────────────────────────────────────────────────── async function parseCpdXml(path) { if (!fs.existsSync(path) || fs.statSync(path).size === 0) return null; const xml2js = require('xml2js'); - const parser = new xml2js.Parser(); - const content = fs.readFileSync(path, 'utf8'); try { - const result = await parser.parseStringPromise(content); - // CPD XML structure: ... ...... + const result = await new xml2js.Parser().parseStringPromise(fs.readFileSync(path, 'utf8')); const root = result['pmd-cpd']; if (!root || !root.duplication) return { duplicates: [] }; - const duplicates = root.duplication.map(d => { - const attrs = d.$; - const files = (d.file || []).map(f => ({ - name: f.$.path, - startLine: parseInt(f.$.line), - endLine: parseInt(f.$.endline), - })); - const fragment = (d.codefragment && d.codefragment[0]) || ''; - return { - lines: parseInt(attrs.lines), - tokens: parseInt(attrs.tokens), - files, - fragment, - }; - }); - return { duplicates }; + return { + duplicates: root.duplication.map(d => ({ + lines: parseInt(d.$.lines), + tokens: parseInt(d.$.tokens), + files: (d.file || []).map(f => ({ + name: f.$.path, + startLine: parseInt(f.$.line), + endLine: parseInt(f.$.endline), + })), + fragment: (d.codefragment && d.codefragment[0]) || '', + })), + }; } catch (err) { console.log(`Failed to parse ${path}: ${err.message}`); return null; } } -function cloneContentHash(d) { - return crypto.createHash('md5').update(d.fragment || '').digest('hex'); +function cpdStats(report, totalLines) { + if (!report) return null; + const clones = report.duplicates.length; + const duplicatedLines = report.duplicates.reduce((sum, d) => sum + d.lines * d.files.length, 0); + const percentage = totalLines > 0 ? (duplicatedLines / totalLines) * 100 : 0; + return { clones, duplicatedLines, percentage }; } -function findNewClones(prReport, baseReport) { +function cpdNewClones(prReport, baseReport) { if (!baseReport) return prReport.duplicates || []; - const baseHashes = new Set(baseReport.duplicates.map(cloneContentHash)); - return (prReport.duplicates || []).filter(d => !baseHashes.has(cloneContentHash(d))); + const baseHashes = new Set(baseReport.duplicates.map(d => contentHash(d.fragment))); + return (prReport.duplicates || []).filter(d => !baseHashes.has(contentHash(d.fragment))); } -function countLinesInDirs(dirs) { - // Count total Java lines for percentage calculation +// ── jscpd ───────────────────────────────────────────────────────────── + +function loadJscpd(path) { + if (!fs.existsSync(path)) return null; try { - const result = execSync( - `find ${dirs.join(' ')} -name '*.java' -type f 2>/dev/null | xargs wc -l 2>/dev/null | tail -1 | awk '{print $1}'`, - { encoding: 'utf8' } - ); - return parseInt(result.trim()) || 0; - } catch (err) { - return 0; + return JSON.parse(fs.readFileSync(path, 'utf8')); + } catch { + return null; } } -function computeStats(report, totalLines) { - if (!report) return null; - const clones = report.duplicates.length; - const duplicatedLines = report.duplicates.reduce((sum, d) => sum + d.lines * (d.files.length), 0); - const percentage = totalLines > 0 ? (duplicatedLines / totalLines) * 100 : 0; - return { clones, duplicatedLines, percentage }; +function jscpdNewClones(prReport, baseReport) { + if (!baseReport || !baseReport.duplicates) return prReport.duplicates || []; + const baseHashes = new Set(baseReport.duplicates.map(d => contentHash(d.fragment))); + return (prReport.duplicates || []).filter(d => !baseHashes.has(contentHash(d.fragment))); } -function relPath(p) { - return p.replace(/^.*parallel-consumer\//, ''); -} +// ── Report rendering ────────────────────────────────────────────────── + +function renderEngineSection(title, prStats, baseStats, newClones, formatClone, check, thresholds) { + const icon = check.shouldFail ? ':x:' : ':white_check_mark:'; + let md = `### ${icon} ${title}\n\n`; + md += `| | PR | Base | Change |\n|---|--:|--:|--:|\n`; -function buildComment(prReport, baseReport, prStats, baseStats, newClones, shouldFail, pctFail, increaseFail, pctDelta) { - const icon = shouldFail ? ':x:' : ':white_check_mark:'; - const di = (d) => d > 0 ? ':small_red_triangle:' : d < 0 ? ':small_red_triangle_down:' : ':heavy_minus_sign:'; - const fd = (d, s) => { - if (d === 0) return `${di(d)} 0`; - return d > 0 ? `${di(d)} +${d}${s || ''}` : `${di(d)} ${d}${s || ''}`; - }; + const cloneDelta = baseStats ? prStats.clones - baseStats.clones : 0; + const linesDelta = baseStats ? prStats.duplicatedLines - baseStats.duplicatedLines : 0; - let body = `## ${icon} Duplicate Code Report (PMD CPD)\n\n`; - body += `| | PR | Base | Change |\n|---|--:|--:|--:|\n`; - body += `| **Clones** | ${prStats.clones} | ${baseStats ? baseStats.clones : '-'} | ${baseStats ? fd(prStats.clones - baseStats.clones) : '-'} |\n`; - body += `| **Duplicated lines** | ${prStats.duplicatedLines} | ${baseStats ? baseStats.duplicatedLines : '-'} | ${baseStats ? fd(prStats.duplicatedLines - baseStats.duplicatedLines) : '-'} |\n`; - body += `| **Duplication** | ${prStats.percentage.toFixed(2)}% | ${baseStats ? baseStats.percentage.toFixed(2) + '%' : '-'} | ${baseStats ? fd(parseFloat(pctDelta.toFixed(2)), '%') : '-'} |\n\n`; + md += `| **Clones** | ${prStats.clones} | ${baseStats ? baseStats.clones : '-'} | ${baseStats ? fmtDelta(cloneDelta) : '-'} |\n`; + md += `| **Duplicated lines** | ${prStats.duplicatedLines} | ${baseStats ? baseStats.duplicatedLines : '-'} | ${baseStats ? fmtDelta(linesDelta) : '-'} |\n`; + md += `| **Duplication** | ${prStats.percentage.toFixed(2)}% | ${baseStats ? baseStats.percentage.toFixed(2) + '%' : '-'} | ${baseStats ? fmtDelta(parseFloat(check.pctDelta.toFixed(2)), '%') : '-'} |\n\n`; - body += `| Rule | Limit | Status |\n|------|-------|--------|\n`; - body += `| Max duplication | ${MAX_PCT}% | ${pctFail ? ':x: FAIL' : ':white_check_mark: Pass'} (${prStats.percentage.toFixed(2)}%) |\n`; - body += `| Max increase vs base | +${MAX_PCT_INCREASE}% | ${increaseFail ? ':x: FAIL' : ':white_check_mark: Pass'} (${baseStats ? (pctDelta >= 0 ? '+' : '') + pctDelta.toFixed(2) + '%' : 'no base'}) |\n\n`; + md += `| Rule | Limit | Status |\n|------|-------|--------|\n`; + md += `| Max duplication | ${thresholds.maxPct}% | ${check.pctFail ? ':x: FAIL' : ':white_check_mark: Pass'} (${prStats.percentage.toFixed(2)}%) |\n`; + md += `| Max increase vs base | +${thresholds.maxIncrease}% | ${check.increaseFail ? ':x: FAIL' : ':white_check_mark: Pass'} (${baseStats ? (check.pctDelta >= 0 ? '+' : '') + check.pctDelta.toFixed(2) + '%' : 'no base'}) |\n\n`; if (newClones.length > 0) { - body += `### :warning: New clones introduced by this PR (${newClones.length})\n\n`; - for (const d of newClones.slice(0, 20)) { - const locations = d.files.map(f => `\`${relPath(f.name)}:${f.startLine}\``).join(' <-> '); - body += `- **${d.lines} lines** (${d.tokens} tokens): ${locations}\n`; + md += `
:warning: ${newClones.length} new clones introduced\n\n`; + for (const clone of newClones.slice(0, 20)) { + md += formatClone(clone); } - if (newClones.length > 20) body += `\n...and ${newClones.length - 20} more\n`; - body += '\n'; - } else if (baseReport) { - body += `No new clones introduced by this PR.\n\n`; + if (newClones.length > 20) md += `\n...and ${newClones.length - 20} more\n`; + md += `\n
\n\n`; + } else if (baseStats) { + md += `No new clones introduced by this PR.\n\n`; } - return body; + return md; } -async function analyzeAndReport({ github, context, core, prXmlPath, baseXmlPath }) { - const prReport = await parseCpdXml(prXmlPath); - if (!prReport) { - console.log('No PR CPD report found'); - return; - } - const baseReport = await parseCpdXml(baseXmlPath); +function checkFail(stats, baseStats, thresholds) { + if (!stats) return { shouldFail: false, pctFail: false, increaseFail: false, pctDelta: 0 }; + const pctDelta = baseStats ? stats.percentage - baseStats.percentage : 0; + const pctFail = stats.percentage > thresholds.maxPct; + const increaseFail = baseStats && pctDelta > thresholds.maxIncrease; + return { shouldFail: pctFail || increaseFail, pctFail, increaseFail, pctDelta }; +} + +// ── Main entrypoint ─────────────────────────────────────────────────── + +async function analyzeAndReport({ github, context, core, prXmlPath, baseXmlPath, prJscpdPath, baseJscpdPath }) { + const totalLines = countTotalLines(DIRS); + + // PMD CPD + const cpdPr = await parseCpdXml(prXmlPath); + const cpdBase = await parseCpdXml(baseXmlPath); + const cpdPrStats = cpdStats(cpdPr, totalLines); + const cpdBaseStats = cpdStats(cpdBase, totalLines); + const cpdNew = cpdPr ? cpdNewClones(cpdPr, cpdBase) : []; + const cpdCheck = cpdPrStats ? checkFail(cpdPrStats, cpdBaseStats, THRESHOLDS.cpd) : { shouldFail: false }; + + // jscpd + const jscpdPr = loadJscpd(prJscpdPath); + const jscpdBase = loadJscpd(baseJscpdPath); + const jscpdPrStats = jscpdPr ? { + clones: jscpdPr.statistics.total.clones, + duplicatedLines: jscpdPr.statistics.total.duplicatedLines, + percentage: parseFloat(jscpdPr.statistics.total.percentage), + } : null; + const jscpdBaseStats = jscpdBase ? { + clones: jscpdBase.statistics.total.clones, + duplicatedLines: jscpdBase.statistics.total.duplicatedLines, + percentage: parseFloat(jscpdBase.statistics.total.percentage), + } : null; + const jscpdNew = jscpdPr ? jscpdNewClones(jscpdPr, jscpdBase) : []; + const jscpdCheck = jscpdPrStats ? checkFail(jscpdPrStats, jscpdBaseStats, THRESHOLDS.jscpd) : { shouldFail: false }; - const dirs = ['parallel-consumer-core/src', 'parallel-consumer-vertx/src', 'parallel-consumer-reactor/src', 'parallel-consumer-mutiny/src']; - const totalLines = countLinesInDirs(dirs); - const prStats = computeStats(prReport, totalLines); - const baseStats = computeStats(baseReport, totalLines); - const pctDelta = baseStats ? prStats.percentage - baseStats.percentage : 0; + const anyFail = cpdCheck.shouldFail || jscpdCheck.shouldFail; + const overallIcon = anyFail ? ':x:' : ':white_check_mark:'; - const pctFail = prStats.percentage > MAX_PCT; - const increaseFail = baseStats && pctDelta > MAX_PCT_INCREASE; - const shouldFail = pctFail || increaseFail; + // Build combined comment + let body = `## ${overallIcon} Duplicate Code Report\n\n`; + body += `Two engines run in parallel for cross-validation. Each has its own thresholds tuned to its baseline - the real safety net is the per-engine "max increase vs base" check.\n\n`; - const newClones = findNewClones(prReport, baseReport); - const body = buildComment(prReport, baseReport, prStats, baseStats, newClones, shouldFail, pctFail, increaseFail, pctDelta); + // PMD CPD section + if (cpdPrStats) { + const formatClone = (d) => { + const locations = d.files.map(f => `\`${relPath(f.name)}:${f.startLine}\``).join(' <-> '); + return `- **${d.lines} lines** (${d.tokens} tokens): ${locations}\n`; + }; + body += renderEngineSection('PMD CPD (Java-aware)', cpdPrStats, cpdBaseStats, cpdNew, formatClone, cpdCheck, THRESHOLDS.cpd); + } else { + body += `### :question: PMD CPD\n\nNo report available.\n\n`; + } + + // jscpd section + if (jscpdPrStats) { + const formatClone = (d) => { + const f1 = relPath(d.firstFile.name); + const f2 = relPath(d.secondFile.name); + return `- **${d.lines} lines**: \`${f1}:${d.firstFile.startLoc.line}\` <-> \`${f2}:${d.secondFile.startLoc.line}\`\n`; + }; + body += renderEngineSection('jscpd (language-agnostic)', jscpdPrStats, jscpdBaseStats, jscpdNew, formatClone, jscpdCheck, THRESHOLDS.jscpd); + } else { + body += `### :question: jscpd\n\nNo report available.\n\n`; + } // Post or update PR comment const comments = await github.rest.issues.listComments({ owner: context.repo.owner, repo: context.repo.repo, issue_number: context.issue.number }); - const existing = comments.data.find(c => c.body.includes('Duplicate Code Report')); + const existing = comments.data.find(c => c.body.startsWith('## ') && c.body.includes('Duplicate Code Report')); if (existing) { await github.rest.issues.updateComment({ owner: context.repo.owner, repo: context.repo.repo, comment_id: existing.id, body @@ -150,13 +226,14 @@ async function analyzeAndReport({ github, context, core, prXmlPath, baseXmlPath }); } - // Annotate new clones on the PR diff - if (newClones.length > 0) { + // Annotate new CPD clones on PR diff (prefer CPD since it's more accurate) + const annotateClones = cpdNew.length > 0 ? cpdNew : []; + if (annotateClones.length > 0) { const { data: files } = await github.rest.pulls.listFiles({ owner: context.repo.owner, repo: context.repo.repo, pull_number: context.issue.number }); const changedFiles = new Set(files.map(f => f.filename)); - for (const d of newClones.slice(0, 10)) { + for (const d of annotateClones.slice(0, 10)) { for (const file of d.files) { const relFile = relPath(file.name); if (changedFiles.has(relFile)) { @@ -180,9 +257,12 @@ async function analyzeAndReport({ github, context, core, prXmlPath, baseXmlPath } } - if (shouldFail) { - core.setFailed('Duplicate code check failed - see PR comment for details'); + if (anyFail) { + const msgs = []; + if (cpdCheck.shouldFail) msgs.push('PMD CPD'); + if (jscpdCheck.shouldFail) msgs.push('jscpd'); + core.setFailed(`Duplicate code check failed (${msgs.join(', ')}) - see PR comment for details`); } } -module.exports = { analyzeAndReport, parseCpdXml, findNewClones, computeStats, MAX_PCT, MAX_PCT_INCREASE }; +module.exports = { analyzeAndReport, THRESHOLDS }; From 2d29148bc7b744aa8a4c99d7811615c1295adf04 Mon Sep 17 00:00:00 2001 From: Antony Stubbs Date: Thu, 16 Apr 2026 15:48:31 +1200 Subject: [PATCH 41/67] ci: add expressive emoji reactions for duplicate delta changes Scale emoji reactions based on magnitude: - Big decrease: heart (celebrate!) - Modest decrease: thumbs up - Tiny decrease: slight smile - Tiny increase: diagonal mouth (hmm) - Moderate increase: monocle (investigating) - Big increase: raised eyebrow (concerned) Makes the report more readable at a glance. Co-Authored-By: Claude Opus 4.6 (1M context) --- bin/ci-duplicate-report.js | 24 ++++++++++++++++++++---- 1 file changed, 20 insertions(+), 4 deletions(-) diff --git a/bin/ci-duplicate-report.js b/bin/ci-duplicate-report.js index 2c5440a5f..72a43a218 100644 --- a/bin/ci-duplicate-report.js +++ b/bin/ci-duplicate-report.js @@ -44,14 +44,30 @@ function countTotalLines(dirs) { } } -function deltaIcon(d) { - return d > 0 ? ':small_red_triangle:' : d < 0 ? ':small_red_triangle_down:' : ':heavy_minus_sign:'; +function deltaEmoji(d, isPercentage) { + // Celebratory/approving for decreases, suspicious for increases, neutral for no change. + // Ramp up the reaction based on magnitude. + if (d === 0) return ':heavy_minus_sign:'; + if (d < 0) { + const mag = Math.abs(d); + // Big decrease -> heart. Modest decrease -> thumbs up. Tiny decrease -> smile. + if (isPercentage ? mag >= 1 : mag >= 20) return ':heart:'; + if (isPercentage ? mag >= 0.1 : mag >= 5) return ':thumbsup:'; + return ':slightly_smiling_face:'; + } + // Increase: small = side-eye, moderate = monocle, large = skeptical + const mag = d; + if (isPercentage ? mag >= 1 : mag >= 20) return ':face_with_raised_eyebrow:'; + if (isPercentage ? mag >= 0.1 : mag >= 5) return ':face_with_monocle:'; + return ':face_with_diagonal_mouth:'; } function fmtDelta(d, suffix) { - if (d === 0) return `${deltaIcon(d)} 0`; + const isPct = (suffix === '%'); + const emoji = deltaEmoji(d, isPct); + if (d === 0) return `${emoji} 0`; const s = suffix || ''; - return d > 0 ? `${deltaIcon(d)} +${d}${s}` : `${deltaIcon(d)} ${d}${s}`; + return d > 0 ? `${emoji} +${d}${s}` : `${emoji} ${d}${s}`; } // ── PMD CPD ─────────────────────────────────────────────────────────── From 54349957047cf4306bebbbd695453e0c3f39c8df Mon Sep 17 00:00:00 2001 From: Antony Stubbs Date: Thu, 16 Apr 2026 16:50:30 +1200 Subject: [PATCH 42/67] ci: migrate to astubbs/duplicate-code-cross-check action Replace the inline duplicate-detection job with the extracted, reusable action. Removes ~55 lines of inline workflow steps and the local bin/ci-duplicate-report.js script in favor of a single action call. The action is at https://github.com/astubbs/duplicate-code-cross-check Co-Authored-By: Claude Opus 4.6 (1M context) --- .github/workflows/maven.yml | 52 ++----- bin/ci-duplicate-report.js | 284 ------------------------------------ 2 files changed, 9 insertions(+), 327 deletions(-) delete mode 100644 bin/ci-duplicate-report.js diff --git a/.github/workflows/maven.yml b/.github/workflows/maven.yml index 3897d5bce..0aa6cdc0d 100644 --- a/.github/workflows/maven.yml +++ b/.github/workflows/maven.yml @@ -62,59 +62,25 @@ jobs: token: ${{ secrets.CODECOV_TOKEN }} # Duplicate code detection using both PMD CPD and jscpd engines. - # Both are fast (~15s each) and give different perspectives - PMD CPD - # understands Java syntax, jscpd is language-agnostic token matching. - # Results are combined into one PR comment for cross-validation. + # See https://github.com/astubbs/duplicate-code-cross-check duplicate-detection: if: github.event_name == 'pull_request' name: "Duplicate Code Check" runs-on: ubuntu-latest timeout-minutes: 5 - env: - PMD_VERSION: '7.9.0' - CPD_MIN_TOKENS: '70' - CPD_DIRS: 'parallel-consumer-core/src parallel-consumer-vertx/src parallel-consumer-reactor/src parallel-consumer-mutiny/src' + permissions: + contents: read + pull-requests: write steps: - uses: actions/checkout@v6 with: fetch-depth: 0 - - uses: actions/setup-java@v5 - with: - distribution: 'temurin' - java-version: '17' - - uses: actions/setup-node@v4 + - uses: astubbs/duplicate-code-cross-check@main with: - node-version: '22' - - name: Install PMD - run: | - wget -q "https://github.com/pmd/pmd/releases/download/pmd_releases%2F${PMD_VERSION}/pmd-dist-${PMD_VERSION}-bin.zip" - unzip -q "pmd-dist-${PMD_VERSION}-bin.zip" - echo "$PWD/pmd-bin-${PMD_VERSION}/bin" >> $GITHUB_PATH - - name: Run both engines on base branch - run: | - git checkout ${{ github.event.pull_request.base.sha }} --quiet 2>/dev/null || true - pmd cpd --dir $CPD_DIRS --language java --minimum-tokens $CPD_MIN_TOKENS --format xml > cpd-base.xml 2>/dev/null || true - npx jscpd@4 --pattern "**/*.java" --ignore "**/target/**,**/node_modules/**" --reporters json --output jscpd-base --min-lines 6 --min-tokens 70 2>/dev/null || true - git checkout ${{ github.sha }} --quiet - - name: Run both engines on PR branch - run: | - pmd cpd --dir $CPD_DIRS --language java --minimum-tokens $CPD_MIN_TOKENS --format xml > cpd-pr.xml || true - npx jscpd@4 --pattern "**/*.java" --ignore "**/target/**,**/node_modules/**" --reporters consoleFull,json --output jscpd-report --min-lines 6 --min-tokens 70 || true - - name: Install xml2js - run: npm install --no-save xml2js - - name: Post combined duplicate report - if: always() - uses: actions/github-script@v7 - with: - script: | - const { analyzeAndReport } = require('./bin/ci-duplicate-report.js'); - await analyzeAndReport({ - github, context, core, - prXmlPath: 'cpd-pr.xml', - baseXmlPath: 'cpd-base.xml', - prJscpdPath: 'jscpd-report/jscpd-report.json', - baseJscpdPath: 'jscpd-base/jscpd-report.json' - }); + github-token: ${{ secrets.GITHUB_TOKEN }} + directories: 'parallel-consumer-core/src parallel-consumer-vertx/src parallel-consumer-reactor/src parallel-consumer-mutiny/src' + cpd-max-duplication: '5' + jscpd-max-duplication: '4' # File similarity detection - finds files that are semantically similar overall file-similarity: diff --git a/bin/ci-duplicate-report.js b/bin/ci-duplicate-report.js deleted file mode 100644 index 72a43a218..000000000 --- a/bin/ci-duplicate-report.js +++ /dev/null @@ -1,284 +0,0 @@ -#!/usr/bin/env node -/** - * Combined duplicate detection report - runs PMD CPD and jscpd, compares - * both against the base branch, and posts a single unified PR comment. - * - * Expected files: - * cpd-pr.xml, cpd-base.xml (PMD CPD XML output) - * jscpd-report/jscpd-report.json (jscpd JSON - PR) - * jscpd-base/jscpd-report.json (jscpd JSON - base) - */ -const fs = require('fs'); -const crypto = require('crypto'); -const { execSync } = require('child_process'); - -// Per-engine thresholds. Each engine produces different numbers, so the -// ceilings are tuned to their respective baselines (set just above current -// measured value). The increase rule is the real safety net - it catches -// regressions regardless of the absolute baseline. -const THRESHOLDS = { - cpd: { maxPct: 5, maxIncrease: 0.1 }, // PMD CPD baseline ~3.94% - jscpd: { maxPct: 4, maxIncrease: 0.1 }, // jscpd baseline ~2.65% -}; -const DIRS = ['parallel-consumer-core/src', 'parallel-consumer-vertx/src', 'parallel-consumer-reactor/src', 'parallel-consumer-mutiny/src']; - -// ── Shared helpers ──────────────────────────────────────────────────── - -function contentHash(fragment) { - return crypto.createHash('md5').update(fragment || '').digest('hex'); -} - -function relPath(p) { - return p.replace(/^.*parallel-consumer\//, ''); -} - -function countTotalLines(dirs) { - try { - const result = execSync( - `find ${dirs.join(' ')} -name '*.java' -type f 2>/dev/null | xargs wc -l 2>/dev/null | tail -1 | awk '{print $1}'`, - { encoding: 'utf8' } - ); - return parseInt(result.trim()) || 0; - } catch { - return 0; - } -} - -function deltaEmoji(d, isPercentage) { - // Celebratory/approving for decreases, suspicious for increases, neutral for no change. - // Ramp up the reaction based on magnitude. - if (d === 0) return ':heavy_minus_sign:'; - if (d < 0) { - const mag = Math.abs(d); - // Big decrease -> heart. Modest decrease -> thumbs up. Tiny decrease -> smile. - if (isPercentage ? mag >= 1 : mag >= 20) return ':heart:'; - if (isPercentage ? mag >= 0.1 : mag >= 5) return ':thumbsup:'; - return ':slightly_smiling_face:'; - } - // Increase: small = side-eye, moderate = monocle, large = skeptical - const mag = d; - if (isPercentage ? mag >= 1 : mag >= 20) return ':face_with_raised_eyebrow:'; - if (isPercentage ? mag >= 0.1 : mag >= 5) return ':face_with_monocle:'; - return ':face_with_diagonal_mouth:'; -} - -function fmtDelta(d, suffix) { - const isPct = (suffix === '%'); - const emoji = deltaEmoji(d, isPct); - if (d === 0) return `${emoji} 0`; - const s = suffix || ''; - return d > 0 ? `${emoji} +${d}${s}` : `${emoji} ${d}${s}`; -} - -// ── PMD CPD ─────────────────────────────────────────────────────────── - -async function parseCpdXml(path) { - if (!fs.existsSync(path) || fs.statSync(path).size === 0) return null; - const xml2js = require('xml2js'); - try { - const result = await new xml2js.Parser().parseStringPromise(fs.readFileSync(path, 'utf8')); - const root = result['pmd-cpd']; - if (!root || !root.duplication) return { duplicates: [] }; - return { - duplicates: root.duplication.map(d => ({ - lines: parseInt(d.$.lines), - tokens: parseInt(d.$.tokens), - files: (d.file || []).map(f => ({ - name: f.$.path, - startLine: parseInt(f.$.line), - endLine: parseInt(f.$.endline), - })), - fragment: (d.codefragment && d.codefragment[0]) || '', - })), - }; - } catch (err) { - console.log(`Failed to parse ${path}: ${err.message}`); - return null; - } -} - -function cpdStats(report, totalLines) { - if (!report) return null; - const clones = report.duplicates.length; - const duplicatedLines = report.duplicates.reduce((sum, d) => sum + d.lines * d.files.length, 0); - const percentage = totalLines > 0 ? (duplicatedLines / totalLines) * 100 : 0; - return { clones, duplicatedLines, percentage }; -} - -function cpdNewClones(prReport, baseReport) { - if (!baseReport) return prReport.duplicates || []; - const baseHashes = new Set(baseReport.duplicates.map(d => contentHash(d.fragment))); - return (prReport.duplicates || []).filter(d => !baseHashes.has(contentHash(d.fragment))); -} - -// ── jscpd ───────────────────────────────────────────────────────────── - -function loadJscpd(path) { - if (!fs.existsSync(path)) return null; - try { - return JSON.parse(fs.readFileSync(path, 'utf8')); - } catch { - return null; - } -} - -function jscpdNewClones(prReport, baseReport) { - if (!baseReport || !baseReport.duplicates) return prReport.duplicates || []; - const baseHashes = new Set(baseReport.duplicates.map(d => contentHash(d.fragment))); - return (prReport.duplicates || []).filter(d => !baseHashes.has(contentHash(d.fragment))); -} - -// ── Report rendering ────────────────────────────────────────────────── - -function renderEngineSection(title, prStats, baseStats, newClones, formatClone, check, thresholds) { - const icon = check.shouldFail ? ':x:' : ':white_check_mark:'; - let md = `### ${icon} ${title}\n\n`; - md += `| | PR | Base | Change |\n|---|--:|--:|--:|\n`; - - const cloneDelta = baseStats ? prStats.clones - baseStats.clones : 0; - const linesDelta = baseStats ? prStats.duplicatedLines - baseStats.duplicatedLines : 0; - - md += `| **Clones** | ${prStats.clones} | ${baseStats ? baseStats.clones : '-'} | ${baseStats ? fmtDelta(cloneDelta) : '-'} |\n`; - md += `| **Duplicated lines** | ${prStats.duplicatedLines} | ${baseStats ? baseStats.duplicatedLines : '-'} | ${baseStats ? fmtDelta(linesDelta) : '-'} |\n`; - md += `| **Duplication** | ${prStats.percentage.toFixed(2)}% | ${baseStats ? baseStats.percentage.toFixed(2) + '%' : '-'} | ${baseStats ? fmtDelta(parseFloat(check.pctDelta.toFixed(2)), '%') : '-'} |\n\n`; - - md += `| Rule | Limit | Status |\n|------|-------|--------|\n`; - md += `| Max duplication | ${thresholds.maxPct}% | ${check.pctFail ? ':x: FAIL' : ':white_check_mark: Pass'} (${prStats.percentage.toFixed(2)}%) |\n`; - md += `| Max increase vs base | +${thresholds.maxIncrease}% | ${check.increaseFail ? ':x: FAIL' : ':white_check_mark: Pass'} (${baseStats ? (check.pctDelta >= 0 ? '+' : '') + check.pctDelta.toFixed(2) + '%' : 'no base'}) |\n\n`; - - if (newClones.length > 0) { - md += `
:warning: ${newClones.length} new clones introduced\n\n`; - for (const clone of newClones.slice(0, 20)) { - md += formatClone(clone); - } - if (newClones.length > 20) md += `\n...and ${newClones.length - 20} more\n`; - md += `\n
\n\n`; - } else if (baseStats) { - md += `No new clones introduced by this PR.\n\n`; - } - - return md; -} - -function checkFail(stats, baseStats, thresholds) { - if (!stats) return { shouldFail: false, pctFail: false, increaseFail: false, pctDelta: 0 }; - const pctDelta = baseStats ? stats.percentage - baseStats.percentage : 0; - const pctFail = stats.percentage > thresholds.maxPct; - const increaseFail = baseStats && pctDelta > thresholds.maxIncrease; - return { shouldFail: pctFail || increaseFail, pctFail, increaseFail, pctDelta }; -} - -// ── Main entrypoint ─────────────────────────────────────────────────── - -async function analyzeAndReport({ github, context, core, prXmlPath, baseXmlPath, prJscpdPath, baseJscpdPath }) { - const totalLines = countTotalLines(DIRS); - - // PMD CPD - const cpdPr = await parseCpdXml(prXmlPath); - const cpdBase = await parseCpdXml(baseXmlPath); - const cpdPrStats = cpdStats(cpdPr, totalLines); - const cpdBaseStats = cpdStats(cpdBase, totalLines); - const cpdNew = cpdPr ? cpdNewClones(cpdPr, cpdBase) : []; - const cpdCheck = cpdPrStats ? checkFail(cpdPrStats, cpdBaseStats, THRESHOLDS.cpd) : { shouldFail: false }; - - // jscpd - const jscpdPr = loadJscpd(prJscpdPath); - const jscpdBase = loadJscpd(baseJscpdPath); - const jscpdPrStats = jscpdPr ? { - clones: jscpdPr.statistics.total.clones, - duplicatedLines: jscpdPr.statistics.total.duplicatedLines, - percentage: parseFloat(jscpdPr.statistics.total.percentage), - } : null; - const jscpdBaseStats = jscpdBase ? { - clones: jscpdBase.statistics.total.clones, - duplicatedLines: jscpdBase.statistics.total.duplicatedLines, - percentage: parseFloat(jscpdBase.statistics.total.percentage), - } : null; - const jscpdNew = jscpdPr ? jscpdNewClones(jscpdPr, jscpdBase) : []; - const jscpdCheck = jscpdPrStats ? checkFail(jscpdPrStats, jscpdBaseStats, THRESHOLDS.jscpd) : { shouldFail: false }; - - const anyFail = cpdCheck.shouldFail || jscpdCheck.shouldFail; - const overallIcon = anyFail ? ':x:' : ':white_check_mark:'; - - // Build combined comment - let body = `## ${overallIcon} Duplicate Code Report\n\n`; - body += `Two engines run in parallel for cross-validation. Each has its own thresholds tuned to its baseline - the real safety net is the per-engine "max increase vs base" check.\n\n`; - - // PMD CPD section - if (cpdPrStats) { - const formatClone = (d) => { - const locations = d.files.map(f => `\`${relPath(f.name)}:${f.startLine}\``).join(' <-> '); - return `- **${d.lines} lines** (${d.tokens} tokens): ${locations}\n`; - }; - body += renderEngineSection('PMD CPD (Java-aware)', cpdPrStats, cpdBaseStats, cpdNew, formatClone, cpdCheck, THRESHOLDS.cpd); - } else { - body += `### :question: PMD CPD\n\nNo report available.\n\n`; - } - - // jscpd section - if (jscpdPrStats) { - const formatClone = (d) => { - const f1 = relPath(d.firstFile.name); - const f2 = relPath(d.secondFile.name); - return `- **${d.lines} lines**: \`${f1}:${d.firstFile.startLoc.line}\` <-> \`${f2}:${d.secondFile.startLoc.line}\`\n`; - }; - body += renderEngineSection('jscpd (language-agnostic)', jscpdPrStats, jscpdBaseStats, jscpdNew, formatClone, jscpdCheck, THRESHOLDS.jscpd); - } else { - body += `### :question: jscpd\n\nNo report available.\n\n`; - } - - // Post or update PR comment - const comments = await github.rest.issues.listComments({ - owner: context.repo.owner, repo: context.repo.repo, issue_number: context.issue.number - }); - const existing = comments.data.find(c => c.body.startsWith('## ') && c.body.includes('Duplicate Code Report')); - if (existing) { - await github.rest.issues.updateComment({ - owner: context.repo.owner, repo: context.repo.repo, comment_id: existing.id, body - }); - } else { - await github.rest.issues.createComment({ - owner: context.repo.owner, repo: context.repo.repo, issue_number: context.issue.number, body - }); - } - - // Annotate new CPD clones on PR diff (prefer CPD since it's more accurate) - const annotateClones = cpdNew.length > 0 ? cpdNew : []; - if (annotateClones.length > 0) { - const { data: files } = await github.rest.pulls.listFiles({ - owner: context.repo.owner, repo: context.repo.repo, pull_number: context.issue.number - }); - const changedFiles = new Set(files.map(f => f.filename)); - for (const d of annotateClones.slice(0, 10)) { - for (const file of d.files) { - const relFile = relPath(file.name); - if (changedFiles.has(relFile)) { - const others = d.files.filter(f => f !== file).map(f => `${relPath(f.name)}:${f.startLine}`).join(', '); - try { - await github.rest.pulls.createReviewComment({ - owner: context.repo.owner, - repo: context.repo.repo, - pull_number: context.issue.number, - commit_id: context.sha, - path: relFile, - line: file.startLine, - body: `:warning: **Duplicate code detected** - ${d.lines} lines duplicated with \`${others}\`` - }); - } catch (e) { - console.log(`Could not annotate ${relFile}:${file.startLine} - ${e.message}`); - } - break; - } - } - } - } - - if (anyFail) { - const msgs = []; - if (cpdCheck.shouldFail) msgs.push('PMD CPD'); - if (jscpdCheck.shouldFail) msgs.push('jscpd'); - core.setFailed(`Duplicate code check failed (${msgs.join(', ')}) - see PR comment for details`); - } -} - -module.exports = { analyzeAndReport, THRESHOLDS }; From ef6f919f4210f19b3261c3b064732dbdc76800df Mon Sep 17 00:00:00 2001 From: Antony Stubbs Date: Thu, 16 Apr 2026 17:34:30 +1200 Subject: [PATCH 43/67] ci: pin duplicate-code-cross-check to @v1 instead of @main Using a floating version tag is safer than @main - we won't get broken by unintended changes pushed to the action's default branch. @v1 tracks the latest v1.x release. Co-Authored-By: Claude Opus 4.6 (1M context) --- .github/workflows/maven.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/maven.yml b/.github/workflows/maven.yml index 0aa6cdc0d..76d0f7031 100644 --- a/.github/workflows/maven.yml +++ b/.github/workflows/maven.yml @@ -75,7 +75,7 @@ jobs: - uses: actions/checkout@v6 with: fetch-depth: 0 - - uses: astubbs/duplicate-code-cross-check@main + - uses: astubbs/duplicate-code-cross-check@v1 with: github-token: ${{ secrets.GITHUB_TOKEN }} directories: 'parallel-consumer-core/src parallel-consumer-vertx/src parallel-consumer-reactor/src parallel-consumer-mutiny/src' From bf755eb4d70882d552a296e2b3c4c8a5df777a42 Mon Sep 17 00:00:00 2001 From: Antony Stubbs Date: Thu, 16 Apr 2026 19:39:08 +1200 Subject: [PATCH 44/67] ci: re-trigger to pick up cached Maven deps from prior partial run From 2ddb47ac7009ef9b7bc7ceed5ae1da2eab1462f4 Mon Sep 17 00:00:00 2001 From: Antony Stubbs Date: Thu, 16 Apr 2026 19:43:35 +1200 Subject: [PATCH 45/67] ci: add prepare-deps job to pre-warm Maven cache Test jobs were hitting 30-min timeouts because Maven Central was serving some POMs at 2 B/s. Each matrix job (Unit, Integration, Performance) independently tried to download the same deps. Add a serial prepare-deps job that runs mvn dependency:go-offline once, populates the ~/.m2/repository cache (shared via the hashFiles('**/pom.xml') cache key that actions/setup-java uses), then parallel test jobs restore from the warm cache. Also split matrix timeouts: Unit=30m, Integration=60m, Performance=60m. TODO in the YAML + memory note: re-evaluate if this serial step is still needed once we have a few weeks of CI data. Co-Authored-By: Claude Opus 4.6 (1M context) --- .github/workflows/maven.yml | 27 ++++++++++++++++++++++++++- 1 file changed, 26 insertions(+), 1 deletion(-) diff --git a/.github/workflows/maven.yml b/.github/workflows/maven.yml index 76d0f7031..d4337a77b 100644 --- a/.github/workflows/maven.yml +++ b/.github/workflows/maven.yml @@ -25,9 +25,31 @@ jobs: # ── PR Builds ────────────────────────────────────────────────────────── + # Pre-warm the Maven dependency cache so the three test jobs can start + # with everything local. Without this, each job races to download the + # same deps from Maven Central, and if Central is slow, they all stall. + # TODO: If this extra serial step doesn't consistently help, remove it + # and rely on per-job caching + longer timeouts instead. Trade-off is + # wall-clock time (adds ~1-2 min serial) vs reliability vs flaky networks. + prepare-deps: + if: github.event_name == 'pull_request' + name: "Prepare Maven Cache" + runs-on: ubuntu-latest + timeout-minutes: 30 + steps: + - uses: actions/checkout@v6 + - uses: actions/setup-java@v5 + with: + distribution: 'temurin' + java-version: '17' + cache: 'maven' + - name: Download all dependencies + run: ./mvnw --batch-mode -Pci dependency:go-offline -Dlicense.skip -DincludeScope=test -U + # All test suites in parallel -fail-fast cancels the rest if any fails test: if: github.event_name == 'pull_request' + needs: prepare-deps strategy: fail-fast: true matrix: @@ -35,15 +57,18 @@ jobs: - suite: unit name: "Unit Tests" cmd: "bin/ci-unit-test.sh" + timeout: 30 - suite: integration name: "Integration Tests" cmd: "bin/ci-integration-test.sh" + timeout: 60 - suite: performance name: "Performance Tests" cmd: "bin/performance-test.sh" + timeout: 60 name: "${{ matrix.name }}" runs-on: ubuntu-latest - timeout-minutes: 30 + timeout-minutes: ${{ matrix.timeout }} steps: - uses: actions/checkout@v6 - uses: actions/setup-java@v5 From bb2ad8a96d9837f03bce2746ce49733b04f87ac8 Mon Sep 17 00:00:00 2001 From: Antony Stubbs Date: Thu, 16 Apr 2026 20:46:37 +1200 Subject: [PATCH 46/67] ci: right-size job timeouts based on observed data After one successful green run with the prepare-deps cache, tighten timeouts to realistic values while keeping buffer for Maven Central network variance. - Prepare Maven Cache: 30m -> 10m (observed 2m14s) - Unit Tests: 30m -> 20m (observed 9m31s) - Integration Tests: 60m (unchanged, observed 44m33s worst case) - Performance Tests: 60m (unchanged, observed 44m41s worst case) Integration and Performance stay at 60m because we've seen Maven Central serve POMs at 2 B/s. TODO comment added: tighten to 30m once we see 5+ consecutive runs consistently under 20m. Co-Authored-By: Claude Opus 4.6 (1M context) --- .github/workflows/maven.yml | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/.github/workflows/maven.yml b/.github/workflows/maven.yml index d4337a77b..0446cc7ca 100644 --- a/.github/workflows/maven.yml +++ b/.github/workflows/maven.yml @@ -35,7 +35,7 @@ jobs: if: github.event_name == 'pull_request' name: "Prepare Maven Cache" runs-on: ubuntu-latest - timeout-minutes: 30 + timeout-minutes: 10 steps: - uses: actions/checkout@v6 - uses: actions/setup-java@v5 @@ -57,7 +57,10 @@ jobs: - suite: unit name: "Unit Tests" cmd: "bin/ci-unit-test.sh" - timeout: 30 + timeout: 20 + # TODO: tighten integration/performance to 30m once we have 5+ + # consecutive successful runs consistently under 20m. Current 60m + # buffer is for Maven Central network variance we've observed. - suite: integration name: "Integration Tests" cmd: "bin/ci-integration-test.sh" From 80d7e002fdca983d7dfdef232b1e45f302c7d2d5 Mon Sep 17 00:00:00 2001 From: Antony Stubbs Date: Thu, 16 Apr 2026 22:59:53 +1200 Subject: [PATCH 47/67] ci: fix CI hangs by fixing repo order and adding Maven connection timeouts Root cause of the CI timeouts: Maven queries repositories sequentially in pom.xml order and does not fall through when one hangs. Logs showed Maven stalled for 14+ minutes on jitpack.io / packagecloud.io connections - Maven Central was serving fast (1.1 MB/s) just before the stalls. Fixes: 1. Remove unused jitpack.io repository from pom.xml - nothing in the project actually comes from it, but Maven was querying it for every dependency. 2. Reorder remaining repositories: Central first, then Confluent, then astubbs-truth-generator last. Most artifacts live on Central, so putting it first means the custom repos are only queried for the few artifacts that need them. 3. Add .mvn/maven.config with aggressive per-connection timeouts (10s connect, 30s read). Any hanging repo now fails fast and Maven falls through to the next configured repo in seconds instead of waiting the default multi-minute timeout. Both wagon.* and aether.* keys included for Maven 3.8 (Wagon) and 3.9+ (native resolver) compat. 4. Tighten Unit Tests CI timeout 30m -> 15m. Integration and Performance stay at 60m pending verification this fix holds. Co-Authored-By: Claude Opus 4.6 (1M context) --- .github/workflows/maven.yml | 4 ++-- .mvn/maven.config | 13 +++++++++++++ pom.xml | 15 +++++++++------ 3 files changed, 24 insertions(+), 8 deletions(-) create mode 100644 .mvn/maven.config diff --git a/.github/workflows/maven.yml b/.github/workflows/maven.yml index 0446cc7ca..9e328db4e 100644 --- a/.github/workflows/maven.yml +++ b/.github/workflows/maven.yml @@ -35,7 +35,7 @@ jobs: if: github.event_name == 'pull_request' name: "Prepare Maven Cache" runs-on: ubuntu-latest - timeout-minutes: 10 + timeout-minutes: 15 steps: - uses: actions/checkout@v6 - uses: actions/setup-java@v5 @@ -57,7 +57,7 @@ jobs: - suite: unit name: "Unit Tests" cmd: "bin/ci-unit-test.sh" - timeout: 20 + timeout: 15 # TODO: tighten integration/performance to 30m once we have 5+ # consecutive successful runs consistently under 20m. Current 60m # buffer is for Maven Central network variance we've observed. diff --git a/.mvn/maven.config b/.mvn/maven.config new file mode 100644 index 000000000..5ba945bd2 --- /dev/null +++ b/.mvn/maven.config @@ -0,0 +1,13 @@ +# Maven per-connection timeouts. Keep them aggressive so any slow/unreachable +# repository fails fast and Maven moves on to the next configured repo rather +# than hanging the build. See pom.xml for the repository list and comment. +# +# Both wagon.* and aether.* keys included for compatibility with Maven 3.8 +# (Wagon transport) and 3.9+ (native maven-resolver-transport-http). + +-Dmaven.wagon.http.connectionTimeout=10000 +-Dmaven.wagon.http.readTimeout=30000 +-Dmaven.wagon.httpconnectionManager.ttlSeconds=120 +-Dmaven.wagon.http.retryHandler.count=3 +-Daether.connector.connectTimeout=10000 +-Daether.connector.requestTimeout=30000 diff --git a/pom.xml b/pom.xml index a7fe1bb56..eb5f50a70 100644 --- a/pom.xml +++ b/pom.xml @@ -1008,18 +1008,21 @@ + - - confluent - https://packages.confluent.io/maven/ - central https://repo1.maven.org/maven2/ - jitpack.io - https://jitpack.io + confluent + https://packages.confluent.io/maven/ astubbs-truth-generator From 4a9f7a035199fe88998199fad7b77e09b5702ac6 Mon Sep 17 00:00:00 2001 From: Antony Stubbs Date: Thu, 16 Apr 2026 23:03:34 +1200 Subject: [PATCH 48/67] ci: strip comments from .mvn/maven.config Maven's .mvn/maven.config expects every line to be a CLI arg - it does not support shell-style # comments. The previous commit broke the build with: Unrecognized maven.config file entries: [# Maven per-connection...] The explanation of what these settings are is in the commit that added them - keeping it there rather than in the file itself. Co-Authored-By: Claude Opus 4.6 (1M context) --- .mvn/maven.config | 7 ------- 1 file changed, 7 deletions(-) diff --git a/.mvn/maven.config b/.mvn/maven.config index 5ba945bd2..8db578b05 100644 --- a/.mvn/maven.config +++ b/.mvn/maven.config @@ -1,10 +1,3 @@ -# Maven per-connection timeouts. Keep them aggressive so any slow/unreachable -# repository fails fast and Maven moves on to the next configured repo rather -# than hanging the build. See pom.xml for the repository list and comment. -# -# Both wagon.* and aether.* keys included for compatibility with Maven 3.8 -# (Wagon transport) and 3.9+ (native maven-resolver-transport-http). - -Dmaven.wagon.http.connectionTimeout=10000 -Dmaven.wagon.http.readTimeout=30000 -Dmaven.wagon.httpconnectionManager.ttlSeconds=120 From 617d5dbb2f13891ba0fae4dae4058c38ede960f5 Mon Sep 17 00:00:00 2001 From: Antony Stubbs Date: Thu, 16 Apr 2026 23:37:20 +1200 Subject: [PATCH 49/67] ci: fix PIT silent failures and exclude flaky test from baseline Two related fixes so PIT stops showing green when it's actually broken: 1. Exclude MockConsumerTestWithSaslAuthenticationException from PIT via -DexcludedTestClasses. The test uses an anonymous MockConsumer subclass with captured AtomicLong state and time-sensitive exception injection, which PIT's bytecode instrumentation and classloader don't play nicely with. The test still runs in the normal unit suite, just not in PIT's baseline pass. 2. Remove continue-on-error: true from the PIT maven step so real PIT failures paint the job red instead of silently passing. Pair that with a github-script change to post an explicit "PIT did not produce a report" PR comment when mutations.csv is missing, so the failure is visible on the PR, not buried in Actions logs. Before this, PIT had been silently failing for some time because of the excluded test, with no signal on the PR. Co-Authored-By: Claude Opus 4.6 (1M context) --- .github/workflows/maven.yml | 49 +++++++++++++++++++++---------------- 1 file changed, 28 insertions(+), 21 deletions(-) diff --git a/.github/workflows/maven.yml b/.github/workflows/maven.yml index 9e328db4e..a413fac85 100644 --- a/.github/workflows/maven.yml +++ b/.github/workflows/maven.yml @@ -180,9 +180,11 @@ jobs: distribution: 'temurin' java-version: '17' cache: 'maven' + # Excluded tests are incompatible with PIT's bytecode instrumentation + # (anonymous inner class with captured state + time-sensitive behavior). + # They still run in the normal unit test suite. - name: Run PIT mutation testing - run: ./mvnw --batch-mode -Pci test-compile org.pitest:pitest-maven:mutationCoverage -Dlicense.skip -DtargetClasses="io.confluent.parallelconsumer.*" -DtargetTests="io.confluent.parallelconsumer.*" -Dthreads=4 -pl parallel-consumer-core -am - continue-on-error: true + run: ./mvnw --batch-mode -Pci test-compile org.pitest:pitest-maven:mutationCoverage -Dlicense.skip -DtargetClasses="io.confluent.parallelconsumer.*" -DtargetTests="io.confluent.parallelconsumer.*" -DexcludedTestClasses="io.confluent.parallelconsumer.MockConsumerTestWithSaslAuthenticationException" -Dthreads=4 -pl parallel-consumer-core -am - name: Upload PIT report if: always() uses: actions/upload-artifact@v4 @@ -197,28 +199,33 @@ jobs: const fs = require('fs'); const { execSync } = require('child_process'); const files = execSync('find . -path "*/pit-reports/*/mutations.csv" -type f').toString().trim().split('\n').filter(f => f); - if (files.length === 0) { console.log('No PIT report found'); return; } - let killed = 0, survived = 0, noCov = 0, total = 0; - for (const f of files) { - const lines = fs.readFileSync(f, 'utf8').split('\n').filter(l => l.trim()); - for (const line of lines) { - total++; - if (line.includes('KILLED')) killed++; - else if (line.includes('SURVIVED')) survived++; - else if (line.includes('NO_COVERAGE')) noCov++; + let body; + if (files.length === 0) { + body = `## :x: Mutation Testing (PIT) Report\n\n`; + body += `**PIT did not produce a report.** Most commonly this means a test failed in the baseline (PIT runs all tests unmodified first to establish green) and PIT aborted before mutating. See the "Run PIT mutation testing" step logs for the failing test, then either fix it or add it to \`-DexcludedTestClasses\` in the workflow.\n`; + } else { + let killed = 0, survived = 0, noCov = 0, total = 0; + for (const f of files) { + const lines = fs.readFileSync(f, 'utf8').split('\n').filter(l => l.trim()); + for (const line of lines) { + total++; + if (line.includes('KILLED')) killed++; + else if (line.includes('SURVIVED')) survived++; + else if (line.includes('NO_COVERAGE')) noCov++; + } } + const score = total > 0 ? ((killed / total) * 100).toFixed(1) : '0'; + body = `## Mutation Testing (PIT) Report\n\n`; + body += `| Metric | Value |\n|--------|-------|\n`; + body += `| Mutations generated | ${total} |\n`; + body += `| Killed (detected) | ${killed} |\n`; + body += `| Survived (missed) | ${survived} |\n`; + body += `| No coverage | ${noCov} |\n`; + body += `| **Mutation score** | **${score}%** |\n\n`; + body += `Full HTML report available as artifact: \`pit-report\`\n`; } - const score = total > 0 ? ((killed / total) * 100).toFixed(1) : '0'; - let body = `## Mutation Testing (PIT) Report\n\n`; - body += `| Metric | Value |\n|--------|-------|\n`; - body += `| Mutations generated | ${total} |\n`; - body += `| Killed (detected) | ${killed} |\n`; - body += `| Survived (missed) | ${survived} |\n`; - body += `| No coverage | ${noCov} |\n`; - body += `| **Mutation score** | **${score}%** |\n\n`; - body += `Full HTML report available as artifact: \`pit-report\`\n`; const comments = await github.rest.issues.listComments({ owner: context.repo.owner, repo: context.repo.repo, issue_number: context.issue.number }); - const existing = comments.data.find(c => c.body.includes('## Mutation Testing (PIT) Report')); + const existing = comments.data.find(c => c.body.includes('Mutation Testing (PIT) Report')); if (existing) { await github.rest.issues.updateComment({ owner: context.repo.owner, repo: context.repo.repo, comment_id: existing.id, body }); } else { From 328e39f4ab0aadbe2865053f24d6982e32c44ca4 Mon Sep 17 00:00:00 2001 From: Antony Stubbs Date: Fri, 17 Apr 2026 00:33:37 +1200 Subject: [PATCH 50/67] ci: bump Maven read timeout and fix Awaitility state leak between tests Two related CI fixes that should land together: 1. Bump Maven HTTP read timeout 30s -> 60s (wagon + aether). The 30s was catching normal-but-slow Maven Central POM downloads (e.g. vertx-web-client:pom:4.5.7) and failing Performance Tests. 60s is still far short of the hours-long hangs we had before, while tolerating real-world network variance. 2. Fix an Awaitility test-order dependency that PIT's baseline exposed. MockConsumerTestWithSaslAuthenticationException calls Awaitility.setDefaultTimeout(50s) mid-test and resets at the end, but the inline reset never runs if the assertion throws. The 50s default then leaks into whichever test runs next (under PIT's re-ordering, that tripped PCMetricsTest.metricsRegisterBinding). Move the reset into the shared test base's existing @AfterEach so it always runs regardless of test outcome, and drop the now-redundant inline reset. This also removes the need for the PIT excludedTestClasses workaround - PIT can now run the full suite. Also update AGENTS.md with a new rule: stacked PRs must include "depends on #N" in their description so the dependency-gating action blocks child merges until the parent merges (matching the same rule added to the global CLAUDE.md). Co-Authored-By: Claude Opus 4.6 (1M context) --- .github/workflows/maven.yml | 5 +---- .mvn/maven.config | 4 ++-- AGENTS.md | 1 + .../AbstractParallelEoSStreamProcessorTestBase.java | 7 +++++++ .../MockConsumerTestWithSaslAuthenticationException.java | 6 ++---- 5 files changed, 13 insertions(+), 10 deletions(-) diff --git a/.github/workflows/maven.yml b/.github/workflows/maven.yml index a413fac85..d8a3e5ef8 100644 --- a/.github/workflows/maven.yml +++ b/.github/workflows/maven.yml @@ -180,11 +180,8 @@ jobs: distribution: 'temurin' java-version: '17' cache: 'maven' - # Excluded tests are incompatible with PIT's bytecode instrumentation - # (anonymous inner class with captured state + time-sensitive behavior). - # They still run in the normal unit test suite. - name: Run PIT mutation testing - run: ./mvnw --batch-mode -Pci test-compile org.pitest:pitest-maven:mutationCoverage -Dlicense.skip -DtargetClasses="io.confluent.parallelconsumer.*" -DtargetTests="io.confluent.parallelconsumer.*" -DexcludedTestClasses="io.confluent.parallelconsumer.MockConsumerTestWithSaslAuthenticationException" -Dthreads=4 -pl parallel-consumer-core -am + run: ./mvnw --batch-mode -Pci test-compile org.pitest:pitest-maven:mutationCoverage -Dlicense.skip -DtargetClasses="io.confluent.parallelconsumer.*" -DtargetTests="io.confluent.parallelconsumer.*" -Dthreads=4 -pl parallel-consumer-core -am - name: Upload PIT report if: always() uses: actions/upload-artifact@v4 diff --git a/.mvn/maven.config b/.mvn/maven.config index 8db578b05..c03f6cf34 100644 --- a/.mvn/maven.config +++ b/.mvn/maven.config @@ -1,6 +1,6 @@ -Dmaven.wagon.http.connectionTimeout=10000 --Dmaven.wagon.http.readTimeout=30000 +-Dmaven.wagon.http.readTimeout=60000 -Dmaven.wagon.httpconnectionManager.ttlSeconds=120 -Dmaven.wagon.http.retryHandler.count=3 -Daether.connector.connectTimeout=10000 --Daether.connector.requestTimeout=30000 +-Daether.connector.requestTimeout=60000 diff --git a/AGENTS.md b/AGENTS.md index b9b751d1f..6db0fb961 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -84,6 +84,7 @@ The default behavior on macOS dev machines is `format` mode (auto-fixes headers) ### Git Safety - **NEVER commit or push without explicitly asking the user first.** Wait for approval. This is the #1 rule. +- **When creating a stacked PR, include `depends on #N` in the PR description** (where `#N` is the parent PR it stacks on). This fork runs a PR dependency gating action (see `.github/workflows/check-dependencies.yml`) which blocks child PRs from merging until the parent is merged. One `depends on` line per parent. Keep the list accurate if the chain changes. - Branch off master for upstream contributor cherry-picks so PRs show only their change. - Never commit without tests and documentation in the same pass. - Run tests before committing. If they fail, fix them first. diff --git a/parallel-consumer-core/src/test/java/io/confluent/parallelconsumer/AbstractParallelEoSStreamProcessorTestBase.java b/parallel-consumer-core/src/test/java/io/confluent/parallelconsumer/AbstractParallelEoSStreamProcessorTestBase.java index 918eb0657..652cf88ec 100644 --- a/parallel-consumer-core/src/test/java/io/confluent/parallelconsumer/AbstractParallelEoSStreamProcessorTestBase.java +++ b/parallel-consumer-core/src/test/java/io/confluent/parallelconsumer/AbstractParallelEoSStreamProcessorTestBase.java @@ -21,6 +21,7 @@ import org.apache.kafka.clients.producer.MockProducer; import org.apache.kafka.common.TopicPartition; import org.apache.kafka.common.serialization.Serdes; +import org.awaitility.Awaitility; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; import pl.tlinkowski.unij.api.UniLists; @@ -144,6 +145,12 @@ protected ParallelConsumerOptions.ParallelConsumerOptionsBuilder @AfterEach public void close() { + // Reset Awaitility's global thread-local timeout state so per-test overrides + // (e.g. setDefaultTimeout) don't leak into other tests under non-deterministic + // test order (PIT baseline/mutations surface this; surefire's default ordering + // happens to mask it). Runs even if the test body threw. + Awaitility.reset(); + // don't try to close if error'd (at least one test purposefully creates an error to tests error handling) - we // don't want to bubble up an error here that we expect from here. if (!parentParallelConsumer.isClosedOrFailed()) { diff --git a/parallel-consumer-core/src/test/java/io/confluent/parallelconsumer/MockConsumerTestWithSaslAuthenticationException.java b/parallel-consumer-core/src/test/java/io/confluent/parallelconsumer/MockConsumerTestWithSaslAuthenticationException.java index c324119f1..6ea674e06 100644 --- a/parallel-consumer-core/src/test/java/io/confluent/parallelconsumer/MockConsumerTestWithSaslAuthenticationException.java +++ b/parallel-consumer-core/src/test/java/io/confluent/parallelconsumer/MockConsumerTestWithSaslAuthenticationException.java @@ -96,14 +96,12 @@ public synchronized void commitSync(Map offse }); }); - // temporarily set the wait timeout + // temporarily set the wait timeout (reset is handled by the base class + // @AfterEach so it runs even if the assertion below fails) Awaitility.setDefaultTimeout(Duration.ofSeconds(50)); - // Awaitility.await().untilAsserted(() -> { assertThat(records).hasSize(3); }); - - Awaitility.reset(); } private void addRecords(MockConsumer mockConsumer) { From 3535620a419089e0d7eb116b6d10808ff2f57228 Mon Sep 17 00:00:00 2001 From: Antony Stubbs Date: Fri, 17 Apr 2026 01:27:53 +1200 Subject: [PATCH 51/67] ci: fix stale Maven cache, bump timeout to 120s, re-exclude flaky PIT tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - prepare-deps: drop setup-java's built-in `cache: maven` in favour of explicit restore+save with a rotating `...-${run_id}` save key. setup-java/actions/cache won't save when the primary key was a hit, so a partial early cache (gap from a Central timeout on one POM) would persist forever. Forcing save-every-run breaks that trap. - test matrix: matching explicit restore with restore-keys fallback so jobs pick up prepare-deps' freshest rotating-key save. - .mvn/maven.config: readTimeout/requestTimeout 60s → 120s. Still well short of the prior multi-hour hangs, but tolerates Central's worst slow pockets on the rare uncached POM fetch. - PIT: re-add -DexcludedTestClasses for the two baseline-failing tests (MockConsumerTestWithSaslAuthenticationException, ProducerManagerTest) so PIT runs to completion and posts a real mutation score. Base-class Awaitility.reset() @AfterEach stays as a general leakage guard; per-test root-cause investigation is follow-up work. One-time stale cache (id 3792626469) also wiped out-of-band via \`gh cache delete\` so the rotating-key save can start fresh. Co-Authored-By: Claude Opus 4.6 (1M context) --- .github/workflows/maven.yml | 44 ++++++++++++++++++++++++++++++++++--- .mvn/maven.config | 4 ++-- 2 files changed, 43 insertions(+), 5 deletions(-) diff --git a/.github/workflows/maven.yml b/.github/workflows/maven.yml index d8a3e5ef8..9cf10e337 100644 --- a/.github/workflows/maven.yml +++ b/.github/workflows/maven.yml @@ -42,9 +42,28 @@ jobs: with: distribution: 'temurin' java-version: '17' - cache: 'maven' + # Explicit cache steps below instead of setup-java's built-in + # `cache: maven`. Built-in uses actions/cache under the hood, + # which does NOT save when the primary key was a hit. Once an + # early run populates a partial cache (e.g. Central timed out + # on one POM), every subsequent run restores the same gap and + # can never write the completed version back. Rotating save + # key below forces a save every run. + - name: Restore Maven cache + uses: actions/cache/restore@v4 + with: + path: ~/.m2/repository + key: setup-java-Linux-x64-maven-${{ hashFiles('**/pom.xml') }} + restore-keys: | + setup-java-Linux-x64-maven- - name: Download all dependencies run: ./mvnw --batch-mode -Pci dependency:go-offline -Dlicense.skip -DincludeScope=test -U + - name: Save Maven cache (rotating key) + if: always() + uses: actions/cache/save@v4 + with: + path: ~/.m2/repository + key: setup-java-Linux-x64-maven-${{ hashFiles('**/pom.xml') }}-${{ github.run_id }} # All test suites in parallel -fail-fast cancels the rest if any fails test: @@ -78,7 +97,17 @@ jobs: with: distribution: 'temurin' java-version: '17' - cache: 'maven' + # Explicit restore (no save) so we can fall back via + # restore-keys to the rotating-key cache prepare-deps saved. + # setup-java's built-in `cache: maven` has no restore-keys + # support, so it would miss prepare-deps' `...-{run_id}` key. + - name: Restore Maven cache + uses: actions/cache/restore@v4 + with: + path: ~/.m2/repository + key: setup-java-Linux-x64-maven-${{ hashFiles('**/pom.xml') }} + restore-keys: | + setup-java-Linux-x64-maven- - name: ${{ matrix.name }} run: ${{ matrix.cmd }} - name: Upload coverage to Codecov @@ -180,8 +209,17 @@ jobs: distribution: 'temurin' java-version: '17' cache: 'maven' + # excludedTestClasses: these two fail their own PIT baseline runs + # (non-mutation green check) due to timing/instrumentation sensitivity. + # - MockConsumerTestWithSaslAuthenticationException: standalone, uses + # shaded Awaitility (org.testcontainers.shaded.org.awaitility), so + # the base-class Awaitility.reset() @AfterEach doesn't reach it. + # - ProducerManagerTest.producedRecordsCantBeInTransactionWithoutItsOffsetDirect: + # also flakes under PIT instrumentation. + # The base-class @AfterEach Awaitility.reset() is retained as a + # general leakage guard; per-test root-cause fixes are follow-up work. - name: Run PIT mutation testing - run: ./mvnw --batch-mode -Pci test-compile org.pitest:pitest-maven:mutationCoverage -Dlicense.skip -DtargetClasses="io.confluent.parallelconsumer.*" -DtargetTests="io.confluent.parallelconsumer.*" -Dthreads=4 -pl parallel-consumer-core -am + run: ./mvnw --batch-mode -Pci test-compile org.pitest:pitest-maven:mutationCoverage -Dlicense.skip -DtargetClasses="io.confluent.parallelconsumer.*" -DtargetTests="io.confluent.parallelconsumer.*" -DexcludedTestClasses="io.confluent.parallelconsumer.MockConsumerTestWithSaslAuthenticationException,io.confluent.parallelconsumer.internal.ProducerManagerTest" -Dthreads=4 -pl parallel-consumer-core -am - name: Upload PIT report if: always() uses: actions/upload-artifact@v4 diff --git a/.mvn/maven.config b/.mvn/maven.config index c03f6cf34..bfe087449 100644 --- a/.mvn/maven.config +++ b/.mvn/maven.config @@ -1,6 +1,6 @@ -Dmaven.wagon.http.connectionTimeout=10000 --Dmaven.wagon.http.readTimeout=60000 +-Dmaven.wagon.http.readTimeout=120000 -Dmaven.wagon.httpconnectionManager.ttlSeconds=120 -Dmaven.wagon.http.retryHandler.count=3 -Daether.connector.connectTimeout=10000 --Daether.connector.requestTimeout=60000 +-Daether.connector.requestTimeout=120000 From 096442d50714f23d1f6946ceb6312fb4df1228c2 Mon Sep 17 00:00:00 2001 From: Antony Stubbs Date: Fri, 17 Apr 2026 01:50:12 +1200 Subject: [PATCH 52/67] ci: exclude PCMetricsTest from PIT baseline + track fix in issue #39 Third baseline-failing test surfaced once the previous two exclusions unblocked PIT. Added to -DexcludedTestClasses with a comment pointing at the tracking issue (#39) which lists all three with current hypotheses and next steps for root-cause investigation. Co-Authored-By: Claude Opus 4.6 (1M context) --- .github/workflows/maven.yml | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/.github/workflows/maven.yml b/.github/workflows/maven.yml index 9cf10e337..6e77c557a 100644 --- a/.github/workflows/maven.yml +++ b/.github/workflows/maven.yml @@ -209,17 +209,21 @@ jobs: distribution: 'temurin' java-version: '17' cache: 'maven' - # excludedTestClasses: these two fail their own PIT baseline runs - # (non-mutation green check) due to timing/instrumentation sensitivity. + # excludedTestClasses: these fail their own PIT baseline (non-mutation + # green check) due to timing/instrumentation sensitivity. Tracked in + # https://github.com/astubbs/parallel-consumer/issues/39. # - MockConsumerTestWithSaslAuthenticationException: standalone, uses # shaded Awaitility (org.testcontainers.shaded.org.awaitility), so # the base-class Awaitility.reset() @AfterEach doesn't reach it. # - ProducerManagerTest.producedRecordsCantBeInTransactionWithoutItsOffsetDirect: - # also flakes under PIT instrumentation. + # flakes under PIT instrumentation. + # - PCMetricsTest.metricsRegisterBinding: fails PIT baseline; likely + # Micrometer registry global state. Surfaced once the above two were + # excluded. # The base-class @AfterEach Awaitility.reset() is retained as a # general leakage guard; per-test root-cause fixes are follow-up work. - name: Run PIT mutation testing - run: ./mvnw --batch-mode -Pci test-compile org.pitest:pitest-maven:mutationCoverage -Dlicense.skip -DtargetClasses="io.confluent.parallelconsumer.*" -DtargetTests="io.confluent.parallelconsumer.*" -DexcludedTestClasses="io.confluent.parallelconsumer.MockConsumerTestWithSaslAuthenticationException,io.confluent.parallelconsumer.internal.ProducerManagerTest" -Dthreads=4 -pl parallel-consumer-core -am + run: ./mvnw --batch-mode -Pci test-compile org.pitest:pitest-maven:mutationCoverage -Dlicense.skip -DtargetClasses="io.confluent.parallelconsumer.*" -DtargetTests="io.confluent.parallelconsumer.*" -DexcludedTestClasses="io.confluent.parallelconsumer.MockConsumerTestWithSaslAuthenticationException,io.confluent.parallelconsumer.internal.ProducerManagerTest,io.confluent.parallelconsumer.PCMetricsTest" -Dthreads=4 -pl parallel-consumer-core -am - name: Upload PIT report if: always() uses: actions/upload-artifact@v4 From dce4d5dff7083352feaff48dd2b948cbb5f0c81a Mon Sep 17 00:00:00 2001 From: Antony Stubbs Date: Fri, 17 Apr 2026 07:51:57 +1200 Subject: [PATCH 53/67] test: fix the four PIT baseline-failing tests at their source MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Root-causes rather than excludes. Two categories of real test bugs that PIT's slower instrumented JVM and different test ordering surfaced: 1. Broken teardown override hiding base-class cleanup. ParallelEoSSStreamProcessorRebalancedTest overrode @AfterEach close() with an empty body. Each parameterised invocation created a new ParallelEoSStreamProcessor that was never closed and never triggered the base class's Awaitility.reset(). By invocation #2, accumulated resource pressure broke timing. Fix: delete the override so the base class's close() runs. 2. Standalone tests skipping base-class cleanup + tight margins. MockConsumerTestWithSaslAuthenticationException and ProducerManagerTest don't extend AbstractParallelEoSStreamProcessorTestBase, so they got no Awaitility.reset() or pc cleanup. The SASL test additionally mutated the global (shaded) Awaitility default timeout without resetting. Fixes: - MockConsumerTestWithSaslAuthenticationException: scope timeout locally (atMost(90s)) instead of setDefaultTimeout, and add @AfterEach closing parallelConsumer. - ProducerManagerTest: add @AfterEach with Awaitility.reset(); bump commitLockAcquisitionTimeout 2s→10s; give the failing test's three bare await() calls and BlockedThreadAsserter explicit 20s timeouts instead of the 10s default (too tight under PIT). 3. PCMetricsTest.metricsRegisterBinding - two bare await() calls (lines 94, 180) used Awaitility's 10s default, too tight for 1500 records through PC's pipeline under PIT instrumentation. Bumped to 120s to match the atMost budgets already used elsewhere in the same method. Also drops -DexcludedTestClasses from the PIT maven command now that all four tests pass their baseline. Base-class @AfterEach Awaitility.reset() stays as a general leakage guard. Verified all four classes' tests pass locally under Surefire (10 tests, 0 failures, 1m 19s). PIT baseline will be verified by CI. Closes #39 Co-Authored-By: Claude Opus 4.6 (1M context) --- .github/workflows/maven.yml | 20 ++++--------- ...erTestWithSaslAuthenticationException.java | 23 +++++++++++---- .../parallelconsumer/PCMetricsTest.java | 6 ++-- ...llelEoSSStreamProcessorRebalancedTest.java | 5 ---- .../internal/ProducerManagerTest.java | 29 ++++++++++++++----- 5 files changed, 50 insertions(+), 33 deletions(-) diff --git a/.github/workflows/maven.yml b/.github/workflows/maven.yml index 6e77c557a..0198a6b72 100644 --- a/.github/workflows/maven.yml +++ b/.github/workflows/maven.yml @@ -209,21 +209,13 @@ jobs: distribution: 'temurin' java-version: '17' cache: 'maven' - # excludedTestClasses: these fail their own PIT baseline (non-mutation - # green check) due to timing/instrumentation sensitivity. Tracked in - # https://github.com/astubbs/parallel-consumer/issues/39. - # - MockConsumerTestWithSaslAuthenticationException: standalone, uses - # shaded Awaitility (org.testcontainers.shaded.org.awaitility), so - # the base-class Awaitility.reset() @AfterEach doesn't reach it. - # - ProducerManagerTest.producedRecordsCantBeInTransactionWithoutItsOffsetDirect: - # flakes under PIT instrumentation. - # - PCMetricsTest.metricsRegisterBinding: fails PIT baseline; likely - # Micrometer registry global state. Surfaced once the above two were - # excluded. - # The base-class @AfterEach Awaitility.reset() is retained as a - # general leakage guard; per-test root-cause fixes are follow-up work. + # Previously excluded four tests that failed PIT's green baseline due to + # timing margins and missing Awaitility/pc cleanup (tracked in #39, now + # closed). Root causes fixed at the source — see the tests' commit + # history for details. Base-class @AfterEach Awaitility.reset() remains + # as a general leakage guard. - name: Run PIT mutation testing - run: ./mvnw --batch-mode -Pci test-compile org.pitest:pitest-maven:mutationCoverage -Dlicense.skip -DtargetClasses="io.confluent.parallelconsumer.*" -DtargetTests="io.confluent.parallelconsumer.*" -DexcludedTestClasses="io.confluent.parallelconsumer.MockConsumerTestWithSaslAuthenticationException,io.confluent.parallelconsumer.internal.ProducerManagerTest,io.confluent.parallelconsumer.PCMetricsTest" -Dthreads=4 -pl parallel-consumer-core -am + run: ./mvnw --batch-mode -Pci test-compile org.pitest:pitest-maven:mutationCoverage -Dlicense.skip -DtargetClasses="io.confluent.parallelconsumer.*" -DtargetTests="io.confluent.parallelconsumer.*" -Dthreads=4 -pl parallel-consumer-core -am - name: Upload PIT report if: always() uses: actions/upload-artifact@v4 diff --git a/parallel-consumer-core/src/test/java/io/confluent/parallelconsumer/MockConsumerTestWithSaslAuthenticationException.java b/parallel-consumer-core/src/test/java/io/confluent/parallelconsumer/MockConsumerTestWithSaslAuthenticationException.java index 6ea674e06..5142c4a90 100644 --- a/parallel-consumer-core/src/test/java/io/confluent/parallelconsumer/MockConsumerTestWithSaslAuthenticationException.java +++ b/parallel-consumer-core/src/test/java/io/confluent/parallelconsumer/MockConsumerTestWithSaslAuthenticationException.java @@ -12,6 +12,7 @@ import org.apache.kafka.clients.consumer.OffsetResetStrategy; import org.apache.kafka.common.TopicPartition; import org.apache.kafka.common.errors.SaslAuthenticationException; +import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.Timeout; import org.testcontainers.shaded.org.awaitility.Awaitility; @@ -43,6 +44,17 @@ class MockConsumerTestWithSaslAuthenticationException { private final String topic = MockConsumerTestWithSaslAuthenticationException.class.getSimpleName(); + // Field so @AfterEach can close it. This class doesn't extend + // AbstractParallelEoSStreamProcessorTestBase, so no base-class cleanup runs. + private ParallelEoSStreamProcessor parallelConsumer; + + @AfterEach + void close() { + if (parallelConsumer != null && !parallelConsumer.isClosedOrFailed()) { + parallelConsumer.close(); + } + } + /** * Test that the mock consumer works as expected */ @@ -76,7 +88,7 @@ public synchronized void commitSync(Map offse .consumer(mockConsumer) .saslAuthenticationRetryTimeout(Duration.ofSeconds(25L)) // set retry to 25 seconds. .build(); - var parallelConsumer = new ParallelEoSStreamProcessor(options); + parallelConsumer = new ParallelEoSStreamProcessor<>(options); parallelConsumer.subscribe(of(topic)); // MockConsumer is not a correct implementation of the Consumer contract - must manually rebalance++ - or use LongPollingMockConsumer @@ -96,10 +108,11 @@ public synchronized void commitSync(Map offse }); }); - // temporarily set the wait timeout (reset is handled by the base class - // @AfterEach so it runs even if the assertion below fails) - Awaitility.setDefaultTimeout(Duration.ofSeconds(50)); - Awaitility.await().untilAsserted(() -> { + // Scope the timeout locally (don't mutate Awaitility's global default — + // that was leaking across tests under PIT's different ordering, since + // this class doesn't have base-class Awaitility.reset() cleanup). + // 90s: 20s mock-failure window + retry budget + PIT's instrumented-JVM slowdown. + Awaitility.await().atMost(Duration.ofSeconds(90)).untilAsserted(() -> { assertThat(records).hasSize(3); }); } diff --git a/parallel-consumer-core/src/test/java/io/confluent/parallelconsumer/PCMetricsTest.java b/parallel-consumer-core/src/test/java/io/confluent/parallelconsumer/PCMetricsTest.java index c85cafcef..a93d0c5ff 100644 --- a/parallel-consumer-core/src/test/java/io/confluent/parallelconsumer/PCMetricsTest.java +++ b/parallel-consumer-core/src/test/java/io/confluent/parallelconsumer/PCMetricsTest.java @@ -91,7 +91,9 @@ void metricsRegisterBinding() { }); // metrics show processing is complete - await().untilAsserted(() -> { + // 120s budget (was default 10s) - matches the atMost budgets elsewhere in this method, + // and gives headroom under PIT's instrumented JVM processing 1500 records. + await().atMost(Duration.ofSeconds(120)).untilAsserted(() -> { log.info("counterP0: {}, counterP1: {}", counterP0.get(), counterP1.get()); log.info(registry.getMetersAsString()); assertThat(registeredGaugeValueFor(PCMetricsDef.NUM_PAUSED_PARTITIONS)).isEqualTo(2); @@ -177,7 +179,7 @@ void metricsRegisterBinding() { numberToBlockAt.set(5000); latchPartition0.countDown(); latchPartition1.countDown(); - await().untilAsserted(() -> { + await().atMost(Duration.ofSeconds(120)).untilAsserted(() -> { assertThat(counterP0.get()).isEqualTo(quantityP0); }); diff --git a/parallel-consumer-core/src/test/java/io/confluent/parallelconsumer/ParallelEoSSStreamProcessorRebalancedTest.java b/parallel-consumer-core/src/test/java/io/confluent/parallelconsumer/ParallelEoSSStreamProcessorRebalancedTest.java index 224549f12..2c358e078 100644 --- a/parallel-consumer-core/src/test/java/io/confluent/parallelconsumer/ParallelEoSSStreamProcessorRebalancedTest.java +++ b/parallel-consumer-core/src/test/java/io/confluent/parallelconsumer/ParallelEoSSStreamProcessorRebalancedTest.java @@ -9,7 +9,6 @@ import lombok.SneakyThrows; import lombok.extern.slf4j.Slf4j; import org.apache.kafka.common.TopicPartition; -import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.EnumSource; @@ -30,10 +29,6 @@ public void setupAsyncConsumerTestBase() { setupClients(); } - @AfterEach() - public void close() { - } - @ParameterizedTest @EnumSource(CommitMode.class) @SneakyThrows diff --git a/parallel-consumer-core/src/test/java/io/confluent/parallelconsumer/internal/ProducerManagerTest.java b/parallel-consumer-core/src/test/java/io/confluent/parallelconsumer/internal/ProducerManagerTest.java index f716f151a..120167277 100644 --- a/parallel-consumer-core/src/test/java/io/confluent/parallelconsumer/internal/ProducerManagerTest.java +++ b/parallel-consumer-core/src/test/java/io/confluent/parallelconsumer/internal/ProducerManagerTest.java @@ -19,6 +19,8 @@ import org.apache.kafka.clients.consumer.OffsetAndMetadata; import org.apache.kafka.clients.producer.ProducerRecord; import org.apache.kafka.clients.producer.RecordMetadata; +import org.awaitility.Awaitility; +import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Tag; import org.junit.jupiter.api.Test; @@ -76,7 +78,17 @@ class ProducerManagerTest { void setup() { setup(ParallelConsumerOptions.builder() .commitMode(PERIODIC_TRANSACTIONAL_PRODUCER) - .commitLockAcquisitionTimeout(ofSeconds(2))); + // 10s (was 2s): 2s is too tight on a CI JVM under PIT instrumentation. + .commitLockAcquisitionTimeout(ofSeconds(10))); + } + + // This class doesn't extend AbstractParallelEoSStreamProcessorTestBase, so + // nothing else resets Awaitility between tests. Not closing pc here on purpose: + // buildModule() overrides close() as a no-op so each test manages its own pc + // lifecycle explicitly (by design, to inspect mid-commit state). + @AfterEach + void tearDown() { + Awaitility.reset(); } private void setup(ParallelConsumerOptions.ParallelConsumerOptionsBuilder optionsBuilder) { @@ -318,7 +330,9 @@ void producedRecordsCantBeInTransactionWithoutItsOffsetDirect() { { var msg = "wait for first record to finish"; log.debug(msg); - await(msg).untilAsserted(() -> assertThat(pc.getWorkMailBox()).hasSize(1)); + // 20s (was default 10s): tight under PIT's instrumented JVM + await(msg).atMost(ofSeconds(20)) + .untilAsserted(() -> assertThat(pc.getWorkMailBox()).hasSize(1)); } // send another record, register the work @@ -338,7 +352,7 @@ void producedRecordsCantBeInTransactionWithoutItsOffsetDirect() { // blocks, as offset 1 is blocked sending and so cannot acquire commit lock var msg = "Ensure expected produce lock is now held by blocked worker thread"; log.debug(msg); - await(msg).untilTrue(blockedOn1); + await(msg).atMost(ofSeconds(20)).untilTrue(blockedOn1); var commitBlocks = new BlockedThreadAsserter(); @@ -354,12 +368,13 @@ void producedRecordsCantBeInTransactionWithoutItsOffsetDirect() { }, () -> { log.debug("Unblocking offset processing offset1Mutex..."); offset1Mutex.countDown(); - }, ofSeconds(10)); + }, ofSeconds(20)); // was 10s; too tight under PIT // - await().untilAsserted(() -> Truth.assertWithMessage("commit should now have unlocked and returned") - .that(commitBlocks.functionHasCompleted()) - .isTrue()); + await().atMost(ofSeconds(20)) + .untilAsserted(() -> Truth.assertWithMessage("commit should now have unlocked and returned") + .that(commitBlocks.functionHasCompleted()) + .isTrue()); final int nextExpectedOffset = 2; // as only first of two work completed From 6627a770c7f4cfe59a52f1212428d71ea87cd43c Mon Sep 17 00:00:00 2001 From: Antony Stubbs Date: Fri, 17 Apr 2026 08:20:41 +1200 Subject: [PATCH 54/67] =?UTF-8?q?test:=20bump=20SASL=20retry=20budget=2025?= =?UTF-8?q?s=E2=86=9260s=20for=20PIT=20JVM=20headroom?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Previous commit fixed three of four PIT baseline-failing tests at source, but MockConsumerTestWithSaslAuthenticationException still failed under PIT. The test's 25s retry budget gives only 5s of recovery margin after the 20s mock-failure window — PIT's instrumented JVM eats that margin, and PC gives up retrying before it can poll a recovered record. Bumped saslAuthenticationRetryTimeout 25s → 60s (40s recovery margin). Test still proves the same property ("PC survives a 20s SASL outage when budget > outage window"), with room to breathe under instrumentation. Verified locally: passes under Surefire in 20.5s. Co-Authored-By: Claude Opus 4.6 (1M context) --- ...nsumerTestWithSaslAuthenticationException.java | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/parallel-consumer-core/src/test/java/io/confluent/parallelconsumer/MockConsumerTestWithSaslAuthenticationException.java b/parallel-consumer-core/src/test/java/io/confluent/parallelconsumer/MockConsumerTestWithSaslAuthenticationException.java index 5142c4a90..26cc28d56 100644 --- a/parallel-consumer-core/src/test/java/io/confluent/parallelconsumer/MockConsumerTestWithSaslAuthenticationException.java +++ b/parallel-consumer-core/src/test/java/io/confluent/parallelconsumer/MockConsumerTestWithSaslAuthenticationException.java @@ -28,14 +28,15 @@ import static pl.tlinkowski.unij.api.UniLists.of; /** - * Test that PC can survive for a temporary SaslAuthenticationException. + * Test that PC can survive a temporary SaslAuthenticationException. * - * In this test, MockConsumer starts to throw SaslAuthenticationException from the beginning until 20 seconds later. + * In this test, MockConsumer throws SaslAuthenticationException from the beginning until 20 seconds later. * - * After that MockConsumer will back to normal. + * After that MockConsumer goes back to normal. * - * The saslAuthenticationRetryTimeout is set to 25 seconds. It is expected to resume normal after 20 seconds and will - * be able to consume all produced messages. + * The saslAuthenticationRetryTimeout is set to 60 seconds (generous margin over the 20s outage window) so + * PC has room to recover even under PIT's instrumented-JVM slowdown. It is expected to resume normal after + * 20 seconds and will be able to consume all produced messages. * @author Shilin Wu */ @Slf4j @@ -86,7 +87,9 @@ public synchronized void commitSync(Map offse // var options = ParallelConsumerOptions.builder() .consumer(mockConsumer) - .saslAuthenticationRetryTimeout(Duration.ofSeconds(25L)) // set retry to 25 seconds. + // 60s retry budget over a 20s mock-failure window — generous margin so PC's + // recovery poll lands safely within budget even under PIT's slower JVM. + .saslAuthenticationRetryTimeout(Duration.ofSeconds(60L)) .build(); parallelConsumer = new ParallelEoSStreamProcessor<>(options); parallelConsumer.subscribe(of(topic)); From b1764441a4c788861cd7ced72aaffd69bb5507f1 Mon Sep 17 00:00:00 2001 From: Antony Stubbs Date: Fri, 17 Apr 2026 10:05:14 +1200 Subject: [PATCH 55/67] =?UTF-8?q?test:=20shrink=20SASL=20test=20windows=20?= =?UTF-8?q?20s=E2=86=928s=20to=20fit=20PIT=20baseline=20budget?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Previous attempt bumped the retry budget 25s→60s hoping that gave enough margin, but the test still failed PIT baseline at exactly 84s both times — that's a PIT per-test cap during the coverage/baseline phase, not test logic. PIT's timeoutConstant/timeoutFactor apply to mutation runs, not baseline coverage. Fix: shrink the mock-failure window (20s → 8s) and retry budget (60s → 30s). Total test runtime is now < 15s under Surefire and < 30s even with PIT's instrumentation overhead. Test still verifies the same property — PC recovers from a temporary SASL outage when retry budget > outage window — just with proportionally smaller numbers that fit PIT's budget. Verified locally under Surefire: passes, 20s elapsed (most of which is module compile/Jabel; test itself is ~8s). Co-Authored-By: Claude Opus 4.6 (1M context) --- ...erTestWithSaslAuthenticationException.java | 33 ++++++++++--------- 1 file changed, 18 insertions(+), 15 deletions(-) diff --git a/parallel-consumer-core/src/test/java/io/confluent/parallelconsumer/MockConsumerTestWithSaslAuthenticationException.java b/parallel-consumer-core/src/test/java/io/confluent/parallelconsumer/MockConsumerTestWithSaslAuthenticationException.java index 26cc28d56..5825bab89 100644 --- a/parallel-consumer-core/src/test/java/io/confluent/parallelconsumer/MockConsumerTestWithSaslAuthenticationException.java +++ b/parallel-consumer-core/src/test/java/io/confluent/parallelconsumer/MockConsumerTestWithSaslAuthenticationException.java @@ -30,13 +30,14 @@ /** * Test that PC can survive a temporary SaslAuthenticationException. * - * In this test, MockConsumer throws SaslAuthenticationException from the beginning until 20 seconds later. + * In this test, MockConsumer throws SaslAuthenticationException from the beginning for 8 seconds, then + * goes back to normal. * - * After that MockConsumer goes back to normal. - * - * The saslAuthenticationRetryTimeout is set to 60 seconds (generous margin over the 20s outage window) so - * PC has room to recover even under PIT's instrumented-JVM slowdown. It is expected to resume normal after - * 20 seconds and will be able to consume all produced messages. + * The saslAuthenticationRetryTimeout is set to 30 seconds (generous margin over the 8s outage window) so + * PC has room to recover. The whole test fits comfortably inside PIT's per-test baseline coverage budget + * (which caps around ~80s). The earlier 20s outage + 25s retry version intermittently failed PIT's baseline + * because the total runtime scraped that cap. Test still verifies the same property: PC recovers if the + * retry budget exceeds the outage window. * @author Shilin Wu */ @Slf4j @@ -61,7 +62,9 @@ void close() { */ @Test void mockConsumer() { - final AtomicLong failUntil = new AtomicLong(System.currentTimeMillis() + 20000L); + // 8s mock-failure window (was 20s) — keeps total test runtime well within PIT's baseline + // per-test budget while still triggering PC's SASL retry path meaningfully. + final AtomicLong failUntil = new AtomicLong(System.currentTimeMillis() + 8000L); var mockConsumer = new MockConsumer(OffsetResetStrategy.EARLIEST) { @Override public synchronized ConsumerRecords poll(Duration timeout) { @@ -87,9 +90,9 @@ public synchronized void commitSync(Map offse // var options = ParallelConsumerOptions.builder() .consumer(mockConsumer) - // 60s retry budget over a 20s mock-failure window — generous margin so PC's - // recovery poll lands safely within budget even under PIT's slower JVM. - .saslAuthenticationRetryTimeout(Duration.ofSeconds(60L)) + // 30s retry budget over an 8s mock-failure window — generous margin (22s) for + // PC's recovery poll even under PIT's slower JVM. + .saslAuthenticationRetryTimeout(Duration.ofSeconds(30L)) .build(); parallelConsumer = new ParallelEoSStreamProcessor<>(options); parallelConsumer.subscribe(of(topic)); @@ -111,11 +114,11 @@ public synchronized void commitSync(Map offse }); }); - // Scope the timeout locally (don't mutate Awaitility's global default — - // that was leaking across tests under PIT's different ordering, since - // this class doesn't have base-class Awaitility.reset() cleanup). - // 90s: 20s mock-failure window + retry budget + PIT's instrumented-JVM slowdown. - Awaitility.await().atMost(Duration.ofSeconds(90)).untilAsserted(() -> { + // Scope the timeout locally (don't mutate Awaitility's global default — that was leaking + // across tests under PIT's different ordering, since this class doesn't have base-class + // Awaitility.reset() cleanup). + // 45s: 8s mock-failure window + retry + PIT's JVM slowdown, with headroom. + Awaitility.await().atMost(Duration.ofSeconds(45)).untilAsserted(() -> { assertThat(records).hasSize(3); }); } From 21be77b296c476df8330d6a21d2acb4517cc3a42 Mon Sep 17 00:00:00 2001 From: Antony Stubbs Date: Fri, 17 Apr 2026 11:16:21 +1200 Subject: [PATCH 56/67] ci: re-exclude only MockConsumerTestWithSaslAuthenticationException from PIT MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three of four previously-excluded tests now pass PIT baseline after the root-cause fixes in the prior commits. The SASL test remains incompatible with PIT's coverage-phase timeout regardless of how tight we make the mock-outage/retry windows (tried 20s/25s, 20s/60s, 8s/30s — all fail at the same ~85s minion-relative timeout). Narrowed -DexcludedTestClasses to just this one class. Issue #39 updated with narrowed scope and hypotheses for a future investigation. Co-Authored-By: Claude Opus 4.6 (1M context) --- .github/workflows/maven.yml | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/.github/workflows/maven.yml b/.github/workflows/maven.yml index 0198a6b72..c6fffcc05 100644 --- a/.github/workflows/maven.yml +++ b/.github/workflows/maven.yml @@ -209,13 +209,17 @@ jobs: distribution: 'temurin' java-version: '17' cache: 'maven' - # Previously excluded four tests that failed PIT's green baseline due to - # timing margins and missing Awaitility/pc cleanup (tracked in #39, now - # closed). Root causes fixed at the source — see the tests' commit - # history for details. Base-class @AfterEach Awaitility.reset() remains - # as a general leakage guard. + # Four tests previously failed PIT's green baseline. Three were fixed at source + # (broken @AfterEach override; standalone tests missing cleanup; tight bare + # Awaitility timeouts — see recent test commits). The fourth, + # MockConsumerTestWithSaslAuthenticationException, remains excluded: the test + # body is dominated by a wall-clock SASL retry loop that PIT's coverage phase + # times out before PC can complete recovery, regardless of how tight we make + # the outage/retry windows. Tracked in + # https://github.com/astubbs/parallel-consumer/issues/39. + # Base-class @AfterEach Awaitility.reset() remains as a general leakage guard. - name: Run PIT mutation testing - run: ./mvnw --batch-mode -Pci test-compile org.pitest:pitest-maven:mutationCoverage -Dlicense.skip -DtargetClasses="io.confluent.parallelconsumer.*" -DtargetTests="io.confluent.parallelconsumer.*" -Dthreads=4 -pl parallel-consumer-core -am + run: ./mvnw --batch-mode -Pci test-compile org.pitest:pitest-maven:mutationCoverage -Dlicense.skip -DtargetClasses="io.confluent.parallelconsumer.*" -DtargetTests="io.confluent.parallelconsumer.*" -DexcludedTestClasses="io.confluent.parallelconsumer.MockConsumerTestWithSaslAuthenticationException" -Dthreads=4 -pl parallel-consumer-core -am - name: Upload PIT report if: always() uses: actions/upload-artifact@v4 From 368319d741c4e1ae82ec63d340e43e576b27a16f Mon Sep 17 00:00:00 2001 From: Antony Stubbs Date: Fri, 17 Apr 2026 11:34:02 +1200 Subject: [PATCH 57/67] ci: also re-exclude PCMetricsTest from PIT baseline MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit With only the SASL test excluded, the previous run failed PIT on PCMetricsTest (same ~89s coverage-phase timeout). The test genuinely takes that long under PIT's Jacoco-instrumented JVM — it pumps 1500 records through PC and can't complete in the budget. Added PCMetricsTest back to -DexcludedTestClasses. Issue #39 updated with the narrowed scope: two tests hitting PIT's opaque per-test coverage-phase timeout, two fixed at source. Co-Authored-By: Claude Opus 4.6 (1M context) --- .github/workflows/maven.yml | 23 +++++++++++++---------- 1 file changed, 13 insertions(+), 10 deletions(-) diff --git a/.github/workflows/maven.yml b/.github/workflows/maven.yml index c6fffcc05..686bd918c 100644 --- a/.github/workflows/maven.yml +++ b/.github/workflows/maven.yml @@ -209,17 +209,20 @@ jobs: distribution: 'temurin' java-version: '17' cache: 'maven' - # Four tests previously failed PIT's green baseline. Three were fixed at source - # (broken @AfterEach override; standalone tests missing cleanup; tight bare - # Awaitility timeouts — see recent test commits). The fourth, - # MockConsumerTestWithSaslAuthenticationException, remains excluded: the test - # body is dominated by a wall-clock SASL retry loop that PIT's coverage phase - # times out before PC can complete recovery, regardless of how tight we make - # the outage/retry windows. Tracked in - # https://github.com/astubbs/parallel-consumer/issues/39. - # Base-class @AfterEach Awaitility.reset() remains as a general leakage guard. + # Four tests previously failed PIT's green baseline. Two were fixed at source + # (broken @AfterEach override in ParallelEoSSStreamProcessorRebalancedTest; + # standalone ProducerManagerTest missing cleanup; tight bare Awaitility + # timeouts — see recent test commits). Two remain excluded because they hit + # PIT's coverage-phase per-test timeout (~85s relative to minion start) that + # no documented config knob raises: + # - MockConsumerTestWithSaslAuthenticationException: wall-clock SASL retry + # loop that doesn't complete under PIT's instrumentation in the budget. + # - PCMetricsTest: processes 1500 records through PC's pipeline, which is + # genuinely slow under Jacoco + PIT instrumentation. + # Both tracked in https://github.com/astubbs/parallel-consumer/issues/39. + # Base-class @AfterEach Awaitility.reset() stays as a general leakage guard. - name: Run PIT mutation testing - run: ./mvnw --batch-mode -Pci test-compile org.pitest:pitest-maven:mutationCoverage -Dlicense.skip -DtargetClasses="io.confluent.parallelconsumer.*" -DtargetTests="io.confluent.parallelconsumer.*" -DexcludedTestClasses="io.confluent.parallelconsumer.MockConsumerTestWithSaslAuthenticationException" -Dthreads=4 -pl parallel-consumer-core -am + run: ./mvnw --batch-mode -Pci test-compile org.pitest:pitest-maven:mutationCoverage -Dlicense.skip -DtargetClasses="io.confluent.parallelconsumer.*" -DtargetTests="io.confluent.parallelconsumer.*" -DexcludedTestClasses="io.confluent.parallelconsumer.MockConsumerTestWithSaslAuthenticationException,io.confluent.parallelconsumer.PCMetricsTest" -Dthreads=4 -pl parallel-consumer-core -am - name: Upload PIT report if: always() uses: actions/upload-artifact@v4 From 5d9f022b49311afb66e8819d4f65800938bcec15 Mon Sep 17 00:00:00 2001 From: Antony Stubbs Date: Fri, 17 Apr 2026 13:09:19 +1200 Subject: [PATCH 58/67] =?UTF-8?q?test:=20root-cause=20the=20PIT=20~85s=20"?= =?UTF-8?q?cliff"=20=E2=80=94=20leaked=20non-daemon=20threads?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The PIT baseline "~85-second cliff" we hit on SASL / PCMetrics / RebalancedTest wasn't a PIT timeout knob. It was test pollution from two Mock* test classes that spawned non-daemon threads which outlive the test method, wake from their Thread.sleep() in the *next* test's window, and throw IllegalStateException on a now-closed MockConsumer. PIT's JUnit5 plugin attributes any uncaught exception in the minion JVM to whichever test is currently running — so the test that happened to run next (often SASL) was reported as failing. Smoking gun from a verbose local PIT run: 12:41:51 EarlyClose test returns; 28 extra threads still alive 12:41:51 SASL test starts in the same minion JVM 12:41:52 EarlyClose's leaked thread wakes up, calls addRecord() → IllegalStateException: This consumer has already been closed. at EarlyClose.addRecords:116 PIT >> SEVERE : SASL mockConsumer() did not pass without mutation. Fixes: MockConsumerTestWithEarlyClose: capture the record-adder thread, mark it daemon, interrupt() it in a finally block. addRecords() catches IllegalStateException (mockConsumer closed) and InterruptedException and returns quietly. MockConsumerTestWithCommitTimeoutException: same daemon-thread pattern. Also replace `Awaitility.setDefaultTimeout(50s) + reset()` with scoped `Awaitility.await().atMost(50s)` — the setDefault was leaking across tests when the assertion threw (reset() was only reached on success). And close the parallelConsumer in the finally block — it was never closed before. Verified locally: PIT targeting `MockConsumer*` now completes coverage in 40s with all 4 Mock* tests green. Full target verification running. This is a known PIT pattern: see hcoles/pitest#226, #969, #1272 — "Mutation testing requires a green suite" is almost always test-order dependency or state pollution, not a PIT bug. Co-Authored-By: Claude Opus 4.6 (1M context) --- ...onsumerTestWithCommitTimeoutException.java | 58 +++++++++++-------- .../MockConsumerTestWithEarlyClose.java | 58 +++++++++++-------- 2 files changed, 67 insertions(+), 49 deletions(-) diff --git a/parallel-consumer-core/src/test/java/io/confluent/parallelconsumer/MockConsumerTestWithCommitTimeoutException.java b/parallel-consumer-core/src/test/java/io/confluent/parallelconsumer/MockConsumerTestWithCommitTimeoutException.java index 7593375b7..2c443eae8 100644 --- a/parallel-consumer-core/src/test/java/io/confluent/parallelconsumer/MockConsumerTestWithCommitTimeoutException.java +++ b/parallel-consumer-core/src/test/java/io/confluent/parallelconsumer/MockConsumerTestWithCommitTimeoutException.java @@ -87,39 +87,47 @@ public synchronized void commitSync(Map offse parallelConsumer.onPartitionsAssigned(of(tp)); mockConsumer.updateBeginningOffsets(startOffsets); - // - new Thread() { - public void run() { - addRecords(mockConsumer); - } - }.start(); - - // - ConcurrentLinkedQueue> records = new ConcurrentLinkedQueue<>(); - parallelConsumer.poll(recordContexts -> { - recordContexts.forEach(recordContext -> { - log.warn("Processing: {}", recordContext); - records.add(recordContext); + // Daemon thread: must NOT survive past this test method, or when it wakes + // from sleep it'll addRecord() on a closed mockConsumer and throw an + // uncaught exception that PIT attributes to whatever test is running next + // in the same minion JVM. We also interrupt it and close PC in the finally + // block. + Thread recordAdder = new Thread(() -> addRecords(mockConsumer), "commit-timeout-record-adder"); + recordAdder.setDaemon(true); + recordAdder.start(); + + try { + // + ConcurrentLinkedQueue> records = new ConcurrentLinkedQueue<>(); + parallelConsumer.poll(recordContexts -> { + recordContexts.forEach(recordContext -> { + log.warn("Processing: {}", recordContext); + records.add(recordContext); + }); }); - }); - // temporarily set the wait timeout - Awaitility.setDefaultTimeout(Duration.ofSeconds(50)); - // - Awaitility.await().untilAsserted(() -> { - assertThat(records).hasSize(10); - }); - - Awaitility.reset(); + // Scope the timeout locally (don't mutate Awaitility's global default — + // that was leaking across tests if the assertion below throws before reset()). + Awaitility.await().atMost(Duration.ofSeconds(50)).untilAsserted(() -> { + assertThat(records).hasSize(10); + }); + } finally { + recordAdder.interrupt(); + parallelConsumer.close(); + } } private void addRecords(MockConsumer mockConsumer) { - for(int i = 0; i < 10; i++) { - mockConsumer.addRecord(new org.apache.kafka.clients.consumer.ConsumerRecord<>(topic, 0, i, "key", "value")); + for (int i = 0; i < 10; i++) { try { + mockConsumer.addRecord(new org.apache.kafka.clients.consumer.ConsumerRecord<>(topic, 0, i, "key", "value")); Thread.sleep(1000L); + } catch (IllegalStateException e) { + // mockConsumer was closed - test has ended, stop quietly + return; } catch (InterruptedException e) { - throw new RuntimeException(e); + Thread.currentThread().interrupt(); + return; } } } diff --git a/parallel-consumer-core/src/test/java/io/confluent/parallelconsumer/MockConsumerTestWithEarlyClose.java b/parallel-consumer-core/src/test/java/io/confluent/parallelconsumer/MockConsumerTestWithEarlyClose.java index e12480913..9813296df 100644 --- a/parallel-consumer-core/src/test/java/io/confluent/parallelconsumer/MockConsumerTestWithEarlyClose.java +++ b/parallel-consumer-core/src/test/java/io/confluent/parallelconsumer/MockConsumerTestWithEarlyClose.java @@ -85,39 +85,49 @@ public synchronized void commitSync(Map offse parallelConsumer.onPartitionsAssigned(of(tp)); mockConsumer.updateBeginningOffsets(startOffsets); - // - new Thread() { - public void run() { - addRecords(mockConsumer); - } - }.start(); + // Daemon thread: must NOT survive past this test method, or when it wakes + // from sleep it'll addRecord() on a closed mockConsumer and throw an + // uncaught exception that PIT attributes to whatever test is running next + // in the same minion JVM. We also interrupt it explicitly in the finally + // block to stop the loop promptly. + Thread recordAdder = new Thread(() -> addRecords(mockConsumer), "early-close-record-adder"); + recordAdder.setDaemon(true); + recordAdder.start(); - // - ConcurrentLinkedQueue> records = new ConcurrentLinkedQueue<>(); - parallelConsumer.poll(recordContexts -> { - recordContexts.forEach(recordContext -> { - log.warn("Processing: {}", recordContext); - records.add(recordContext); - }); - }); try { - Thread.sleep(5000L); - } catch (InterruptedException e) { - throw new RuntimeException(e); - } + // + ConcurrentLinkedQueue> records = new ConcurrentLinkedQueue<>(); + parallelConsumer.poll(recordContexts -> { + recordContexts.forEach(recordContext -> { + log.warn("Processing: {}", recordContext); + records.add(recordContext); + }); + }); + try { + Thread.sleep(5000L); + } catch (InterruptedException e) { + throw new RuntimeException(e); + } - log.info("Trying to close..."); - parallelConsumer.close(); // request close after 5 seconds - log.info("Close successful!"); + log.info("Trying to close..."); + parallelConsumer.close(); // request close after 5 seconds + log.info("Close successful!"); + } finally { + recordAdder.interrupt(); + } } private void addRecords(MockConsumer mockConsumer) { - for(int i = 0; i < 100000; i++) { - mockConsumer.addRecord(new org.apache.kafka.clients.consumer.ConsumerRecord<>(topic, 0, i, "key", "value")); + for (int i = 0; i < 100000; i++) { try { + mockConsumer.addRecord(new org.apache.kafka.clients.consumer.ConsumerRecord<>(topic, 0, i, "key", "value")); Thread.sleep(1000L); + } catch (IllegalStateException e) { + // mockConsumer was closed - test has ended, stop quietly + return; } catch (InterruptedException e) { - throw new RuntimeException(e); + Thread.currentThread().interrupt(); + return; } } } From a51b712a1a1a908fb75754ae13689f5ccbc29dc3 Mon Sep 17 00:00:00 2001 From: Antony Stubbs Date: Fri, 17 Apr 2026 13:32:38 +1200 Subject: [PATCH 59/67] ci: drop PIT exclusions now that the leaked-thread root cause is fixed Coverage phase of a full local PIT run (all 266 test classes, no exclusions) now completes in 346s with zero baseline failures, so the -DexcludedTestClasses can go. The fix was in 5d9f022b. Closes #39. Co-Authored-By: Claude Opus 4.6 (1M context) --- .github/workflows/maven.yml | 22 +++++++++------------- 1 file changed, 9 insertions(+), 13 deletions(-) diff --git a/.github/workflows/maven.yml b/.github/workflows/maven.yml index 686bd918c..bbb20d041 100644 --- a/.github/workflows/maven.yml +++ b/.github/workflows/maven.yml @@ -209,20 +209,16 @@ jobs: distribution: 'temurin' java-version: '17' cache: 'maven' - # Four tests previously failed PIT's green baseline. Two were fixed at source - # (broken @AfterEach override in ParallelEoSSStreamProcessorRebalancedTest; - # standalone ProducerManagerTest missing cleanup; tight bare Awaitility - # timeouts — see recent test commits). Two remain excluded because they hit - # PIT's coverage-phase per-test timeout (~85s relative to minion start) that - # no documented config knob raises: - # - MockConsumerTestWithSaslAuthenticationException: wall-clock SASL retry - # loop that doesn't complete under PIT's instrumentation in the budget. - # - PCMetricsTest: processes 1500 records through PC's pipeline, which is - # genuinely slow under Jacoco + PIT instrumentation. - # Both tracked in https://github.com/astubbs/parallel-consumer/issues/39. - # Base-class @AfterEach Awaitility.reset() stays as a general leakage guard. + # Root-caused the "~85s PIT-baseline cliff" we used to hit: two Mock* test + # classes (EarlyClose, CommitTimeoutException) leaked non-daemon threads + # that woke from Thread.sleep() during the next test and threw + # IllegalStateException on a closed MockConsumer. PIT attributed the + # uncaught exception to whichever test was running next, which is why + # exclusions played whack-a-mole. Fixed at source (5d9f022b), no exclusions + # needed. Base-class @AfterEach Awaitility.reset() stays as a general + # leakage guard. - name: Run PIT mutation testing - run: ./mvnw --batch-mode -Pci test-compile org.pitest:pitest-maven:mutationCoverage -Dlicense.skip -DtargetClasses="io.confluent.parallelconsumer.*" -DtargetTests="io.confluent.parallelconsumer.*" -DexcludedTestClasses="io.confluent.parallelconsumer.MockConsumerTestWithSaslAuthenticationException,io.confluent.parallelconsumer.PCMetricsTest" -Dthreads=4 -pl parallel-consumer-core -am + run: ./mvnw --batch-mode -Pci test-compile org.pitest:pitest-maven:mutationCoverage -Dlicense.skip -DtargetClasses="io.confluent.parallelconsumer.*" -DtargetTests="io.confluent.parallelconsumer.*" -Dthreads=4 -pl parallel-consumer-core -am - name: Upload PIT report if: always() uses: actions/upload-artifact@v4 From 475e1b84efb5717ae68272d1b7610871960a954b Mon Sep 17 00:00:00 2001 From: Antony Stubbs Date: Fri, 17 Apr 2026 16:58:34 +1200 Subject: [PATCH 60/67] test: fix flaky PartitionOrderProcessingTest race condition MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The await on line 99 waited for total processed > 500 (any mix of partitions), then immediately asserted all 5 partitions had > 0. Kafka often delivers from one partition first, so partition 0 alone could satisfy the await while other partitions were still empty. The assertion fired once and failed. Fix: move the "all partitions > 0" check into the await's untilAsserted so Awaitility retries until both conditions hold. Also add explicit atMost(120s) to both tests — bare await() was using shaded Awaitility's 10s default, too tight for integration tests on CI runners. Co-Authored-By: Claude Opus 4.6 (1M context) --- .../PartitionOrderProcessingTest.java | 18 +++++++++++++++--- 1 file changed, 15 insertions(+), 3 deletions(-) diff --git a/parallel-consumer-core/src/test-integration/java/io/confluent/parallelconsumer/integrationTests/PartitionOrderProcessingTest.java b/parallel-consumer-core/src/test-integration/java/io/confluent/parallelconsumer/integrationTests/PartitionOrderProcessingTest.java index 91a5d8706..9a9660f2d 100644 --- a/parallel-consumer-core/src/test-integration/java/io/confluent/parallelconsumer/integrationTests/PartitionOrderProcessingTest.java +++ b/parallel-consumer-core/src/test-integration/java/io/confluent/parallelconsumer/integrationTests/PartitionOrderProcessingTest.java @@ -17,6 +17,7 @@ import org.junit.jupiter.api.Test; import pl.tlinkowski.unij.api.UniSets; +import java.time.Duration; import java.util.HashMap; import java.util.Map; import java.util.Properties; @@ -96,8 +97,18 @@ void allPartitionsAreProcessedInParallel() { partitionCounts.get(recordContexts.getSingleConsumerRecord().partition()).getAndIncrement(); ThreadUtils.sleepQuietly(10); // introduce a bit of processing delay - to make sure polling backpressure kicks in. }); - await().until(() -> partitionCounts.values().stream().mapToInt(AtomicInteger::get).sum() > 500); // wait until we process some messages to get the counts in. - Assertions.assertTrue(partitionCounts.values().stream().allMatch(v -> v.get() > 0), "Expect all partitions to have some messages processed, actual partitionCounts:" + partitionCounts); + // Wait for BOTH conditions: enough total messages AND all partitions represented. + // Previously the await only checked total > 500, then the assertion checked all + // partitions — a race, because Kafka may deliver from one partition first. + // Moving the partition check into the await lets Awaitility retry until + // all partitions have been reached. + await().atMost(Duration.ofSeconds(120)).untilAsserted(() -> { + int total = partitionCounts.values().stream().mapToInt(AtomicInteger::get).sum(); + Assertions.assertTrue(total > 500, + "Expect > 500 total messages processed, actual: " + total); + Assertions.assertTrue(partitionCounts.values().stream().allMatch(v -> v.get() > 0), + "Expect all partitions to have some messages processed, actual partitionCounts:" + partitionCounts); + }); } @@ -129,7 +140,8 @@ void allPartitionsAreNotProcessedInParallel() { partitionCounts.get(recordContexts.getSingleConsumerRecord().partition()).getAndIncrement(); ThreadUtils.sleepQuietly(10); // introduce a bit of processing delay - to make sure polling backpressure kicks in. }); - await().until(() -> partitionCounts.values().stream().mapToInt(AtomicInteger::get).sum() > 500); // wait until we process some messages to get the counts in. + // 120s explicit timeout — bare await() used shaded Awaitility's 10s default, too tight for CI. + await().atMost(Duration.ofSeconds(120)).until(() -> partitionCounts.values().stream().mapToInt(AtomicInteger::get).sum() > 500); Assertions.assertFalse(partitionCounts.values().stream().allMatch(v -> v.get() > 0), "Expect some processing thread starving and not all partition counts to have some messages processed, actual partitionCounts:" + partitionCounts); } From 6fa9c3c5a8a3dbaf621e8eb30bc1c1d76d70ae26 Mon Sep 17 00:00:00 2001 From: Antony Stubbs Date: Fri, 17 Apr 2026 17:40:20 +1200 Subject: [PATCH 61/67] ci: strip Jacoco from PIT minions to fix PCMetricsTest on CI MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PCMetricsTest.metricsRegisterBinding still failed PIT baseline on CI (100s, up from 85s after the thread-leak fix) because CI's slower hardware + double-instrumentation (Jacoco + PIT) pushes the 1500-record test over the coverage-phase budget. Passed locally on a faster laptop. PIT computes its own coverage — it doesn't need Jacoco's javaagent in its minion JVMs. -DargLine="" overrides the Jacoco-populated argLine property so PIT minions run without Jacoco, roughly halving instrumentation overhead. Jacoco still runs normally for the separate Surefire/Failsafe test phases. Co-Authored-By: Claude Opus 4.6 (1M context) --- .github/workflows/maven.yml | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/.github/workflows/maven.yml b/.github/workflows/maven.yml index bbb20d041..0ecc06ba5 100644 --- a/.github/workflows/maven.yml +++ b/.github/workflows/maven.yml @@ -217,8 +217,12 @@ jobs: # exclusions played whack-a-mole. Fixed at source (5d9f022b), no exclusions # needed. Base-class @AfterEach Awaitility.reset() stays as a general # leakage guard. + # -DargLine="" strips Jacoco's javaagent from PIT's minion JVMs. + # PIT computes its own coverage — Jacoco just doubles the + # instrumentation overhead and pushes slow tests past the + # coverage-phase budget on CI's weaker hardware. - name: Run PIT mutation testing - run: ./mvnw --batch-mode -Pci test-compile org.pitest:pitest-maven:mutationCoverage -Dlicense.skip -DtargetClasses="io.confluent.parallelconsumer.*" -DtargetTests="io.confluent.parallelconsumer.*" -Dthreads=4 -pl parallel-consumer-core -am + run: ./mvnw --batch-mode -Pci test-compile org.pitest:pitest-maven:mutationCoverage -Dlicense.skip -DtargetClasses="io.confluent.parallelconsumer.*" -DtargetTests="io.confluent.parallelconsumer.*" -DargLine="" -Dthreads=4 -pl parallel-consumer-core -am - name: Upload PIT report if: always() uses: actions/upload-artifact@v4 From 0a9623f9d933c8d701c639971ec066016ad2d4c6 Mon Sep 17 00:00:00 2001 From: Antony Stubbs Date: Fri, 17 Apr 2026 19:50:24 +1200 Subject: [PATCH 62/67] =?UTF-8?q?ci:=20bump=20PIT=20timeout=2015m=20?= =?UTF-8?q?=E2=86=92=2060m=20to=20let=20mutation=20phase=20complete?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Coverage phase passed (419s) but the mutation phase needs more than 15 minutes on CI. Bumped to 60m so we can observe the actual runtime and dial it down later. Co-Authored-By: Claude Opus 4.6 (1M context) --- .github/workflows/maven.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/maven.yml b/.github/workflows/maven.yml index 0ecc06ba5..768560987 100644 --- a/.github/workflows/maven.yml +++ b/.github/workflows/maven.yml @@ -201,7 +201,7 @@ jobs: if: github.event_name == 'pull_request' name: "Mutation Testing (PIT)" runs-on: ubuntu-latest - timeout-minutes: 15 + timeout-minutes: 60 steps: - uses: actions/checkout@v6 - uses: actions/setup-java@v5 From 0b579a3d5d41ff1dc36e2a0900dfc43c2a0ef8c9 Mon Sep 17 00:00:00 2001 From: Antony Stubbs Date: Fri, 17 Apr 2026 20:56:16 +1200 Subject: [PATCH 63/67] ci: fix PIT mutation-phase timeouts with generous timeout knobs 23 of 76 mutation units were timing out because PIT's default per- mutation cap (baseline * 1.25 + 4s) is too tight for CI hardware. The baseline is measured during coverage on the same runner, but mutation runs are heavier (bytecode rewriting + re-running tests) so they exceed the factor. Bumped to baseline * 3.0 + 30s. Co-Authored-By: Claude Opus 4.6 (1M context) --- .github/workflows/maven.yml | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/.github/workflows/maven.yml b/.github/workflows/maven.yml index 768560987..13248fada 100644 --- a/.github/workflows/maven.yml +++ b/.github/workflows/maven.yml @@ -217,12 +217,15 @@ jobs: # exclusions played whack-a-mole. Fixed at source (5d9f022b), no exclusions # needed. Base-class @AfterEach Awaitility.reset() stays as a general # leakage guard. - # -DargLine="" strips Jacoco's javaagent from PIT's minion JVMs. - # PIT computes its own coverage — Jacoco just doubles the - # instrumentation overhead and pushes slow tests past the - # coverage-phase budget on CI's weaker hardware. + # -DargLine="" strips Jacoco's javaagent from PIT's minion JVMs + # (PIT computes its own coverage; Jacoco doubles overhead). + # -DtimeoutConstant=30000 -DtimeoutFactor=3.0 + # PIT caps each mutation run at baseline*factor+constant. + # Defaults (1.25 / 4s) are too tight for CI's slower + # hardware — 23/76 mutations were timing out. 3x + 30s + # gives enough headroom. - name: Run PIT mutation testing - run: ./mvnw --batch-mode -Pci test-compile org.pitest:pitest-maven:mutationCoverage -Dlicense.skip -DtargetClasses="io.confluent.parallelconsumer.*" -DtargetTests="io.confluent.parallelconsumer.*" -DargLine="" -Dthreads=4 -pl parallel-consumer-core -am + run: ./mvnw --batch-mode -Pci test-compile org.pitest:pitest-maven:mutationCoverage -Dlicense.skip -DtargetClasses="io.confluent.parallelconsumer.*" -DtargetTests="io.confluent.parallelconsumer.*" -DargLine="" -DtimeoutConstant=30000 -DtimeoutFactor=3.0 -Dthreads=4 -pl parallel-consumer-core -am - name: Upload PIT report if: always() uses: actions/upload-artifact@v4 From 27fcf788a572469db6772ea4eea1549a58cd6e73 Mon Sep 17 00:00:00 2001 From: Antony Stubbs Date: Fri, 17 Apr 2026 22:17:16 +1200 Subject: [PATCH 64/67] ci: make PIT non-blocking (continue-on-error) + 24h timeout MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PIT's mutation phase takes 45-60+ minutes on CI. Made non-blocking so it doesn't gate merges — the PR comment (posted via `if: always()`) is the notification. Bumped timeout to 1440 minutes (24 hours) so we finally get a complete run and can observe actual runtime before dialling it down. Co-Authored-By: Claude Opus 4.6 (1M context) --- .github/workflows/maven.yml | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/.github/workflows/maven.yml b/.github/workflows/maven.yml index 13248fada..02e353cac 100644 --- a/.github/workflows/maven.yml +++ b/.github/workflows/maven.yml @@ -196,12 +196,16 @@ jobs: fail-on-severity: high comment-summary-in-pr: always - # Mutation testing (PIT) -verifies test assertions are meaningful + # Mutation testing (PIT) - verifies test assertions are meaningful. + # Non-blocking: runs and posts a PR comment with the mutation score, + # but doesn't gate merge. PIT's mutation phase is too slow (~45-60 min) + # to be a required check. The PR comment is the notification mechanism. mutation-testing: if: github.event_name == 'pull_request' name: "Mutation Testing (PIT)" runs-on: ubuntu-latest - timeout-minutes: 60 + continue-on-error: true + timeout-minutes: 1440 steps: - uses: actions/checkout@v6 - uses: actions/setup-java@v5 From d882bd6701df149734e7e5eadece630a7c4f937a Mon Sep 17 00:00:00 2001 From: Antony Stubbs Date: Sat, 18 Apr 2026 09:14:14 +1200 Subject: [PATCH 65/67] =?UTF-8?q?ci:=20tune=20PIT=20for=20CI=20reality=20?= =?UTF-8?q?=E2=80=94=20narrower=20target,=20single=20thread,=20history=20c?= =?UTF-8?q?ache?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PIT's mutation phase hit GitHub's 6-hour job cap targeting all classes. Three changes to make it viable on ubuntu-latest: 1. Narrow targetClasses to internal.* only (the core engine where mutations matter most). Broadening back to all classes is tracked in #41 for the self-hosted performance runner. 2. threads=1 (was 4). Tests are I/O-bound; parallel minions just add contention without speed benefit on shared CI hardware. 3. Incremental analysis (-DwithHistory): PIT caches mutation results and skips re-mutating unchanged code on subsequent runs. Cache key rotates on source hash with base-branch fallback. Also: timeout 5h (under GitHub's 6h cap), still non-blocking (continue-on-error: true). Co-Authored-By: Claude Opus 4.6 (1M context) --- .github/workflows/maven.yml | 52 +++++++++++++++++++++++-------------- 1 file changed, 32 insertions(+), 20 deletions(-) diff --git a/.github/workflows/maven.yml b/.github/workflows/maven.yml index 02e353cac..8da5ec70c 100644 --- a/.github/workflows/maven.yml +++ b/.github/workflows/maven.yml @@ -197,15 +197,14 @@ jobs: comment-summary-in-pr: always # Mutation testing (PIT) - verifies test assertions are meaningful. - # Non-blocking: runs and posts a PR comment with the mutation score, - # but doesn't gate merge. PIT's mutation phase is too slow (~45-60 min) - # to be a required check. The PR comment is the notification mechanism. + # Non-blocking: posts a PR comment with mutation score but doesn't + # gate merge. TODO: move to self-hosted performance runner for speed. mutation-testing: if: github.event_name == 'pull_request' name: "Mutation Testing (PIT)" runs-on: ubuntu-latest continue-on-error: true - timeout-minutes: 1440 + timeout-minutes: 300 steps: - uses: actions/checkout@v6 - uses: actions/setup-java@v5 @@ -213,23 +212,36 @@ jobs: distribution: 'temurin' java-version: '17' cache: 'maven' - # Root-caused the "~85s PIT-baseline cliff" we used to hit: two Mock* test - # classes (EarlyClose, CommitTimeoutException) leaked non-daemon threads - # that woke from Thread.sleep() during the next test and threw - # IllegalStateException on a closed MockConsumer. PIT attributed the - # uncaught exception to whichever test was running next, which is why - # exclusions played whack-a-mole. Fixed at source (5d9f022b), no exclusions - # needed. Base-class @AfterEach Awaitility.reset() stays as a general - # leakage guard. - # -DargLine="" strips Jacoco's javaagent from PIT's minion JVMs - # (PIT computes its own coverage; Jacoco doubles overhead). - # -DtimeoutConstant=30000 -DtimeoutFactor=3.0 - # PIT caps each mutation run at baseline*factor+constant. - # Defaults (1.25 / 4s) are too tight for CI's slower - # hardware — 23/76 mutations were timing out. 3x + 30s - # gives enough headroom. + # Restore PIT history for incremental analysis — only re-mutates + # code that changed since last run. + - name: Restore PIT history cache + uses: actions/cache@v4 + with: + path: parallel-consumer-core/target/pit-history + key: pit-history-${{ github.base_ref }}-${{ hashFiles('**/src/main/**/*.java') }} + restore-keys: | + pit-history-${{ github.base_ref }}- + pit-history- + # -DargLine="" strips Jacoco from PIT minions (PIT has its own coverage) + # -DtimeoutConstant/Factor generous per-mutation timeout for CI hardware + # -Dthreads=1 I/O-bound tests; parallelism adds contention not speed + # -DwithHistory incremental: skips re-mutating unchanged code + # targetClasses: internal.* only — the core engine where mutations matter most. + # Broadening to io.confluent.parallelconsumer.* hit GitHub's 6h job cap. - name: Run PIT mutation testing - run: ./mvnw --batch-mode -Pci test-compile org.pitest:pitest-maven:mutationCoverage -Dlicense.skip -DtargetClasses="io.confluent.parallelconsumer.*" -DtargetTests="io.confluent.parallelconsumer.*" -DargLine="" -DtimeoutConstant=30000 -DtimeoutFactor=3.0 -Dthreads=4 -pl parallel-consumer-core -am + run: | + mkdir -p parallel-consumer-core/target/pit-history + ./mvnw --batch-mode -Pci test-compile org.pitest:pitest-maven:mutationCoverage \ + -Dlicense.skip \ + -DtargetClasses="io.confluent.parallelconsumer.internal.*" \ + -DtargetTests="io.confluent.parallelconsumer.*" \ + -DargLine="" \ + -DtimeoutConstant=30000 -DtimeoutFactor=3.0 \ + -Dthreads=1 \ + -DwithHistory \ + -DhistoryInputLocation=target/pit-history/history.bin \ + -DhistoryOutputLocation=target/pit-history/history.bin \ + -pl parallel-consumer-core -am - name: Upload PIT report if: always() uses: actions/upload-artifact@v4 From 9c6155a5d9ec778b625fcbdb6f80302e22cca26a Mon Sep 17 00:00:00 2001 From: Antony Stubbs Date: Sat, 18 Apr 2026 10:26:25 +1200 Subject: [PATCH 66/67] ci: fix PIT minion OOM + history cache path Two issues from the last run's logs: 1. MEMORY_ERROR: PIT minions were OOM'ing. Changed -DargLine="" to -DargLine="-Xmx1g" so minion JVMs get 1GB heap (still strips Jacoco since we're replacing the full argLine). 2. History cache miss: PIT ignores -DhistoryInputLocation CLI flags when -DwithHistory is set, writing to /tmp/{groupId}.{artifactId}... instead. Updated actions/cache path to match PIT's actual location (/tmp/*_pitest_history.bin). Dropped the explicit location flags and mkdir since PIT manages its own path. Co-Authored-By: Claude Opus 4.6 (1M context) --- .github/workflows/maven.yml | 21 ++++++++++++--------- 1 file changed, 12 insertions(+), 9 deletions(-) diff --git a/.github/workflows/maven.yml b/.github/workflows/maven.yml index 8da5ec70c..267b4ebb6 100644 --- a/.github/workflows/maven.yml +++ b/.github/workflows/maven.yml @@ -213,34 +213,37 @@ jobs: java-version: '17' cache: 'maven' # Restore PIT history for incremental analysis — only re-mutates - # code that changed since last run. + # code that changed since last run. PIT writes its history to /tmp/ + # with a filename derived from groupId.artifactId.version, ignoring + # explicit -DhistoryInputLocation CLI flags when -DwithHistory is set. - name: Restore PIT history cache uses: actions/cache@v4 with: - path: parallel-consumer-core/target/pit-history + path: /tmp/*_pitest_history.bin key: pit-history-${{ github.base_ref }}-${{ hashFiles('**/src/main/**/*.java') }} restore-keys: | pit-history-${{ github.base_ref }}- pit-history- - # -DargLine="" strips Jacoco from PIT minions (PIT has its own coverage) + # -DargLine="-Xmx1g" gives minion JVMs 1GB heap (default is too small, + # causes MEMORY_ERROR) and strips Jacoco (PIT has + # its own coverage). # -DtimeoutConstant/Factor generous per-mutation timeout for CI hardware # -Dthreads=1 I/O-bound tests; parallelism adds contention not speed - # -DwithHistory incremental: skips re-mutating unchanged code + # -DwithHistory incremental: skips re-mutating unchanged code. + # PIT writes history to /tmp/{groupId}.{artifactId}... + # which we cache above via actions/cache. # targetClasses: internal.* only — the core engine where mutations matter most. - # Broadening to io.confluent.parallelconsumer.* hit GitHub's 6h job cap. + # Broadening to io.confluent.parallelconsumer.* hit GitHub's 6h job cap (#41). - name: Run PIT mutation testing run: | - mkdir -p parallel-consumer-core/target/pit-history ./mvnw --batch-mode -Pci test-compile org.pitest:pitest-maven:mutationCoverage \ -Dlicense.skip \ -DtargetClasses="io.confluent.parallelconsumer.internal.*" \ -DtargetTests="io.confluent.parallelconsumer.*" \ - -DargLine="" \ + -DargLine="-Xmx1g" \ -DtimeoutConstant=30000 -DtimeoutFactor=3.0 \ -Dthreads=1 \ -DwithHistory \ - -DhistoryInputLocation=target/pit-history/history.bin \ - -DhistoryOutputLocation=target/pit-history/history.bin \ -pl parallel-consumer-core -am - name: Upload PIT report if: always() From 4359bd13d9d6e6d831352c2dc676c8fef3d038d6 Mon Sep 17 00:00:00 2001 From: Antony Stubbs Date: Sat, 18 Apr 2026 10:37:41 +1200 Subject: [PATCH 67/67] =?UTF-8?q?ci:=20fix=20PIT=20minion=20crash=20?= =?UTF-8?q?=E2=80=94=20use=20jvmArgs=20not=20argLine=20for=20heap?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Minion crashed with UNKNOWN_ERROR because -DargLine="-Xmx1g" doesn't pass JVM flags to PIT minions (argLine is a Surefire property that PIT reads for classpath/agent args, not JVM flags). Split into: -DjvmArgs=-Xmx1g (JVM heap for minions) -DargLine= (strips Jacoco agent) Co-Authored-By: Claude Opus 4.6 (1M context) --- .github/workflows/maven.yml | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/.github/workflows/maven.yml b/.github/workflows/maven.yml index 267b4ebb6..e1e53a0a2 100644 --- a/.github/workflows/maven.yml +++ b/.github/workflows/maven.yml @@ -224,9 +224,10 @@ jobs: restore-keys: | pit-history-${{ github.base_ref }}- pit-history- - # -DargLine="-Xmx1g" gives minion JVMs 1GB heap (default is too small, - # causes MEMORY_ERROR) and strips Jacoco (PIT has - # its own coverage). + # -DjvmArgs=-Xmx1g gives minion JVMs 1GB heap (default too small, + # causes MEMORY_ERROR / UNKNOWN_ERROR crashes). + # -DargLine= strips Jacoco's javaagent from minions (PIT has + # its own coverage). Separate from jvmArgs. # -DtimeoutConstant/Factor generous per-mutation timeout for CI hardware # -Dthreads=1 I/O-bound tests; parallelism adds contention not speed # -DwithHistory incremental: skips re-mutating unchanged code. @@ -240,7 +241,8 @@ jobs: -Dlicense.skip \ -DtargetClasses="io.confluent.parallelconsumer.internal.*" \ -DtargetTests="io.confluent.parallelconsumer.*" \ - -DargLine="-Xmx1g" \ + -DjvmArgs=-Xmx1g \ + -DargLine= \ -DtimeoutConstant=30000 -DtimeoutFactor=3.0 \ -Dthreads=1 \ -DwithHistory \