Skip to content

test: add hermetic coverage for scripts/sync-skills.sh (ADR-0026) - #123

Merged
bketelsen merged 4 commits into
mainfrom
cockpit/worker-3479163bfa3ea977
Aug 25, 2026
Merged

test: add hermetic coverage for scripts/sync-skills.sh (ADR-0026)#123
bketelsen merged 4 commits into
mainfrom
cockpit/worker-3479163bfa3ea977

Conversation

@bketelsen

Copy link
Copy Markdown
Contributor

Summary

  • Adds test/sync-skills.test.mjs, a hermetic test harness for scripts/sync-skills.sh that stubs git clone and gh via PATH shims so the script runs against local fixture repos with no network access and no real GH_TOKEN.
  • Automatically wired into npm run check (npm test already globs test/*.test.mjs) — no other files changed.

Coverage

  • An up-to-date consumer (destination already matches core) performs no git push or gh invocation.
  • A changed managed skill checks out chore/sync-core-skills, commits, pushes, calls gh pr list then gh pr create with the expected repo/branch/title, and the pushed branch tip carries the synced skill content and marker.

Test plan

  • node --test test/sync-skills.test.mjs passes (2/2).
  • npm run check passes (28/28 tests across the repo) — verified locally with a minimal local-only rsync shim, since this sandbox lacks a system rsync binary; the pre-existing sync-skills-containment.test.mjs has the same sandbox-only dependency and is unaffected by this change. GitHub's ubuntu-latest runners (used by this repo's docs-gate CI job) ship rsync by default.
  • Confirmed the new tests actually catch regressions: temporarily broke the sync script's up-to-date early-exit and separately its git push step — both times the new tests failed with the expected assertion, then the script was restored with no diff.

Risk tier: 1 — adds test coverage only, no production code changed.

result.model: claude-sonnet-5

Stubs git clone and gh via PATH shims so the sync script's control flow
runs against local fixture repos with no network access or real
GH_TOKEN, and wires the new test/*.test.mjs file into npm run check's
existing `npm test` step.
@bketelsen
bketelsen marked this pull request as ready for review August 25, 2026 01:01
@bketelsen
bketelsen added this pull request to the merge queue Aug 25, 2026
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Aug 25, 2026
@bketelsen
bketelsen added this pull request to the merge queue Aug 25, 2026
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Aug 25, 2026
bketelsen and others added 3 commits August 24, 2026 21:20
#124 changed scripts/lib/skills-sync-repo.sh to clone and push as
`git -c credential.helper=<helper> <subcommand> ...`. The hermetic
harness's fake git only recognised `clone` as $1, so the real
github.com URL passed straight through to the real binary and CI's
docs-gate failed both tests with an authentication error.

- Locate the subcommand by skipping `-c <value>` pairs and flags (the
  same detection test/sync-skills-run-repo.test.mjs uses), and rewrite
  the frostyard clone URL by pattern rather than by argv position.
- Accept the credential-helper prefix on the asserted push line.
- Carry the sibling test's rsync fallback so the harness stays runnable
  in sandboxes without a system rsync.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@bketelsen
bketelsen added this pull request to the merge queue Aug 25, 2026
Merged via the queue into main with commit 65cb039 Aug 25, 2026
3 checks passed
@bketelsen
bketelsen deleted the cockpit/worker-3479163bfa3ea977 branch August 25, 2026 01:46
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