Repository navigation
Make fixtures the single source of truth for ECPay wire payloads - #12
Merged
Merged
Conversation
provider.test.ts duplicated the request wire-payload assertions that fixtures.test.ts already checks against fixtures/ecpay/*.json. Remove the duplication so the wire contract lives in one place: - Delete four tests whose entire assertion was a wire payload already covered by a fixture case: tax-type mapping, B2B 統編 fields, item ItemRemark, and allowance email-notify. Also drop the redundant query-by-invoiceNumber test (wire in fixtures, result identical to query-by-orderId). - Trim the wire toMatchObject/toEqual blocks from the mixed issue / void / allowance / voidAllowance / query tests, keeping their result-parsing, status-derivation and envelope (MerchantID) assertions — the response half fixtures deliberately don't model. - Keep everything fixtures don't cover: the e2e error-mapping tests (5000022/5070357/5070450/5070453), validation short-circuits, the default-InvoiceDate (dynamic) behavior, and transport errors. - Add Items to the basic issue fixture so the one assertion that moved (Items length/shape) stays covered. Every removed wire assertion maps to a named fixture case; coverage moves to fixtures.test.ts rather than being lost. Full ecpay suite green.
There was a problem hiding this comment.
Pull request overview
Refactors the ECPay adapter test suite so that wire-payload expectations live exclusively in the language-neutral JSON fixtures (consumed by fixtures.test.ts), while provider.test.ts focuses on response parsing, status derivation, and error/validation behavior.
Changes:
- Removed duplicated inline wire-payload assertions from
provider.test.tsin favor of fixture-driven wire contract tests. - Deleted several pure-wire tests now covered by
fixtures/ecpay/*.json. - Extended the
fixtures/ecpay/issue.json“basic issue” case to include anItemsexpectation.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| packages/einvoice-ecpay/src/tests/provider.test.ts | Trims request wire assertions, keeping response parsing/error/validation behaviors. |
| fixtures/ecpay/issue.json | Adds Items to the baseline issue fixture to keep item mapping covered by fixtures. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
55
to
+59
| expect(res.invoiceNumber).toBe("JU11082062"); | ||
| expect(res.randomCode).toBe("3136"); | ||
| expect(res.invoiceDate.getFullYear()).toBe(2026); | ||
| expect(res.status).toBe("ISSUED"); | ||
|
|
||
| expect(captured?.merchantId).toBe(MERCHANT); | ||
| expect(captured?.data).toMatchObject({ | ||
| RelateNumber: "ORDER_1", | ||
| CarrierType: "1", // MEMBER → ECPay carrier | ||
| Print: "0", | ||
| Donation: "0", | ||
| TaxType: "1", | ||
| SalesAmount: 100, | ||
| InvType: "07", | ||
| }); | ||
| expect(captured?.data.Items).toHaveLength(1); | ||
| }); | ||
|
|
||
| it("maps the tax type onto the wire TaxType (TAXABLE→1 / ZERO_RATED→2 / TAX_FREE→3)", async () => { | ||
| let captured: Awaited<ReturnType<typeof parseRequest>> | undefined; | ||
| server.use( | ||
| http.post(url(ECPAY_ENDPOINTS.issue), async ({ request }) => { | ||
| captured = parseRequest(await request.text()); | ||
| return HttpResponse.json(ecSuccess(ISSUE_OK)); | ||
| }), | ||
| ); | ||
| const cases: Array<[IssueInvoiceInput["taxType"], string]> = [ | ||
| ["TAXABLE", "1"], | ||
| ["ZERO_RATED", "2"], | ||
| ["TAX_FREE", "3"], | ||
| ]; | ||
| for (const [taxType, code] of cases) { | ||
| await testProvider().issue( | ||
| // ECPay requires a customs-clearance mark for zero-rated invoices. | ||
| issueInput({ | ||
| taxType, | ||
| ...(taxType === "ZERO_RATED" ? { providerOptions: { clearanceMark: "1" } } : {}), | ||
| }), | ||
| ); | ||
| expect(captured?.data.TaxType).toBe(code); | ||
| } | ||
| }); | ||
|
|
||
| it("sends B2B fields (CustomerIdentifier, Print=1) for a 統編 buyer", async () => { | ||
| let data: Record<string, unknown> | undefined; | ||
| server.use( | ||
| http.post(url(ECPAY_ENDPOINTS.issue), async ({ request }) => { | ||
| data = parseRequest(await request.text()).data; | ||
| return HttpResponse.json(ecSuccess(ISSUE_OK)); | ||
| }), | ||
| ); | ||
| await testProvider().issue( | ||
| issueInput({ | ||
| buyer: { ubn: "53538851", name: "測試公司", address: "台北市測試路1號", email: "b@x.com" }, | ||
| carrier: undefined, | ||
| }), | ||
| ); | ||
| expect(data?.CustomerIdentifier).toBe("53538851"); | ||
| expect(data?.Print).toBe("1"); | ||
| expect(data?.CarrierType).toBe(""); | ||
| }); | ||
|
|
||
| it("maps an item's remark to ItemRemark (omitted when absent)", async () => { | ||
| let data: Record<string, unknown> | undefined; | ||
| server.use( | ||
| http.post(url(ECPAY_ENDPOINTS.issue), async ({ request }) => { | ||
| data = parseRequest(await request.text()).data; | ||
| return HttpResponse.json(ecSuccess(ISSUE_OK)); | ||
| }), | ||
| ); | ||
| await testProvider().issue( | ||
| issueInput({ | ||
| items: [ | ||
| { description: "有備註", quantity: 1, unitPrice: 60, amount: 60, remark: "備註1" }, | ||
| { description: "無備註", quantity: 1, unitPrice: 40, amount: 40 }, | ||
| ], | ||
| amount: { salesAmount: 100, taxAmount: 0, totalAmount: 100 }, | ||
| }), | ||
| ); | ||
| const items = data?.Items as Array<Record<string, unknown>>; | ||
| expect(items[0]?.ItemRemark).toBe("備註1"); | ||
| expect(items[1]).not.toHaveProperty("ItemRemark"); | ||
| expect(captured?.merchantId).toBe(MERCHANT); // envelope MerchantID, not part of Data |
Contributor
Author
There was a problem hiding this comment.
The premise here — that the fixtures runner doesn't assert array length — isn't correct. Vitest's toMatchObject does enforce array length: an actual array with 2 elements does not match an expected array with 1, even if the first element matches. Verified empirically:
expect({ Items: [{ItemName:'a'}, {ItemName:'b'}] }).toMatchObject({ Items: [{ItemName:'a'}] })
// → AssertionError: expected { Items: [ …(2) ] } to match object { Items: [ { ItemName: 'a' } ] }
So the fixture's Items: [ …one element… ] already pins the length to 1 — a dropped item (length 0) or duplicated item (length 2) fails the fixture. Removing the local toHaveLength(1) lost no coverage, and re-adding it would re-introduce exactly the duplication this PR removes.
I did adopt the underlying point that the contract wording was ambiguous: the README called expect.data a 'subset match', which reads as loose for arrays. e936a6f spells out that arrays match by length + position (so the Ruby consumer enforces line-item count identically). No code change to the assertion.
The consumption steps called expect.data a 'subset match', which reads as loose for arrays too. Spell out that objects ignore extra keys but arrays match by length and position (mirroring Vitest's toMatchObject) — so every SDK's consumer enforces the same line-item count and a dropped/duplicated item fails. Prompted by review feedback questioning whether removing a local Items length assertion weakened coverage: it does not, because the fixture's Items array already pins the length.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Follow-up to #11. Now that
fixtures/ecpay/*.json+fixtures.test.tsverify the ECPay wire contract against the real adapter, the same wire-payload assertions were still duplicated inline inprovider.test.ts. This removes that duplication so the wire contract lives in one place.What moved
issue.json), B2B 統編 fields (→issue.json), itemItemRemark(→issue.json), allowance email-notify (→void-allowance.json). Plus the redundant query-by-invoiceNumber test (wire inquery.json; result identical to query-by-orderId).toMatchObject/toEqualblocks from the mixed issue/void/allowance/voidAllowance/query tests, keeping their result-parsing, status-derivation, and envelope (MerchantID) assertions — the response half fixtures deliberately don't model.InvoiceDatebehavior, and transport errors.Itemsto the basic issue fixture so the one assertion that genuinely moved (Items length/shape) stays covered.No coverage lost
Every removed wire assertion maps to a named fixture case — coverage moves to
fixtures.test.ts, it isn't dropped. Net:provider.test.ts−139 lines;fixtures.test.tsunchanged; the full ecpay suite is green (173 passed / 27 live-gated skipped).Why
This is the point of the fixtures: a wire-mapping change now updates one place (the fixture), and the Ruby port asserts the same JSON. Duplicated inline expectations would drift.
Noted follow-up (not in this PR)
errors.jsondoesn't yet include the 5070357重覆variant or the code-table-based reason path (ecpayErrorReason(msg, rtnCode)); the consumer calls the keyword path only. Worth enriching the shared error fixtures separately.