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>
This commit is contained in:
Rsclub2_2andClaude Fable 5.1 authored and GitHub committed 2026-10-11 12:13:31 +02:00
1 parent 4a90ae7d30
commit 5491cbcf8b
42 files changed
+1205 -2025

No files matched your search

+101
View File
@@ -25,9 +25,104 @@ permissions:
packages: write
jobs:
# Same job as in pr-compile-check.yml, copied rather than shared: a reusable workflow
# would be one more indirection to read, and the comments belong next to the steps they
# explain. Every packaging job below needs this one, so a commit whose tests fail builds
# no artifact (#62). Edit all three copies together.
test:
name: Test (Java 21, headless)
runs-on: ubuntu-latest
steps:
- name: Checkout
uses: actions/checkout@v7
- name: Set up Java 21
uses: actions/setup-java@v6.0.1
with:
distribution: temurin
java-version: "21"
- name: Ensure gradlew is executable
run: chmod +x gradlew
# Skiko's native library links against libGL, so it has to be loadable even though
# nothing here renders through it: the drawing tests use Skia's software backend and
# runComposeUiTest never opens a window. Without this the whole Compose half of the
# suite dies with "libskiko-linux-x64.so: libGL.so.1: cannot open shared object
# file" -- which reads like a missing display and is not one. Found by running this
# job under act, where the container has no libGL; GitHub's own runner image happens
# to ship it, so the step is what makes the job reproducible off GitHub.
- name: Install the OpenGL client library skiko links against
run: |
sudo apt-get update
sudo apt-get install -y --no-install-recommends libgl1
# No Xvfb, deliberately. Everything that pins behaviour runs without a display:
# runComposeUiTest reaches the semantics tree headless (measured before this job was
# written), and the ImageComposeScene drawing tests render in software. The handful
# of tests that open a real window guard themselves with assumeFalse(isHeadless) and
# skip here; the summary below prints the skip count, so that is visible rather than
# assumed.
#
# Software GL under Xvfb was considered and rejected: a missing Skia GL context is
# the hardest failure in this codebase to read, and importing that class of flake
# into CI would cost more than the few window tests are worth here.
# All three modules. The translation generator is a Gradle module and not buildSrc
# precisely so that its 42 tests run here: buildSrc tests do not run on `./gradlew
# build` -- measured, not assumed. The summary below globs */build/test-results, so
# the module was counted the moment it existed.
- name: Test
run: ./gradlew -S test
- name: Summarise the test results
if: always()
run: |
set -euo pipefail
shopt -s nullglob
files=(*/build/test-results/test/TEST-*.xml)
if [ ${#files[@]} -eq 0 ]; then
echo "No test result XML was produced -- the test task did not run." >&2
exit 1
fi
# Counted from the XML and not from Gradle's console output: this project's own
# rule is never to infer "all tests passed" from an exit code.
summary=$(grep -h "<testsuite " "${files[@]}" \
| sed 's/.*tests="\([0-9]*\)".*skipped="\([0-9]*\)".*failures="\([0-9]*\)".*errors="\([0-9]*\)".*/\1 \2 \3 \4/' \
| awk '{t+=$1; s+=$2; f+=$3; e+=$4} END {print t" "s" "f" "e}')
read -r total skipped failures errors <<< "$summary"
echo "tests=$total skipped=$skipped failures=$failures errors=$errors"
{
echo "### Tests"
echo ""
echo "| total | skipped | failures | errors |"
echo "|---|---|---|---|"
echo "| $total | $skipped | $failures | $errors |"
echo ""
echo "Skipped are the tests that need a display, plus the deliberately"
echo "disabled reference-data generator. This job runs headless on purpose."
} >> "$GITHUB_STEP_SUMMARY"
if [ "$failures" != "0" ] || [ "$errors" != "0" ]; then
echo "The result XML reports failures or errors." >&2
exit 1
fi
- name: Keep the test reports on a failure
if: failure()
uses: actions/upload-artifact@v6
with:
name: test-results
path: |
*/build/test-results/test/
*/build/reports/tests/test/
if-no-files-found: ignore
retention-days: 7
build-windows-zip:
name: Build Windows ZIP
runs-on: windows-latest
needs: test
steps:
- name: Checkout
@@ -80,6 +175,7 @@ jobs:
build-linux-appimage:
name: Build Linux AppImage
runs-on: ubuntu-latest
needs: test
steps:
- name: Checkout
@@ -153,6 +249,7 @@ jobs:
build-linux-deb:
name: Build Debian package
runs-on: ubuntu-latest
needs: test
steps:
- name: Checkout
@@ -198,6 +295,7 @@ jobs:
build-linux-rpm:
name: Build Fedora package
runs-on: ubuntu-latest
needs: test
steps:
- name: Checkout
@@ -243,6 +341,7 @@ jobs:
build-linux-arch:
name: Build Arch Linux package
runs-on: ubuntu-latest
needs: test
steps:
- name: Checkout
@@ -354,6 +453,7 @@ jobs:
build-flatpak:
name: Build Flatpak
runs-on: ubuntu-latest
needs: test
steps:
- name: Checkout
@@ -538,6 +638,7 @@ jobs:
build-macos-dmg:
name: Build macOS DMG (${{ matrix.os }})
runs-on: ${{ matrix.os }}
needs: test
strategy:
fail-fast: false
matrix:
+52
View File
@@ -141,6 +141,14 @@ jobs:
- name: Test
run: ./gradlew -S test
# Coverage is a report, not a gate: no threshold, nothing in it can fail on a
# percentage. Deliberately not `if: always()` -- without a completed test run the
# report would be partial, and after a failed test the result XML below is what matters.
# jacocoTestReport writes the per-module XML the summary reads; the aggregate merges
# core and app-desktop (i18n-generator is build tooling and stays out).
- name: Coverage report
run: ./gradlew -S jacocoTestReport jacocoAggregatedReport
- name: Summarise the test results
if: always()
run: |
@@ -173,6 +181,50 @@ jobs:
exit 1
fi
- name: Summarise the coverage
if: always()
run: |
set -euo pipefail
# The last LINE/BRANCH counter in a JaCoCo XML is the report-level total; the
# per-package and per-class counters come before it.
percent() {
local file="$1" type="$2"
{ grep -o "<counter type=\"$type\"[^>]*/>" "$file" || true; } | tail -1 \
| sed 's/.*missed="\([0-9]*\)".*covered="\([0-9]*\)".*/\1 \2/' \
| awk 'NF == 2 { t=$1+$2; if (t==0) print "n/a"; else printf "%.1f%% (%d/%d)", 100*$2/t, $2, t }'
}
row() {
local label="$1" file="$2"
if [ -f "$file" ]; then
echo "| $label | $(percent "$file" LINE) | $(percent "$file" BRANCH) |"
else
echo "| $label | no report | no report |"
fi
}
{
echo ""
echo "### Coverage"
echo ""
echo "| module | lines | branches |"
echo "|---|---|---|"
row "core" core/build/reports/jacoco/test/jacocoTestReport.xml
row "app-desktop" app-desktop/build/reports/jacoco/test/jacocoTestReport.xml
row "core + app-desktop" build/reports/jacoco/aggregated/jacoco.xml
echo ""
echo "Report only: nothing fails on a percentage. The aggregated HTML report is the"
echo "coverage-report artifact of this run. i18n-generator is build tooling and is"
echo "not counted."
} >> "$GITHUB_STEP_SUMMARY"
- name: Keep the coverage report
if: always()
uses: actions/upload-artifact@v6
with:
name: coverage-report
path: build/reports/jacoco/aggregated/html/
if-no-files-found: ignore
retention-days: 14
- name: Keep the test reports on a failure
if: failure()
uses: actions/upload-artifact@v6
+101
View File
@@ -16,9 +16,104 @@ env:
FORCE_JAVASCRIPT_ACTIONS_TO_NODE24: true
jobs:
# Same job as in pr-compile-check.yml, copied rather than shared: a reusable workflow
# would be one more indirection to read, and the comments belong next to the steps they
# explain. Every packaging job below needs this one, so a commit whose tests fail builds
# no artifact (#62). Edit all three copies together.
test:
name: Test (Java 21, headless)
runs-on: ubuntu-latest
steps:
- name: Checkout
uses: actions/checkout@v7
- name: Set up Java 21
uses: actions/setup-java@v6.0.1
with:
distribution: temurin
java-version: "21"
- name: Ensure gradlew is executable
run: chmod +x gradlew
# Skiko's native library links against libGL, so it has to be loadable even though
# nothing here renders through it: the drawing tests use Skia's software backend and
# runComposeUiTest never opens a window. Without this the whole Compose half of the
# suite dies with "libskiko-linux-x64.so: libGL.so.1: cannot open shared object
# file" -- which reads like a missing display and is not one. Found by running this
# job under act, where the container has no libGL; GitHub's own runner image happens
# to ship it, so the step is what makes the job reproducible off GitHub.
- name: Install the OpenGL client library skiko links against
run: |
sudo apt-get update
sudo apt-get install -y --no-install-recommends libgl1
# No Xvfb, deliberately. Everything that pins behaviour runs without a display:
# runComposeUiTest reaches the semantics tree headless (measured before this job was
# written), and the ImageComposeScene drawing tests render in software. The handful
# of tests that open a real window guard themselves with assumeFalse(isHeadless) and
# skip here; the summary below prints the skip count, so that is visible rather than
# assumed.
#
# Software GL under Xvfb was considered and rejected: a missing Skia GL context is
# the hardest failure in this codebase to read, and importing that class of flake
# into CI would cost more than the few window tests are worth here.
# All three modules. The translation generator is a Gradle module and not buildSrc
# precisely so that its 42 tests run here: buildSrc tests do not run on `./gradlew
# build` -- measured, not assumed. The summary below globs */build/test-results, so
# the module was counted the moment it existed.
- name: Test
run: ./gradlew -S test
- name: Summarise the test results
if: always()
run: |
set -euo pipefail
shopt -s nullglob
files=(*/build/test-results/test/TEST-*.xml)
if [ ${#files[@]} -eq 0 ]; then
echo "No test result XML was produced -- the test task did not run." >&2
exit 1
fi
# Counted from the XML and not from Gradle's console output: this project's own
# rule is never to infer "all tests passed" from an exit code.
summary=$(grep -h "<testsuite " "${files[@]}" \
| sed 's/.*tests="\([0-9]*\)".*skipped="\([0-9]*\)".*failures="\([0-9]*\)".*errors="\([0-9]*\)".*/\1 \2 \3 \4/' \
| awk '{t+=$1; s+=$2; f+=$3; e+=$4} END {print t" "s" "f" "e}')
read -r total skipped failures errors <<< "$summary"
echo "tests=$total skipped=$skipped failures=$failures errors=$errors"
{
echo "### Tests"
echo ""
echo "| total | skipped | failures | errors |"
echo "|---|---|---|---|"
echo "| $total | $skipped | $failures | $errors |"
echo ""
echo "Skipped are the tests that need a display, plus the deliberately"
echo "disabled reference-data generator. This job runs headless on purpose."
} >> "$GITHUB_STEP_SUMMARY"
if [ "$failures" != "0" ] || [ "$errors" != "0" ]; then
echo "The result XML reports failures or errors." >&2
exit 1
fi
- name: Keep the test reports on a failure
if: failure()
uses: actions/upload-artifact@v6
with:
name: test-results
path: |
*/build/test-results/test/
*/build/reports/tests/test/
if-no-files-found: ignore
retention-days: 7
build-windows-zip:
name: Build Windows ZIP
runs-on: windows-latest
needs: test
steps:
- name: Checkout
@@ -61,6 +156,7 @@ jobs:
build-linux-appimage:
name: Build Linux AppImage
runs-on: ubuntu-latest
needs: test
steps:
- name: Checkout
@@ -127,6 +223,7 @@ jobs:
build-linux-deb:
name: Build Debian package
runs-on: ubuntu-latest
needs: test
steps:
- name: Checkout
@@ -165,6 +262,7 @@ jobs:
build-linux-rpm:
name: Build Fedora package
runs-on: ubuntu-latest
needs: test
steps:
- name: Checkout
@@ -203,6 +301,7 @@ jobs:
build-linux-arch:
name: Build Arch Linux package
runs-on: ubuntu-latest
needs: test
steps:
- name: Checkout
@@ -305,6 +404,7 @@ jobs:
build-flatpak:
name: Build Flatpak
runs-on: ubuntu-latest
needs: test
steps:
- name: Checkout
@@ -430,6 +530,7 @@ jobs:
build-macos-dmg:
name: Build macOS DMG (${{ matrix.os }})
runs-on: ${{ matrix.os }}
needs: test
strategy:
fail-fast: false
matrix: