Repository navigation
docs(build): put bootJar before test in the local verification order - #81
Merged
Merged
Conversation
The guide's "good example" ran `clean test` first and `bootJar` second. The container and OS-isolation tests mount the fat -boot.jar and self-skip without it, and `clean` deletes it -- so that order never ran those 16 tests. The run still reported green, over 580 tests instead of 596, with nothing in the output saying so. Measured on this branch: 16 skipped in the documented order, 0 in the reversed one. CI is not affected and is not changed here. Its split is deliberate: the `test` job runs without a bootJar so those tests skip, and a dedicated `isolation-test` job builds the jar and runs exactly them. The gap was local-only, where there is no second job to fall back on. Three surfaces carried the stale order, so all three move together: the guide (en + ko), which is the stated source of truth, and the PR template, whose checklist asked for `clean test` and then accepted "Docker tests run or knowingly skipped" -- a box you could tick truthfully while never learning the tests had skipped for a reason unrelated to Docker. That box now asks whether they actually ran, and for the reason if not. The build says it out loud as well: `test` now logs a note when build/libs holds no -boot.jar, naming the consequence and the fix. It does not fail the build -- CI hits this path deliberately on every run -- but the gap no longer hides inside a skip count nobody reads. Documented the cost of the new order too, since it is real: one local invocation now runs Docker alongside everything else, and worker-spawn health checks can time out under that load. One of two full runs here failed WorkerPoolTest.hot_swap_removes_the_emptied_old_worker_synchronously that way; it passed alone, and the second full run was clean. Local maxRetries is 0 by design, so the guide says to re-run the class before believing such a failure. Co-Authored-By: htjulia <htjulia1@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What & why
Found while verifying #80. The operations guide's "good example" told you to run
clean testfirst andbootJarsecond. The container and OS-isolation testsmount the fat
-boot.jarand self-skip without it(
assumeTrue(bootJarExists())), andcleandeletes it — so that order never ranthose 16 tests. The run still reported green: green over 580 tests instead of
596, with nothing in the output saying which reading applied.
Measured on this branch, same machine, Docker up throughout:
clean test→bootJar(as documented)clean bootJar→test(this PR)CI is unaffected and is not changed here. Its split is deliberate — the
testjob runs without a bootJar so those tests skip there, and a dedicatedisolation-testjob builds the jar and runs exactly them(
.github/workflows/ci.ymlsays so in a comment). The gap was local-only, wherethere is no second job to fall back on. A developer running the documented
command before pushing simply did not exercise those 16 tests, and had no way to
notice.
Three surfaces carried the stale order, so all three move together:
docs/guide/11-operations.md+.ko.md— the stated source of truth. Thereversed order is now the good example, the old one is shown as a bad example
with its consequence, and the CI difference is written down so nobody
"fixes" the workflow to match.
.github/PULL_REQUEST_TEMPLATE.md— the checklist asked forclean testand then accepted "Docker-dependent tests run or knowingly skipped (Docker
absent)". That box could be ticked truthfully while the tests had skipped for
a reason that has nothing to do with Docker — which is exactly what happened on
feat(web): ack a console stream connection with a ready frame #80. It now asks whether they actually ran, and for the reason if not.
build.gradle—testlogs a note whenbuild/libsholds no-boot.jar, naming the consequence and the fix. It does not fail thebuild, because CI takes that path deliberately on every run; it just stops the
gap from hiding inside a skip count nobody reads.
The OOM constraint that motivated the original wording is untouched: these are
still two separate gradle invocations, only reordered.
The new order has a cost, and the guide now says so
One local invocation now runs Docker alongside everything else, so it is heavier
than CI's split. Worker-spawn health checks (
WorkerProcessIsolation.waitHealthy)can time out under that load. Of two full runs here, one failed
WorkerPoolTest.hot_swap_removes_the_emptied_old_worker_synchronouslywithworker health timeout: port 58759; it passed when re-run alone, and the secondfull run was clean (596 / 0 failures / 0 skipped). Local
maxRetriesis 0 bydesign so flakiness stays visible, so the guide now says to re-run the class by
itself before treating such a failure as real.
Type of change
fix— bug fix (no new API)feat— new capability (dynamic loading, gates, isolation, MCP, debug, data access)perf— performance, no observable behaviour changedocs— documentation onlyrefactor/chore/test/ci— no behaviour change (the build.gradle note)Correctness & safety
No production code path is touched. The only non-documentation change is a
doFirston thetesttask that logs a line; it adds no dependency betweentestandbootJar(which would reintroduce the OOM the separation exists toavoid) and cannot fail a build.
Verification
gradle clean bootJarassemblesgradle testpasses — 596 tests, 0 failures, 0 skippeddocs/guide/11-operations.md+.ko.md16 that silently skipped under the old order now execute.
./gradlew cleanthen./gradlew testprints it and the build still succeeds.
Related
Follow-up to #80, where the old order caused 16 tests to skip unnoticed.