Skip to content

test: cover the configuration the JS client forwards into each request - #17

Merged
dmccoystephenson merged 1 commit into
mainfrom
test/client-config-reaches-request
Oct 3, 2026
Merged

dmccoystephenson merged 1 commit into
mainfrom
test/client-config-reaches-request

Conversation

@dmccoystephenson

Copy link
Copy Markdown
Contributor

Summary

Stage B (unit-test expansion) on js/src/client.ts. Characterization tests only — no production code is changed.

Several configuration options and branches in the client were accepted but never asserted to reach the outgoing request. Tests were added for:

  • ref — a non-default ref reaches the raw-host URL (and is trimmed); a blank ref falls back to HEAD; on the Contents API it is sent as an encoded ?ref= query parameter.
  • apiBase — a GitHub Enterprise base (with a trailing slash) is used for API requests.
  • A forced raw transport with a token configured — the token is not sent to raw.githubusercontent.com (headers are exactly {Accept: 'text/plain'}). This is a credential-handling property that had no asserting test.
  • Contents API headers — Accept: application/vnd.github.raw, the pinned X-GitHub-Api-Version, and a token arriving with surrounding whitespace is trimmed before it goes into Authorization.
  • Target trimming — fetchMarkdown(' rules ') resolves the catalogue entry.
  • Size-limit boundary — a body of exactly maxDocumentBytes is served.
  • The standalone fetchers — fetchRawMarkdown applies its own maxDocumentBytes and passes an abort signal; fetchApiMarkdown sends no Authorization header for an undefined, null, empty, or blank token.

While reading fetchMarkdown, the size check was found to compare string.length after the full body is buffered, so it bounds neither bytes nor memory. Per the Stage B rule that production code is not changed under a test-expansion cycle, no test was written to lock that behavior in; it is filed as #16.

Half: js/ only.

Deferred issues: #5 and #6 are release/publishing prerequisites that require operator actions outside this repository (npm token and scope, PyPI trusted publisher, environments), so neither can be closed by a code change.

No tracking issue — coverage gap found during triage.

Test plan

  • CI js (18 | 20 | 22) green on the head SHA: npm run typecheck (the tests are included in tsconfig.json) and npm test
  • CI python legs green (unaffected)
  • Local anchor: UNVERIFIED — node is not installed in the session sandbox, so the new tests were not run locally; CI on the PR head SHA is the anchor.

This PR description was drafted during a Gardener session (https://github.com/Stephenson-Software/gardener).

🤖 Generated with Claude Code


drafted by Claude on behalf of Daniel Stephenson

ref, apiBase, a forced raw transport with a token, token trimming, the
Contents API headers, target trimming, the size-limit boundary and the
two standalone fetchers had no asserting test.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@dmccoystephenson

Copy link
Copy Markdown
Contributor Author

Self-review rubric (anchored on CI run 37108483239 on the head SHA and on gh pr diff 17):

  • Scope: PASS — the diff touches only js/test/client.test.ts (+153/−1; the one deletion is the import line, extended to bring in fetchApiMarkdown / fetchRawMarkdown). No production code, README, or manifest changed.
  • Tests-new: PASS — no new public symbol is added. The tests now exercise the two exported fetchers, which previously had no direct test.
  • Tests-fix: N/A — this is not a bug fix. The tests describe current behavior. The one apparent defect found (the size limit is checked by string length after the full body is buffered) was filed as maxDocumentBytes is checked against string length after the whole body is read, so it bounds neither bytes nor memory #16, and no test was written for it.
  • Sibling structure: PASS — the new describe blocks follow the file's established style: prose it names, fetchImpl injected via vi.fn(), the shared REPO/DOCS/TOKEN fixtures, and explanatory comments that give reasons.
  • Sibling renames: N/A — nothing renamed.
  • Docs: PASS — no behavior change, so no Phase 7 row is affected. The new assertions agree with the js/README.md Configuration table (ref default 'HEAD', apiBase for GitHub Enterprise, transport selection, maxDocumentBytes boundary).
  • Issue resolution: N/A — no Closes. The PR body records that no tracking issue exists.
  • CI: PASS — all 8 legs green. The js (18) log shows test/client.test.ts (35 tests) (24 before this PR, so 11 new tests ran) and Tests 76 passed (76). npm run typecheck passed too, and it includes test/**/*.ts under noUnusedLocals.
  • No-leak (js): PASS — no src/ change. The new raw-transport test also asserts that TOKEN appears nowhere in the raw-host request headers.
  • No-leak (python) / Stdlib-only / Sentinel intact: PASS — python/ is untouched, and the JS TOKEN sentinel is reused unchanged.
  • Result-not-exception / No runtime dependency / Export parity / Support-matrix parity: PASS — no source, manifest, or lockfile change.

Judgment call flagged: js/test/client.test.ts — fetchRawMarkdown bounds the request with an abort signal only checks that a signal is passed, not that it fires. The timeout firing is already covered through the client by gives up after the timeout by aborting the request, so a second timer-based test was not added.

Local anchor: UNVERIFIED (node is absent from the sandbox). The CI run on the exact head SHA served as the anchor.

This comment was drafted during a Gardener session (https://github.com/Stephenson-Software/gardener).


drafted by Claude on behalf of Daniel Stephenson

@dmccoystephenson
dmccoystephenson merged commit 4c184ec into main Oct 3, 2026
16 checks passed
@dmccoystephenson
dmccoystephenson deleted the test/client-config-reaches-request branch October 3, 2026 08:05
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