Skip to content

Commit 64beb19

Browse files
committed
fix(build): give PIT the same SLF4J exclusion as Surefire, and cover FlashAttn
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 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01AnNYn8W1xuVxVJtyL34GyH
1 parent df58f4a commit 64beb19

3 files changed

Lines changed: 63 additions & 5 deletions

File tree

‎CLAUDE.md‎

Lines changed: 18 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1667,11 +1667,24 @@ It runs twice: in the `package` job over `llama/target` (every classifier jar pl
16671667
jar, as early as they exist), and again in `smoke-fatjar-linux` over the downloaded `fatjars/` —
16681668
`package-fatjars` rewrites those zips, and they are the artifacts users actually download.
16691669

1670-
**Surefire excludes `org.slf4j:slf4j-simple` from the test classpath** (`classpathDependencyExcludes`).
1671-
Runtime scope is on the test classpath too, and LogCaptor (test scope) requires logback specifically —
1672-
with both providers present it fails with *"SLF4J Logger implementation should be of the type
1673-
[ch.qos.logback.classic.Logger]"*. The exclusion leaves logback the sole provider in tests and does
1674-
not touch the artifact.
1670+
**Surefire AND PIT both exclude `org.slf4j:slf4j-simple` from the test classpath**
1671+
(`classpathDependencyExcludes` in each). Runtime scope is on the test classpath too, and LogCaptor
1672+
(test scope) requires logback specifically — with both providers present SLF4J's `ServiceLoader`
1673+
picks one arbitrarily and the LogCaptor assertions fail. The exclusions leave logback the sole
1674+
provider in tests and do not touch the artifact: the jar and the fat jar still ship slf4j-simple.
1675+
1676+
**The PIT half is not redundant, and forgetting it is a trap worth naming.** `spotbugs:check` and
1677+
`spotless:check` bind to `verify`, so `mvn test` misses them — a different trap. This one is
1678+
sharper: **PIT builds its own classpath and never reads Surefire's configuration**, so a
1679+
Surefire-only exclusion leaves the mutation run with two providers. It then aborts the whole gate
1680+
with *"N tests did not pass without mutation … requires a green suite"* — a red gate on a suite
1681+
Surefire had just reported green, which reads like a PIT bug rather than a classpath one. This
1682+
shipped once (#411 added the binding with only the Surefire exclusion; the five affected
1683+
LogCaptor tests reddened `Java Tests Ubuntu` and the failure went unseen because every publish run
1684+
in between was cancelled). The sibling repo srcmorph hit the identical thing and solved it a
1685+
different way — there both providers arrive transitively, so it excludes at the dependency instead.
1686+
Same rule either way: **one SLF4J provider on the test classpath, enforced everywhere a test
1687+
classpath is built.**
16751688

16761689
## SpotBugs Suppressions
16771690

‎llama/pom.xml‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -832,6 +832,25 @@ SPDX-License-Identifier: MIT
832832
</targetTests>
833833
<mutationThreshold>100</mutationThreshold>
834834
<timeoutConstant>30000</timeoutConstant>
835+
<!--
836+
The same exclusion Surefire carries above, and it has to be repeated here
837+
because PIT builds its OWN classpath and does not read Surefire's
838+
configuration. Two SLF4J providers are on the test classpath: slf4j-simple
839+
arrives at runtime scope (it is the binding shipped in the fat jar) and
840+
logback-classic at test scope (LogCaptor requires logback specifically).
841+
SLF4J's ServiceLoader then picks one arbitrarily; when it picks
842+
slf4j-simple, every LogCaptor assertion fails and PIT aborts the whole run
843+
with "N tests did not pass without mutation ... requires a green suite" —
844+
a red gate on a suite Surefire had just run green, which is a genuinely
845+
confusing failure to land on.
846+
847+
The parameter is pitest-maven's own, modelled on Surefire's and taking the
848+
same "groupId:artifactId" form. It changes nothing about the artifact: the
849+
jar and the fat jar still ship slf4j-simple.
850+
-->
851+
<classpathDependencyExcludes>
852+
<classpathDependencyExclude>org.slf4j:slf4j-simple</classpathDependencyExclude>
853+
</classpathDependencyExcludes>
835854
</configuration>
836855
</plugin>
837856
</plugins>
Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
// SPDX-FileCopyrightText: 2026 Bernard Ladenthin <bernard.ladenthin@gmail.com>
2+
//
3+
// SPDX-License-Identifier: MIT
4+
5+
package net.ladenthin.llama.args;
6+
7+
import java.util.Arrays;
8+
import java.util.Collection;
9+
10+
/**
11+
* The three {@code --flash-attn} values, pinned to the exact tokens llama.cpp's parser accepts.
12+
*
13+
* <p>These strings are a wire contract, not labels: since llama.cpp b10273 the option takes a
14+
* mandatory value, so a wrong or empty token is not a cosmetic defect — the parser consumes the
15+
* following argv entry and the model load fails naming a flag the caller never set.</p>
16+
*/
17+
public class FlashAttnTest extends AbstractCliArgEnumTest<FlashAttn> {
18+
19+
public static Collection<Object[]> data() {
20+
return Arrays.asList(new Object[][] {
21+
{FlashAttn.ON, "on", 3},
22+
{FlashAttn.OFF, "off", 3},
23+
{FlashAttn.AUTO, "auto", 3},
24+
});
25+
}
26+
}

0 commit comments

Comments
 (0)