feat: local install recipe, CI coverage gate, and unskip integration tests - #28
Merged
Conversation
Compiles for the host platform and installs to ~/.local/bin (falling back to ~/.upkeep/bin), overridable via UPKEEP_INSTALL_DIR, warning if the destination is not on PATH. Previously the only way to exercise a local build was `just build` plus invoking ./dist/upkeep by path; scripts/install.sh only fetches released binaries.
bunfig.toml's coverageThreshold does not work for this: verified on Bun 1.3.9, the table forms are not enforced at all (an impossible 0.99 threshold still exits 0), and the single-number form is enforced per file rather than against the aggregate, so any non-zero threshold fails the build on src/lib/utils/exec.ts (0% functions). scripts/check-coverage.sh parses the aggregate row itself instead, and fails closed if that row is missing or unparseable. Floors are set at 83% functions / 75% lines, just under the current 84.14% / 76.05%. CI now runs the gate as its Test step. `just coverage-check` runs the same gate locally.
405 pass / 15 skip becomes 420 pass / 0 skip. All 15 were verified to pass; nothing was broken or flaky. The real cost is about 11 seconds across the two files (663ms to 11.9s), and the stale comments claiming they were "skipped by default" were rewritten. These tests add zero measured coverage: runCli uses Bun.spawn, and instrumentation does not follow into a child process, so running with and without them produces byte-identical coverage numbers. All of tests/cli/ is invisible to coverage, not just these blocks. They are enabled for end-to-end verification, not to move the coverage number.
llbbl
force-pushed
the
feat/install-local-and-coverage
branch
from
August 23, 2026 20:51
dafe904 to
6a4e9d3
Compare
The integration tests unskipped in this PR shell out to whichever package manager the fixture declares, and tests/fixtures/sample-project ships a pnpm-lock.yaml. The ubuntu-latest runner provides npm and yarn but not pnpm, so the CLI produced empty stdout and 12 tests failed on JSON.parse with "Unexpected EOF". They pass locally only because pnpm is installed there. Adds a pinned pnpm/action-setup@v6 step (pnpm 11.9.0) before dependency installation. Note this failure was only visible once this PR was retargeted from its stacked base onto main -- CI is scoped to pull_request: branches: [main], so the stacked PR never ran tests.
The pull_request trigger was scoped to branches: [main], so a PR stacked on another PR received no test run at all. Worse, GitHub then reports mergeStateStatus: CLEAN for such a PR, which reads as "checks passed" when it actually means "nothing is blocking" -- this PR sat looking mergeable with 12 failing tests until it was retargeted onto main and CI ran for the first time. Removing the base-branch filter means every PR gets tested regardless of what it targets.
2 tasks
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 #26
Summary
just install-localto build and install a local binary onto PATHscripts/check-coverage.sh+just coverage-check) and wire it into CIChanges
1.
just install-localCompiles for the host platform and installs to
~/.local/bin(falling back to~/.upkeep/bin), overridable viaUPKEEP_INSTALL_DIR, and warns if the destination is not on PATH.Previously the only way to exercise a local build was
just buildplus invoking./dist/upkeepby path;scripts/install.shonly fetches released binaries. Verified by installing into a scratch directory and running the result (upkeep v0.5.0).install-localrecipe2. Coverage gate — and why it's a script, not Bun config
This is the part worth a reviewer's attention. The intent was
bunfig.tomlwithcoverageThreshold. That does not work here, verified on Bun 1.3.9:coverageThreshold = { line = ..., function = ... }, and a[test.coverageThreshold]section) are not enforced at all — an impossible 0.99 threshold still exits 0.src/lib/utils/exec.tssits at 0% functions, any non-zero threshold fails the build — verified down to 0.02.A silently inert gate is worse than no gate, so
bunfig.tomlwas dropped andscripts/check-coverage.shparses the aggregate row itself. It fails closed: if theAll filesrow is missing or unparseable it exits non-zero rather than reporting success. Floors are set at 83% functions / 75% lines, just under the current 84.14% / 76.05%. All three paths were verified: passes at baseline, fails when floors are raised to 90%, and fails closed when the summary row cannot be found.coverage-checkrecipe3. Unskip the CLI integration tests
405 pass / 15 skip becomes 420 pass / 0 skip. Investigated before enabling: they all pass, nothing was broken or flaky, and the real cost is about 11 seconds (the two files go from 663ms to 11.9s). The stale comments claiming they are "skipped by default" were rewritten.
Important caveat: these tests add zero measured coverage. Running coverage with and without them produces byte-identical numbers, because
runCliusesBun.spawnand instrumentation does not follow into a child process. That means all oftests/cli/is invisible to coverage, not just these blocks. They are enabled for end-to-end verification, not to move the coverage number — worth being explicit so nobody later mistakes the CLI suite for coverage.Follow-up
src/lib/utils/exec.tsat 0% functions / 3.95% lines is tracked in #27. It is the shared subprocess layer under audit, deps, risk, and dependabot, and it needs real unit tests — that is what would actually raise coverage, and what would make a per-file threshold viable.Test plan
just checkpasses (lint, typecheck, 420 tests)actionlintclean on the workflowsjust --listparses