-
Notifications
You must be signed in to change notification settings - Fork 380
Add e2e tests for discount codes #3427
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
|
Oxygen deployed a preview of your
Learn more about Hydrogen's GitHub integration. |
6ca9a08 to
44c2b83
Compare
kdaviduik
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Needs a rebase to fix E2E tests in CI but other than that LGTM! :)
| await storefront.tryApplyDiscountCode(inactiveDiscountCode); | ||
|
|
||
| const appliedCodes = await storefront.getAppliedDiscountCodes(); | ||
| expect(appliedCodes).not.toContain(inactiveDiscountCode); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This verifies the code isn't applied, but doesn't verify that helpful feedback is shown to the user (which is indeed the case right now).
Since we're going to be fixing this soon, it would be great to add a TODO comment here for our future selves to add an assertion that an error message is displayed - this catches both functionality and UX regressions.
| }); | ||
|
|
||
| test('Applies discount via URL', async ({storefront}) => { | ||
| await storefront.goto(`/discount/${activeDiscountCode}`); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oooh i forgot this route even existed, nice catch
| @@ -0,0 +1,59 @@ | |||
| import {execSync} from 'node:child_process'; | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
CI looks like it's failing because this file isn't using the secrets properly. I've now merged this PR so if you rebase with main and keep my version of this file then things should work for you :)
WHY are these changes introduced?
Closes developer-tools-team/issues/978
WHAT is this pull request doing?
Adds e2e tests for discount counts that verify:
HOW to test your changes?
Run tests (covered by CI)
Notes for Reviewers
Checklist