feat(coverage): enable 80% coverage gate via 28+ new tests (Story 15)#38
Conversation
…ory 15) Closes Task #15. Builds on Story 15.1 (PR #37 @ 23694dd) which unblocked this work by fixing the EIP-55 checksum + 0x-prefix issues in src/micropayment-engine.ts. Track A — extend tests/x402-v2.test.ts (tests #14-#26) - on402Response: happy / non-402 status / missing+malformed header - submitPayment: 200 OK / non-2xx / network error rewrap - verifyReceipt: 200+verified / non-2xx / network error (no throw) - createPaymentIntentHeader: happy / optional fields undef - negotiatePaymentIntent: happy / TODO branch locked - src/x402-v2.ts: 63.74% → 97.32% line coverage Track B — create tests/micropayment-engine.test.ts (B-1 through B-18) - constructor (3): 0x-guard, defaults, custom thresholds - queuePayment (3): paymentId shape, usdToBaseUnits errors, auto-settle - batchPayments (3): EIP-712 BatchSettlement sign / missing IDs / skipGas - getGasEstimate (2): 6-decimal USD, inverse scaling - setBatchWindow (2): s→ms conversion, window-boundary observable - getQueueStatistics (2): empty zeroes, multi-window aggregation - clearOldPayments (2): nothing-to-clear, settled+old removal - getPaymentStatus (1): find-or-undefined cross-window - src/micropayment-engine.ts: 0% → 100% line coverage Track C — .github/workflows/ci.yml - Add `Coverage gate` step (npm run coverage) between Run tests + Build - Add `Upload coverage report` step (actions/upload-artifact@ea165f8 v4) with if: always(), 14-day retention, per matrix Node version Test count: 115 baseline + 13 Track A + 18 Track B = 146 tests, all PASS Coverage: global 97.52% lines / 86.75% branches / 100% funcs / 97.52% stmts (exceeds vitest.config.ts thresholds 80/80/70/80 → npm run coverage exit 0) Determinism: all tests use vi.useFakeTimers + vi.setSystemTime + frozen Math.random; fetch is stubbed via vi.stubGlobal; zero real network I/O. Verbatim source preservation: 0 changes to src/ (verified git diff --stat). Zero skipped tests, zero .todo/xit/xdescribe/.only. Story chain: full SINKRA — @sm draft (v0.1) → @po GO-conditional → @sm patch (v0.2) → @dev attempt 1 → bug escalation → @aios-master roundtable → Story 15.1 source fix (PR #37) → @dev resume (this PR) → @qa pending → @devops pending merge.
|
Warning Review limit reached
Your plan currently allows 1 review/hour. Refill in 40 minutes and 11 seconds. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more review capacity refills, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than trial, open-source, and free plans. In all cases, review capacity refills continuously over time. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces comprehensive unit tests for the MicropaymentEngine and X402Handler, significantly increasing test coverage. The added tests verify core functionalities such as payment queuing, batching, gas estimation, and 402 response handling using deterministic fixtures. Feedback suggests improving test robustness by avoiding direct mutation of internal state in MicropaymentEngine tests and strengthening header assertions in X402Handler tests through round-trip verification.
| const item1 = engine.getPaymentStatus(id1)!; | ||
| item1.status = 'settled'; |
There was a problem hiding this comment.
Directly mutating the status property of the object returned by getPaymentStatus relies on the internal implementation detail that the engine returns a direct reference to the object stored in its private queue Map. While effective for unit testing the clearOldPayments logic (which only targets 'settled' items), it creates a tight coupling with the internal state management of the MicropaymentEngine class.
| expect(init.method).toBe('POST'); | ||
| expect(init.headers['Content-Type']).toBe('application/json'); | ||
| expect(init.headers['Authorization']).toBe('Bearer bearer-token-xyz'); | ||
| expect(init.headers['Payment-Intent-Authorization']).toMatch(/^x402:v2:/); |
There was a problem hiding this comment.
The assertion for Payment-Intent-Authorization only verifies the prefix. Since this header is a critical part of the protocol transit, consider adding a round-trip verification or checking that the base64-encoded payload contains the expected protocol and signature fields to ensure encodeSignedPayment is working correctly.
|
Merge authorization per .claude/rules/automated-pr-merge-authority.md (8th application — second story-driven merge of the day):
Closes Task #15. Enables 80% coverage gate in CI. Non-blocking observations (backlog candidates):
Pattern note — coverage work surfaced 2 product bugs today (Tasks #14 + #18), now both fixed:
Precedent chain (8 PRs today):
This makes 10 paybot ecosystem merges in a single day. |
1 participant
Summary
Closes Task #15. Enables the 80% coverage gate in CI on
paybot-sdkby adding 31 new tests across two modules and wiringnpm run coverageinto the CI build matrix. Unblocks the 0.3.0 release (Story 14 §Out of Scope).This PR builds on Story 15.1 (PR #37 @
23694dd), which fixed the EIP-55 checksum and 0x-prefix bugs that the first Story 15 attempt surfaced insrc/micropayment-engine.ts. Without the 15.1 fix, Track B (the newmicropayment-engine.test.tssuite) could not run.Why this matters
vitest.config.tsdefined coverage thresholds at80/80/70/80since project start, butnpm test(what CI runs) never enforced them. Two large untested surfaces blocked the gate from being turned on:src/x402-v2.tssrc/micropayment-engine.tsPer-file detail (vitest v8 reporter, on this commit):
npm run coveragenow exits 0 locally and (expected) in CI.What's included
Track A —
tests/x402-v2.test.ts(extended; tests #14-#26)13 new tests on previously-untested public surface. Reuses Story 14's determinism scaffolding (fake timers + module-mocked
generateEIP3009Nonce):on402Response: happy / non-402 status / missing+malformed header (covers privateextractPaymentIntentHeader+parsePaymentIntent+parsePaymentResponseBodytransitively)submitPayment: 200 OK with full Receipt parse / non-2xx error rewrap / network error rewrap (fetchstubbed viavi.stubGlobal)verifyReceipt: 200 OK + verified:true / non-2xx returns false / network error returns false without throwing (locks the differing contract fromsubmitPayment)createPaymentIntentHeader: happy round-trip / optionalmerchant+metaundefinednegotiatePaymentIntent: deterministicintent_<ts>_<rnd>shape / TODO branch locked (currently always returnsprotocol='dual', ignoring_supportedProtocols— flagged by Story 14 QA, deferred per @sm)Track B —
tests/micropayment-engine.test.ts(new file; B-1 through B-18)18 new tests covering all 7 public methods of
MicropaymentEngine+ transitive private helpers:constructor(B-1..B-3): 0x-prefix guard / default thresholds (60s/100/$1.0) / custom thresholds honoredqueuePayment(B-4..B-6): paymentId shapemp_<ts>_<rnd>with deterministic Math.random /usdToBaseUnitsregex errors (empty / non-numeric / leading-dot) / auto-settle triggered when thresholds metbatchPayments(B-7..B-9): EIP-712 BatchSettlement signing produces a 130-hex-char signature + correct totals/recipientCount/expiresAt + 'pending' status mutation / missing-IDs throw /skipGasEstimate: trueshort-circuits to '0.000000'getGasEstimate(B-10..B-11): 6-decimal USD string / inverse scaling with paymentCountsetBatchWindow(B-12..B-13): seconds→ms conversion / observable window-boundary change viavi.advanceTimersByTimegetQueueStatistics(B-14..B-15): empty-queue zeroes / multi-window aggregation + per-window auto-settle contract (only current windowKey batches)clearOldPayments(B-16..B-17): nothing-to-clear returns 0 / removes onlystatus==='settled'items older than cutoff and deletes empty window keysgetPaymentStatus(B-18): find-or-undefined contract across multiple windowsTrack C —
.github/workflows/ci.ymlTwo new steps inside the existing
buildmatrix job, betweenRun testsandBuild:Coverage gate— runsnpm run coverage(enforces 80/80/70/80 thresholds)Upload coverage report—actions/upload-artifact@ea165f8d65b6e75b540449e92b4886f43607fa02(v4, pinned per supply-chain hygiene policy from PR ci(security): add CodeQL, Dependabot, and OSV-Scanner bundle #11),if: always(), 14-day retention, namedcoverage-report-node-${{ matrix.node-version }}The coverage step runs inside the existing matrix job, so the GitHub check context name remains
build (18)/build (20)— no newrequired_status_checksentry expected. @devops to confirm post-merge.Test count breakdown
tests/x402-v2.test.tstests/micropayment-engine.test.tsHow the previously-failing tests now pass
The first Story 15 attempt produced 5 failing Track B tests, all rooted in two source bugs:
src/micropayment-engine.ts:232verifyingContracthad wrong EIP-55 checksum → viem 2.49+ rejected.generateNonce()returned 64 hex chars without0xprefix → mismatch with the0x${string}cast.Both fixed by Story 15.1 (PR #37 @ 23694dd). The 5 previously-failing tests (B-3, B-6, B-7, B-9, B-15) now exercise the full signing path successfully against the corrected source.
One additional fixture adjustment in this PR (NOT a source change):
RECIPIENT_Btest fixture corrected from'0x...FACE'(wrong checksum) to'0x...Face'(viem-validated). This exposed Story 15.1's deferred backlog item — line 240 ofsrc/micropayment-engine.tsis an undefendedrecipient as 0x${string}re-cast, meaning caller-supplied recipient addresses must be EIP-55 valid until the source-side guard lands. Test fixtures now respect this constraint; the broader source hardening is on the post-Story-15 backlog per @dev (Story 15.1 implementer).Acceptance Criteria mapping
src/x402-v2.ts≥80% line coveragesrc/micropayment-engine.ts≥80% line coverage, all 7 public methodstests/micropayment-engine.test.tsexists as new filenpm run coverageexits 0.github/workflows/ci.ymlCoverage gate step in build matrixRun testsandBuild, this PRactions/upload-artifact@ea165f8v4,if: always(), 14-day retentionrequired_status_checksupdatebuild (18)/build (20)).todo/xit/xdescribevitest.config.tsthresholds NOT loweredgit diff --stat)Files changed (3)
Source-file modification policy honored: zero changes to
src/orvitest.config.ts. Confirmed viagit diff --stat origin/mainfrom this branch.Test plan
npm test— 146/146 PASS, zero skipped, zero .todonpm run coverage— exit 0; global ≥80% lines/functions/statements, ≥70% branchesnpm run lint— cleannpm run type-check— cleangit diff --stat: only tests + ci.yml)fetch; no real network, no Date.now leakageChain governance
Full SINKRA — story-driven:
DO NOT MERGE. Per
automated-pr-merge-authority.md: this PR awaits @qa PASS verdict, then @devops clicks merge. Per Orion's pre-approval pattern for Story 15: once @qa issues PASS, @devops merges directly without re-asking operator.Out of scope (unchanged from Story 15 spec)
paybot-sdk@0.3.0— happens AFTER this PR lands.recipient as 0x${string}cast on line 240 — Story 15.1 deferred to backlog._supportedProtocolsinnegotiatePaymentIntent— Story 14 QA TODO, separate future story.