fix: decouple known-customer & free-shipping fulfillment tests from sample behavior - #79
Conversation
…xtures The known-customer and free-shipping tests in fulfillment_test hardcoded flower-shop sample data in test code: buyer identities (John Doe/Jane Doe and their emails), stored-address ids (addr_1/addr_2) and literal contents, destinations[0].address_country == "US", the option id "std-ship", the word "Free" in option titles, and the USD 100 subtotal threshold. None of these is a 2026-04-08 spec requirement: specification/fulfillment.md never mandates stored-address injection (saved addresses are an optional personalization capability per identity-linking.md), option ids are the server's own to assign, and the title rules only require that a title distinguishes an option from its siblings — not that it contains "Free". A fully conformant server with its own customers, address ids, option ids, titles, or promotion rules failed these checks. Following the configuration precedent of Universal-Commerce-Protocol#71/Universal-Commerce-Protocol#72 and the suite's existing skip-when-absent convention (valid_discount_code_2, valid_fixed_discount_code): - new test_fixtures keys: known_customer (identity + stored addresses), known_customer_without_address, free_shipping_min_subtotal, free_shipping_item_sku, with DynamicFixtureContext getters; tests skip when the server under test declares no such data - stored addresses are located by content, ids are read from the server's response instead of asserting addr_1/addr_2 literals; the reuse test learns the server-assigned id from the injection round and asserts the resubmitted content keeps it - zero-cost options are found by totals (total == 0) instead of the "std-ship" id and "Free" title - order quantity for the threshold test derives from the configured threshold and item price instead of a hardcoded quantity of 3 - the unknown-customer test uses a randomized email so it cannot collide with a customer the server actually knows - client-side destination ids are randomized per run so re-running against a persistent server database cannot collide with stored state The checks still fail genuinely non-conformant servers: declared stored addresses that are not injected (wrong customer or wrong content), a declared free-shipping threshold or eligible item that is not honored, and injection for unknown buyers all still fail. Verified against both the Python and Node.js reference servers, including a promotion-stripped server variant for the free-shipping kill tests.
damaz91
added
status:needs-triage
| - PUT /checkout-sessions/{id} | ||
| """ | ||
|
|
||
| def _buyer_payload(self, customer: dict) -> dict: |
There was a problem hiding this comment.
would be nice to have data types where possible so not all functions take / return dicts.
There was a problem hiding this comment.
nit: would rename to something like _to_buyer_payload or _build_buyer_payload
There was a problem hiding this comment.
Renamed to _build_buyer_payload. On types: added CustomerFixture and AddressFixture TypedDicts in integration_test_utils for the fixture seam this PR introduced, so the known-customer getters and this helper now share a named shape. Server responses stay as plain dicts deliberately: the suite asserts on raw wire JSON, and parsing responses through typed models could coerce or hide exactly the deviations these tests exist to catch. A wider typing pass over DynamicFixtureContext, whose pre-existing getters all return dict[str, Any], would touch every module, so I left that for a separate change if you want it.
| (d for d in destinations if self._address_matches(d, address)), None | ||
| ) | ||
|
|
||
| def _zero_cost_option(self, options: list[dict]) -> dict | None: |
There was a problem hiding this comment.
nit: would rename to something like _get_free_shipping_option()
There was a problem hiding this comment.
Renamed to _get_free_shipping_option.
|
|
||
| def _zero_cost_option(self, options: list[dict]) -> dict | None: | ||
| """Find a fulfillment option whose total amount is zero.""" | ||
| for option in options: |
There was a problem hiding this comment.
can we simplify this?
e.g.
for option in options:
if any(t["type"] == "total" and t["amount"] == 0 for t in option.get("totals", [])):
return option
return None
There was a problem hiding this comment.
Simplified to the any() form as suggested. Behavior is unchanged for spec-legal options, since a totals array carries exactly one entry of type total. Re-ran the full suite against the booted Python reference after the change: 17 modules OK.
Rename _buyer_payload to _build_buyer_payload and _zero_cost_option to _get_free_shipping_option, and simplify the free shipping finder to a direct any() scan of totals entries. Type the fixture seam this PR introduced: CustomerFixture and AddressFixture TypedDicts in integration_test_utils, returned by the known-customer getters and consumed by _build_buyer_payload. Wire responses stay as raw dicts on purpose: the suite asserts on the raw JSON so that model parsing cannot coerce or mask a deviation. Full suite re-run against the booted Python reference: 17 modules OK.
3 participants
Description
The known-customer and free-shipping fulfillment tests
(
fulfillment_test.py, the ~233–579 family) encode flower-shop samplebehavior as test-code literals: buyer identities and emails, stored-address
ids (
addr_1/addr_2), option-id strings (std-ship), anassertIn("Free", title), and free-shipping thresholds. None of these is arequirement in the pinned 2026-04-08 spec — so the tests pass only against a
server that happens to reproduce the reference sample's choices, and can fail a
differently-but-conformantly-built server.
The circularity is direct: the Node reference hardcodes
isKnownCustomer = buyer?.email === "john.doe@example.com"(
samples/rest/nodejs/src/api/checkout.ts:203) — the reference was written tothe test, and the test to the reference.
None of the encoded literals is spec-grounded
fulfillment.md(~L175–179) requires the title only todistinguish an option from its siblings and suffice for the buyer's
decision; it says nothing about the word "Free".
known-customer language exists in
fulfillment.md,checkout.md, orcheckout-rest.md. Saved addresses are personalization that"upgrades the experience, it does not gate it" and is user-authenticated
(
identity-linking.md~L29–33). (Consistent with the direction of mergedfix: remove out-of-scope address persistence test #60, which removed an out-of-scope address-persistence test.)
std-ship/exp-ship-*appear only in the sample servers.is server pricing policy (the flower shop's CSVs).
Change
The behaviors move out of test code and into optional fixtures, using the
suite's existing
test_fixtures.json+ skip-when-absent convention (the #71/#72precedent; mirrors the existing
valid_discount_code_2keys). New keys:known_customer,known_customer_without_address,free_shipping_min_subtotal,free_shipping_item_sku(documented in the README fixtures section). When afixture is absent the relevant test skips rather than false-failing.
Assertions are rewritten to check the spec-relevant invariant, not the sample
literal:
back from the response (no
addr_1/addr_2);total == 0, not by id or the word "Free";threshold // price + 1);cannot collide with real data on an arbitrary server.
Soundness preserved
The relaxed tests still catch a genuinely non-conformant server — verified with
kill-tests on both the Python and Node references: a server that doesn't
store the declared address → fails; wrong address content (bad postal) → fails;
a declared eligible item or subtotal threshold left unhonored → fails; absent
fixtures → clean skips with no false pass. Full suite stays 17/17 on both
references; CI keeps full coverage because
--fixture_configdefaults to thepopulated
test_data/flower_shop/test_fixtures.json.Scope
This covers the known-customer / free-shipping family.
test_dynamic_fulfillmentstill references the
exp-ship-us/exp-ship-intlliterals (same file, ~L236/269);it's the same class but needs a second-destination fixture to decouple properly,
so I've left it as a deliberate follow-up rather than widen this PR. The
retained (now fixture-gated) address-id reuse check is likewise sample
behavior verified only when configured, not a spec MUST.
Category (Required)