fix: assert a seed-independent variant invariant instead of SKU

[khoinguyen-mindera]

Ticket

N/A — test fix

Summary

• ProductDetailsIntegrationTests.test_getProduct_variantsSurface fails on main against a local BFF, because it asserts product.defaultVariant.sku.isEmpty == false
• Every variant in the seed Shopify store has an empty SKU — verified by querying the BFF directly across the whole women collection (sandals-lv, black-winter-coat, capucines-bb, high-waist-jeans, mini-v-neck-pleat-dress — all return sku: ""). Most are "Default Title" variants, so SKU was never populated
• The variants themselves surface correctly, so the first assertion was always fine; only the SKU one was unsatisfiable
• Replaced it with a seed-independent invariant: the default variant must be one of the product's variants. Product.Variant has no id, so that was not an option
• ./Alfie/scripts/verify.sh now reports ✅ FULL VERIFICATION PASSED (build + unit + integration); integration is 9/9

Screenshot

[Copilot]

Pull request overview

Updates the Product Details BFF integration test to avoid asserting on SKU values that are empty in the current seed Shopify store, replacing it with a seed-independent invariant to keep the test meaningful and stable.

Changes:

• Removed the unsatisfiable assertion that product.defaultVariant.sku is non-empty.
• Added an assertion that the defaultVariant is included in product.variants.

[Prathimaboggu]

Diagnosis is right — the SKU assertion was unsatisfiable, and ProductDetailsConverterTests.swift:240 already asserts defaultVariant.sku == "" as an expected converter shape, so an integration test demanding a non-empty SKU was always wrong. Two things to fix.

1. The replacement assertion can never fail. In ProductDetails+Converter.swift: line 66 returns variants.isEmpty ? nil : 0, so the index is non-nil whenever there's a variant; line 20 sets defaultVariant = domainVariants[index], an element of that array; line 47 assigns the same array to product.variants. So variants.contains(defaultVariant) is true by construction, and the only case that falsifies it — empty variants — is already asserted on the line above. (Variant's synthesised == is reflexive here; Money.amount is Int, so no NaN edge.)

The old assertion was unsatisfiable, the new one is unfalsifiable — test_getProduct_variantsSurface now substantively asserts only non-emptiness, nothing about the default variant. Something a live response could actually falsify, still seed-independent:

// the resolution rule the converter documents at line 63
if product.variants.contains(where: { $0.stock > 0 }) {
    XCTAssertGreaterThan(product.defaultVariant.stock, 0)
}
// or the media fallback at line 115, which has a comment about the carousel going blank
XCTAssertFalse(product.defaultVariant.media.isEmpty)

2. The empty SKUs deserve a ticket, not just a test comment. Three places treat SKU as variant identity:

• SelectedProduct.swift:43 — id = "\(product.id)-\(selectedVariant.sku)", so with empty SKUs every variant of a product collapses to one id (and SelectedProduct is Identifiable, Hashable)
• ProductDetailsViewModel.swift:270 — variants.first { $0.sku == persistedSku } matches the first variant regardless of what the user picked, so Bag/Wishlist → PDP silently switches the selection, which is the opposite of the "keeping the user's choice" comment above it
• PersistedProductDTO.swift:48/:100 — persists and rehydrates by variantSku

Product.swift:63 also documents sku as "A unique identifier for the variant", which the seed data violates. Real Shopify data usually does populate SKU and the seed's "Default Title" variants don't, so this may not bite in production — but nothing in the code defends against it either way. Worth raising rather than leaving as a benign-sounding note in a test.

Minor: the comment states a positive fact about a mutable external seed store, in a file whose header says it "stays seed-agnostic" — "may be empty / not guaranteed by the seed" ages better.

Context: this target isn't in CI's test plan (SCAN_TESTPLAN=Alfie, which excludes BFFIntegrationTests), so verify.sh locally is the only signal — which is how the broken assertion sat on main.

Nothing here makes anything worse and it does unblock verify.sh; I'd just swap the assertion for one with actual coverage first. No Xcode or local BFF on my side, so the 9/9 is unverified — the above is from the converter and model code.