Skip to content

Test the client's untested fallback and failure branches - #2

Merged
dmccoystephenson merged 1 commit into
mainfrom
feat/test-uncovered-branches
Sep 27, 2026
Merged

dmccoystephenson merged 1 commit into
mainfrom
feat/test-uncovered-branches

Conversation

@dmccoystephenson

@dmccoystephenson dmccoystephenson commented Sep 24, 2026 •

Copy link
Copy Markdown
Member

Summary

A unit-test expansion cycle (Stage B). It adds characterization tests for branches of trace-client.ts that no test covered before. trace-client.ts itself is unchanged.

  • Constructor timeoutMs fallback: 0, -1, NaN and Infinity fall back to TraceClient.TIMEOUT_MS. A stub fetch checks that the request's signal has not aborted after 100 ms. If any of those values were used as-is, the request would abort.
  • Constructor errors name the argument: both throws mention baseUrl / application, including for non-string input.
  • serialize() tag cleaning: null/undefined tag values are dropped and other values are coerced to strings. A tags object left empty is omitted. ±Infinity values are omitted, and value: 0 is kept, which guards against a falsy check.
  • report() serialize-failure path: a tag whose toString() throws is logged as could not serialize <name>. The call resolves and nothing is sent.
  • send() success paths: a 202 counts as delivered, and so does a 2xx whose body can't be read. Neither produces a debug line.
  • describe(): an Error with a cause is logged as Error: <message> (<cause message>), and a non-Error rejection is logged via String().
  • bounded(): flush() and close() given -1, NaN or Infinity return promptly and don't wait on a hung request.
  • pagePath(): a non-string input or an unparseable absolute URL (http://[not-a-host/...) returns /.

No test showed a bug. Every assertion matches the code's current behavior as read from trace-client.ts.

No tracking issue: when this cycle started, the repo had no open issues. The gap was found during triage using the dev loop's Stage B checklist. A documentation sweep of README, CHANGELOG and TSDoc against trace-client.ts was done first and found no drift, so this cycle expands the tests instead.

Test plan

  • CI test job on this head: npm ci, npm run typecheck, npm test on Node 22 (run 35968520940 on 5b2f341: # tests 40, # pass 40, # fail 0)
  • Only tests/trace-client.test.ts changed. Its new imports are none. It uses only the existing node:* imports and ../trace-client.ts.
  • Timing bounds are generous (≥1.5 s for waits that should be ~0 ms, and a 100 ms stub delay against a 5 s default timeout).

The local anchor is UNVERIFIED. Node isn't installed in the environment that drafted this PR, so the suite couldn't be run locally. The CI check on this PR's head SHA is the verification.

Cross-client parity and changelog

  • trace-client-python / trace-client-java: no change needed, because no behavior visible to another client changed (tests only).
  • CHANGELOG.md: no entry, because nothing changes for users. Only tests were added.

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

Characterization tests only; trace-client.ts is unchanged. Covers the
timeoutMs fallback, serialize()'s tag cleaning and its failure path, any
2xx and an unreadable body counting as delivered, describe() naming an
Error's cause, bounded() with a negative or non-finite timeout, the
constructor naming the bad argument, and pagePath() on a non-string or
an unparseable absolute URL.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@dmccoystephenson

Copy link
Copy Markdown
Member Author

Self-review rubric (anchored on CI run 35968520940 on this head: test job green, # tests 40, # pass 40, # fail 0, all 8 new tests listed as ok):

  • Scope: PASS. git diff --name-only origin/main lists only tests/trace-client.test.ts.
  • Tests-new: PASS (not applicable). No new public members were added. The new tests cover existing branches: constructor, serialize, send, describe, bounded and pagePath.
  • Tests-fix: PASS (not applicable). This is a characterization-only change with no production fix to revert. No test showed a bug.
  • Sibling structure: PASS. The new it(...) blocks sit in the existing TraceClient and pagePath describe blocks. They reuse serve(new Capture()), the debug/log pair and sleep, and close every enabled client.
  • Sibling renames: PASS (not applicable). Nothing was renamed.
  • Docs: PASS. No behavior changed. Before choosing this work, the README, CHANGELOG and TSDoc were swept against trace-client.ts and no drift was found.
  • Issue resolution: PASS (not applicable). There is no Closes #N. The repo had no open issues.
  • CI: PASS. Green on the head SHA.
  • Zero-dependency: PASS. No imports were added. Imports are still only node:* and ../trace-client.ts.
  • Edge gate intact: PASS. git diff origin/main -- tsconfig.json trace-client.ts package.json is empty.
  • Never-throws / no console / export surface / version agreement: PASS. trace-client.ts is untouched.
  • Changelog: PASS. No entry is needed because nothing changes for users.
  • Cross-client parity noted: PASS. The PR body says no counterpart change is needed.

Findings:

  • tests/trace-client.test.ts (flush() and close() treat a negative or non-finite timeout as zero) — the Infinity case barely tells the two behaviors apart. Without the clamp in bounded(), Node would treat setTimeout(resolve, Infinity) as about 1 ms anyway, so that case would pass either way. The -1 and NaN cases have the same limitation. The assertion that does carry weight is that flush()/close() return well before the client's 3 s timeout, i.e. they don't wait on the hung request. This is left as is: it documents current behavior correctly.
  • tests/trace-client.test.ts is not type-checked (tsconfig.json include covers only trace-client.ts), so annotations such as RequestInit in the new stubs are stripped rather than checked. This was already true before this PR. It is noted here, not changed.
  • The local anchor is UNVERIFIED because the drafting environment has no Node. The CI run on the exact head SHA above is the verification.

This review 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 27c31f7 into main Sep 27, 2026
1 check passed
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