Conversation
|
Oxygen deployed a preview of your
Learn more about Hydrogen's GitHub integration. |
ec3a36a to
6e22e3e
Compare
6e22e3e to
aadd3e9
Compare
e2e/fixtures/storefront.ts
Outdated
| await this.closeCartAside(); | ||
|
|
||
| // Navigate directly to cart page | ||
| await this.page.goto('/cart'); | ||
| await this.page.waitForLoadState('networkidle'); | ||
| // Extract locale from current URL to preserve it | ||
| const currentUrl = new URL(this.page.url()); | ||
| const pathPrefix = | ||
| currentUrl.pathname.match(/^\/[A-Z]{2}-[A-Z]{2}/)?.[0] || ''; | ||
| const targetUrl = `${pathPrefix}/cart`; |
There was a problem hiding this comment.
i dont love this
there’s a lot of flaky logic here that can break; it ignores subdomain localization, and accepts locales anywhere in the URL
did you choose this over clicking the cart button for a reason?
There was a problem hiding this comment.
The cart button doesn't go to the cart page, it opens the drawer. I can do this inline with the hard-coded localized url in the test rather than updating the helper to handle locales since that's markets specific. We don't actually have a button that goes to the cart page itself (which is why we have this helper).
e2e/specs/recipes/markets.spec.ts
Outdated
| const priceElement = page | ||
| .locator('div') | ||
| .filter({hasText: LOCALES.default.currencyFormat}) | ||
| .first(); |
There was a problem hiding this comment.
this works, but i’d love if we could have more "fail fast" tests rather than this type
by fail fast I mean dont let playwright wait until a timeout to see if a thing is there or not
e.g.:
- if we had a way to spot that price element (like a screen reader only label?) we could find it straight away without checking for the text yet
- then we can assert the price’s text is X
this is all made much easier the less logic we have in our tests. navigateToFirstProduct is really making our life tough here because it’s so non deterministic in a real backend situation.
i’m going to start a discussion about this in our channel
tl;dr: it works but flaky and makes CI hang, lets discuss improvements in a followup
5d73419 to
c8faa74
Compare
bb2ae84 to
e0f830d
Compare
WHY are these changes introduced?
Fixes https://github.com/Shopify/developer-tools-team/issues/1041 (Localization/Markets tests)
Part of https://github.com/Shopify/developer-tools-team/issues/1057 (Recipe tests)
Fixes a bug where add to cart button wasn't locale aware.
Establishes reusable foundation for automated end-to-end testing for cookbook recipes.
Adds comprehensive test coverage for the Markets recipe (which handles localization.)
WHAT is this pull request doing?
Recipe Testing Infrastructure
New test framework (
e2e/fixtures/recipe.ts):setRecipeFixture()helper that applies recipes on-demand, builds them, and starts a dev serverDocumentation (
e2e/specs/recipes/README.md):Markets Recipe Tests (
e2e/specs/recipes/markets.spec.ts)Tests validate:
Recipe Bug Fixes
Fixed cart currency bug and improved type safety (AddToCartButton.tsx.6e553f.patch + i18n.ts):
Added accessibility (
CountrySelector.tsx.dc473b.patch):aria-label="Country selector"on details elementaria-label="Current locale: ${label}"on summary element (dynamically shows current locale)aria-label="Switch to ${locale}"on locale switch buttons (dynamically shows target locale)Test Fixture Improvements
Locale-aware helpers (
e2e/fixtures/storefront.ts):navigateToFirstProduct()now waits for URL change (prevents flaky tests)navigateToCart()now preserves locale prefix (/FR-CA/cartvs/cart)HOW to test your changes?
Run the Markets recipe tests
Expected results:
Verify recipe bug fixes
Apply the Markets recipe:
Start dev server in skeleton template:
cd templates/skeleton npm run devNavigate to
/FR-CA/products/snowboardAdd to cart
Verify cart shows CAD pricing (e.g.,
CA$#,###.), not USDPost-merge steps
None required. Tests run in CI as part of the
recipesproject.Checklist