diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 5f4529f..37d3037 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -3,8 +3,12 @@ name: CI on: push: branches: [main] + # Deliberately unfiltered by base branch. Scoping this to `branches: [main]` + # meant a PR stacked on another PR received no test run at all -- and a + # `mergeStateStatus` of CLEAN then reads as "nothing is blocking" rather than + # "tests passed". A stacked PR sat green-looking with 12 failing tests until + # it was retargeted onto main. pull_request: - branches: [main] jobs: test: @@ -20,6 +24,16 @@ jobs: with: bun-version: 1.4.0 + # The CLI integration tests shell out to whichever package manager the + # fixture declares, and tests/fixtures/sample-project ships a + # pnpm-lock.yaml. ubuntu-latest provides npm and yarn but not pnpm, so + # without this the audit/deps integration tests get an empty stdout and + # fail on JSON.parse. + - name: Setup pnpm (required by the CLI integration tests) + uses: pnpm/action-setup@v6 + with: + version: 11.9.0 + - name: Install dependencies run: bun install --frozen-lockfile @@ -29,8 +43,8 @@ jobs: - name: Type check run: bun run typecheck - - name: Test - run: bun test + - name: Test and coverage gate + run: ./scripts/check-coverage.sh # Compiling here is what stops a build break from reaching main. Lint, # typecheck and tests all pass on code that `bun build --compile` cannot diff --git a/docs/DEVELOPMENT.md b/docs/DEVELOPMENT.md index 66851ce..173314f 100644 --- a/docs/DEVELOPMENT.md +++ b/docs/DEVELOPMENT.md @@ -14,8 +14,17 @@ cd upkeep just install ``` -`just install` installs dependencies. It does not install the binary onto your -`PATH` — use `just build` and run `./dist/upkeep` to exercise a local build. +`just install` installs dependencies. To put a locally built binary on your +`PATH` — for trying a change without waiting on a release and the tap's 24-hour +hold — use: + +```bash +just install-local +``` + +That compiles for the host platform and installs to `~/.local/bin` (falling back +to `~/.upkeep/bin`). Override the destination with `UPKEEP_INSTALL_DIR`. It warns +if the target directory is not on your `PATH`. ## Commands @@ -32,6 +41,8 @@ Run `just --list` for the full set. The ones you will use most: | `just lint-fix` | Lint and apply fixes | | `just check` | Lint, typecheck, and test — run this before pushing | | `just build` | Compile the binary for the host platform to `dist/upkeep` | +| `just install-local` | Compile and install onto your `PATH` for local testing | +| `just coverage-check` | Run tests with coverage and enforce the floor (what CI runs) | | `just build-all` | Compile for all five release targets | | `just clean` | Remove `dist/` | @@ -72,6 +83,37 @@ Two consequences worth knowing: - The `typecheck` script points at `./node_modules/typescript/bin/tsc` explicitly, because both packages ship a `tsc` binary. +## Coverage + +```bash +just test-coverage # report only +just coverage-check # report and enforce the floor (what CI runs) +``` + +CI runs `scripts/check-coverage.sh`, which fails the build if aggregate coverage +drops below the floor recorded in that script. + +The floor is enforced by our own script rather than Bun's built-in +`coverageThreshold` for two reasons, both verified on Bun 1.3.9: + +- The table forms (`coverageThreshold = { line = ..., function = ... }` and a + `[test.coverageThreshold]` section) are not enforced at all — an impossible + 0.99 threshold still exits 0. +- The single-number form *is* enforced, but **per file**, not against the + aggregate. Since `src/lib/utils/exec.ts` is at 0% functions, any non-zero + threshold fails the build (verified down to 0.02). + +A gate that silently does nothing is worse than no gate, so the script parses the +aggregate row itself and fails closed if it cannot find or parse it. Once +`exec.ts` has real tests (issue #27), switching back to the built-in per-file +threshold becomes realistic. + +One thing worth knowing when reading coverage numbers: `tests/cli/` runs the CLI +through `Bun.spawn`, and instrumentation does not follow into a child process. +Those tests contribute **nothing** to the percentages — everything measured comes +from `tests/lib/`. The CLI tests earn their place as end-to-end verification, not +as coverage. + ## Project structure ``` diff --git a/justfile b/justfile index 0dc95de..1bf8a6d 100644 --- a/justfile +++ b/justfile @@ -18,6 +18,26 @@ build: build-all: bun run build:all +# Compile for the host platform and install onto PATH, for testing local changes +# without waiting on a release (and then on the Homebrew tap's 24h hold window). +# Override the destination with UPKEEP_INSTALL_DIR. +install-local: build + @INSTALL_DIR="${UPKEEP_INSTALL_DIR:-}"; \ + if [ -z "$INSTALL_DIR" ]; then \ + if [ -d "$HOME/.local/bin" ]; then \ + INSTALL_DIR="$HOME/.local/bin"; \ + else \ + INSTALL_DIR="$HOME/.upkeep/bin"; \ + fi; \ + fi; \ + mkdir -p "$INSTALL_DIR"; \ + install -m 755 dist/upkeep "$INSTALL_DIR/upkeep"; \ + echo "Installed $("$INSTALL_DIR/upkeep" --version) to $INSTALL_DIR/upkeep"; \ + case ":$PATH:" in \ + *":$INSTALL_DIR:"*) ;; \ + *) echo "NOTE: $INSTALL_DIR is not on your PATH; add it to use \`upkeep\` directly." ;; \ + esac + test: bun test @@ -27,6 +47,10 @@ test-watch: test-coverage: bun run test:coverage +# Run tests with coverage and enforce the aggregate floor (what CI runs). +coverage-check: + ./scripts/check-coverage.sh + typecheck: bun run typecheck diff --git a/scripts/check-coverage.sh b/scripts/check-coverage.sh new file mode 100755 index 0000000..cbc1015 --- /dev/null +++ b/scripts/check-coverage.sh @@ -0,0 +1,82 @@ +#!/usr/bin/env bash +# +# Run the test suite with coverage and enforce an aggregate floor. +# +# Why this exists rather than Bun's built-in `coverageThreshold`: +# that setting is enforced PER FILE, not against the aggregate. Because +# src/lib/utils/exec.ts currently sits at 0% functions (see issue #27), any +# non-zero per-file threshold fails the build -- verified down to 0.02. The +# table forms (`coverageThreshold = { line = ..., function = ... }` and a +# `[test.coverageThreshold]` section) are additionally not enforced at all on +# Bun 1.3.9: an impossible 0.99 still exits 0. A silently inert gate is worse +# than no gate, so we parse the aggregate ourselves. +# +# Swap back to the built-in once exec.ts is covered and a per-file floor is +# realistic. +# +# Floors can be overridden for local experimentation: +# MIN_LINES=80 MIN_FUNCS=85 ./scripts/check-coverage.sh + +set -euo pipefail + +MIN_LINES="${MIN_LINES:-75}" +MIN_FUNCS="${MIN_FUNCS:-83}" + +output=$(bun test --coverage 2>&1) && test_status=0 || test_status=$? +echo "$output" + +if [ "$test_status" -ne 0 ]; then + echo "" + echo "Tests failed (exit $test_status); not evaluating coverage." >&2 + exit "$test_status" +fi + +# The summary row Bun prints for the whole project. +summary=$(printf '%s\n' "$output" | grep -E '^All files' | head -1 || true) + +# Fail closed. If the table format ever changes, this must not quietly pass. +if [ -z "$summary" ]; then + echo "" + echo "ERROR: could not find the 'All files' coverage summary row." >&2 + echo "The coverage output format may have changed. Refusing to report success." >&2 + exit 1 +fi + +funcs=$(printf '%s' "$summary" | awk -F'|' '{gsub(/[[:space:]]/, "", $2); print $2}') +lines=$(printf '%s' "$summary" | awk -F'|' '{gsub(/[[:space:]]/, "", $3); print $3}') + +for pair in "funcs:$funcs" "lines:$lines"; do + name=${pair%%:*} + value=${pair#*:} + case "$value" in + ''|*[!0-9.]*) + echo "" + echo "ERROR: could not parse a numeric $name percentage (got '$value')." >&2 + echo "Refusing to report success." >&2 + exit 1 + ;; + esac +done + +echo "" +echo "Coverage gate" +echo " functions: ${funcs}% (floor ${MIN_FUNCS}%)" +echo " lines: ${lines}% (floor ${MIN_LINES}%)" + +failed=0 +if awk -v v="$funcs" -v m="$MIN_FUNCS" 'BEGIN { exit (v + 0 >= m + 0) ? 1 : 0 }'; then + echo " FAIL: function coverage ${funcs}% is below the ${MIN_FUNCS}% floor." >&2 + failed=1 +fi +if awk -v v="$lines" -v m="$MIN_LINES" 'BEGIN { exit (v + 0 >= m + 0) ? 1 : 0 }'; then + echo " FAIL: line coverage ${lines}% is below the ${MIN_LINES}% floor." >&2 + failed=1 +fi + +if [ "$failed" -ne 0 ]; then + echo "" + echo "Coverage regressed. Add tests, or lower the floor deliberately in this script." >&2 + exit 1 +fi + +echo " OK" diff --git a/tests/cli/commands/audit.test.ts b/tests/cli/commands/audit.test.ts index b4102f1..3296f10 100644 --- a/tests/cli/commands/audit.test.ts +++ b/tests/cli/commands/audit.test.ts @@ -38,10 +38,15 @@ describe("upkeep audit", () => { }); }); - // Note: The following tests require running actual package manager audit - // commands which can be slow and require network access. They are skipped - // by default but can be enabled for integration testing by removing .skip. - describe.skip("integration tests (require package manager)", () => { + // These shell out to a real package manager, so they are slower than the rest + // of the suite (~11s for the integration blocks across this file and + // deps.test.ts). They run everywhere -- locally and in CI -- because they are + // the only tests that exercise the CLI end to end. + // + // Note they contribute nothing to the coverage numbers: runCli uses + // Bun.spawn, and coverage instrumentation does not follow into a child + // process. Their value is end-to-end verification, not coverage. + describe("integration tests (require package manager)", () => { describe("output format", () => { test("outputs valid JSON", async () => { const { stdout } = await runCli(["audit"], getFixturePath("sample-project")); diff --git a/tests/cli/commands/deps.test.ts b/tests/cli/commands/deps.test.ts index dbd50ad..05aa2b8 100644 --- a/tests/cli/commands/deps.test.ts +++ b/tests/cli/commands/deps.test.ts @@ -54,10 +54,15 @@ describe("upkeep deps", () => { }); }); - // Note: The following tests require running actual package manager commands - // which can be slow. They are skipped by default but can be enabled for - // integration testing by removing the .skip. - describe.skip("integration tests (require package manager)", () => { + // These shell out to a real package manager, so they are slower than the rest + // of the suite (~11s for the integration blocks across this file and + // deps.test.ts). They run everywhere -- locally and in CI -- because they are + // the only tests that exercise the CLI end to end. + // + // Note they contribute nothing to the coverage numbers: runCli uses + // Bun.spawn, and coverage instrumentation does not follow into a child + // process. Their value is end-to-end verification, not coverage. + describe("integration tests (require package manager)", () => { describe("output format", () => { test("outputs valid JSON", async () => { const { stdout } = await runCli(["deps"], getFixturePath("sample-project"));