From bbd47ab639a2f36a045fa7cd6ebabeac8424c502 Mon Sep 17 00:00:00 2001 From: Marco Schaeck Date: Fri, 14 Aug 2026 07:31:55 +0200 Subject: [PATCH 1/4] test: add PIT mutation testing with per-module score gates Wire the info.solidsoft.pitest plugin into the four logic-bearing modules (core, runtime, web, testing) following the existing JaCoCo convention, with a hard per-module mutationThreshold gate. Runs locally via ./gradlew pitest and as an opt-in nightly/manual CI workflow. Suppress Kotlin synthetic-mutant noise (null intrinsics, serializers, inlined comparators) via avoidCallsTo and excludedClasses, and strengthen the runtime and testing suites to 100% / 98% mutation coverage. --- .github/workflows/mutation-testing.yml | 56 ++++++++ bpmn-to-code-core/build.gradle.kts | 9 ++ bpmn-to-code-runtime/build.gradle.kts | 6 + .../miragon/bpmn/runtime/RuntimeTypesTest.kt | 74 ++++++++++- bpmn-to-code-testing/build.gradle.kts | 5 + .../miragon/bpmn/testing/BpmnValidatorTest.kt | 65 +++++++++ bpmn-to-code-web/build.gradle.kts | 11 ++ build.gradle.kts | 44 +++++++ .../adr/019-mutation-testing-with-pit.md | 124 ++++++++++++++++++ docs/contributing/adr/index.md | 1 + docs/contributing/index.md | 15 +++ gradle/libs.versions.toml | 4 + 12 files changed, 411 insertions(+), 3 deletions(-) create mode 100644 .github/workflows/mutation-testing.yml create mode 100644 docs/contributing/adr/019-mutation-testing-with-pit.md diff --git a/.github/workflows/mutation-testing.yml b/.github/workflows/mutation-testing.yml new file mode 100644 index 00000000..2d3e9f7a --- /dev/null +++ b/.github/workflows/mutation-testing.yml @@ -0,0 +1,56 @@ +name: Mutation Testing + +# Opt-in mutation testing (PIT). Not part of the PR gate — it is far slower than the unit +# tests, so it runs on a nightly schedule and on demand. The per-module mutationThreshold +# configured in each build.gradle.kts fails the job when mutation coverage regresses. +on: + workflow_dispatch: + schedule: + # Nightly at 03:00 UTC (off-peak). + - cron: '0 3 * * *' + +concurrency: + group: ${{ github.workflow }}-${{ github.ref }} + cancel-in-progress: true + +jobs: + pitest: + name: Run PIT + runs-on: ubuntu-latest + + steps: + # Step 1: Checkout code + - name: Checkout Code + uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6 + + # Step 2: Setup Gradle + - name: Setup Gradle + uses: gradle/actions/setup-gradle@3f131e8634966bd73d06cc69884922b02e6faf92 # v6 + + # Step 3: Set up JDK 21 + - name: Set up JDK 21 + uses: actions/setup-java@1bcf9fb12cf4aa7d266a90ae39939e61372fe520 # v5 + with: + java-version: '21' + distribution: 'temurin' + + # Step 4: Run mutation testing for the logic-bearing modules. + # --no-configuration-cache: the pitest task is not configuration-cache compatible. + # --continue: run every module even if one regresses below its threshold, so all + # reports are produced before the job fails. + - name: Run PIT + run: | + ./gradlew --no-configuration-cache --continue --warning-mode all \ + :bpmn-to-code-core:pitest \ + :bpmn-to-code-runtime:pitest \ + :bpmn-to-code-web:pitest \ + :bpmn-to-code-testing:pitest + + # Step 5: Publish HTML/XML reports for inspection (even when the gate fails). + - name: Upload PIT reports + if: always() + uses: actions/upload-artifact@ea165f8d65b6e75b540449e92b4886f43607fa02 # v4.6.2 + with: + name: pitest-reports + path: '**/build/reports/pitest/**' + if-no-files-found: warn diff --git a/bpmn-to-code-core/build.gradle.kts b/bpmn-to-code-core/build.gradle.kts index 3a6635d2..107bd09b 100644 --- a/bpmn-to-code-core/build.gradle.kts +++ b/bpmn-to-code-core/build.gradle.kts @@ -1,6 +1,7 @@ plugins { alias(libs.plugins.kotlin.jvm) alias(libs.plugins.kotlin.serialization) + alias(libs.plugins.pitest) jacoco } @@ -63,3 +64,11 @@ tasks.jacocoTestCoverageVerification { files(classDirectories.files.map { fileTree(it) { exclude(coverageExclusions) } }) ) } + +pitest { + excludedClasses.addAll( + "io.miragon.bpmn.adapter.outbound.engine.*Constants*", + "io.miragon.bpmn.adapter.outbound.json.model.*", + ) + mutationThreshold.set(80) +} diff --git a/bpmn-to-code-runtime/build.gradle.kts b/bpmn-to-code-runtime/build.gradle.kts index 675b4a3c..11e7d2c1 100644 --- a/bpmn-to-code-runtime/build.gradle.kts +++ b/bpmn-to-code-runtime/build.gradle.kts @@ -2,6 +2,7 @@ plugins { alias(libs.plugins.kotlin.jvm) alias(libs.plugins.mavenPublish) alias(libs.plugins.dokka) + alias(libs.plugins.pitest) jacoco } @@ -27,6 +28,11 @@ tasks.withType().configureEach { exclude("**/io/miragon/bpmn/runtime/path/example/**") } +pitest { + excludedClasses.addAll("io.miragon.bpmn.runtime.path.example.*") + mutationThreshold.set(95) +} + mavenPublishing { publishToMavenCentral() diff --git a/bpmn-to-code-runtime/src/test/kotlin/io/miragon/bpmn/runtime/RuntimeTypesTest.kt b/bpmn-to-code-runtime/src/test/kotlin/io/miragon/bpmn/runtime/RuntimeTypesTest.kt index aa3f9b4b..5590bb92 100644 --- a/bpmn-to-code-runtime/src/test/kotlin/io/miragon/bpmn/runtime/RuntimeTypesTest.kt +++ b/bpmn-to-code-runtime/src/test/kotlin/io/miragon/bpmn/runtime/RuntimeTypesTest.kt @@ -13,6 +13,14 @@ class RuntimeTypesTest { assertThat(SignalName("CancelRequested").toString()).isEqualTo("CancelRequested") } + @Test + fun `identifier wrappers expose the raw value property`() { + assertThat(ProcessId("order-process").value).isEqualTo("order-process") + assertThat(ElementId("place-order").value).isEqualTo("place-order") + assertThat(MessageName("OrderPlaced").value).isEqualTo("OrderPlaced") + assertThat(SignalName("CancelRequested").value).isEqualTo("CancelRequested") + } + @Test fun `identifier wrappers implement value equality`() { assertThat(ProcessId("a")).isEqualTo(ProcessId("a")) @@ -44,11 +52,29 @@ class RuntimeTypesTest { @Test fun `BpmnFlow defaults nullable fields to null`() { val flow = BpmnFlow(id = "f1", sourceRef = "s", targetRef = "t") + assertThat(flow.id).isEqualTo("f1") + assertThat(flow.sourceRef).isEqualTo("s") + assertThat(flow.targetRef).isEqualTo("t") assertThat(flow.name).isNull() assertThat(flow.condition).isNull() assertThat(flow.isDefault).isFalse() } + @Test + fun `BpmnFlow retains a default flag and its labelled fields`() { + val flow = BpmnFlow( + id = "f2", + name = "yes", + sourceRef = "gateway", + targetRef = "approve", + condition = "\${approved}", + isDefault = true, + ) + assertThat(flow.name).isEqualTo("yes") + assertThat(flow.condition).isEqualTo("\${approved}") + assertThat(flow.isDefault).isTrue() + } + @Test fun `BpmnRelations retains list and nullable fields`() { val relations = BpmnRelations( @@ -60,12 +86,30 @@ class RuntimeTypesTest { attachedElements = listOf("boundary-timer"), elementType = "USER_TASK", ) + assertThat(relations.name).isEqualTo("Approve") + assertThat(relations.parentId).isNull() + assertThat(relations.attachedToRef).isNull() assertThat(relations.previousElements).containsExactly("start") assertThat(relations.followingElements).containsExactly("end") assertThat(relations.attachedElements).containsExactly("boundary-timer") assertThat(relations.elementType).isEqualTo("USER_TASK") } + @Test + fun `BpmnRelations exposes parent and boundary host for nested elements`() { + val relations = BpmnRelations( + name = "Send reminder", + previousElements = listOf("start"), + followingElements = listOf("end"), + parentId = "confirmation-subprocess", + attachedToRef = "confirm-task", + attachedElements = emptyList(), + elementType = "TIMER_BOUNDARY_EVENT", + ) + assertThat(relations.parentId).isEqualTo("confirmation-subprocess") + assertThat(relations.attachedToRef).isEqualTo("confirm-task") + } + @Test fun `BpmnEngine covers all supported dialects`() { assertThat(BpmnEngine.entries).containsExactly( @@ -77,9 +121,33 @@ class RuntimeTypesTest { @Test fun `BpmnTimer, BpmnError, BpmnEscalation carry their pair of strings`() { - assertThat(BpmnTimer("Duration", "PT5M").timerValue).isEqualTo("PT5M") - assertThat(BpmnError("NotFound", "E_404").code).isEqualTo("E_404") - assertThat(BpmnEscalation("OutOfHours", "E_HRS").name).isEqualTo("OutOfHours") + val timer = BpmnTimer("Duration", "PT5M") + assertThat(timer.type).isEqualTo("Duration") + assertThat(timer.timerValue).isEqualTo("PT5M") + + val error = BpmnError("NotFound", "E_404") + assertThat(error.name).isEqualTo("NotFound") + assertThat(error.code).isEqualTo("E_404") + + val escalation = BpmnEscalation("OutOfHours", "E_HRS") + assertThat(escalation.name).isEqualTo("OutOfHours") + assertThat(escalation.code).isEqualTo("E_HRS") + } + + @Test + fun `AbstractFlowNode derives identity and hash from the element id`() { + val node = flowNode("approve-task") + val same = flowNode("approve-task") + val other = flowNode("reject-task") + + assertThat(node).isEqualTo(same) + assertThat(node).isNotEqualTo(other) + assertThat(node.hashCode()).isEqualTo(ElementId("approve-task").hashCode()) + assertThat(node.hashCode()).isEqualTo(same.hashCode()) + } + + private fun flowNode(id: String): AbstractFlowNode { + return object : AbstractFlowNode(ElementId(id), "SERVICE_TASK") {} } @Test diff --git a/bpmn-to-code-testing/build.gradle.kts b/bpmn-to-code-testing/build.gradle.kts index 4dcbfeff..2ad3420f 100644 --- a/bpmn-to-code-testing/build.gradle.kts +++ b/bpmn-to-code-testing/build.gradle.kts @@ -2,6 +2,7 @@ plugins { alias(libs.plugins.kotlin.jvm) alias(libs.plugins.mavenPublish) alias(libs.plugins.dokka) + alias(libs.plugins.pitest) jacoco } @@ -37,6 +38,10 @@ tasks.named("test") { useJUnitPlatform() } +pitest { + mutationThreshold.set(95) +} + mavenPublishing { publishToMavenCentral() diff --git a/bpmn-to-code-testing/src/test/kotlin/io/miragon/bpmn/testing/BpmnValidatorTest.kt b/bpmn-to-code-testing/src/test/kotlin/io/miragon/bpmn/testing/BpmnValidatorTest.kt index 72bf6d66..992914ed 100644 --- a/bpmn-to-code-testing/src/test/kotlin/io/miragon/bpmn/testing/BpmnValidatorTest.kt +++ b/bpmn-to-code-testing/src/test/kotlin/io/miragon/bpmn/testing/BpmnValidatorTest.kt @@ -4,7 +4,9 @@ import io.miragon.bpmn.domain.shared.ProcessEngine import io.miragon.bpmn.domain.validation.SingleModelValidationRule import io.miragon.bpmn.domain.validation.model.Severity import io.miragon.bpmn.domain.validation.model.SingleModelValidationContext +import io.miragon.bpmn.domain.validation.model.ValidationPhase import io.miragon.bpmn.domain.validation.model.ValidationViolation +import org.assertj.core.api.Assertions.assertThat import org.assertj.core.api.Assertions.assertThatThrownBy import org.junit.jupiter.api.Test import org.junit.jupiter.api.io.TempDir @@ -99,6 +101,69 @@ class BpmnValidatorTest { .assertNoErrors() } + @Test + fun `failOnWarning promotes warnings to errors`() { + val result = BpmnValidator + .fromClasspath("bpmn/valid-process.bpmn") + .engine(ProcessEngine.CAMUNDA_7) + .withRules(AlwaysViolatingRule("warn-rule", Severity.WARN)) + .failOnWarning() + .validate() + .result() + + assertThat(result.errors.map { it.ruleId }).contains("warn-rule") + assertThat(result.warnings).isEmpty() + } + + @Test + fun `warnings in the pre-merge phase do not short-circuit post-merge rules`() { + val result = BpmnValidator + .fromClasspath("bpmn/valid-process.bpmn") + .engine(ProcessEngine.CAMUNDA_7) + .withRules( + AlwaysViolatingRule("pre-warn", Severity.WARN, ValidationPhase.PRE_MERGE), + AlwaysViolatingRule("post-warn", Severity.WARN, ValidationPhase.POST_MERGE), + ) + .validate() + .result() + + assertThat(result.violations.map { it.ruleId }).contains("pre-warn", "post-warn") + } + + @Test + fun `an error in the pre-merge phase short-circuits post-merge rules`() { + val result = BpmnValidator + .fromClasspath("bpmn/valid-process.bpmn") + .engine(ProcessEngine.CAMUNDA_7) + .withRules( + AlwaysViolatingRule("pre-error", Severity.ERROR, ValidationPhase.PRE_MERGE), + AlwaysViolatingRule("post-warn", Severity.WARN, ValidationPhase.POST_MERGE), + ) + .validate() + .result() + + assertThat(result.violations.map { it.ruleId }).contains("pre-error") + assertThat(result.violations.map { it.ruleId }).doesNotContain("post-warn") + } + + private class AlwaysViolatingRule( + override val id: String, + override val severity: Severity, + override val phase: ValidationPhase = ValidationPhase.PRE_MERGE, + ) : SingleModelValidationRule { + override fun validate(context: SingleModelValidationContext): List { + return listOf( + ValidationViolation( + ruleId = id, + severity = severity, + elementId = null, + processId = context.model.processId, + message = "violation from $id", + ), + ) + } + } + private class AlwaysFailingMandatoryRule : SingleModelValidationRule { override val id = "always-failing-mandatory" override val severity = Severity.ERROR diff --git a/bpmn-to-code-web/build.gradle.kts b/bpmn-to-code-web/build.gradle.kts index 32382441..cacab83a 100644 --- a/bpmn-to-code-web/build.gradle.kts +++ b/bpmn-to-code-web/build.gradle.kts @@ -2,6 +2,7 @@ plugins { alias(libs.plugins.kotlin.jvm) alias(libs.plugins.kotlin.serialization) alias(libs.plugins.ktor) + alias(libs.plugins.pitest) application jacoco } @@ -117,6 +118,16 @@ tasks.jacocoTestCoverageVerification { ) } +pitest { + excludedClasses.addAll( + "io.miragon.bpmn.web.routes.*", + "io.miragon.bpmn.web.Application*", + "io.miragon.bpmn.web.config.*", + "io.miragon.bpmn.web.model.ConfigResponse*", + ) + mutationThreshold.set(22) +} + tasks.named("processResources") { val projectVersion = project.version.toString() filesMatching("version.properties") { diff --git a/build.gradle.kts b/build.gradle.kts index 99701005..980a62ab 100644 --- a/build.gradle.kts +++ b/build.gradle.kts @@ -1,3 +1,4 @@ +import info.solidsoft.gradle.pitest.PitestPluginExtension import io.gitlab.arturbosch.detekt.extensions.DetektExtension import org.gradle.testing.jacoco.tasks.JacocoCoverageVerification import org.gradle.testing.jacoco.tasks.JacocoReport @@ -9,8 +10,13 @@ plugins { alias(libs.plugins.mavenPublish) apply false alias(libs.plugins.detekt) apply false alias(libs.plugins.ktlint) apply false + alias(libs.plugins.pitest) apply false } +// Captured here: the `libs` accessor is not resolvable inside the subprojects {} block below. +val pitestCoreVersion = libs.versions.pitestCore.get() +val pitestJunit5Version = libs.versions.pitestJunit5.get() + allprojects { repositories { mavenCentral() @@ -70,4 +76,42 @@ subprojects { } } } + + plugins.withId("info.solidsoft.pitest") { + configure { + pitestVersion.set(pitestCoreVersion) + junit5PluginVersion.set(pitestJunit5Version) + + targetClasses.set(listOf("io.miragon.*")) + outputFormats.set(listOf("HTML", "XML")) + timestampedReports.set(false) + threads.set(Runtime.getRuntime().availableProcessors()) + + // Suppress mutations of Kotlin compiler-generated null/resource intrinsics + // (unkillable synthetic checks), keeping PIT's logging defaults. + avoidCallsTo.set( + listOf( + "kotlin.jvm.internal.Intrinsics", + "kotlin.io.CloseableKt", + "java.util.logging", + "org.slf4j", + "org.apache.log4j", + "org.apache.commons.logging", + ), + ) + + excludedClasses.set( + listOf( + "*\$DefaultImpls", + "*\$Companion", + "*\$WhenMappings", + "*\$\$serializer", + "*\$\$inlined\$*", + "io.miragon.bpmn.domain.shared.*", + "io.miragon.bpmn.domain.validation.model.*", + "io.miragon.bpmn.application.port.*", + ), + ) + } + } } diff --git a/docs/contributing/adr/019-mutation-testing-with-pit.md b/docs/contributing/adr/019-mutation-testing-with-pit.md new file mode 100644 index 00000000..ed25a275 --- /dev/null +++ b/docs/contributing/adr/019-mutation-testing-with-pit.md @@ -0,0 +1,124 @@ +# ADR 019: Mutation Testing with PIT + +## Status +Accepted + +## Context + +The project already gates on JaCoCo line coverage (≥ 75% per class, see the `subprojects` +block in the root `build.gradle.kts`). Line coverage only proves a line *executed* during a +test — it says nothing about whether any assertion would *notice* if that line's behaviour +changed. A test that calls a method but asserts nothing still counts as full coverage. + +We wanted a metric that measures the fault-detection strength of the suite itself: +[PIT (pitest)](https://pitest.org/) mutation testing. PIT introduces small changes +("mutants") into the compiled bytecode — negate a conditional, swap a return value, remove a +void call — and re-runs the tests. A mutant that no test catches ("survived") marks a real +gap; the *mutation score* is the share of mutants killed. + +Two facts about this repo shaped the approach: + +- **JUnit 6.** The build runs JUnit Jupiter / Platform **6.1.0**, a very new line. PIT drives + tests through the JUnit Platform launcher via `pitest-junit5-plugin`, whose documentation + only lists JUnit 5 / Platform 1.x. Compatibility was therefore proven empirically before + committing to the integration, on `bpmn-to-code-core`, and confirmed working. +- **Kotlin bytecode.** Kotlin emits synthetic members (`$DefaultImpls`, `$Companion`, + `$WhenMappings`, `kotlinx.serialization`'s `$$serializer` classes, and `$$inlined$…` + comparators from `sortedBy {}`) plus compiler-generated null/resource intrinsics + (`Intrinsics.checkNotNull…`, `CloseableKt.closeFinally` from `.use {}`). PIT mutates all of + these, but no hand-written test can kill them — they inflated the survivor count enough to + understate the real suite by 6–15 points per module. The commercial Arcmutate Kotlin plugin + solves this at the engine level; we do not use it and instead exclude the synthetic classes + and suppress the intrinsic calls via `avoidCallsTo`. + +## Decision + +Adopt the `info.solidsoft.pitest` Gradle plugin (`gradle-pitest-plugin`) with a **hard +per-module mutation-score gate**, wired to follow the existing JaCoCo convention exactly. + +### Scope + +Applied to the four logic-bearing modules only — the same set that carries JaCoCo: +`bpmn-to-code-core`, `bpmn-to-code-runtime`, `bpmn-to-code-web`, `bpmn-to-code-testing`. +`bpmn-to-code-gradle` (TestKit), `bpmn-to-code-maven` (thin Java Mojos) and +`bpmn-to-code-architecture-tests` (Konsist, no main sources) are excluded. + +### Implementation + +- Versions live in `gradle/libs.versions.toml` (`pitest`, `pitestCore`, `pitestJunit5`). +- The plugin is declared `apply false` at the root and applied per-module in each target's own + `plugins {}` block — mirroring how `jacoco` is wired. Shared configuration sits in a + `plugins.withId("info.solidsoft.pitest")` block inside `subprojects {}`, next to the + existing `plugins.withId("jacoco")` block: PIT/engine versions, `targetClasses = io.miragon.*`, + HTML+XML reports, and the common `excludedClasses`. +- **Excluded classes** mirror the JaCoCo `coverageExclusions` plus the Kotlin/serialization + synthetics (`*$DefaultImpls`, `*$Companion`, `*$WhenMappings`, `*$$serializer`, + `*$$inlined$*`). Each module adds its own extras via `excludedClasses.addAll(...)` so they + extend, not replace, the shared list. `avoidCallsTo` additionally suppresses mutations of the + Kotlin null/resource intrinsics (while keeping PIT's logging-framework defaults). +- **Thresholds** are per-module `mutationThreshold` values, set a few points below each module's + coverage so the gate ratchets against regression rather than chasing a target: + + | Module | Mutation coverage | Threshold | + |---|---|---| + | `bpmn-to-code-core` | 84% | 80 | + | `bpmn-to-code-runtime` | 100% | 95 | + | `bpmn-to-code-testing` | 98% | 95 | + | `bpmn-to-code-web` | 28% | 22 | + + `bpmn-to-code-web` is the outstanding gap: mutation testing surfaced that its service layer + (`LibrarySourceProvider`, `WebGenerationService`, `WebJsonGenerationService`) is weakly + covered by mutation standards even though it clears the 75% JaCoCo line gate. The threshold + records that reality as a floor; it should be raised as those tests improve. The `core` + figure is likewise still climbing — its remaining survivors are concentrated in the BPMN + parsing/dialect adapters (`BpmnStructureReader`, `CamundaDialect`, `ForeignXmlReader`). + +- **Configuration cache.** The build enables `org.gradle.configuration-cache` globally. The + `pitest` task is not configuration-cache compatible, so it must be run with + `--no-configuration-cache` (the same flag the publish workflows already use). Applying the + plugin does **not** break the normal cached build — only the `pitest` task needs the flag. + +### Running it + +Local runs are manual and on demand — PIT is far slower than the unit suite, so it is **not** +added to the Lefthook pre-push hook: + +```bash +./gradlew pitest --no-configuration-cache # all four modules +./gradlew :bpmn-to-code-core:pitest --no-configuration-cache # one module +``` + +In CI it runs as a separate, opt-in workflow (`.github/workflows/mutation-testing.yml`) on a +nightly schedule and via manual `workflow_dispatch`, not on the PR path. The job uploads the +HTML reports as an artifact and fails when any module regresses below its threshold. + +## Consequences + +### Positive +- Adds a fault-detection metric that line coverage cannot provide, and turns it into an + enforceable regression floor. +- Already surfaced a concrete gap (the web service layer) that the 75% line gate hid. +- Follows the established JaCoCo wiring pattern, so the build stays consistent and the + per-module exclusion lists have a single obvious home. + +### Negative +- PIT is slow, which is why it is nightly/opt-in rather than a PR gate — a regression can land + on `main` and only be caught by the next scheduled run. +- Kotlin synthetic mutants force an exclusion list that must be kept in sync with the JaCoCo + exclusions by hand (Arcmutate would remove this need, at a licence cost). +- The `web` threshold is low enough to be a weak gate until its service tests improve. +- Running PIT requires remembering `--no-configuration-cache`; forgetting it fails fast with a + clear cache error rather than silently. + +## Alternatives Considered + +**Report-only (no gate).** Simplest and never blocks, but a score nobody enforces tends to +drift; the project already prefers hard gates (JaCoCo, Detekt), so a gate is consistent. + +**Run on every PR.** Best feedback latency, but PIT's runtime would dominate the PR build for a +metric that changes slowly — a poor trade for a fast-moving PR queue. Nightly + on-demand keeps +signal without taxing every PR. + +**Adopt Arcmutate for first-class Kotlin support.** Would eliminate the synthetic-mutant noise +and the exclusion list, but it is a commercial plugin; the free exclusion approach is adequate +for the current surface. diff --git a/docs/contributing/adr/index.md b/docs/contributing/adr/index.md index c9b1d8a2..846d4be7 100644 --- a/docs/contributing/adr/index.md +++ b/docs/contributing/adr/index.md @@ -75,3 +75,4 @@ Document decisions that: ### Testing - [ADR 013: Testing Module](013-testing-module.md) - Arch-Unit style BPMN model validation framework +- [ADR 019: Mutation Testing with PIT](019-mutation-testing-with-pit.md) - PIT mutation testing with a hard per-module score gate diff --git a/docs/contributing/index.md b/docs/contributing/index.md index 5e4b9791..c814512c 100644 --- a/docs/contributing/index.md +++ b/docs/contributing/index.md @@ -41,6 +41,21 @@ plus the pre-push hook — no baseline, no silent suppressions. The only scoped ktor wildcard-import allowance and no hard line-length limit (`.editorconfig`) and the generated runtime fixture (excluded in both `.editorconfig` and `bpmn-to-code-runtime/build.gradle.kts`). +## Mutation testing (PIT) + +[PIT](https://pitest.org/) measures how well the tests actually detect faults, complementing +the line-coverage gate. It runs on demand locally (it is slower than the unit suite, so it is +not part of the pre-push hook) and as a nightly / manual CI workflow. See +[ADR 019](adr/019-mutation-testing-with-pit.md) for scope and thresholds. + +```bash +./gradlew pitest --no-configuration-cache # all instrumented modules +./gradlew :bpmn-to-code-core:pitest --no-configuration-cache # one module +``` + +`--no-configuration-cache` is required: the `pitest` task is not configuration-cache +compatible. HTML reports land in each module's `build/reports/pitest/`. + ## Skipping hooks ```bash diff --git a/gradle/libs.versions.toml b/gradle/libs.versions.toml index 5cc3ed7d..5af42bbe 100644 --- a/gradle/libs.versions.toml +++ b/gradle/libs.versions.toml @@ -21,6 +21,9 @@ jsonSchemaValidator = "1.5.6" dokka = "2.2.0" detekt = "1.23.8" ktlintPlugin = "0.14.0" +pitest = "1.19.0" +pitestCore = "1.19.4" +pitestJunit5 = "1.2.3" [libraries] # Plugin dependencies @@ -73,3 +76,4 @@ shadow = { id = "com.gradleup.shadow", version.ref = "shadow" } dokka = { id = "org.jetbrains.dokka", version.ref = "dokka" } detekt = { id = "io.gitlab.arturbosch.detekt", version.ref = "detekt" } ktlint = { id = "io.github.usefulness.ktlint-gradle-plugin", version.ref = "ktlintPlugin" } +pitest = { id = "info.solidsoft.pitest", version.ref = "pitest" } From d2ca9e8367633075cc1c5b9133d323912755a495 Mon Sep 17 00:00:00 2001 From: Marco Schaeck Date: Fri, 14 Aug 2026 07:39:52 +0200 Subject: [PATCH 2/4] test(web): raise web mutation coverage to 95% and fix version lookup MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add kotlin-logging (io.github.oshai) to avoidCallsTo — the bulk of web's survivors were logging-call noise — add real LibrarySourceProvider tests plus response-DTO assertions, and raise the web mutationThreshold to 90. Fix a bug the new tests surfaced: LibrarySourceProvider read the wrong properties key (projectVersion instead of version), so the playground's runtime-dependency snippet always rendered "unknown". --- bpmn-to-code-web/build.gradle.kts | 2 +- .../bpmn/web/service/LibrarySourceProvider.kt | 2 +- .../web/service/LibrarySourceProviderTest.kt | 42 +++++++++++++++++++ .../web/service/WebGenerationServiceTest.kt | 6 +++ .../service/WebJsonGenerationServiceTest.kt | 2 + build.gradle.kts | 1 + .../adr/019-mutation-testing-with-pit.md | 22 ++++++---- 7 files changed, 66 insertions(+), 11 deletions(-) create mode 100644 bpmn-to-code-web/src/test/kotlin/io/miragon/bpmn/web/service/LibrarySourceProviderTest.kt diff --git a/bpmn-to-code-web/build.gradle.kts b/bpmn-to-code-web/build.gradle.kts index cacab83a..05724a55 100644 --- a/bpmn-to-code-web/build.gradle.kts +++ b/bpmn-to-code-web/build.gradle.kts @@ -125,7 +125,7 @@ pitest { "io.miragon.bpmn.web.config.*", "io.miragon.bpmn.web.model.ConfigResponse*", ) - mutationThreshold.set(22) + mutationThreshold.set(90) } tasks.named("processResources") { diff --git a/bpmn-to-code-web/src/main/kotlin/io/miragon/bpmn/web/service/LibrarySourceProvider.kt b/bpmn-to-code-web/src/main/kotlin/io/miragon/bpmn/web/service/LibrarySourceProvider.kt index b79df663..8f05e8f4 100644 --- a/bpmn-to-code-web/src/main/kotlin/io/miragon/bpmn/web/service/LibrarySourceProvider.kt +++ b/bpmn-to-code-web/src/main/kotlin/io/miragon/bpmn/web/service/LibrarySourceProvider.kt @@ -53,7 +53,7 @@ class LibrarySourceProvider { val properties = stream.use { Properties().apply { load(it) } } - return properties.getProperty("projectVersion")?.takeIf { it.isNotBlank() } ?: "unknown" + return properties.getProperty("version")?.takeIf { it.isNotBlank() } ?: "unknown" } private fun readResource(path: String): String? { diff --git a/bpmn-to-code-web/src/test/kotlin/io/miragon/bpmn/web/service/LibrarySourceProviderTest.kt b/bpmn-to-code-web/src/test/kotlin/io/miragon/bpmn/web/service/LibrarySourceProviderTest.kt new file mode 100644 index 00000000..aa75e47a --- /dev/null +++ b/bpmn-to-code-web/src/test/kotlin/io/miragon/bpmn/web/service/LibrarySourceProviderTest.kt @@ -0,0 +1,42 @@ +package io.miragon.bpmn.web.service + +import org.assertj.core.api.Assertions.assertThat +import org.junit.jupiter.api.Test + +class LibrarySourceProviderTest { + + private val underTest = LibrarySourceProvider() + + @Test + fun `libraryFiles exposes the bundled runtime Kotlin sources`() { + val files = underTest.libraryFiles() + + assertThat(files).isNotEmpty() + assertThat(files).allSatisfy { + assertThat(it.fileName).endsWith(".kt") + assertThat(it.content).isNotBlank() + assertThat(it.processId).isEqualTo("bpmn-to-code-runtime") + } + } + + @Test + fun `libraryFiles are loaded once and cached`() { + assertThat(underTest.libraryFiles()).isSameAs(underTest.libraryFiles()) + } + + @Test + fun `runtimeDependency describes the published runtime artifact with a resolved version`() { + val dependency = underTest.runtimeDependency() + + assertThat(dependency.group).isEqualTo("io.miragon") + assertThat(dependency.artifact).isEqualTo("bpmn-to-code-runtime") + assertThat(dependency.version).isNotEqualTo("unknown") + assertThat(dependency.version).matches("\\d+\\..*") + assertThat(dependency.gradleSnippet) + .isEqualTo("implementation(\"io.miragon:bpmn-to-code-runtime:${dependency.version}\")") + assertThat(dependency.mavenSnippet) + .contains("io.miragon") + .contains("bpmn-to-code-runtime") + .contains("${dependency.version}") + } +} diff --git a/bpmn-to-code-web/src/test/kotlin/io/miragon/bpmn/web/service/WebGenerationServiceTest.kt b/bpmn-to-code-web/src/test/kotlin/io/miragon/bpmn/web/service/WebGenerationServiceTest.kt index 45f07ddd..6a469214 100644 --- a/bpmn-to-code-web/src/test/kotlin/io/miragon/bpmn/web/service/WebGenerationServiceTest.kt +++ b/bpmn-to-code-web/src/test/kotlin/io/miragon/bpmn/web/service/WebGenerationServiceTest.kt @@ -31,6 +31,7 @@ class WebGenerationServiceTest { val response = underTest.generate(request) // then: a Kotlin file is generated containing the process constant + assertThat(request.files.first().fileName).isEqualTo("c8-subscribe-newsletter.bpmn") assertThat(response.success).describedAs("Generation should succeed").isTrue() assertThat(response.files).describedAs("Should generate at least one file").isNotEmpty() assertThat(response.error).describedAs("Should not have errors").isNull() @@ -38,6 +39,11 @@ class WebGenerationServiceTest { assertThat(generatedFile.fileName).describedAs("Should generate Kotlin file").endsWith(".kt") assertThat(generatedFile.content).describedAs("Should contain Kotlin object declaration").contains("object") assertThat(generatedFile.content).describedAs("Should contain process ID").contains("newsletterSubscription") + assertThat(generatedFile.processId).describedAs("Should carry the process id").isEqualTo("newsletterSubscription") + + // and: the bundled runtime sources and dependency snippet ride along with the response + assertThat(response.libraryFiles).describedAs("Should bundle runtime library sources").isNotEmpty() + assertThat(response.runtimeDependency).describedAs("Should include the runtime dependency").isNotNull() } @Test diff --git a/bpmn-to-code-web/src/test/kotlin/io/miragon/bpmn/web/service/WebJsonGenerationServiceTest.kt b/bpmn-to-code-web/src/test/kotlin/io/miragon/bpmn/web/service/WebJsonGenerationServiceTest.kt index bd310a14..42f0a2f5 100644 --- a/bpmn-to-code-web/src/test/kotlin/io/miragon/bpmn/web/service/WebJsonGenerationServiceTest.kt +++ b/bpmn-to-code-web/src/test/kotlin/io/miragon/bpmn/web/service/WebJsonGenerationServiceTest.kt @@ -29,12 +29,14 @@ class WebJsonGenerationServiceTest { val response = underTest.generate(request) // then: generation succeeds + assertThat(request.files.first().fileName).isEqualTo("c8-newsletter.bpmn") assertThat(response.success).describedAs("Generation should succeed but got: ${response.error}").isTrue() assertThat(response.files).isNotEmpty() assertThat(response.error).isNull() val file = response.files.first() assertThat(file.fileName).endsWith(".json") assertThat(file.processId).isEqualTo("newsletterSubscription") + assertThat(file.content).describedAs("Should carry the generated JSON body").isNotBlank() } @Test diff --git a/build.gradle.kts b/build.gradle.kts index 980a62ab..515dc58b 100644 --- a/build.gradle.kts +++ b/build.gradle.kts @@ -93,6 +93,7 @@ subprojects { listOf( "kotlin.jvm.internal.Intrinsics", "kotlin.io.CloseableKt", + "io.github.oshai.kotlinlogging", "java.util.logging", "org.slf4j", "org.apache.log4j", diff --git a/docs/contributing/adr/019-mutation-testing-with-pit.md b/docs/contributing/adr/019-mutation-testing-with-pit.md index ed25a275..b80c0305 100644 --- a/docs/contributing/adr/019-mutation-testing-with-pit.md +++ b/docs/contributing/adr/019-mutation-testing-with-pit.md @@ -55,7 +55,8 @@ Applied to the four logic-bearing modules only — the same set that carries JaC synthetics (`*$DefaultImpls`, `*$Companion`, `*$WhenMappings`, `*$$serializer`, `*$$inlined$*`). Each module adds its own extras via `excludedClasses.addAll(...)` so they extend, not replace, the shared list. `avoidCallsTo` additionally suppresses mutations of the - Kotlin null/resource intrinsics (while keeping PIT's logging-framework defaults). + Kotlin null/resource intrinsics and the `kotlin-logging` (`io.github.oshai`) calls, while + keeping PIT's logging-framework defaults. - **Thresholds** are per-module `mutationThreshold` values, set a few points below each module's coverage so the gate ratchets against regression rather than chasing a target: @@ -64,14 +65,16 @@ Applied to the four logic-bearing modules only — the same set that carries JaC | `bpmn-to-code-core` | 84% | 80 | | `bpmn-to-code-runtime` | 100% | 95 | | `bpmn-to-code-testing` | 98% | 95 | - | `bpmn-to-code-web` | 28% | 22 | + | `bpmn-to-code-web` | 95% | 90 | - `bpmn-to-code-web` is the outstanding gap: mutation testing surfaced that its service layer - (`LibrarySourceProvider`, `WebGenerationService`, `WebJsonGenerationService`) is weakly - covered by mutation standards even though it clears the 75% JaCoCo line gate. The threshold - records that reality as a floor; it should be raised as those tests improve. The `core` - figure is likewise still climbing — its remaining survivors are concentrated in the BPMN - parsing/dialect adapters (`BpmnStructureReader`, `CamundaDialect`, `ForeignXmlReader`). + `bpmn-to-code-web` started at 28%: most of that was `kotlin-logging` call noise (now handled + by `avoidCallsTo`), and the rest was a genuinely untested `LibrarySourceProvider`. Testing it + properly surfaced a real bug — `loadProjectVersion()` read the wrong properties key + (`projectVersion` instead of `version`), so the playground's runtime-dependency snippet always + showed `unknown`; fixed here. Its three residual survivors are defensive "resource missing" + fallbacks unreachable in a correctly built artifact (test strength is 100%). The `core` figure + is still climbing — its remaining survivors are concentrated in the BPMN parsing/dialect + adapters (`BpmnStructureReader`, `CamundaDialect`, `ForeignXmlReader`). - **Configuration cache.** The build enables `org.gradle.configuration-cache` globally. The `pitest` task is not configuration-cache compatible, so it must be run with @@ -97,7 +100,8 @@ HTML reports as an artifact and fails when any module regresses below its thresh ### Positive - Adds a fault-detection metric that line coverage cannot provide, and turns it into an enforceable regression floor. -- Already surfaced a concrete gap (the web service layer) that the 75% line gate hid. +- Already paid for itself: surfaced (and fixed) a real bug in the web playground's + runtime-dependency version lookup that the 75% line gate had missed. - Follows the established JaCoCo wiring pattern, so the build stays consistent and the per-module exclusion lists have a single obvious home. From a85b9c37854bbc05a3ab054cc021403fb44c3086 Mon Sep 17 00:00:00 2001 From: Marco Schaeck Date: Fri, 14 Aug 2026 07:42:28 +0200 Subject: [PATCH 3/4] chore: drop explanatory comments from PIT build config --- .github/workflows/mutation-testing.yml | 3 --- build.gradle.kts | 4 ---- 2 files changed, 7 deletions(-) diff --git a/.github/workflows/mutation-testing.yml b/.github/workflows/mutation-testing.yml index 2d3e9f7a..e5b40049 100644 --- a/.github/workflows/mutation-testing.yml +++ b/.github/workflows/mutation-testing.yml @@ -35,9 +35,6 @@ jobs: distribution: 'temurin' # Step 4: Run mutation testing for the logic-bearing modules. - # --no-configuration-cache: the pitest task is not configuration-cache compatible. - # --continue: run every module even if one regresses below its threshold, so all - # reports are produced before the job fails. - name: Run PIT run: | ./gradlew --no-configuration-cache --continue --warning-mode all \ diff --git a/build.gradle.kts b/build.gradle.kts index 515dc58b..a644682b 100644 --- a/build.gradle.kts +++ b/build.gradle.kts @@ -13,7 +13,6 @@ plugins { alias(libs.plugins.pitest) apply false } -// Captured here: the `libs` accessor is not resolvable inside the subprojects {} block below. val pitestCoreVersion = libs.versions.pitestCore.get() val pitestJunit5Version = libs.versions.pitestJunit5.get() @@ -86,9 +85,6 @@ subprojects { outputFormats.set(listOf("HTML", "XML")) timestampedReports.set(false) threads.set(Runtime.getRuntime().availableProcessors()) - - // Suppress mutations of Kotlin compiler-generated null/resource intrinsics - // (unkillable synthetic checks), keeping PIT's logging defaults. avoidCallsTo.set( listOf( "kotlin.jvm.internal.Intrinsics", From 86c50c7558d195e62aaa50007b71040389e0abd6 Mon Sep 17 00:00:00 2001 From: Marco Schaeck Date: Fri, 14 Aug 2026 07:48:40 +0200 Subject: [PATCH 4/4] style: apply ktlint formatting to new PIT tests --- .../miragon/bpmn/runtime/RuntimeTypesTest.kt | 4 +--- .../miragon/bpmn/testing/BpmnValidatorTest.kt | 20 +++++++++---------- 2 files changed, 10 insertions(+), 14 deletions(-) diff --git a/bpmn-to-code-runtime/src/test/kotlin/io/miragon/bpmn/runtime/RuntimeTypesTest.kt b/bpmn-to-code-runtime/src/test/kotlin/io/miragon/bpmn/runtime/RuntimeTypesTest.kt index 5590bb92..f95fa2e7 100644 --- a/bpmn-to-code-runtime/src/test/kotlin/io/miragon/bpmn/runtime/RuntimeTypesTest.kt +++ b/bpmn-to-code-runtime/src/test/kotlin/io/miragon/bpmn/runtime/RuntimeTypesTest.kt @@ -146,9 +146,7 @@ class RuntimeTypesTest { assertThat(node.hashCode()).isEqualTo(same.hashCode()) } - private fun flowNode(id: String): AbstractFlowNode { - return object : AbstractFlowNode(ElementId(id), "SERVICE_TASK") {} - } + private fun flowNode(id: String): AbstractFlowNode = object : AbstractFlowNode(ElementId(id), "SERVICE_TASK") {} @Test fun `InputOutputMapping keeps target plus source or sourceExpression`() { diff --git a/bpmn-to-code-testing/src/test/kotlin/io/miragon/bpmn/testing/BpmnValidatorTest.kt b/bpmn-to-code-testing/src/test/kotlin/io/miragon/bpmn/testing/BpmnValidatorTest.kt index 992914ed..0cfa3569 100644 --- a/bpmn-to-code-testing/src/test/kotlin/io/miragon/bpmn/testing/BpmnValidatorTest.kt +++ b/bpmn-to-code-testing/src/test/kotlin/io/miragon/bpmn/testing/BpmnValidatorTest.kt @@ -151,17 +151,15 @@ class BpmnValidatorTest { override val severity: Severity, override val phase: ValidationPhase = ValidationPhase.PRE_MERGE, ) : SingleModelValidationRule { - override fun validate(context: SingleModelValidationContext): List { - return listOf( - ValidationViolation( - ruleId = id, - severity = severity, - elementId = null, - processId = context.model.processId, - message = "violation from $id", - ), - ) - } + override fun validate(context: SingleModelValidationContext): List = listOf( + ValidationViolation( + ruleId = id, + severity = severity, + elementId = null, + processId = context.model.processId, + message = "violation from $id", + ), + ) } private class AlwaysFailingMandatoryRule : SingleModelValidationRule {