Skip to content

fix: run the whole test suite in CI, and repair the break it was hiding - #335

Merged
chubes4 merged 1 commit into
mainfrom
ci-blind-spot
Aug 5, 2026
Merged

fix: run the whole test suite in CI, and repair the break it was hiding#335
chubes4 merged 1 commit into
mainfrom
ci-blind-spot

Conversation

@chubes4

@chubes4 chubes4 commented Aug 5, 2026

Copy link
Copy Markdown
Member

The problem

19 of 42 test files were never run by CI. Present in the repo, referenced nowhere in .github/workflows/shell.yml, executed only when someone remembered to by hand.

This is how #334 shipped 20/20 green while leaving main red:

  1. fix: use environment for Kimaki lock port #334 changed how the Kimaki lock port reaches a systemd unit
  2. It correctly updated tests/kimaki-multi-instance.sh to match
  3. That test is one of the 19 → CI never ran it
  4. Merged green. Test fails on main.

CI reported green over red. It only surfaced because I happened to run the full suite afterwards for an unrelated reason.

The break

#334 removed the --lock-port argument that bridge_render_systemd appended to ExecStart, moving it to an Environment= line. But the renderer only interpolates the env block its caller hands it.

So a guarantee the renderer enforced unconditionally became something every caller has to remember. A caller rendering with an env block that lacks the line silently drops the port, and the instance falls back to the default — two Kimaki instances then contend for one lock, which is precisely what multi-instance support exists to prevent.

Fixed at the layer that always owned it: the renderer re-asserts the line when KIMAKI_LOCK_PORT is set and the block doesn't already carry it.

The structural fix

tests/ci-coverage.sh — this is the part that matters.

A workflow that lists jobs by hand fails this way silently and permanently: the person adding a test is the same person who must remember to wire it, and nothing complains when they don't. The guard fails loudly instead, and carries an EXCLUDED list so that deliberately skipping a test is a visible decision in review rather than an omission.

It checks for the test name anywhere in the workflow rather than one exact invocation, so a matrix, a composite job, or a future restructure all still satisfy it. The claim is "CI knows about this file", not "CI runs it this particular way".

The 21 suites are wired as a matrix rather than 21 more hand-written jobs, so adding a test stays a one-line change. They run via bash because several of these files lack the executable bit — invoking ./tests/x.sh produces a Permission denied that reads exactly like a test failure. (That cost me a false "4 regressions" reading earlier today.)

Expect some of the newly-wired jobs to be informative

These have never run in CI. If any fail, that is the point — they are reporting real state that was previously invisible. Locally, everything passes except three environment-only failures that also fail on origin/main: this host has a real /usr/bin/kimaki that two tests resolve instead of their fixtures, and no studio CLI for a third.

19 of 42 test files were present in the repo and referenced nowhere in
.github/workflows/shell.yml — nearly half the suite, run only when someone
remembered to run it by hand.

That is not theoretical. #334 changed how the Kimaki lock port reaches a
systemd unit, updated tests/kimaki-multi-instance.sh to match, merged with
20/20 checks green, and left that test failing on main, because it was one
of the 19. CI reported green over red. The only reason it surfaced was an
unrelated full-suite run afterwards.

The break itself: #334 removed the `--lock-port` argument that
bridge_render_systemd used to append to ExecStart, in favour of an
Environment= line. But the renderer only interpolates the env block its
caller hands it, so the guarantee moved from something the RENDERER
enforced unconditionally to something every CALLER has to remember. A
caller that renders with an env block lacking the line silently drops the
port, and the instance falls back to the default — two Kimaki instances
then contend for one lock, which is the exact failure multi-instance
support exists to prevent. The renderer now re-asserts the line when
KIMAKI_LOCK_PORT is set and the block does not already carry it, at the
same layer that always owned it.

The suites are wired in as a matrix rather than 21 more hand-written jobs,
so adding a test is a one-line change. They run through `bash` because
several of these files do not carry the executable bit — invoking them as
./tests/x.sh reports a Permission denied that reads exactly like a test
failure.

tests/ci-coverage.sh is the part that matters. A workflow that lists jobs
by hand fails silently and permanently: the person adding a test is the
same person who must remember to wire it, and nothing complains when they
do not. The guard fails loudly instead, and has an EXCLUDED list so that
deliberately skipping a test is a visible decision rather than an
omission. It checks for the test name anywhere in the workflow rather than
one exact invocation, so a matrix, a composite job, or a future
restructure all still satisfy it — the claim is "CI knows about this
file", not "CI runs it this particular way".

Local suite is green apart from three environment-only failures that also
fail on origin/main: this host has a real /usr/bin/kimaki, which two tests
resolve instead of their fixtures, and no studio CLI for a third.
@chubes4
chubes4 merged commit b62d969 into main Aug 5, 2026
42 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant