ci(conformance): cache the installs the verify job spends its time on - #171
Open
Bccorb wants to merge 1 commit into
Open
ci(conformance): cache the installs the verify job spends its time on#171Bccorb wants to merge 1 commit into
Bccorb wants to merge 1 commit into
Conversation
The verify job has been going red since #167 without a test failing. Every step reports success, the job runs 30m00s exactly, and GitHub records that as cancelled, which reads as a failure in the checks list. It is the job timeout, and what pushed it there is the install layer rather than anything in the matrix. Comparing the last green run to the first red one: building the CLI went 6s to 214s, installing the SDK dependencies 15s to 307s, installing the harness and browser 32s to 215s, and the run itself 249s to 1066s. Nothing in any repo makes npm ci thirty-six times slower; a degraded registry does, and one was returning 503s on the API repo at the same time. So cache what the job re-downloads on every run. setup-node warms ~/.npm for the three npm installs, the pnpm store is cached separately because the server install does not touch ~/.npm, and the Playwright chromium download is cached on the harness lockfile. The harness now installs with npm ci rather than npm install, which is what the committed lockfile is for. Raise the timeout to 45 as well. A healthy run is about 5 minutes, so 30 was not tight until it was; the caching is the fix and the headroom is so the next bad day is a slow check rather than a red one.
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.
The
verify / verifycheck has been red on every PR since #167, in this repo and inseamless-auth-api, without a single test failing.What is actually happening
It is the job timeout, and GitHub records a timed-out job as
cancelled, which shows inthe checks list as a failure. On the
seamless-auth-apirun I dug into, every stepreported success, including
Run seamless verify, and the job ran 00:42:12 to 01:12:12:30m00s against
timeout-minutes: 30.So the conformance matrix was passing. The job just ran out of wall clock.
What got slow
Not the matrix. Comparing the last green run with the first red one, step by step:
Build the CLIisnpm ci && npm run build. Nothing merged in any repo makes thatthirty-six times slower. A degraded registry does, and one was:
npm auditon the APIrepo's own
testjob was returning503 Service Unavailablefromregistry.npmjs.orgin the same window. Every step here is install-bound, so they allinflated together and the total crossed the cap.
The change
Cache what the job re-downloads on every run:
setup-nodewithcache: npm, keyed on all three lockfiles the job installs from(the CLI, the harness, the React SDK). One warm
~/.npmcovers all three.~/.npm, so the setup-nodecache does nothing for the
seamless-auth-serverinstall. Gated oninputs.local, thesame condition as the install it serves.
--with-depsstillruns, since the apt packages are not cacheable, but the browser download is skipped on a
hit.
restore-keysis deliberate: a stale hit is still a win, becauseplaywright installthen fetches only what the pinned version needs.npm ciinstead ofnpm installin the harness. The lockfile is committed and insync (
npm ci --dry-runclean), so this is both faster and reproducible.Also raises
timeout-minutesfrom 30 to 45. The caching is the fix; the headroom is sothe next bad registry day is a slow check rather than a red one. A healthy run is about 5
minutes end to end, so 45 is not a licence to let it drift.
Correction: this is not the fix I thought it was
I claimed caching would fix the timeout. I then measured it, and it does not. Keeping the
original reasoning above for the record, but the numbers below are what should decide this
review.
A re-run of this PR's own conformance job with all three caches warm:
Every cache hit and restored correctly. The job still took 39m32s, worse than the
25m37s cold run before it. The caching works and does not matter.
Where the time actually goes. In that 39m job,
Run seamless verifywas 28m02s, andthe tests inside it took 23.7 seconds:
The rest is Docker image building, and the log shows the whole stack coming up twice:
The stack is rebuilt from scratch once per web template. Nothing on the host, cached or
not, touches that: those builds happen inside Docker, which has no layer cache in CI.
I had dismissed Docker earlier in this PR on the grounds that
Run seamless verifywas249s in a green run. That was one healthy data point and I over-generalised from it. The
step has since been 249s, 880s, 1067s, 1267s and 1682s. It is the dominant and growing
cost.
So what is this PR worth now? The
timeout-minutes: 30to45change is what made therun above pass; at 39m32s it would have been cancelled under the old cap. The caches are
correct, cheap and harmless, and they remove real work on the host, but they are not the
reason the check goes green.
Reviewer's call on whether that is worth merging as-is. I would suggest yes, as the
headroom is doing real work today, with the actual fix tracked separately: build the stack
once and reuse it across web templates, and/or give the image builds a layer cache
(
docker/setup-buildx-actionplus a GHA cache backend). Both are changes to howseamless verifydrives compose, not to this workflow, so neither belongs in this diff.Verification
actionlint1.7.12 withshellcheck0.10.0 passes, and the YAML parses.conformance.ymlin this repo calls the reusable workflow by local path, so theverify / verifycheck here exercises the modified workflow directly. Cold run: 25m37s,pass. Warm re-run: 39m32s, pass, with all three caches confirmed hit. Both green; see the
correction above for what that does and does not demonstrate.