Files
kst4contest/docs/superpowers/specs/2026-10-11-test-coverage-design.md
T
Rsclub2_2andClaude Fable 5.1 5491cbcf8b Test suite cleanup, JaCoCo coverage report and tests before packaging (#62) (#112)
* docs: add the test-coverage design spec (#62)

Records the state found for issue #62 (JUnit classes under src/main that
never run, test libraries in the shipped app image, dead 2023/2024 scratch
tests, no coverage figure), the decisions taken (delete, move to src/test,
JaCoCo report-only without a threshold, tests before packaging), what is
deliberately left to separate issues, and the definition of done.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

* test: move the JUnit classes out of src/main and drop the dead 2023 tests (#62)

Gradle's test task scans src/test only, so the six JUnit classes under
src/main compiled on every build and were never executed. They move to
src/test of the module that owns the class under test, keeping their
package names: On4KstProtocolTest, On4KstSocketThreadTest and
ChatMemberDirectionOpportunityTest within core; Utils4KSTFrequencyTest,
FrequencyTextParserTest and ChatPreferencesStationMapVisibilityTest from
app-desktop into core, next to the sibling tests that already live there,
because they test core classes only. All 26 of them run and pass.

The fully commented-out MessageBusManagementThreadFrequencyContextTest is
deleted. Eight files under core/src/test/java/kst4contest/test from
2023/2024 go as well: they carry no assertion, five of them annotate a
static main() with @Test, which Jupiter never runs, and some write the real
preferences file, send real UDP broadcasts or open the real user database
when started by hand. The five main() scratch programs under src/main
(ReadUDPByWintestThreadTest, PatternMatcherTest, mapTest, RegexTester,
locatorUtils/TestLocatorUtils) and MockKstServer, which only an unused
import in ChatController still referenced, are removed too.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

* build: make JUnit and Mockito test-only dependencies (#62)

With no test class left under src/main, the implementation declarations of
junit-jupiter-api and mockito-core in core and app-desktop lose their only
justification. They put junit-jupiter-api, junit-platform-commons,
opentest4j, mockito-core, byte-buddy, byte-buddy-agent and objenesis on the
runtime classpath and into lib/app of every app image, Flatpak and
installer. The duplicate catalog entries junit-jupiter-api,
mockito-core-compile and the mockitoCompile version go with them.

Verified: `:app-desktop:dependencies --configuration runtimeClasspath` no
longer lists any of the seven artifacts.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

* docs: state that test failures fail the build and tests live under src/test (#62)

AGENTS.md and docs/PROJECT_CONTEXT.md still said the build configuration
had allowed test failures to pass. That was true of the Maven build; under
Gradle a test failure fails the build, and only PMD and SpotBugs run with
ignoreFailures. Both documents now say so, note that test classes live under
src/test only and that JUnit and Mockito are test-only dependencies, and
keep the rule to read the test-result XML rather than the exit code.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

* build: add a JaCoCo coverage report (#62)

The jacoco plugin is applied to every module from the root build with the
tool version held in the catalog. jacocoTestReport now depends on test and
writes XML and HTML per module; a root task jacocoAggregatedReport merges
core and app-desktop into build/reports/jacoco/aggregated/ (XML plus HTML).
i18n-generator is excluded: it is a build tool and its coverage says nothing
about the shipped application. Kotlin classes are counted, because the
Kotlin plugin adds its output to the main source set the report reads.

Report only, on purpose. There is no jacocoTestCoverageVerification and no
threshold: a minimum set at today's number would trip on the next unrelated
change, and one set at a target would fail every build until tests exist for
the number's sake. The figure is published so it can be read when deciding
what to test next; the reasoning is in
docs/superpowers/specs/2026-10-11-test-coverage-design.md.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

* ci: run the tests before packaging and publish coverage (#62)

The nightly and tagged-release workflows built seven package formats from
commits nobody had tested on the way: pr-compile-check.yml runs the suite
for pull requests only, and main can be pushed to directly. Both workflows
now carry a copy of that test job, including the libgl1 step and the
XML-based summary, and every packaging job declares `needs: test`. Copied
rather than factored into a reusable workflow on purpose: the comments in
the job explain decisions the packaging workflows should carry with them,
and three copies read better than one indirection. build-docs-pdf runs
pandoc only and is left alone; publish-flatpak-repo and release-tag are
gated through the jobs they already need.

The PR test job additionally runs jacocoTestReport and
jacocoAggregatedReport once the tests have run (not `if: always()`: a
report from an incomplete run would mislead), prints a line/branch coverage
table per module and aggregated into the job summary from the JaCoCo XML,
and uploads the aggregated HTML report as the coverage-report artifact for
14 days whenever it exists.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

* docs: describe the coverage report in the project context (#62)

A short paragraph on JaCoCo in docs/PROJECT_CONTEXT.md: report only and
why, where the per-module and aggregated reports land, which Gradle tasks
write them, that i18n-generator is excluded, how the PR job summary and the
coverage-report artifact show the figure, and that the nightly and
tagged-release workflows run the tests before any packaging job.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

* test: round-trip every ChatPreferences field through the XML file (#62)

Drive all public single-argument setters by reflection to non-default
values, write preferences.xml, read it back and compare every getter.
Fields that are not persisted on purpose are listed with their evidence;
two real reader/writer gaps (second-category locator, sniffer words) are
documented with a BUG marker instead of being fixed here. Two further
tests load the bundled template and a legacy file without configVersion.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

* test: cover the DBController schema migration from older user_version values (#62)

Open the seeded template database and hand-built release 1.1 and 1.3
schema fixtures with a DBController and verify that PRAGMA user_version
reaches the current level, every column of the current CREATE TABLE
statements exists, legacy callsign keys are merged to base callsigns and
existing rows and flags survive. A further test proves that reopening an
up-to-date database changes neither schema nor rows.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

* chore: drop the committed JVM crash report and ignore future ones (#62)

hs_err_pid26344.log in the repository root was a HotSpot crash dump that
had been committed by accident. It carries no information about the
project and must not travel with the sources.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

* fix: drop the dead QSO sniffer word list that corrupted the callsign list on load (#103)

ChatPreferences carried a second sniffer list, lstNotify_QSOSniffer_sniffedWordsList,
that no UI, no manual and no consumer outside ChatPreferences ever touched. It was
still written to preferences.xml as <snifferWords><w>, and the matching reader block
was a copy of the <snifferCallSigns> block: it cleared the word list and then added
every word, normalised as a callsign, to lstNotify_QSOSniffer_sniffedCallSignList.
Any entry that ever reached the word list therefore leaked into QSO monitoring on
every load.

Remove the field, its accessors, the write block and the read block. A <snifferWords>
element in an older preferences file is now ignored like any unknown element; the
callsign list and its persistence are unchanged.

ChatPreferencesSnifferListTest loads a file that carries both lists and asserts that
only the <snifferCallSigns> entry is monitored. The round-trip test loses its
exclusion entry for the removed setter.

Fixes #103

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

* fix: remove the second-category locator that could never be set (#104)

ChatPreferences declared stn_loginLocatorSecondCat with the compiled-in default
"jn49fk". Nothing wrote it to or read it from preferences.xml, no settings tab
exposed it and no caller ever invoked the setter, so the field always held its
default. On4KstConnectionManager.configureMainChat nevertheless compared that
default with the station's locator and, with the second chat enabled, raised the
warning "ON4KST uses one locator per TCP session. The second-chat locator ... is
ignored" whenever the station's locator was not jn49fk, which is practically every
installation.

ON4KST uses one locator per TCP session by design, so a second-category locator can
never become meaningful. Remove the field, its accessors, the comparison and the
warning; the rest of configureMainChat is unchanged. The round-trip test loses its
exclusion entry for the removed setter.

No automated test covers configureMainChat: it is private, works on the manager's
private Session and needs a mocked ChatController plus a live socket handshake, so a
harness would far exceed the scope of this fix. The compile of core and app-desktop
and the shorter exclusion list are the evidence that nothing references the member.

Fixes #104

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

---------

Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>
2026-10-11 12:13:31 +02:00

11 KiB

Konzept: Testabdeckung (#62)

Status: Konzept, freigegeben am 2026-10-11, umgesetzt im Zweig feature/62-test-coverage. Alle Zeilenanker beziehen sich auf main bei 4a90ae7d.

Issue #62 verlangt einen Blick auf die Testsuite als Ganzes: Was läuft, was sieht nur so aus, als liefe es, und wie viel des Codes erreicht die Suite tatsächlich. Dieses Dokument hält den vorgefundenen Stand fest, die getroffenen Entscheidungen und das, was bewusst eigenen Issues überlassen wird.

Das Problem

Vier Befunde, gemessen am Stand 4a90ae7d.

Sieben JUnit-Klassen liegen unter src/main und werden nie ausgeführt. Gradles test-Task durchsucht nur src/test. Deshalb werden core/src/main/java/kst4contest/controller/On4KstProtocolTest.java, On4KstSocketThreadTest.java, MessageBusManagementThreadFrequencyContextTest.java (vollständig auskommentiert), core/src/main/java/kst4contest/model/ChatMemberDirectionOpportunityTest.java und die drei Klassen unter app-desktop/src/main/java/kst4contest/test/ (Utils4KSTFrequencyTest, FrequencyTextParserTest, ChatPreferencesStationMapVisibilityTest) bei jedem Build kompiliert und zählen für nichts. Der Protokolltest darunter nagelt die exakten LOGINC|-, SDONE|- und ACHAT|-Frames fest, der Socket-Test genau ein CR/LF je Frame. Beides sind Invarianten, die AGENTS.md ausdrücklich nennt, und keine davon hat seit der Maven-Zeit ein Build geprüft.

Wegen dieser Klassen wird JUnit und Mockito mit der Anwendung ausgeliefert. core und app-desktop deklarieren implementation(libs.junit.jupiter.api) und implementation(libs.mockito.core.compile). Damit liegen junit-jupiter-api, junit-platform-commons, opentest4j, mockito-core, byte-buddy, byte-buddy-agent und objenesis auf dem Laufzeit-Klassenpfad und in lib/app jedes App-Image, Flatpak und Installers. docs/CONCEPT_IN_APP_UPDATE.md nennt das bereits als Vorbedingung für einen signierten Update-Pfad: Ein Bytecode-Manipulations-Agent in einem signierten Produktionsbinary ist das Falsche, was man erklären müsste.

Acht Dateien unter core/src/test/java/kst4contest/test/ sind toter Scratch-Code von 2023/2024. TestChatPreferences, TestReadUDPASListenerThread, TestReadUDPUCXListenerThread, UDPAirScoutSendMessages, testTimeGeneration, TestApplicationFileUtils, TestDBController und TestLocatorUtils enthalten keine einzige Assertion. Fünf davon tragen @Test an einer public static void main, die Jupiter nie ausführt; die anderen drei haben gar keine Annotation. Manche schreiben beim manuellen Start die echte Preferences-Datei, senden echten UDP-Broadcast oder öffnen die echte Nutzerdatenbank. Fünf weitere main()-Programme derselben Art liegen unter src/main (ReadUDPByWintestThreadTest, PatternMatcherTest, mapTest, RegexTester, locatorUtils/TestLocatorUtils), dazu MockKstServer, auf den nur noch ein ungenutzter Import in ChatController verweist.

Es gibt keine Abdeckungszahl. Die Suite ist durch die Compose-Migration auf mehrere hundert Tests gewachsen, aber niemand kann sagen, welchen Teil von core sie erreicht. Drei Bereiche sind bekannt dünn: der XML-Roundtrip von ChatPreferences jenseits der Handvoll Felder, die Einzeltests festnageln, die Schema-Migration in DBController, und die Netzwerk-Threads (ReadThread/WriteThread, die UDP-Listener), die bestenfalls je einen Loopback-Test haben.

Entscheidungen

Tote Tests löschen, nicht retten. Ein Test ohne Assertion dokumentiert nichts, und die mit Seiteneffekten sind eine Gefahr, sobald jemand „alle Tests“ aus der IDE startet. Ihre Gegenstände sind entweder anderswo abgedeckt (ChatPreferences*Test, DBControllerProfileDatabaseTest, DirectionUtilsConsoleOutputTest) oder werden zu den zwei neuen Testbereichen unten. Der auskommentierte MessageBusManagementThreadFrequencyContextTest wird ebenfalls gelöscht; was er abgedeckt hätte (MessageBusManagementThread.hasExplicitBareFrequencyContext und Band.fromPrefix), ist im Umsetzungsbericht vermerkt, damit es mit der Frequenz-Parsing-Arbeit aufgegriffen und nicht stillschweigend verloren wird.

Tests liegen ausschließlich unter src/test. Die sechs lebenden JUnit-Klassen wandern nach src/test des Moduls, dem die getestete Klasse gehört, mit unveränderten Paketnamen, neben die Geschwistertests, die dort schon liegen (FrequencyTextParserRegressionTest, ChatPreferencesStationMapClusteringTest). Die drei Klassen aus app-desktop testen ausschließlich core-Klassen und ziehen deshalb nach core. Ohne Testklasse unter src/main entfallen die beiden implementation-Deklarationen und die doppelten Katalogeinträge (junit-jupiter-api, mockito-core-compile, Version mockitoCompile). JUnit und Mockito sind nur noch testImplementation, was die vorhandenen testImplementation-Zeilen ohnehin schon sagten. Nachgewiesen über :app-desktop:dependencies --configuration runtimeClasspath ohne die sieben Artefakte und über die Jar-Liste von createDistributable.

JaCoCo nur als Bericht, ohne Schwellwert. Das jacoco-Plugin wird aus dem Root-Build auf jedes Modul angewandt, jacocoTestReport läuft nach test und schreibt XML und HTML je Modul, und ein Root-Task jacocoAggregatedReport führt core und app-desktop zu einem Bericht unter build/reports/jacoco/aggregated/ zusammen. i18n-generator bleibt außen vor: Es ist ein Bauwerkzeug, seine Abdeckung sagt nichts über die ausgelieferte Anwendung. Es gibt bewusst keine jacocoTestCoverageVerification und keinen Mindestprozentsatz. Ein heute gesetzter Schwellwert stünde bei der Zahl, die zufällig gerade herauskommt, und wäre damit ein Stolperdraht für die nächste unabhängige Änderung statt einer Aussage über Qualität; ein Schwellwert auf Zielniveau ließe jeden Build scheitern, bis jemand Tests um der Zahl willen schreibt. Der Bericht existiert, damit die Zahl in jedem Pull Request sichtbar ist und bei der Entscheidung hilft, was als Nächstes zu testen ist. Sie zu heben ist Sache der unten gelisteten Issues, nicht der Build-Konfiguration.

Tests laufen vor dem Packaging. Der Nightly- und der Release-Workflow bauen sieben Paketformate aus einem Commit, den unterwegs niemand getestet hat: pr-compile-check.yml führt die Suite nur für Pull Requests aus, und main kann direkte Pushes erhalten. Beide Workflows bekommen eine Kopie des test-Jobs aus pr-compile-check.yml, einschließlich des libgl1-Schritts und der XML-basierten Zusammenfassung, und jeder Packaging-Job deklariert needs: test. Der Job wird dupliziert statt in einen wiederverwendbaren Workflow ausgelagert: Drei Kopien von vierzig Zeilen lesen sich leichter als eine Indirektion, und die Kommentare im Job erklären Entscheidungen, die die Packaging-Workflows bei sich tragen sollen. Der PR-Workflow führt zusätzlich jacocoAggregatedReport aus, schreibt aus dem JaCoCo-XML eine Abdeckungstabelle in die Job-Zusammenfassung und lädt den aggregierten HTML-Bericht für vierzehn Tage als Artefakt coverage-report hoch.

Zwei neue Testbereiche entstehen im selben Zweig, aber in eigenen Commits. Issue #62 nennt den ChatPreferences-Roundtrip und die DBController-Schema-Migration als die zwei ungetesteten Risikobereiche, die sich jetzt schließen lassen. Es sind gewöhnliche Testergänzungen, die vom Aufräumen nicht abhängen; sie landen nach dem Aufräumen, damit ihre Wirkung im aggregierten Bericht sichtbar wird.

Bewusst außerhalb des Umfangs

Jeder dieser Punkte ist eine echte Lücke, die unterwegs gefunden wurde, und jeder wird ein eigenes Issue, weil jeder eine Entscheidung braucht, die dieses Aufräumen nicht nebenbei treffen sollte:

  • Refactoring von MessageBusManagementThread für Testbarkeit. Frequenzextraktion, Worked-Prüfung und der DX-Cluster-Spot-Pfad sind heute nur über einen konstruierten Thread mit gemockten Mitspielern erreichbar. Sie in testbare Einheiten zu teilen berührt die Nachrichten-Pipeline und braucht ein eigenes Konzept.
  • Loopback-Tests für die Netzwerk-Threads. On4KstSocketThreadTest deckt ReadThread und WriteThread ab; die UDP-Listener für Win-Test, UCXLog und AirScout und der PSTRotator-Client haben kein Gegenstück. Ein Loopback-Harness mit Port-0-Socket je Protokoll ist die richtige Form, aber das Framing jedes Protokolls verdient eine Prüfung, bevor ein Test es festnagelt.
  • Compose-Screen-Tests. Die State-Tests und die zwei Settings-Abdeckungsnetze prüfen, dass Setter aufgerufen werden; nur runComposeUiTest kann prüfen, dass ein Wert den Bildschirm erreicht hat. Welche Screens diesen Aufwand wert sind, ist eine Oberflächenentscheidung.
  • Pfadanalyse-Mathematik. PathPropagationAssessment und die Terrainprofil-Berechnung haben Layout-Tests, aber keine numerischen Referenzfälle. Referenzwerte brauchen eine Quelle, die sich zitieren lässt.
  • Entfernen von Thread.sleep. Mehrere Tests und einige Produktionspfade warten noch auf die Wanduhr. Sie durch injizierbare Uhren oder Latches zu ersetzen ist mechanisch, aber breit.

Definition von „fertig“ für #62

  • Unter keinem src/main bleibt eine mit @Test annotierte Klasse oder ein Import von org.junit oder org.mockito.
  • ./gradlew clean build führt die verschobenen Tests aus; die Ergebnis-XML unter */build/test-results/test/ meldet null Fehlschläge und null Fehler.
  • ./gradlew :app-desktop:dependencies --configuration runtimeClasspath listet keines von junit, mockito, byte-buddy, objenesis, opentest4j, und app-desktop/build/compose/binaries/main/app/KST4Contest/lib/app enthält kein Jar dieser Namen.
  • ./gradlew jacocoAggregatedReport schreibt build/reports/jacoco/aggregated/jacoco.xml und einen HTML-Bericht; die PR-Job-Zusammenfassung zeigt eine Abdeckungstabelle und bietet das Artefakt coverage-report an.
  • nightly-artifacts.yml und tagged-release.yml führen den Test-Job aus, und kein Packaging-Job startet, bevor er bestanden ist.
  • AGENTS.md und docs/PROJECT_CONTEXT.md behaupten nicht mehr, Testfehler könnten den Build passieren; sie sagen, dass nur PMD und SpotBugs mit ignoreFailures laufen.
  • Die zwei neuen Testbereiche (ChatPreferences-Roundtrip, DBController-Schema-Migration) liegen im Zweig, und die Punkte außerhalb des Umfangs existieren als Issues.

Was unverändert bleibt

  • Testverhalten: Keine Assertion wird abgeschwächt und kein Test deaktiviert, damit der Umzug kompiliert.
  • Der Produktionscode, bis auf den entfernten MockKstServer-Import in ChatController.
  • PMD und SpotBugs behalten ignoreFailures; ihre Befunde werden wie bisher aus den Berichten gelesen.
  • Die Packaging-Jobs selbst: Es kommt nur needs: test hinzu.
  • Keine neue Laufzeitabhängigkeit. JaCoCo ist ein Gradle-Plugin, das die Test-JVM instrumentiert; nichts davon erreicht das Jar.