Skip to content

fix: decouple known-customer & free-shipping fulfillment tests from sample behavior - #79

Open
vishkaty wants to merge 1 commit into
Universal-Commerce-Protocol:mainfrom
vishkaty:fix/fulfillment-fixture-decoupling
Open

fix: decouple known-customer & free-shipping fulfillment tests from sample behavior#79
vishkaty wants to merge 1 commit into
Universal-Commerce-Protocol:mainfrom
vishkaty:fix/fulfillment-fixture-decoupling

Conversation

@vishkaty

@vishkaty vishkaty commented Aug 8, 2026

Copy link
Copy Markdown
Contributor

Description

The known-customer and free-shipping fulfillment tests
(fulfillment_test.py, the ~233–579 family) encode flower-shop sample
behavior as test-code literals
: buyer identities and emails, stored-address
ids (addr_1/addr_2), option-id strings (std-ship), an
assertIn("Free", title), and free-shipping thresholds. None of these is a
requirement 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 to
the test, and the test to the reference.

None of the encoded literals is spec-grounded

  • Option titlefulfillment.md (~L175–179) requires the title only to
    distinguish an option from its siblings and suffice for the buyer's
    decision; it says nothing about the word "Free".
  • Stored-address injection / known-customer — no injection or
    known-customer language exists in fulfillment.md, checkout.md, or
    checkout-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 merged
    fix: remove out-of-scope address persistence test #60, which removed an out-of-scope address-persistence test.)
  • Option ids — the spec uses only illustrative ids ("standard"/"express");
    std-ship/exp-ship-* appear only in the sample servers.
  • Free-shipping thresholds — no promotion/threshold text in the spec; this
    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/#72
precedent; mirrors the existing valid_discount_code_2 keys). New keys:
known_customer, known_customer_without_address, free_shipping_min_subtotal,
free_shipping_item_sku (documented in the README fixtures section). When a
fixture is absent the relevant test skips rather than false-failing.

Assertions are rewritten to check the spec-relevant invariant, not the sample
literal:

  • stored addresses located by content, with the server-assigned id read
    back from the response (no addr_1/addr_2);
  • the zero-cost option found by total == 0, not by id or the word "Free";
  • the free-shipping threshold derived from the fixture (threshold // price + 1);
  • unknown-customer email and client destination ids randomized per run so they
    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_config defaults to the
populated test_data/flower_shop/test_fixtures.json.

Scope

This covers the known-customer / free-shipping family. test_dynamic_fulfillment
still references the exp-ship-us/exp-ship-intl literals (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)

  • Conformance: Conformance test suite updates.

…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 damaz91 added the status:needs-triage Signal that the PR is ready for human triage label Aug 8, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

status:needs-triage Signal that the PR is ready for human triage

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants