Skip to content

fix: use configured fixed-discount amount in business_logic_test - #71

Merged
damaz91 merged 1 commit into
Universal-Commerce-Protocol:mainfrom
XiaolongZhang-TT:fix/fixed-discount-hardcoded-amount
Jul 31, 2026
Merged

fix: use configured fixed-discount amount in business_logic_test#71
damaz91 merged 1 commit into
Universal-Commerce-Protocol:mainfrom
XiaolongZhang-TT:fix/fixed-discount-hardcoded-amount

Conversation

@XiaolongZhang-TT

Copy link
Copy Markdown
Contributor

Description

test_fixed_amount_discount in business_logic_test.py fetched the expected reduction into expected_discount and already passed it to assert_totals_consistent(...), but the separate assertion on discounts.applied[0].amount hardcoded 500 instead of using that variable:

expected_discount = self.fixture_ctx.get_expected_fixed_discount_reduction()
...
self.assert_totals_consistent(..., expected_discount=expected_discount)   # uses it
...
self.assertEqual(discounts_obj.applied[0].amount, 500)                    # ignores it

get_expected_fixed_discount_reduction() already returns the value in minor units (expected_fixed_discount_reduction major units × 100), so the hardcoded 500 only matched the default flower_shop fixture (5.00 → 500) by coincidence. Any non-default fixture (e.g. a configured 7.50 fixed reduction → expected 750) would pass the totals-consistency check but fail the applied[0].amount assertion for the same checkout, an internal contradiction.

Fix: assert against expected_discount, making the two assertions in the same test consistent and server-agnostic.

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_fixed_amount_discount fetched the expected reduction into
expected_discount and already passed it to assert_totals_consistent, but the
separate assertion on discounts.applied[0].amount hardcoded 500 instead of
using that variable. Any non-default fixture (e.g. a 7.50 fixed reduction)
would fail the second assertion even though the totals check passed.

get_expected_fixed_discount_reduction() already returns minor units, so the
hardcoded 500 only matched the default flower_shop fixture by coincidence.
@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 31, 2026
@damaz91

damaz91 commented Jul 31, 2026

Copy link
Copy Markdown
Contributor

Nice catch, thanks @XiaolongZhang-TT !

@damaz91
damaz91 merged commit f252c0d into Universal-Commerce-Protocol:main Jul 31, 2026
16 checks passed
@XiaolongZhang-TT
XiaolongZhang-TT deleted the fix/fixed-discount-hardcoded-amount branch August 3, 2026 02:41
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.

3 participants