Skip to content

fix: accept valid repeated checkout cancellation responses#57

Merged
damaz91 merged 1 commit into
Universal-Commerce-Protocol:mainfrom
ShuoRen-TT:fix/idempotent-cancel-response
Jul 15, 2026
Merged

fix: accept valid repeated checkout cancellation responses#57
damaz91 merged 1 commit into
Universal-Commerce-Protocol:mainfrom
ShuoRen-TT:fix/idempotent-cancel-response

Conversation

@ShuoRen-TT

Copy link
Copy Markdown
Contributor

Description

This fixes #54 by updating the repeated checkout cancellation conformance test to accept both protocol-compatible outcomes:

  • a 4xx client error, as recommended for an already canceled checkout
  • a 200 response containing the same checkout in the canceled state, for implementations that treat cancellation as an idempotent operation

The previous assertion rejected every 200 response while accidentally allowing redirects and server errors. The revised test validates the returned checkout when cancellation succeeds and limits error responses to the 4xx range.

Fixes #54.

Validation

  • Ran all 11 tests in checkout_lifecycle_test.py against the latest Flower Shop sample server
  • Verified that 200 + canceled is accepted
  • Verified that a mismatched successful state and 503 are rejected
  • Ran Ruff check, Ruff format check, Codespell, and git diff --check

Type of change

  • Bug fix (non-breaking change which fixes an issue)

Is this a Breaking Change or Removal?

No.

Breaking Changes / Removal Justification

Not applicable.


Checklist

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • The new logic is self-explanatory and does not require additional comments
  • Documentation changes are not required; this aligns the test with existing protocol behavior
  • My changes generate no new warnings
  • I have updated the conformance test to prove the fix is effective
  • New and existing tests pass locally with my changes
  • No dependent downstream changes are required

Pull Request Title

This PR uses the required Conventional Commits format.

@ShuoRen-TT
ShuoRen-TT marked this pull request as ready for review July 14, 2026 21:09
@damaz91 damaz91 added status:needs-triage Signal that the PR is ready for human triage status:under-review and removed status:needs-triage Signal that the PR is ready for human triage labels Jul 14, 2026
)
self.assertNotEqual(

if response.status_code == 200:

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

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.

@damaz91
damaz91 merged commit f9de367 into Universal-Commerce-Protocol:main Jul 15, 2026
17 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: test_cancel_is_idempotent rejects idempotent cancellation — re-cancel returning the canceled resource with 200 fails the test

3 participants