Skip to content

test: type check the test tree and fix fixture drift - #954

Merged
robwoodgate merged 2 commits into
mainfrom
test/typecheck-gate
Aug 12, 2026
Merged

test: type check the test tree and fix fixture drift#954
robwoodgate merged 2 commits into
mainfrom
test/typecheck-gate

Conversation

@robwoodgate

@robwoodgate robwoodgate commented Aug 12, 2026

Copy link
Copy Markdown
Collaborator

Description

Nothing has ever type checked test/. Vitest strips types via esbuild without checking them, and test/tsconfig.json existed only to give eslint's typed rules a project covering the test tree, which catches rule violations rather than assignability. src/ was covered all along, indirectly: api:check runs vite-plugin-dts, which type checks it.

The result was 89 errors across 12 files, accumulated silently as the library types moved. The great majority is one field: method became required on MintQuoteBaseResponse and MeltQuoteBaseResponse, and mint quotes also gained amount_paid / amount_issued / updated_at.

Changes

  • npm run check-types runs tsc -p test/tsconfig.json. The tests import the library source directly rather than the built package, so tsc follows those imports and pulls all 87 files in src/ into the same program. One command therefore covers both trees, and there is no need for a second project for src/. test/tsconfig.json gained rootDir and noEmit so a bare tsc -p behaves the same on the command line as it already did in an editor.
  • test/consumer/ is excluded. Those harnesses deliberately import the package by its published name to simulate an external consumer, so they only resolve after a build. They are covered by consumers.yml, which builds first, and eslint skips them for the same reason.
  • Wired into prtasks, a types.yml workflow following the existing lint.yml shape, and the pre-push hook. A full pass takes about 3.5 seconds.
  • Fixtures realigned with the current types. Unpaid quotes get 0/0 accounting, which validateMintQuoteAvailableAmount short-circuits on, so those tests take the same runtime path as before. Deliberately malformed fixtures kept their hostile shape behind an explicit cast rather than being "fixed".
  • Fixed the pre-push condition. It read if ! npm run check-lint && npm run check-format, where ! binds only to the first command. It aborted only when lint failed and format passed: when lint passed it short-circuited and format never ran, and when both failed it did nothing.
  • AGENTS-CONTRIBUTING.md said Proof.amount is bigint and to construct with Amount.from(x).toBigInt(). That was correct when written but has been wrong since refactor: replace raw bigint with Amount VO on Proof and BlindedMessage #609 moved it to Amount, and it is where the payment request fixtures came from.

Reviewer Notes

  • No files under src/ are touched and etc/cashu-ts.api.md is unchanged, so there is no public API movement here.
  • The obvious alternative was shared fixture factories, so that a future field addition would need fixing in one place instead of 47. I left them out on purpose: with the gate in place, the next type change breaks the build immediately, at however many sites exist that day, rather than piling up unnoticed until someone opens the file. Fixing one or two sites as they break is cheaper than maintaining factories, and the factories would not have prevented today's cleanup anyway.
  • The gate was checked for teeth rather than assumed: planting a Proof with a raw bigint amount makes check-types exit 2 and the hook exit 1, and both return to 0 once removed.
  • The sweep also surfaced several src/ defects where a declared type asserts more than the code delivers. Those are deliberately not in this PR; they follow in a separate one stacked on this branch.

Vitest strips types without checking them, and test/tsconfig.json only fed
eslint's typed rules, so nothing ever ran tsc over test/. src/ was already
covered indirectly via api:check. Fixtures had drifted from the library types
in 89 places across 12 files.

- add check-types (tsc over test/, which pulls src/ in through its imports),
  wired into prtasks, a types.yml workflow and the pre-push hook
- realign quote fixtures: method is required on the mint and melt quote bases,
  and mint quotes also carry the accounting fields
- correct the pre-push condition. '! a && b' only aborted when lint failed and
  format passed, so format was never gated
- AGENTS-CONTRIBUTING still described Proof.amount as bigint; it has been an
  Amount since #609, which is where the payment request fixtures came from
@codecov

codecov Bot commented Aug 12, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 95.76%. Comparing base (20a29e2) to head (ced752f).
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #954   +/-   ##
=======================================
  Coverage   95.76%   95.76%           
=======================================
  Files          55       55           
  Lines        5781     5781           
  Branches     1465     1465           
=======================================
  Hits         5536     5536           
  Misses        104      104           
  Partials      141      141           
Flag Coverage Δ
integration 37.75% <ø> (+0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

test/consumer imports the built package by its published name, so it only
resolves once lib/ exists. A clean checkout has no build, which is why this
passed locally and failed in CI. Those harnesses have their own workflow
(consumers.yml) and eslint already skips them.
@robwoodgate
robwoodgate merged commit 134598d into main Aug 12, 2026
19 checks passed
@robwoodgate
robwoodgate deleted the test/typecheck-gate branch August 12, 2026 19:22
robwoodgate added a commit that referenced this pull request Aug 12, 2026
> Stacked on #954. Base is `test/typecheck-gate`, so review only the
second commit; GitHub will retarget to `main` once #954 merges.

## Description

Type checking the test tree (#954) surfaced five places in `src/` where
a declared type asserts more than the implementation delivers. Four were
held open by an `as` cast, which is why the compiler never objected: `as
unknown as` appears exactly twice in the whole of `src/`, and one of the
two was a defect.

## Changes

- **`PaymentRequest.decodePayload`** returned `amount:
Amount.from(x).toBigInt()`, a raw bigint, behind `as unknown as Proof`,
while `PaymentRequestPayload.proofs` has always declared `Proof[]` (so
`amount: Amount`). A consumer following the types into
`payload.proofs[0].amount.toBigInt()` got a runtime TypeError, and
`encodePayload` disagreed with its own decoder about what a payload
proof is. It now normalizes to `Amount` and the cast is gone. The API
report is unchanged for this: the declared type was always right, only
the implementation lied.
- **`PrepareMeltConfig` folded into `MeltProofsConfig` and deleted.** It
was added incidentally to widen one `prepareMelt` signature when
`nut08Change` arrived, carried no docblock, and had a single use. The
four public melt helpers took `MeltProofsConfig` and forwarded it
wholesale, so `nut08Change` worked at runtime through all of them but
could not be expressed in TypeScript. Since the field is optional,
folding it in changes nothing for existing callers, and it fixes the gap
by making the API smaller rather than widening four signatures.
- **`createLockedMintQuote`** throws unless the mint echoes a pubkey,
then returns it, but declared `MintQuoteBolt11Response` where `pubkey`
is optional. The return now carries it, removing the non-null assertions
at call sites.
- **`NUT10Option.tags` and `RawNUT10Option.t` are now optional.** NUT-10
marks `tags` optional and NUT-18 says "`t`: optional NUT-10 payment
tags". CTS's own internal `Nut10SpendingCondition` in `utils/tlv.ts`
already declared it optional, with a parser that deliberately returns
`undefined` when absent, and every read site guards with `?? []`.
Decoding a spec-valid request without tags therefore produced an object
that violated its own declared type.
- **The bolt11 mint quote `state` is marked `@deprecated`**, pointing at
`amount_paid` / `amount_issued`. The docblock already said "deprecated"
in prose, but nothing machine-readable expressed it. It stays populated
for backwards compatibility.

## Reviewer Notes

- `decodePayload` is the only behavioural change. Exactly the two tests
that had asserted the bigint shape failed, and are updated along with
the doc line that described it.
- Compatibility: `decodePayload` now hands back `Amount` where it used
to hand back `bigint`, and `PrepareMeltConfig` is removed from the
public surface. Both only affect the v5 rc line, so I have not marked
this a breaking change
- The stale `as PrepareMeltConfig` workaround introduced in #954 comes
back out here, which is a decent check that the gap is genuinely closed.
- Not fixed, deliberately: `CTSError.cause` is declared `readonly` but
defined `writable: true` at runtime. That mismatch is intentional and a
test asserts the descriptor.
@github-project-automation github-project-automation Bot moved this from Backlog to Done in cashu-ts Aug 12, 2026
robwoodgate added a commit that referenced this pull request Aug 12, 2026
## Description

Backport of the type-check gate added on `main` in #954. Nothing has
ever run `tsc` over `test/` on this line either: vitest strips types via
esbuild without checking them, and `test/tsconfig.json` existed only to
give eslint's typed rules a project covering the test tree, which
catches rule violations rather than assignability. `src/` was covered
all along, indirectly, since `api:check` runs `vite-plugin-dts`.

v4 had drifted far less than main did: 21 errors across 10 files,
against main's 89 across 12. With v4 remaining the GA line until v5
ships, that seemed worth closing.

## Changes

- `npm run check-types` runs `tsc -p test/tsconfig.json`. The tests
import the library source directly rather than the built package, so tsc
follows those imports and pulls all 81 files in `src/` into the same
program. One command therefore covers both trees. `test/tsconfig.json`
gained `rootDir` and `noEmit` so a bare `tsc -p` behaves the same on the
command line as it already did in an editor.
- Wired into `prtasks`, a `types.yml` workflow following this branch's
existing `lint.yml` shape (including its `actions/*@v4` pins), and the
pre-push hook.
- `test/consumer/` is excluded. Those harnesses deliberately import the
package by its published name to simulate an external consumer, so they
only resolve after a build. They are covered by `consumers.yml`, which
builds first, and eslint skips them for the same reason.
- Fixtures realigned with the current types. The bulk is `Logger`
gaining a `log` method after the mock helpers were written. The rest:
`MintQuoteBolt12Response` has no `state` and `MintQuoteOnchainResponse`
no `amount` (both use the accounting fields), an optional `pubkey`
needing an assertion, and two stale arguments left over from older
signatures, including an `idempotent` request option that exists nowhere
in `src/`.
- `AGENTS.md` and `CONTRIBUTING.md` updated for the new command.

## Reviewer Notes

- No files under `src/` are touched and `etc/cashu-ts.api.md` is
unchanged, so there is no public API movement.
- Fixes deliberately mirror the shapes used on main so that future
backports apply cleanly. The only divergence is prettier line wrapping
where adding `log: vi.fn()` crossed the print width.
- Verified with `lib/` moved aside, to simulate a clean checkout: that
is exactly the case that slipped through locally on main and failed CI
there, because a previous build had made `test/consumer/` resolve.
- `npm run prtasks` passes end to end: 179 test files, 6097 tests.
robwoodgate added a commit that referenced this pull request Aug 14, 2026
Main now type checks the test tree (#954); annotate the leaf fixture,
cast the deliberately wrong-typed amounts, and drop an ES2022 method.
robwoodgate added a commit that referenced this pull request Aug 19, 2026
Main now type checks the test tree (#954); annotate the leaf fixture,
cast the deliberately wrong-typed amounts, and drop an ES2022 method.
robwoodgate added a commit that referenced this pull request Aug 24, 2026
Main now type checks the test tree (#954); annotate the leaf fixture,
cast the deliberately wrong-typed amounts, and drop an ES2022 method.
robwoodgate added a commit that referenced this pull request Aug 25, 2026
Main now type checks the test tree (#954); annotate the leaf fixture,
cast the deliberately wrong-typed amounts, and drop an ES2022 method.
robwoodgate added a commit that referenced this pull request Aug 27, 2026
Main now type checks the test tree (#954); annotate the leaf fixture,
cast the deliberately wrong-typed amounts, and drop an ES2022 method.
robwoodgate added a commit that referenced this pull request Aug 28, 2026
Main now type checks the test tree (#954); annotate the leaf fixture,
cast the deliberately wrong-typed amounts, and drop an ES2022 method.
robwoodgate added a commit that referenced this pull request Aug 28, 2026
Main now type checks the test tree (#954); annotate the leaf fixture,
cast the deliberately wrong-typed amounts, and drop an ES2022 method.
robwoodgate added a commit that referenced this pull request Sep 1, 2026
Main now type checks the test tree (#954); annotate the leaf fixture,
cast the deliberately wrong-typed amounts, and drop an ES2022 method.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

1 participant