Skip to content

setup: type-check test/ during npm run build - #13

Merged
0dillon merged 1 commit into
ledgerkeep:mainfrom
enny791:setup/typecheck-tests
Aug 7, 2026
Merged

setup: type-check test/ during npm run build#13
0dillon merged 1 commit into
ledgerkeep:mainfrom
enny791:setup/typecheck-tests

Conversation

@enny791

@enny791 enny791 commented Aug 7, 2026

Copy link
Copy Markdown
Contributor

Summary

tsconfig.json excluded test/, so npm run build never type-checked test files, and CI never caught type errors in tests (Vitest transpiles without type-checking). This PR makes npm run build type-check test/ under the exact same strict settings as src/ — without emitting test files into dist/ — and fixes the real type errors that the stricter check surfaced.

What changed

  • tsconfig.test.json (new): extends tsconfig.json, so it inherits strict, noUncheckedIndexedAccess, noImplicitOverride, noUnusedLocals, noUnusedParameters, and moduleResolution: "NodeNext". It sets noEmit: true and includes test/**/*.ts (plus src/**/*.ts so imports resolve in one program).
  • tsconfig.json: dropped "test" from exclude. Its include is already src/**/*.ts, so nothing else changes; keeping "test" there would have documented exactly the stance this PR reverses.
  • package.json: "build": "tsc -p tsconfig.json && tsc -p tsconfig.test.json". The source emit step is unchanged; the second step only type-checks tests.
  • test/ttl.test.ts: annotated two key arrays as [xdr.LedgerKey, xdr.LedgerKey] tuples so keys[0]/keys[1] keep their element types under noUncheckedIndexedAccess.
  • test/discover.test.ts: constructs contractSpec.Err with { message: "LimitTooLarge" }, matching the installed SDK's Err<E extends ErrorMessage> where ErrorMessage = { message: string } — verified against the pinned @stellar/stellar-sdk@16.2.0 copy, not against documentation.
  • CONTRIBUTING.md: removed the "test/ is not type-checked" warning.

Key design decisions

  • Two configs + && in the build script rather than project references: tsc -b requires composite projects, which cannot set noEmit, so references would have forced test files to emit somewhere. The two-config approach keeps dist/ byte-for-byte identical (verified by md5sum) and makes a test-file type error fail npm run build.
  • No config weakening and no any/non-null assertions: the existing errors are fixed properly; both constructs are lint errors in this repository.
  • A tuple annotation, not as const: as const produces a readonly tuple, which is not assignable to buildReadings' xdr.LedgerKey[] parameter.

Acceptance criteria

  • test/ is type-checked under the same strict settings as src/tsconfig.test.json extends tsconfig.json; no test file is emitted into dist/ (noEmit, and find dist shows no test output).
  • Existing errors fixed without weakening noUncheckedIndexedAccess or using any/non-null assertions — tuple annotations in test/ttl.test.ts; typed Err construction in test/discover.test.ts.
  • A deliberate type error in a test file fails npm run build — planted const _bad: number = "definitely a string", build exited 2 with TS2322, then reverted.
  • npm run build produces exactly the same dist/ contents — md5sum of all 46 dist files identical before/after.
  • The CONTRIBUTING.md warning is removed.

Test output

format:check = 0   (prettier: all matched files use Prettier code style)
lint       = 0
build      = 0     (tsc -p tsconfig.json && tsc -p tsconfig.test.json)
test       = 0     (vitest run: 9 files, 100 tests passed)

No coverage threshold is configured in this repository; CI runs the same four commands on Node 22 and 24.

Note on the issue's error listing

The issue names test/ttl.test.ts:59, but enabling the check surfaced three real errors, not one: test/ttl.test.ts:59 and :73 (both noUncheckedIndexedAccess indexing) and test/discover.test.ts:89 (Err constructor typing). All three are fixed; none contradicts runtime behavior.

Follow-ups (not in scope)

  • Editors/IDEs: test files are not in the root tsconfig.json include, so an editor falls back to an inferred project for them rather than these strict settings. npm run build enforces the check; wiring the editor project is a separate concern.

Security

No signing path, key handling, or transaction logic was touched. No secrets involved.

Closes #12

@0dillon
0dillon merged commit 75b9722 into ledgerkeep:main Aug 7, 2026
2 checks 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.

fix(setup): type-check the test directory

2 participants