fix: relax over-strict discount applied[] count/position assertions

[XiaolongZhang-TT]

Description

Two discount tests in business_logic_test.py asserted exact lengths (and one a fixed position) on discounts.applied[], but per discount.md the applied list carries code-based discounts plus any automatic discounts, whose count is not fixed and whose code is null. A conformant business that also applies an automatic discount would fail these assertions even though every submitted code is correctly present.

test_multiple_discounts_accepted

self.assertTrue(discounts_obj and len(discounts_obj.applied) == 2)   # too strict
applied_codes = [d.code for d in discounts_obj.applied]
self.assertIn(valid_code_1, applied_codes)
self.assertIn(valid_code_2, applied_codes)

The two assertIn(code) checks already prove both submitted codes were applied, so the len == 2 adds nothing except a failure mode when an automatic discount coexists. Relaxed to assertTrue(discounts_obj.applied).

test_multiple_discounts_one_rejected

self.assertTrue(discounts_obj and len(discounts_obj.applied) == 1)   # too strict
self.assertEqual(discounts_obj.applied[0].code, valid_code)          # assumes position

Two issues: the exact count, and applied[0] assuming the code-based discount is first — an automatic discount ordered ahead of it has code = null and fails the equality. Replaced with a position-independent membership check.

assertIn is safe against a list that may contain None entries (automatic discounts), since the submitted code is always a string.

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

[damaz91]

Hi @XiaolongZhang-TT,

Thanks for the PR! Relaxing these assertions is definitely the right move to support servers that apply automatic discounts.

I noticed a few places where we can improve the tests further to be robust against automatic discounts while maintaining coverage:

1. Verify invalid code rejection: In test_multiple_discounts_one_rejected, since we no longer assert len(applied) == 1, we should explicitly assert that "INVALID_CODE" is NOT in the applied list to ensure it was rejected:

   self.assertNotIn("INVALID_CODE", applied_codes)

2. Relax index assumptions in other tests:

   • In test_discount_flow, discounts_obj.applied[0].code assumes the valid discount is at index 0, which might not be true if automatic discounts are applied. We should use self.assertIn(valid_code, applied_codes) instead.
   • In test_fixed_amount_discount, we should also avoid assuming index 0. We can map the applied discounts by code and verify the amount for the expected code:

     applied_discounts = {d.code: d for d in discounts_obj.applied if d.code}
     self.assertIn(fixed_code, applied_discounts)
     self.assertEqual(applied_discounts[fixed_code].amount, expected_discount)

I have pushed a branch containing these fixes to the upstream repository at origin/fix/relax-discount-applied-count-assertions (commit a5fc51bae02d783bd99a2af8df2be3ba28c0739a). You can pull these changes into your branch to update the PR, or apply them manually.

Thanks!

[XiaolongZhang-TT]

Thanks @damaz91! I've pulled your commit in — the expanded coverage (assertNotIn for the invalid code, plus relaxing the index assumptions in test_discount_flow and test_fixed_amount_discount) looks great. Appreciate the help getting this over the line.

[damaz91]

The changes look good. Relaxing the assertions on the applied discounts list to check for membership () rather than exact count or fixed positions is a correct and robust change. This allows server implementations to return automatic discounts alongside code-based ones as permitted by the UCP spec, without failing conformance tests.