Skip to content

Validate end-to-end, test the untested modules, ship as a gh extension - #1

Open
aditya-dua wants to merge 1 commit into
mainfrom
aditya-dua-e2e-validation-and-tests
Open

aditya-dua wants to merge 1 commit into
mainfrom
aditya-dua-e2e-validation-and-tests

Conversation

@aditya-dua

Copy link
Copy Markdown
Owner

The pure functions were well covered, but init, save, store.ts and stack.ts had never run against real GitHub. I created a throwaway private source repo with a three-layer stack plus a standalone PR, and a throwaway private store, and ran the whole path. That turned up one substantial defect and several smaller ones.

Stack resolution never worked

stack.ts read a stack object from the pull request REST resource. REST has no such field — .stack is absent on both the single-PR and list endpoints — so head.stack.number was always null and getStack early-returned a single layer. Every stack silently collapsed to one layer, and no test caught it because nothing exercised the network path.

Stacks are only exposed via GraphQL:

pullRequest(number: $number) {
  stackEntry { position stack { number size baseRefName entries(first: 50) { ... } } }
}

That also returns every member's position in one round-trip, so resolving a stack is now a single call.

GitHub further reports a native stack only for pull requests created through its stacking flow. A hand-chained stack — each PR targeting the one below — has stackEntry: null. Those are now reconstructed by walking base-to-head links from the trunk, stopping at any branch point since a tree is not a stack, and guarded against cycles. Inferred layers carry source: "inferred" so the distinction is never lost.

Verified live: getStack on PRs #1/#2/#3 of the scratch stack now returns #1@1/3 #2@2/3 #3@3/3, where previously each returned only itself.

Other defects found by actually running it

  • gh api --paginate emits one JSON array per page, concatenated — not valid JSON. JSON.parse would have thrown on any repo with more than 100 open PRs. Now --paginate --jq '.[]' and parsed as JSON lines.
  • -f body= mangles multi-line markdown. The brief suspected this; it's correct. Comment bodies are now sent as JSON on stdin via --input -. Verified a table, backticks, quotes and emoji round-trip byte-for-byte.
  • A store created without --add-readme has an unborn default branch, so the clone has no HEAD and the first push could publish whatever init.defaultBranch happens to be locally rather than the repo's own.
  • save was not idempotent. capturedAt is regenerated every run, dirtying the tree and pushing a commit on every invocation. Records are now compared ignoring that field, using a key-order-independent serializer — key order matters because stored records are reparsed from disk.
  • The fallback git identity was applied unconditionally. git -c user.email=… overrides configured values, so this would have hijacked a real identity. It is now supplied only when the machine genuinely has none. (This machine has none, which is how it got noticed.)
  • ?? does not short-circuit on an explicit null, so a missing adapter was re-probed once per layer.

Also validated: init on both the create and already-exists paths, a standalone PR correctly treated as a stack of one, and an empty session DB producing "nothing saved" rather than empty records.

Testing the untested modules

Testing needed a seam. src/exec.ts adds a CommandRunner that every module touching gh or git accepts, so the whole subprocess surface can be faked without a network. The fake throws on an unrouted command rather than returning empty output — which is how I caught three of my own tests that had forgotten to inject it and were quietly hitting the real API.

62 new tests (90 total) across stack.ts, store.ts, capture.ts and the new extension module, including the three behaviours called out in the brief:

  • a standalone PR is captured as a stack of one
  • stack members sort bottom-to-top by position
  • a branch with no recorded session yields no record, not an empty one

The re-probing test was initially vacuous, so I mutation-checked it: reverting the guard in capture.ts does make it fail.

Two incidental improvements fell out. Tests run against source, which required relative imports to carry .ts extensions so Node's type stripping can resolve them — Node never rewrites .js→.ts, which is why no existing test had ever crossed a runtime import (distill.ts and redact.ts only use import type, which is erased). rewriteRelativeImportExtensions restores .js on emit, so the published package is byte-identical in shape. And tests are now type-checked via tsconfig.test.json, which immediately caught a fixture that had drifted from the real CaptureSource and RedactionReport shapes.

Packaging as a gh extension

gh extension install requires the repository to be named gh-*; this one is named stacklore. I verified the constraint empirically — gh extension install . on a directory named stacklore fails with extension name must start with 'gh-'.

Rather than rename the package, stacklore gh-install writes the extension shim itself into gh's extension directory, so it works for a global npm install, npx, or a source checkout alike. A bash shim rather than a symlink, because gh execs the file directly and a .js file has no interpreter of its own. Paths are JSON-quoted so a home directory with spaces can't split the command.

Verified: gh stacklore status runs, flags pass through (gh stacklore list --json), reinstall is idempotent, and uninstall is clean. stacklore status now reports whether the extension is installed.

A gh-stacklore script at the repo root is the entry point for a companion gh-stacklore repository, documented as a follow-up.

Verification

npm run typecheck, npm test (90 passing), and npm run build all green; node dist/cli.js runs from a clean build.

Cleanup needed

The throwaway repos could not be deleted — this token lacks the delete_repo scope:

gh auth refresh -h github.com -s delete_repo
gh repo delete aditya-dua/stacklore-e2e-src --yes
gh repo delete aditya-dua/stacklore-e2e-store --yes

Both are private. No new adapters were added, per the brief.

The pure functions were well covered, but `init`, `save`, `store.ts` and
`stack.ts` had never run against real GitHub. Exercising them turned up one
substantial defect and several smaller ones.

Stack resolution never worked. `stack.ts` read a `stack` object from the
pull request REST resource, but REST has no such field — `.stack` is absent
on both the single and list endpoints, so every stack silently collapsed to
a single layer. Stacks are only exposed via GraphQL (`PullRequest.stackEntry`),
which also returns every member's position in one round-trip. GitHub further
reports a native stack only for pull requests created through its stacking
flow, so a hand-chained stack is now reconstructed by walking base-to-head
links, stopping at any branch point since a tree is not a stack.

Other fixes found by running the thing:

- `gh api --paginate` concatenates one JSON array per page, which is not
  valid JSON; parsing now goes through `--jq '.[]'` and reads JSON lines.
- Comment bodies went through `-f body=`, which mangles multi-line markdown.
  They are now sent as JSON on stdin via `--input -`.
- A store created without `--add-readme` has an unborn default branch, so the
  first push could publish the local `init.defaultBranch` instead.
- `capturedAt` moves every run, so `save` pushed a commit even when nothing
  changed. Records are now compared ignoring that field.
- A fallback git identity was applied unconditionally; `-c user.email=`
  overrides configured values, so it is now used only when none is set.
- `??` does not short-circuit on an explicit null, so a missing adapter was
  re-probed once per layer.

Testing required a seam: `src/exec.ts` introduces a `CommandRunner` that every
module taking `gh` or `git` now accepts, so the subprocess surface can be faked
without a network. Tests run against source, which meant relative imports had
to carry `.ts` extensions — `rewriteRelativeImportExtensions` restores `.js` on
emit, so the published package is unchanged. Tests are now type-checked too,
which immediately caught fixture drift from the real record shape.

Finally, `gh extension install` requires the repository to be named `gh-*`,
which this one is not. Rather than rename, `stacklore gh-install` writes the
extension shim itself, so `gh stacklore ...` works for a global npm install or
a source checkout alike.

62 new tests, 90 total.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
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