fix(scripts): check.sh says what it covered, on stdout (#336) - #372
Merged
Conversation
The script aborted correctly on a missing prerequisite and said so on stderr, leaving stdout empty — so a caller capturing stdout could not tell a partial run from a full one. The exit code was right, and nobody reads an exit code out of a transcript. Every run now ends with one machine-readable line on stdout naming what completed, what did not, and the verdict, printed from a trap so no exit path can skip it. INCOMPLETE is kept apart from FAILED: the checks not happening and the checks finding something are different news. The browser banner follows what ran rather than what was requested, which is what its own comment always claimed, and stays quiet when nothing ran at all. tests/scripts/check_stages.test.mjs guards the line and holds the group roster to the dispatch, since the way this rots is a stage quietly leaving the loop while everything left still passes.
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.
Closes #336.
The issue's first criterion was already met by PR #249 — the script aborts on a missing
node_modulesrather than limping on. What survived is that it says so on stderr, leaving stdout empty, so a caller capturing stdout sees a partial run and a full one as the same thing: some green pytest output, then silence. The exit code is right, and nobody reads an exit code out of a transcript. It is the same false-calm failure the script's own header warns about for| tail, arriving from the other direction.What changed
One last line on stdout, from a
trap … EXIT— so arequire_node_modulesthree groups deep cannot skip it, which is the exact run that most needed to say something:ran=is what completed, never what was asked for — recorded after each group's function returns, so a group that aborted partway through is never counted as covered.skipped=is every group the script knows that is not in it.FAILEDis a step reporting a problem;INCOMPLETEis the run leaving early, where nothing was found wrong with the tree and the checks simply did not happen.frontend. It stays quiet when nothing ran, sounknown group 'nope'is not buried under twelve lines about Playwright.Written without arrays for the accumulator (
ranis a comma string): macOS ships bash 3.2, where an empty array underset -uis an unbound variable. That constraint is already documented in the timing block; this way there is nothing to remember.Found, not fixed
Nothing.
Test plan
New
tests/scripts/check_stages.test.mjs— 8 cases, ~0.2s, no new dependency. Two halves that fail differently:generated→ exit 2 with the verdict line on stdout alone;python→FAILED ran=python; an unknown group →INCOMPLETE ran=none; the banner quiet with nothing run and loud whenbrowserwas requested but never reached. The subprocess gets aPATHwithoutuvorpnpm, so every step fails identically on every machine rather than depending on what happens to be installed.case, the default list,--fast, and the trap. This half exists because the way the script rots is not a deleted line but a group quietly leaving the loop: coverage shrinks, every remaining stage passes, and the verdict says PASSED about a smaller run than the reader believes.Mutation checks, anchors asserted before and after each patch
trap summary EXITcommented outrannever recordedbrowserdropped fromALL_GROUPSbrowser) run_browser ;;deleted from the dispatchgeneratedThe fourth is the one the issue is really about: every behavioural test stays green while the script silently stops running a quarter of the gate. That is why the static half is there.
Full gate, staged against the harness's ~10-minute ceiling
Test directories derived from
ls tests/at run time.pytest tests/architecture tests/cli tests/examples tests/formats tests/jobs tests/packaging tests/scripts tests/test_versioning.pypytest tests/kernel tests/mcp tests/serverruff check ./ruff format --check ./mypy src/visionset/lint-importscheck.sh frontend generatedcheck.sh: PASSED ran=frontend,generated skipped=python,browserCI=1 npx playwright testCI=1 npx playwright test -c playwright.cycle.config.ts