fix(ci): make samples/minimal test evidence loud, not silent (#198) - #201
Conversation
Gradle's Kotlin/Native test task prints no per-test lines by default, and a test task with no matching sources succeeds silently — so the samples-minimal workflow could go green whether nativeTest ran 3 tests or zero, with nothing in the log to tell the difference. - build.gradle.kts: wire testLogging(passed/skipped/failed) onto every AbstractTestTask so per-test PASSED/FAILED lines show up in the log. - workflow: parse the JUnit XML nativeTest produces and fail the job outright if the parsed test count is 0 (or if failures/errors slipped through). Upload the XML as a workflow artifact for human review. - workflow: add timeout-minutes: 15 so a wedged run can't sit for the inherited 360-minute GitHub default. Co-Authored-By: Claude Opus 5 <[email protected]> Claude-Session: https://claude.ai/code/session_01NLWsoRAe7f1RdQrYJ2ZXqm
|
Independent reproduction (own worktree, not the author's): moved Ran the exact assertion-step shell logic from this PR's workflow against that state: it correctly detects the missing-XML case (the "no XML at all" branch, not just "XML with 0 tests") and exits 1 with Continuing with: false-PASS attempts on the hand-rolled bash/grep parsing, confirming |
monkopedia-reviewer
left a comment
There was a problem hiding this comment.
Reviewed: 6a2eaec
The guard fires (independently reproduced)
Own worktree, off the author's. Moved GeometryTest.kt out of src/nativeTest/kotlin, ran ./gradlew nativeTest --no-daemon: reproduces the exact silent-zero case from the earlier reviewer finding — nativeTest SKIPPED, exit 0, no build/test-results directory produced at all (not "XML with 0 tests", no XML whatsoever). Ran the workflow's actual assertion shell logic against that state: it hits the "no XML files matched" branch and exits 1 with ::error::No JUnit XML found.... This is the failure mode that actually occurred, and it's covered — good, since a script that only checked "0 tests inside an XML" would have missed it.
if: always() — verified live, not just read
Pushed a scratch branch off this PR's head with a deliberate Kotlin syntax error in the test source, dispatched the workflow via workflow_dispatch (run 30483425431), watched it live, then deleted the branch:
:nativeTeststep: failed (compile error, no test-results produced)Assert nativeTest actually ran testsstep: still ran (thanks toif: always()) and correctly emitted::error::No JUnit XML found..., exit 1Upload nativeTest results: ran, warned "no files found" (as configured, doesn't fail the job)- Job: red overall
So a compile failure can't skip the guard, and the extra signal/artifact-upload happens even when the primary step already failed the job by itself.
Tried to construct a false PASS — couldn't
- Regenerated real JUnit XML from a passing run and inspected it: root
<testsuite tests="3" failures="0" errors="0" ...>attributes are all on the first line, before any<testcase>/<system-out>content, sogrep -o ... | head -1reliably captures the suite root regardless of what test output/messages appear later in the file. - Added a second test class (one passing, one deliberately failing assertion) and reran: Kotlin/Native writes one XML file per test class (
TEST-nativeTest.GeometryTest.xml,TEST-nativeTest.ScratchFailingTest.xml), confirming the "aggregate multiple<testsuite>blocks into one file, only count the first" risk doesn't apply to this toolchain's actual output shape. The per-file loop correctly summedtests=5 failures=1 errors=0across both files and exited 1. - Stale-XML-from-a-previous-run risk is structurally absent here: no
actions/cacheonbuild/, and each job runs from a freshactions/checkout, so there's no way for a passing run's leftover XML to paper over a later failure within this workflow. (This would be a footgun for a developer manually re-running the assertion locally against a stalebuild/dir without a clean, but that's outside what this CI guard promises.) - Didn't find a way to make the parser under- or over-count in a way that flips a real failure to a reported pass.
Runs on this PR itself
Confirmed via gh run view --log on this PR's own CI run (30482965828, triggered by pull_request, path filters correctly include .github/workflows/samples-minimal.yml itself): per-test PASSED lines now appear from testLogging, and the assertion step logged build/test-results/nativeTest/TEST-nativeTest.GeometryTest.xml: tests=3 failures=0 errors=0 / TOTAL across 1 report(s): tests=3 failures=0 errors=0, all steps green.
Everything else
timeout-minutes: 15present, reasonable for a ~2 min job.testLogging(events = passed/skipped/failed)produces exactly 3 cleanPASSEDlines locally — doesn't drown the log.- Ran
samples/minimal:nativeTestlocally at this exact SHA: 3/3 pass. - Scope: diff touches only
.github/workflows/samples-minimal.ymlandsamples/minimal/build.gradle.kts— no overlap withcompiler/**/plugin source (#192).
No defects found. Merging.
One line — `version` in the root `gradle.properties`. That is the whole change, and it is the direct payoff of #192/#202: the five other version declarations (three per-module `version = "..."` lines and the hand-written `PLUGIN_VERSION` literal in plugin source) no longer exist. During 0.3.4 prep the same operation touched six places, five were bumped, and the sixth was caught only by a reviewer's grep. The three consumed pins (root `settings.gradle.kts`, `samples/minimal`, `samples/multiproject/bindings`) DELIBERATELY stay at 0.3.4. The repo self-hosts one release behind, so they can only move once 0.3.5 exists on the Portal. `verifyConsumedPluginPins` (also from #202) was run and accepts this as a legal one-release lag. ## What 0.3.5 ships * **#194 / #200 — the reason for this release.** 0.3.4 is BROKEN for multi-project consumers: the plugin picked up whatever kotlinx-coroutines the root buildscript classloader owned (1.8.0 via kotlin-compiler-runner) rather than the 1.11.0 it resolves, and ktor-io calls a method coroutines 1.9+ moved onto the interface -> `NoSuchMethodError: Job.invokeOnCompletion$default`. The plugin's ksrpc/ktor/coroutines runtime is now shaded and relocated under `com.monkopedia.kplusplus.compiler.shaded`, so it is immune to the buildscript classloader's contents. `samples/multiproject` is a new canary reproducing the exact topology that shipped broken. * **#196 / #199** — drop-ledger diagnostics: structurally-expected drops (DEDUP, the operator== capability gate) demoted to INFO so the capped WARNING budget carries actionable entries. Totals still reported per-severity. * **#195 / #197** — `samples/minimal:nativeTest` repaired (it had not compiled since the context-parameter migration) plus the repo's first PR-triggered CI. * **#198 / #201** — that CI now asserts a non-zero test count from the JUnit XML and uploads it; previously it went green whether 3 tests ran or none. * **#192 / #202** — this release's own prep, made a one-line operation. ## Verification Full gate re-run on main @ `a65d066` immediately before the bump: krapper 304 / 0 featuregen 195 / 0 cppfixture 2 / 0 noncopyableDeterminismCheck green BUILD SUCCESSFUL in 4m 39s (34/34 tasks executed) `-p compiler check` green at 0.3.5, including `apiCheck` and the new `verifyConsumedPluginPins`. ## Follow-up, immediately after this publishes The post-release migration is one change: bump the three consumed pins 0.3.4 -> 0.3.5 AND delete the root `build.gradle.kts` `apply false` workaround (#193/#194), whose comment names exactly this condition. It cannot go earlier — `release.yml` builds `:krapper` on main to cut the release, and until 0.3.5 exists the pin cannot move off the unshaded 0.3.4. Co-Authored-By: Claude Opus 5 <[email protected]> Claude-Session: https://claude.ai/code/session_01NLWsoRAe7f1RdQrYJ2ZXqm
Closes #198.
The gap
.github/workflows/samples-minimal.yml(added by #197) ran./gradlew nativeTest --no-daemonand treatedBUILD SUCCESSFULas proof the tests passed. It isn't: Kotlin/Native's test task prints no per-test lines by default, uploads no report, and — critically — a Gradle test task with no matching sources exits 0. The log for "ran 3 tests" and "ran 0 tests" was byte-for-byte indistinguishable.What this PR adds
samples/minimal/build.gradle.kts:tasks.withType<AbstractTestTask>().configureEach { testLogging { events("passed", "skipped", "failed") } }— per-testPASSED/FAILED/SKIPPEDlines now show up directly in thenativeTeststep's log..github/workflows/samples-minimal.yml:if: always(), runs right after thenativeTeststep): globssamples/minimal/build/test-results/**/*.xml, sums the JUnittests=/failures=/errors=attributes across every report file, andexit 1s if the total test count is 0, or if any failure/error is present. This is the actual guarantee — everything else here is evidence for a human. Plain bash +grep, no new action/dependency.actions/upload-artifact@v4,if: always()) uploadssamples/minimal/build/test-results/**so the raw JUnit XML is attached to the run for anyone who wants to double-check by hand, the way the fix(samples): repair samples/minimal:nativeTest and guard it in CI (#195) #197 reviewer had to build locally to do.timeout-minutes: 15on the job (was inheriting GitHub's 360-minute default; the job normally takes ~2 minutes)..github/workflows/samples-minimal.ymlitself (from fix(samples): repair samples/minimal:nativeTest and guard it in CI (#195) #197), so this PR's own CI run exercises the changed workflow — confirmed no change needed there.Demonstration: the assertion actually fires
Ran this locally (
JAVA_HOME=java-21-openjdk, LLVM 22 on PATH, same toolchain the workflow installs) to reproduce the exact failure class #198 describes, then reverted:1. Baseline — real tests, before this fix:
No per-test lines.
build/test-results/nativeTest/TEST-nativeTest.GeometryTest.xmlhastests="3" failures="0" errors="0"— but nothing in the console said so.2. Reproduced the exact silent-zero scenario by temporarily moving
src/nativeTest/kotlin/GeometryTest.ktout of the source tree (an empty native test source set — the scenario named in the issue's acceptance criteria) and re-running:Gradle exits 0. No
build/test-resultsdirectory is produced at all — this is precisely "green" and "ran nothing" being indistinguishable, reproduced on demand.Running the new "Assert nativeTest actually ran tests" step's script against that output:
exit code 1 — the job would fail, where before it went green.
(Side note from the same investigation: if test sources exist but contain zero
@Testmethods, Gradle's ownfailOnNoDiscoveredTestsguard already catches that case and fails the build — so the real, previously-uncovered gap was specifically "no test sources/no report at all," which is what's demonstrated above.)3. Reverted the moved file (
git statusconfirms only the workflow + build.gradle.kts diffs are part of this PR — the test source tree is untouched) and re-ran to confirm the normal path:Per-test lines now visible in the log, and the assertion script reports:
exit code 0 — job passes, with the count of 3 now visible both in the log and (via the new artifact upload) in the run's attached JUnit XML.
Notes
grepper the "no heavy actions/dependencies" guidance — only new third-party action isactions/upload-artifact@v4, which is the standard artifact mechanism.if: always()on both new steps so the artifact/assertion still run (and give a clear signal) even when an earlier step in the job fails outright.compiler/**or other plugin source — scope is.github/workflows/samples-minimal.ymland the test-logging block insamples/minimal/build.gradle.kts, per the coordination note (another session ownscompiler/**/No guard against a partial version bump (version lives in 6 places) #192 concurrently).