Skip to content

fix: use configured base price in fulfillment total assertion - #72

Open
XiaolongZhang-TT wants to merge 1 commit into
Universal-Commerce-Protocol:mainfrom
XiaolongZhang-TT:fix/fulfillment-hardcoded-base-price
Open

fix: use configured base price in fulfillment total assertion#72
XiaolongZhang-TT wants to merge 1 commit into
Universal-Commerce-Protocol:mainfrom
XiaolongZhang-TT:fix/fulfillment-hardcoded-base-price

Conversation

@XiaolongZhang-TT

Copy link
Copy Markdown
Contributor

Description

test_fulfillment_flow in fulfillment_test.py hardcoded 3500 as the item base price when computing the expected total:

expected_total = 3500 + option_cost  # Base 3500 + shipping

Every other test in the suite resolves the item price via fixture_ctx.get_test_price(). The hardcoded 3500 only matches the default flower_shop fixture (valid_item.expected_price 35.00 → 3500); a fixture configured with a different item price would fail the total assertion even though the server behaved correctly — the same class of "default-only" bug as the recently-fixed hardcoded discount amount.

get_test_price() already returns minor units, so this is a pure substitution with no unit conversion.

expected_total = self.fixture_ctx.get_test_price() + option_cost  # base price + shipping

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected, including removal of schema files or fields)
  • Documentation update

Is this a Breaking Change or Removal?

N/A — test-only fix, no schema/field removal.

Checklist

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation (N/A — test-only fix)
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes
  • Any dependent changes have been merged and published in downstream modules

test_fulfillment_flow hardcoded 3500 as the item base price when computing
expected_total, while every other test resolves it via
fixture_ctx.get_test_price(). The hardcoded 3500 only matches the default
flower_shop fixture (35.00 -> 3500); any fixture configured with a
different item price would fail the total assertion even though the server
behaved correctly.

get_test_price() already returns minor units, so this is a pure
substitution with no unit conversion.
@damaz91 damaz91 added the status:needs-triage Signal that the PR is ready for human triage label Aug 3, 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