Skip to content

fix: accept valid repeated checkout cancellation responses #57

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
damaz91 merged 1 commit into from
Jul 15, 2026
Merged

fix: accept valid repeated checkout cancellation responses #57

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
44 changes: 30 additions & 14 deletions checkout_lifecycle_test.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -199,34 +199,50 @@ def _cancel_checkout(self, checkout_id):
self.assert_response_status(response, 200)
return response

def test_cancel_is_idempotent(self):
"""Test that cancellation is idempotent.
def test_repeated_cancel(self):
"""Test repeated cancellation behavior.

Given a checkout session that has already been canceled,
When another cancel request is sent,
Then the server should reject it with a 409 Conflict (or handle idempotency
if key matches, but here we test state conflict logic).
Then the server should return a client error, as recommended by the spec,
or return the same canceled resource as an idempotent success.
"""
response_json = self.create_checkout_session()
checkout_id = checkout.Checkout(**response_json).id

# 1. Cancel
self._cancel_checkout(checkout_id)

# 2. Cancel again - should likely fail with 409 or be idempotent (200)
# depending on implementation. The original test expected NotEqual 200
# (implying 409 Conflict).
# checkout_service.py says:
# if checkout.status in [COMPLETED, CANCELED]:
# raise CheckoutNotModifiableError -> 409
response = self.client.post(
self.get_shopping_url(f"/checkout-sessions/{checkout_id}/cancel"),
headers=integration_test_utils.get_headers(),
)
self.assertNotEqual(

if response.status_code == 200:

@nicholasjameshall nicholasjameshall Jul 15, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: afaik there should be a standard library in Python for these errors

@ShuoRen-TT ShuoRen-TT Jul 15, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for the suggestion and for reviewing this! You are right that http.HTTPStatus.OK would be clearer than the literal 200. For the 4xx range, the repository supports Python 3.10, while HTTPStatus.is_client_error is only available from Python 3.12, so the compatible cleanup would be to use HTTPStatus.OK together with httpx.Response.is_client_error. I will keep that pattern in mind when the suite status assertions are consolidated. I really appreciate you pointing this out.

repeated_checkout = checkout.Checkout(**response.json())
self.assertEqual(
repeated_checkout.id,
checkout_id,
msg="Repeated cancellation returned a different checkout.",
)
self.assertEqual(
repeated_checkout.status,
"canceled",
msg=(
"Repeated cancellation returned status "
f"'{repeated_checkout.status}', expected 'canceled'."
),
)
return

self.assertGreaterEqual(
response.status_code,
200,
msg="Should not be able to cancel an already canceled checkout.",
400,
msg="Repeated cancellation must return 200 or a client error.",
)
self.assertLess(
response.status_code,
500,
msg="Repeated cancellation must not produce a server error.",
)

def test_cannot_update_canceled_checkout(self):
Expand Down
Loading