chore: Correct the packaging, run tests in CI, add the test setup - #8
Merged
Conversation
`requires-python` claimed `>=3.10`, but the code uses `typing.Self` and `enum.StrEnum`, both new in 3.11 — the import fails there. Verified against 3.10 through 3.14. Adds `py.typed` so the annotations are visible to consumers at all (PEP 561), along with the `package-data` entry it needs to reach the wheel, and the `Typing :: Typed` classifier. Test and dev dependencies move into extras. The Pipfile went with them: it was never used for releases (those run in CI) and its lock file had not been touched since the commit that introduced it. A library does not pin its dependencies anyway. - requires-python >=3.11, classifiers for 3.11 to 3.14 - optional-dependencies: `testing` and `dev` - Development Status, Topic and Typing classifiers, Documentation url - .gitignore: .idea/, .vscode/ and the git-ignored docs/superpowers/
The pep8 workflow only ever checked the diff of the current change. Anything that slipped through once was never reported again, which is how five violations came to sit in the tree unnoticed — they are fixed here. New test job over Python 3.11 to 3.14. It also builds the wheel, installs it and imports it from outside the source tree, which is what catches a wrong `requires-python` or a missing `py.typed`. The sandbox tests are deselected by the addopts in pyproject.toml, so CI needs no credentials. The publish workflow had `permissions: write-all` for the release job, a debug step that echoed a boolean, and a commented-out guard — so a `v-test-*` tag created a public GitHub release instead of only going to TestPyPI.
Two modes. Mocked by default: `responses` serves every call from the JSON files in tests/fixtures, so the suite needs no network and no credentials. And opt-in against the real sandbox with `pytest -m sandbox`, which is necessary because a mock can only ever confirm what we believed while writing it — and Unzer's documentation is wrong often enough that belief is not enough. The fixtures are captured from real responses, not written by hand. A hand-written one had `action: "charge"` where the API answers `"CHARGE"`. Guards, because the sandbox tests create real transactions on whatever account they are pointed at: the suite refuses any key that is not `s-priv-`, and the sandbox tests are deselected unless asked for, so a leftover .env cannot turn a plain `pytest` into API traffic. Credentials come from the environment or a git-ignored .env.
This was referenced Aug 21, 2026
Without a single test file, `pytest` exits with code 5 ("no tests collected")
and the new CI job fails — which it did. But the fixtures and the sandbox guard
are code either way, and the guard is the one piece that must not quietly stop
working: it is what keeps the suite from creating real transactions on a
production account.
Covers the fixture loader including its error message, that the client fixtures
really have retries disabled, that every fixture file is valid JSON, and that
the guard rejects production, public and malformed keys.
sveneberth
added a commit
that referenced
this pull request
Aug 21, 2026
Stacked on #8 — merge that first, GitHub then retargets this to `develop`. Ten bugs, every one reproduced against the sandbox rather than deduced from the docs. Four of them only surfaced while writing the tests and examples in this PR. | Bug | Effect before | |---|---| | Payment page interpolated the `Action` enum into the path | `405 API.000.000.006` — `createPaymentPage` and `getPaymentPage` both dead | | `card3ds` validated with `isinstance(..., bool)` against a `None` default | every `PaymentPage()` without it raised | | `Action` knew 2 of 8 transaction types | `getPayment()` raised for any payment ever cancelled, shipped or paid out | | Transaction `action`/`status` stayed strings | `getChargedTransactions()` always returned `[]` | | `billingAddress: ""` for a missing address | `400 API.410.300.007` — no customer without an address | | `expiresAt` read as seconds, API sends milliseconds | `getPaylaterInstallmentPlans()` raised on every call, so Installment was unusable | | Amounts serialised unrounded | `12.3 - 10.0 - 2.3` went out as `8.88e-16`, in scientific notation | | `get_channel_id()` took the first matching keypair entry | wrong channel for AMEX on an account that configures `card` twice | | `"authorize.pendin"` / `"charge.pendin"` | those two webhooks could not be registered | | Retry ignored the HTTP method (#7) | a timed out POST could charge a customer twice | Smaller things along the way: the transaction URL regex was pinned to `api.unzer.com/v1` and silently dropped hyphenated sub-operations such as `chargeback-reversal`; `listWebhooks()` returned a single-use `map` where the docstring promised a list; three `assert` used for runtime validation, which vanish under `python -O`; `getPaymentTypeFromTypeId` raised `IndexError` for an id without a hyphen; and `urllib3` was imported but never declared as a dependency, shadowing the builtin `TimeoutError` and missing `ConnectTimeout`. **On unknown enum values:** they raise, without a fallback. It means the SDK is behind the API, which is worth seeing — and `unknown` is a real value elsewhere in this API (`salutation`), so a placeholder of that name could not be told apart from one. **Not fixed here, on purpose:** `DEBUG` logging still writes full payloads including IBANs. That needs masking rather than a one-line change, and is tracked separately. 251 unit tests and 19 sandbox tests, green against three sandbox accounts with different payment methods enabled.
sveneberth
added a commit
that referenced
this pull request
Aug 21, 2026
…#10) Stacked on #9 — merge #8 and #9 first. The README was eleven lines: a title, two badges, one sentence. It is also the landing page on PyPI. It now covers install, authentication, a quickstart, the sandbox, error handling, the supported payment methods, logging, and — explicitly — what the SDK does *not* do, so "can it refund?" is answerable without reading `client.py`. **Every code sample was run against the sandbox.** One of them corrected the text: a fresh payment sits in `CREATE`, not `PENDING`. `docs/payment-methods.md` takes the detail that outgrew the README: how a payment type comes into being, which 7 of the 24 accept server-side data and which 17 are built in the browser, and the peculiarities per method. It replaces a dated design document that was only ever git-ignored, and therefore invisible to anyone but its author. `AGENTS.md` is mostly a warning list, because this API's documentation and its official SDKs are wrong often enough that neither counts as evidence. Fifteen measured cases back that up. `examples/` has five scripts, one flow each, all actually executed. The set deliberately skips card and Klarna: those payment types are created in the browser, so a server-side script cannot demonstrate them honestly — the fifth script explains that instead. Writing them is what found the installment timestamp bug in #9, which is the argument for having them.
sveneberth
added a commit
that referenced
this pull request
Sep 2, 2026
…that survive an unexpected body (#13) Three findings from tracking down a failing Klarna checkout, all measured against the sandbox. ## How a discount has to reach a basket Neither schema accepts a basket item with a negative amount — so a discount cannot be sent as its own negative `voucher` line, which is the shape a consumer arrives at naturally (one item per article, one for the voucher, the grosses adding up to the order total). It belongs in a positive discount field of the item it reduces, and the two schemas verify very different amounts of it: | | v1 | v3 | |---|---|---| | negative item amounts | `API.600.200.131` + `API.600.410.018` | `API.600.200.131` | | discount field | `amountDiscount` (per line or per unit is **unverified**, see below) | `amountDiscountPerUnitGross`, **per unit** | | item amount | `amountGross` stays the pre-discount gross | `amountPerUnitGross` minus the discount must stay positive | | total | **not checked at all** | exact to the cent, `API.600.410.062` | | `vat` per item | optional, stored as 0 | mandatory, `API.600.410.052` | | discount above the item | accepted | refused, `API.600.410.064` | `TestBasket` grows from 2 to 13 tests, one per row plus the per-unit semantics of the v3 discount. `AGENTS.md` carries the same as prose, with the v1 tolerance marked as a warning rather than a licence: a charge does not compare the basket to the payment amount either, but a method that hands the basket to a partner system may well be stricter. Two things about the last row are worth stating precisely, because a first version of it was wrong. The per-item cap only shows itself when the basket total stays positive: with a single over-discounted line the total goes negative too and the API answers the generic `API.600.200.131` "Amount has to be positive", which the negative total alone explains. With a second, larger item it names the line instead — `API.600.410.064`, "Basket item i1 'amountDiscountPerUnitGross' does not equal to 'amountPerUnitGross'". And the charge-does-not-check-the-basket claim is now backed by Prepayment rather than iDEAL, which this repository states cannot be driven server-side: against one basket worth 817.02, charges of 817.02, 726.24, 907.80 and **1.00** were all accepted and booked at face value. The two existing basket tests are fixed along the way: they passed `type="goods"`, but the parameter is `kind`. `BaseModel.__init__` swallows unknown kwargs, so `"type": ""` went out and no item type was ever exercised. ## Error responses that survive an unexpected body ```py errorResponse = ErrorResponse.fromDict(r.json()) # ← raised JSONDecodeError ``` Not every 4xx comes from the API. A gateway in front of it answers with an HTML page, and parsing that as JSON raised a bare `JSONDecodeError` from inside the SDK — losing the status code and the body, and looking like an SDK bug rather than a refused request. Guarding that turned out to need three steps rather than one, because the first attempt was both too narrow and too wide: - **Too narrow.** `fromDict` indexed `timestamp`, `url` and `errors`, so a 4xx whose body *is* JSON but is not this API's envelope raised `KeyError`/`TypeError` — not a `ValueError`. Measured, these all still escaped `_request` bare: `{"message": "Forbidden"}` (the canonical API-gateway body), `null`, `[]`, `"Forbidden"`. - **Too wide.** `fromDict` parsed the timestamp with one hard-coded format while the API is known to use two, and that `ValueError` landed in the same handler. A well-formed API error with a different format was reported as "non-JSON body" — while quoting JSON — and arrived with an empty `errors` list. The second is the one that mattered: `UnzerClient.createOrUpdateCustomer` falls back to `updateCustomer` on `errors[0].code == "API.410.200.010"`, so an emptied list turns a recoverable checkout into a failed one, silently. That path is exercised routinely in production. So `_request` now guards only `r.json()` and reports a body that decodes but does not fit the schema separately, and `ErrorResponse.fromDict` treats only `errors` as required — it is what identifies the body as an error envelope. Everything else missing or unreadable costs that field alone: the timestamp goes through `utils.parseDateTime` and falls back to `None` with a warning, and `Error` gained defaults so an entry missing `merchantMessage` keeps its code. A body that is not an envelope still raises, so callers keep the distinction between "the API refused this" and "something else answered". Measured across `2026-08-21 10:15:32`, `21.08.2026 10:15:32`, `2026-08-21T10:15:32`, `2026-08-21 10:15:32.123` and a missing timestamp: the codes arrive in all five and the `createOrUpdateCustomer` fallback fires again. Closes #14. The trigger that surfaced all this is worth knowing for anyone developing locally: | `returnUrl` | Result | |---|---| | `http://localhost:8080/…` | **403, nginx HTML** | | `http://127.0.0.1:8080/…`, `https://localhost:8080/…` | **403, nginx HTML** | | `http://192.168.1.5:8080/…` | **403, nginx HTML** | | `https://example.com/…` | API answers | | `http://lvh.me:8080/…`, `http://localtest.me:8080/…`, `http://127-0-0-1.nip.io:8080/…` | API answers | The gateway refuses localhost and private IP literals before the API sees the request. Hostnames that merely *resolve* to 127.0.0.1 pass, so the check is on the string. A rate limit is ruled out: `localhost` still gets a 403 right after a successful call with a public host. ## Documentation claims corrected by measurement Three assertions in the docstrings and in `AGENTS.md` did not survive being checked: - **The Pay later methods do not require v3.** A sandbox *authorize* with `PaylaterInstallment` and with Klarna succeeds on a v1 basket carrying `amountDiscount` exactly as it does on v3, booking the requested amount unchanged. The `Basket` docstring and the schema table both claimed otherwise. - **`vat` is mandatory in v3**, not "optional" as `BasketItem` said. - **Mixing the schemas fails silently in one direction.** A v3 item in a v1 basket is accepted with a 201 and every item amount stored as `0.0000`; the reverse is refused loudly with `API.600.410.051`. A basket is also only readable through the schema it was created with (`API.600.410.024`), and single use — `API.330.200.152` on a second charge, so a retry after a failed authorize needs a new one. The silent-zeroing case is filed as #16, since enforcing it belongs in `Basket.serialize`. The v1 `amountDiscount` semantics are now labelled *unverified* rather than asserted: the table said per line, `basketItem.py:47` says per unit, every v1 discount test uses `quantity=1` where the two are indistinguishable, and v1 reconciles nothing — so the basket endpoint cannot settle it. Only the rendered basket or the partner system can. ## Verification - `pytest -m sandbox tests/sandbox/test_live_api.py::TestBasket` — 13 passed against the real sandbox. - `pytest` — 291 passed, including the new mocked cases for the HTML body, the five foreign-JSON shapes, the five timestamp formats and the incomplete error entry. - `pycodestyle --diff` — clean. - One pre-existing sandbox test is red and untouched by this branch (verified by stashing): `TestPaymentPage::test_redirect_url_points_at_the_sandbox` expects `sbx-` in the redirect URL, the API now answers `payment.test.unzer.com`. ## Filed rather than fixed here #15 (the remaining `_request` branches: retry-exhausted `statusCode == 0`, the unguarded and doubled `r.json()` on 2xx, `<= 201` sending a 204 into the error branch, `r` not reset after a transport failure), #16 (schema mix), #17 (`BaseModel` swallows unknown kwargs — `examples/03_installment_plans.py:57` still carries the `type=` bug this branch fixed in the tests), #18 (`BasketItem.serialize` does not round its amounts while `Basket.serialize` does). ## Note on the target branch Against `develop`, not `main`, although the commits are patch level: `main` carries neither `tests/` nor `AGENTS.md` yet — #8 through #12 are still only on `develop`, so the files this branch changes do not exist there.
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
1 participant
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.
Groundwork for the two PRs that follow. No behaviour changes to the SDK itself.
Packaging.
requires-pythonclaimed>=3.10, buttyping.Selfandenum.StrEnummake3.11 the actual minimum — the import fails on 3.10. Verified across 3.10 to 3.14. Adds
py.typed, without which none of the annotations are visible to consumers, plus thepackage-dataentry it needs to reach the wheel. Test and dev dependencies become extras andthe Pipfile goes: it was unused for releases, and its lock file had not been touched since the
commit that added it.
CI. The pep8 workflow only checked the diff, so a violation that slipped through once was
never reported again — five had accumulated in the tree and are fixed here. New test job over
3.11–3.14 that also installs the built wheel and imports it from outside the source tree,
which is what catches a wrong
requires-pythonor a missingpy.typed. The publish workflowhad
write-allpermissions, a leftover debug step, and a commented-out guard that letv-test-*tags create public releases.Test setup.
responsesplus JSON fixtures for the mocked default mode, and an opt-insandbox mode. Sandbox tests are deselected unless asked for and the suite refuses any key that
is not
s-priv-, so neither CI nor a leftover.envcan create real transactions.Verified: lint clean over the whole tree, package builds, installs and imports on 3.11 and 3.14.