Skip to content

fix(openapi): document GET /fees, and make the coverage test deterministic and fast (W082) - #250

Merged
Miracle656 merged 3 commits into
Miracle656:mainfrom
Glorious21:fix/w082-openapi-coverage
Oct 18, 2026
Merged

Miracle656 merged 3 commits into
Miracle656:mainfrom
Glorious21:fix/w082-openapi-coverage

Conversation

@Glorious21

Copy link
Copy Markdown
Contributor

Summary

Makes src/__tests__/openapiCoverage.test.ts green on main, deterministic and fast.

Routes that were missing

Only one: GET /fees. It shipped in f89a303 without a spec entry. It is now documented, including the optional ?amount= query and the per-asset fee shape (enabled, fees keyed by asset code), and openapi.json is regenerated.

Why it flapped

The route check itself is deterministic: 10 runs in a row on main gave the same result. What changed between runs was the line endings in openapi.json, not the route table.

On Windows, git's core.autocrlf=true checks openapi.json out with CRLF line endings, so the byte-for-byte comparison failed. npm run docs:openapi writes LF, so it then passed until the next checkout. I reproduced this on one commit: 1 failure after running the generator, 2 after a fresh checkout.

Fixes:

  • .gitattributes pins openapi.json (and docs/openapi.json) to eol=lf.
  • The test compares content with line endings normalised.

Checked both: a fresh Windows checkout is now LF, and a file forced to CRLF still passes.

Speed

About 90% of the suite's time is loading the app (Stellar SDK, Apollo, Prisma). Walking the routes and building the spec takes about 100 ms. ts-jest was also type-checking that whole import graph on every cold run, which CI already does in npm run typecheck (it covers src/**/*, tests included) before npm test. tsconfig.jest.json now sets isolatedModules, so Jest only transpiles.

before after
cold, with coverage (as CI runs it) 23 s 13 s
warm ~9 s ~2 s
full npm test — 37 s

This setting applies to every suite, not just this one. Flagging it in case you'd rather keep ts-jest type checking.

Acceptance criteria

  • The suite passes on main. Full npm test: 55/55 suites, 700 passed, 1 skipped.
  • Ten consecutive runs give the same result. 10/10 passed, each in about 1.8 s.
  • Adding an undocumented route fails the suite. efadd0d adds GET /w082-undocumented-probe, and the suite fails with + "get /w082-undocumented-probe". 4a130f5 reverts it.
  • Every missing route is named: GET /fees was the only one.

Found along the way, not fixed here

src/__tests__/routes/accounts.test.ts makes a real network call on every GET: the stale-data middleware calls checkRpcHealth → getLatestLedger against the Soroban RPC, and nothing mocks it. On a slow connection, the balance tests hit Jest's 5 s timeout. I saw this once during this work; it then passed 4 times in a row. It probably needs its own issue.

Related issue

Closes #239

Type of change

  • Bug fix
  • New feature
  • Refactor
  • Docs
  • Tests
  • CI / tooling

Checklist

  • I have read CONTRIBUTING.md
  • npx tsc --noEmit passes
  • npm run build passes
  • I added / updated tests where relevant
  • I updated docs where relevant

🤖 Generated with Claude Code

Glorious21 and others added 3 commits October 15, 2026 20:38
…istic and fast (W082)

The coverage suite was red on main for one route: GET /fees shipped
without a spec entry. It is now documented, with its optional ?amount=
query and the per-asset fee shape.

The suite looked flaky, but the route walk is deterministic; the
environment was not. On a Windows checkout core.autocrlf rewrote
openapi.json to CRLF, so the byte comparison failed until someone ran
`npm run docs:openapi` (which writes LF) and then failed again after
the next checkout. .gitattributes pins the spec to LF, and the test
compares content with line endings normalised.

Speed: tsconfig.jest.json now sets isolatedModules, so ts-jest
transpiles instead of type-checking the whole app graph a second time.
CI already type-checks src/ and the tests in `npm run typecheck`. A
cold run of this suite drops from 23s to 13s; warm, from ~9s to ~2s.

Closes Miracle656#239

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…catches it (W082, reverted next)

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…e suite catches it"

The probe did its job: with it in place the coverage suite failed with
`+ "get /w082-undocumented-probe"`. Removed again.

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

drips-wave Bot commented Oct 15, 2026

Copy link
Copy Markdown

@Glorious21 Great news! 🎉 Based on an automated assessment of this PR, the linked Wave issue(s) no longer count against your application limits.

You can now already apply to more issues while waiting for a review of this PR. Keep up the great work! 🚀

Learn more about application limits

@Miracle656 Miracle656 left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Merging this first because it fixes main. CI has been red there on exactly this:

● OpenAPI route coverage (W085) › every route registered on the app appears in the generated spec
+   "get /fees",
Test Suites: 1 failed, 55 passed | Tests: 1 failed, 710 passed

Every one of the 33 open PRs in this repo has been inheriting that red, so nobody could tell a real failure from the inherited one. Verified locally on your branch merged with current main: tsc --noEmit clean, openapiCoverage.test.ts 7 passed.

The flake diagnosis is the best part of this PR.

It also appeared to flap, but the route walk is deterministic — ten runs in a row agree. What changed between runs was the environment, not the route table.

And then both causes, named: openapi.json with CRLF on a Windows checkout so the byte comparison failed until someone ran npm run docs:openapi (which writes LF) and then failed again after the next checkout; and before that, reading the gitignored docs/openapi.json, which only existed on machines that had already run the generator. Fixed with .gitattributes rather than by loosening the comparison.

That is the difference between fixing a flaky test and deleting the signal. The easy version of this PR relaxes the byte comparison to a normalised diff and nobody ever notices the spec drifting again.

The test got stricter, not looser — get /fees added to the route-key assertions and a dedicated documents /fees. A PR titled "make the coverage test deterministic and fast" that comes out with more assertions than it went in with is the right shape.

And the speed fix is honest about what it trades. isolatedModules in tsconfig.jest.json means ts-jest transpiles instead of type-checking the whole import graph on every cold run — which is fine precisely because npm run typecheck already does that in its own job. Removing duplicated work rather than removing coverage.

Merging.

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.

The OpenAPI coverage test has been red, so the spec is drifting unnoticed

2 participants