-
Notifications
You must be signed in to change notification settings - Fork 402
feat(testing): Add checkout and pricingTable page objects #5776
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
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
🦋 Changeset detectedLatest commit: c852a00 The changes in this PR will be included in the next version bump. Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
alexcarpenter
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.
Looks good! Two small non-blocking questions.
| await u.po.signIn.signInWithEmailAndInstantPassword({ email: fakeUser.email, password: fakeUser.password }); | ||
| await u.po.page.goToRelative('/pricing-table'); | ||
|
|
||
| await u.po.pricingTable.startCheckout({ planSlug: 'pro', shouldSwitch: true }); |
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.
Not a super strong opinion, but shouldSwitch is not super obvious. switchPlans maybe? Again not super strong opinion here.
| }, | ||
| continueWithSavedCard: async () => { | ||
| await page | ||
| .locator('.cl-disclosureRoot') |
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.
do we need to select the disclosureRoot here vs just selecting the button trigger?
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.
The button to pay with a saved card and the button to pay with a new card use the same labels, so we need to distinguish between them. This selector is "select the subscribe/pay button that's a child of a cl-disclosureRoot containing a button with the name Payment Methods" which distinguishes it from the Pay $x.xx button within the Add new payment method form (which is still in the DOM, but not visible). I wanted this method to work even if the Add new payment method disclosure was open.
| return { | ||
| page: app, | ||
| clerk: createClerkPageObject(testArgs), | ||
| checkout: createCheckoutPageObject(testArgs), |
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.
@dstaley since we only expose checkout as an internal component currently, shall we follow the pattern here as well ? We can drop the internal prefix later once, checkout is a standalone component.
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.
I think given that Checkout is intended to be a public component in the near future we're fine to leave this as just checkout. We're under an unstable import so I don't think we really need to double up on the warnings.

Description
This PR adds new page objects to the
@clerk/testing/playwright/unstablepackage for<Checkout />and<PricingTable />, in addition to adding a few new e2e tests for billing.Checklist
pnpm testruns as expected.pnpm buildruns as expected.Type of change