dhaga.blog
Engineering

The CI that waited on itself

Twenty-nine minutes of CI, of which twenty-six were one test suite and the other three sat in a queue behind it. Splitting the job and sharding the suite took it to about ten — but the two changes that mattered most weren't the fast ones, they were the two that stopped a green tick from lying.

The short version

Our CI took 29 minutes. One test suite was 26 of them; everything else finished in three and then queued behind it for no reason. Splitting the job four ways and sharding the suite got it to about ten. The interesting part wasn't the speedup — it was noticing that the obvious way to split the job would have silently disabled our branch protection, and that the obvious way to write the replacement gate would have let a failed run report success.

Where the time actually went

The run log is worth reading before touching anything. Ours said:

Duration 1556.89s (transform 16.23s, import 581.38s, tests 890.89s)
Test Files  598
Tests       3436

Twenty-six minutes for apps/web. The whole rest of CI — four typecheck passes, two other test suites, a browser-extension build, lint — came to about three.

So it was one serial job where 90% of the wall clock was a single step, and every cheap step sat behind it. A lint error took 29 minutes to report. That's the real cost: not the number, but that the fastest feedback was gated behind the slowest work.

Note the split inside that number, because it changes what you'd do next: import 581s against tests 890s. Nearly ten minutes was spent loading modules, not running assertions. Each of our test files boots its own in-memory Postgres, and that cold start is paid 598 times. That's a real target — but it's an invasive change to shared test setup, and there was a much cheaper win available first.

The cheap win: stop queueing

Two changes, no cleverness in either.

Split by what can run at the same time. The test suite, the lint-and-build pair, and everything outside the web app have nothing to say to each other. Three jobs, in parallel.

Shard the suite. Vitest takes --shard=i/N and splits by file, so four matrix jobs each run about 150 files. fail-fast: false on the matrix, because one shard failing shouldn't cancel the others — whether a failure is in one file or spread across the suite is most of the triage.

Roughly 29 minutes to roughly 10, bounded by one shard plus its install.

Two things worth saying plainly, because "we sharded it" invites both:

  • npm ci now runs six times instead of once. In parallel, against the npm cache, so it costs wall clock nothing and some runner minutes. That's a real trade, not a free lunch.
  • Shards split by file count, not by duration. Past a point, the single slowest file sets your floor and extra shards buy only more installs. Four was chosen deliberately; it isn't a number to keep raising.

The part that nearly went wrong

Here's what makes this worth a post rather than a commit message.

Branch protection required a status check named verify — the name of the job we were about to delete. And a required check that no longer exists doesn't block anything. GitHub doesn't warn you; the rule just sits there waiting for a check that will never report, and pull requests become mergeable with nothing enforcing them.

You would not notice from a green PR. You'd notice weeks later, from a merge that shouldn't have happened.

So verify stays — as a gate, not as work. It does nothing except depend on the three real jobs and assert they passed.

Then the second trap, inside the fix. Write that gate the obvious way and it's still wrong:

verify:
  needs: [web-tests, web-build, packages]
  steps:
    - run: echo "ok"

When a dependency fails, this job doesn't fail. It's skipped. And a skipped required check is not a failed one — so the pull request sits there looking mergeable with a red suite behind it. You have replaced a check that didn't exist with one that never says no.

The gate has to run unconditionally and read the results itself:

verify:
  needs: [web-tests, web-build, packages]
  if: always()
  steps:
    - run: |
        results="${{ needs.web-tests.result }} ${{ needs.web-build.result }} ${{ needs.packages.result }}"
        for result in $results; do
          if [ "$result" != "success" ]; then exit 1; fi
        done

!= "success" rather than == "failure" is the point. cancelled and skipped are also not passes, and a cancelled run must never render as a tick.

Both traps have the same shape: a check that isn't running looks exactly like a check that's passing. Speed changes are unusually good at creating that state, because the whole exercise is removing things from the critical path.

While we were in there

A concurrency group, which we should have had regardless:

concurrency:
  group: ci-${{ github.workflow }}-${{ github.ref }}
  cancel-in-progress: true

Push twice in a minute and you were paying for two full runs, with the stale one still reporting a status against your branch. Now the first is cancelled.

The one that got away

We shipped this the same week we merged a change whose CI run was still in progress when the pull request was merged. The run went red about a minute later, on main.

The failure was ours and legitimate — a page had grown a second <h1> in a branch, and an SEO test that greps route source for exactly one heading caught it. Correctly. It cannot evaluate branches, so one-heading-per-file is the only rule it can enforce.

The lesson isn't "wait for CI", which everyone already knows. It's that a 29-minute suite makes not waiting feel reasonable, every single time. Slow CI doesn't just cost you 29 minutes; it quietly converts your merge gate into a suggestion. That's the actual return on this work, and it's the one that doesn't show up in the duration graph.

(A small coda, for anyone who enjoys this genre: the first fix for that double heading also failed — because the explanatory comment we added contained the literal tag, and the test greps raw source, comments included. The test was right both times.)

What we'd suggest

  • Read the duration breakdown before optimising. import versus tests points at completely different fixes.
  • Split by what can run concurrently before making anything individually faster. It's less satisfying and usually wins bigger.
  • When you rename or split a job, go and read your branch protection rules. A required check that no longer exists is not enforcing anything.
  • Any aggregate gate needs if: always(), and should treat everything that isn't success as failure.
  • The point of fast CI isn't the minutes. It's that people stop routing around it.
Share

Discussion

On this page