From 64beb19f79c7b8974053924f2367cc8c9c8305c2 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 5 Sep 2026 09:37:45 +0000 Subject: [PATCH] fix(build): give PIT the same SLF4J exclusion as Surefire, and cover FlashAttn MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two separate failures were stacked on the `Java Tests Ubuntu` job, and the first was hiding the second. 1. PIT aborted with "5 tests did not pass without mutation ... requires a green suite" — on a suite Surefire had just reported green (1729 tests, 0 failures). Cause: two SLF4J providers on the test classpath. slf4j-simple arrives at runtime scope because it is the binding shipped in the fat jar, logback-classic at test scope because LogCaptor requires logback specifically; SLF4J's ServiceLoader picks one arbitrarily and every LogCaptor assertion fails when it picks slf4j-simple. #411 added the binding and the Surefire `classpathDependencyExcludes` for exactly this, but PIT builds its OWN classpath and never reads Surefire's configuration, so the mutation run still saw both. pitest-maven has the same parameter, modelled on Surefire's; it is now set to match. Nothing about the artifact changes. It went unseen because every publish run between #411 and now was cancelled; run 897 is the first to complete since. 2. With that cleared, PIT actually ran and surfaced the survivor that run 887 had already reported before #411 ever landed: "Mutation score of 99 is below threshold of 100". `net.ladenthin.llama.args.*` is on the PIT target list at threshold 100, and FlashAttn — added in #408 — was the one enum in that package with no test, so `getArgValue()`'s `return argValue` had no killer. Adds FlashAttnTest in the shape every sibling enum already uses. The values are a wire contract, not labels: since llama.cpp b10273 `--flash-attn` takes a mandatory value, so a wrong or empty token makes the parser eat the next argv entry and the load fails naming a flag the caller never set. Verified locally, both directions: the failure reproduces on the parent commit with the exact same five test names, and with the fix PIT reports 319/319 mutations killed (100%) and `mvn clean verify` is green at 1486 tests. CLAUDE.md now names the PIT half of the rule explicitly, next to the existing "spotbugs/spotless bind to verify" trap, including why srcmorph solves the same clash by excluding at the dependency instead (there both providers arrive transitively). This does NOT address the macOS 15 failures in the same run; those are a separate, bump-introduced regression and are reported alongside this change. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01AnNYn8W1xuVxVJtyL34GyH --- CLAUDE.md | 23 ++++++++++++---- llama/pom.xml | 19 ++++++++++++++ .../ladenthin/llama/args/FlashAttnTest.java | 26 +++++++++++++++++++ 3 files changed, 63 insertions(+), 5 deletions(-) create mode 100644 llama/src/test/java/net/ladenthin/llama/args/FlashAttnTest.java diff --git a/CLAUDE.md b/CLAUDE.md index 8708f501..cd7905e4 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -1667,11 +1667,24 @@ It runs twice: in the `package` job over `llama/target` (every classifier jar pl jar, as early as they exist), and again in `smoke-fatjar-linux` over the downloaded `fatjars/` — `package-fatjars` rewrites those zips, and they are the artifacts users actually download. -**Surefire excludes `org.slf4j:slf4j-simple` from the test classpath** (`classpathDependencyExcludes`). -Runtime scope is on the test classpath too, and LogCaptor (test scope) requires logback specifically — -with both providers present it fails with *"SLF4J Logger implementation should be of the type -[ch.qos.logback.classic.Logger]"*. The exclusion leaves logback the sole provider in tests and does -not touch the artifact. +**Surefire AND PIT both exclude `org.slf4j:slf4j-simple` from the test classpath** +(`classpathDependencyExcludes` in each). Runtime scope is on the test classpath too, and LogCaptor +(test scope) requires logback specifically — with both providers present SLF4J's `ServiceLoader` +picks one arbitrarily and the LogCaptor assertions fail. The exclusions leave logback the sole +provider in tests and do not touch the artifact: the jar and the fat jar still ship slf4j-simple. + +**The PIT half is not redundant, and forgetting it is a trap worth naming.** `spotbugs:check` and +`spotless:check` bind to `verify`, so `mvn test` misses them — a different trap. This one is +sharper: **PIT builds its own classpath and never reads Surefire's configuration**, so a +Surefire-only exclusion leaves the mutation run with two providers. It then aborts the whole gate +with *"N tests did not pass without mutation … requires a green suite"* — a red gate on a suite +Surefire had just reported green, which reads like a PIT bug rather than a classpath one. This +shipped once (#411 added the binding with only the Surefire exclusion; the five affected +LogCaptor tests reddened `Java Tests Ubuntu` and the failure went unseen because every publish run +in between was cancelled). The sibling repo srcmorph hit the identical thing and solved it a +different way — there both providers arrive transitively, so it excludes at the dependency instead. +Same rule either way: **one SLF4J provider on the test classpath, enforced everywhere a test +classpath is built.** ## SpotBugs Suppressions diff --git a/llama/pom.xml b/llama/pom.xml index 4b77c3bd..a290ddad 100644 --- a/llama/pom.xml +++ b/llama/pom.xml @@ -832,6 +832,25 @@ SPDX-License-Identifier: MIT 100 30000 + + + org.slf4j:slf4j-simple + diff --git a/llama/src/test/java/net/ladenthin/llama/args/FlashAttnTest.java b/llama/src/test/java/net/ladenthin/llama/args/FlashAttnTest.java new file mode 100644 index 00000000..382dfb97 --- /dev/null +++ b/llama/src/test/java/net/ladenthin/llama/args/FlashAttnTest.java @@ -0,0 +1,26 @@ +// SPDX-FileCopyrightText: 2026 Bernard Ladenthin +// +// SPDX-License-Identifier: MIT + +package net.ladenthin.llama.args; + +import java.util.Arrays; +import java.util.Collection; + +/** + * The three {@code --flash-attn} values, pinned to the exact tokens llama.cpp's parser accepts. + * + *

These strings are a wire contract, not labels: since llama.cpp b10273 the option takes a + * mandatory value, so a wrong or empty token is not a cosmetic defect — the parser consumes the + * following argv entry and the model load fails naming a flag the caller never set.

+ */ +public class FlashAttnTest extends AbstractCliArgEnumTest { + + public static Collection data() { + return Arrays.asList(new Object[][] { + {FlashAttn.ON, "on", 3}, + {FlashAttn.OFF, "off", 3}, + {FlashAttn.AUTO, "auto", 3}, + }); + } +}