lint-once.test.ts misses the path CI actually runs, and its lockfile assertion checks the wrong branch
#33
Reference in New Issue
Block a user
Delete Branch "%!s()"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Problem
#29 added
test/packaging/lint-once.test.tsto enforce that prettier runs exactly once. The test is the deliverable of that issue — the point was to stop relying on a claim in a PR body. Adversarial review of #31 demonstrated, by mutation, that it does not enforce what its header says it enforces. Both of the first two findings were shown red-to-green: the mutation was applied, the suite stayed green.1. The walk never reaches
script/cibuildAppending
RUN yarn run prettier --check .toDockerfileafterRUN make buildleft the suite green at 258/258. That givesscript/cibuild— the thing.gitea/workflows/check.ymlactually executes — two prettier passes: exactly the duplication this test exists to prevent, on the one path where it matters most. The header comment's claim that a duplicate is caught wherever it is added overstates the coverage.2. The
--frozen-lockfileassertion misses the branch the container takesscript/bootstraphas two install sites. The container hits the second one, becausenode:22-alpineships yarn. Changing that site to a bareyarn installand leaving the first alone kept the suite green — so the assertion that guarantees lockfile-pinned installs inside the container passes while the container's install has stopped being pinned. A whole-file substring check cannot distinguish the branches.3. Prettier is counted per line, not per occurrence
RUN yarn run prettier --check . && yarn run prettier --check srccounts as one invocation (verified). The samecontinuealso drops any script/make/yarn edge that shares a line with a counted one.4. Stale comment
test/packaging/entrypoints.test.ts:3still saysmake checkruns test, lint and fmt-check. #29 made that false.Definition of done
script/cibuildand theDockerfilereachability case; the mutation in 1 turns the suite red.make checkgreen;script/cibuildgreen and shown to have actually executed rather than served from cache.Not a 1.0.0 blocker, but it should land before anything else relies on this test, since it currently reports a guarantee it does not provide.
Implementation plan
Working on
nextin a fresh clone; one commit, landing on #31.1. Walk the path CI actually runs. Add a
workflow:<path>node kind whose commands are therun:steps of.gitea/workflows/check.yml, and root a walk there. That reachesscript/cibuild, which reaches bothdocker:Dockerfile.lint(viascript/lint) anddocker:Dockerfile(the baredocker build .). Assertions: the CI path invokes prettier exactly once, and its reached set containsscript/cibuild,docker:Dockerfileanddocker:Dockerfile.lint. Rooting the walk in the workflow file rather than in a hand-picked entrypoint is what makes "the path CI executes" an assertion rather than an assumption.2. Lockfile assertion targets the executed branch. Resolve
install_js_depsout ofscript/bootstrapand split it at themissing yarnguard, so the two install sites are separately addressable.node:22-alpineships yarn, so the container takes theelsebranch; assert that everyyarn installoccurrence in that branch carries--frozen-lockfile, and separately that the nvm branch does too. Per occurrence, so a bareyarn installalongside a pinned one cannot hide.3. Count per occurrence, keep the edges. Replace the per-line boolean plus
continuewith an occurrence count, and always run edge extraction on the same line. Extract the counter and the edge extractor so both can be asserted directly:yarn run prettier --check . && yarn run prettier --check srccounts 2, and a line that both invokes prettier and callsscript/lintstill yields thescript/lintedge.4. Anti-vacuity.
commandsOfthrows when a node resolves to no commands (currently a missingpackage.jsonscript silently yields[""]), plus explicit tests that an unknownmake:/yarn:/script/node throws and that a prettier-free subgraph reports0, not a silently satisfied expectation.5. Correct the stale
make checkcomment attest/packaging/entrypoints.test.ts:3.Verification. Every assertion in the file gets mutation-tested individually, including the ones already there, with the named test recorded for each. The two mutations from the issue body are reproduced first — the
Dockerfileprettier append and the bareyarn installat the second bootstrap site — and shown to turn the suite red for their own reason.make checkgreen, andscript/cibuildgreen with per-layer timings and the epoch guard lines shown, so the pass is demonstrably executed rather than cached.Out of scope, untouched:
REPO_POLICIES.mddrift and theLINT_EPOCHnaming, both #32.Done in
2bfa11c, pushed tonextand carried by #31. The full mutation matrix is in the PR comment there.1. The walk reaches
script/cibuild. Aworkflow:<path>node kind resolves therun:steps of.gitea/workflows/check.yml, and a second walk is rooted there, so the graph under test is read out of the repo rather than assumed. It reachesscript/cibuild, and through itdocker:Dockerfile.lintanddocker:Dockerfile. All four are asserted reached; the CI build is asserted to invoke prettier exactly once; and the test and build image is asserted to invoke it zero times. AppendingRUN yarn run prettier --check .toDockerfileafterRUN make buildnow fires both of those — before the fix it left the suite green at 258/258, which I reproduced first.2. The lockfile assertion targets the branch the container executes.
install_js_depsis resolved out ofscript/bootstrapand split at itsmissing yarnguard. The container takes theelsebranch, and everyyarn installoccurrence in it must carry--frozen-lockfile; the nvm branch is asserted separately. Changing the second site to a bareyarn installnow firesinstalls from the lockfile on the branch the container takesand nothing else — and mutating the nvm site instead fires only the other test, which is the cross-check that the two are genuinely distinguished rather than both matching a substring. A companion assertion requires the lint image to reachscript/bootstrapat all, so the lockfile claims cannot end up describing a script the container never runs.3. Counting is per occurrence.
countPrettiercounts matches on a line, and thecontinueis gone soedgesOfruns on counted lines too.RUN yarn run prettier --check . && yarn run prettier --check srcinDockerfile.lintnow fires five separate count assertions; it scored 1 before.4. The
make checkcomment attest/packaging/entrypoints.test.ts:3now says the suite and the lint container.Vacuity. Checked explicitly rather than assumed, since the
node.slice("make:")/NaNdraft is the precedent. Theyarn:resolver used to return[""]for a missing script — a silent zero of the same family; that and four other ways of reaching nothing (unknown Makefile target, missing script file, node with no commands, unknown node kind) now throw, and each is tested. Renaming the Makefilecheck:target makes the whole file error withno such Makefile target: checkinstead of quietly reporting zero invocations. A legitimately prettier-free subgraph (make:clean) is asserted to report 0.Every assertion mutation-tested, all 35 — the ones I added and the ones already there. Each was changed to assert something else and confirmed to fail for its own named reason, with the failing set matching the mutated set exactly in every run. Nothing survived, so there is no assertion left that reads as a guarantee without being one. All mutations reverted; the tree is byte-identical to the committed one.
Verification.
make checkgreen.script/cibuildexit 0 and shown to have executed rather than been served: distinct epoch guard values per build, eslint 2.7s and prettier 4.5s in the lint image,make test13.8s reporting 279 passed inside the test image,make buildverifying the entrypoints.CACHEDappears only on the bootstrap and dependency-install layers. Everything went throughmaketargets andscript/entrypoints; nodocker builder pruneand no cache invalidation beyond the epoch arguments.