Skip to content

e2e test subscription flows fix #5731

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Jul 21, 2025
Merged

e2e test subscription flows fix #5731

merged 1 commit into from
Jul 21, 2025

Conversation

vpremamozilla
Copy link
Collaborator

  • fixes e2e tests
  • subscription page had issues with datatest-id being repeated

groovecoder
groovecoder approved these changes Jul 21, 2025
View reviewed changes
Copy link
Member

@groovecoder groovecoder left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

question (non-blocking): Why did we seem to remove the isVersion3() check from only a couple of the tests, but we left it in-place for the other one?

e2e-tests/pages/subscriptionPaymentPage.ts
@@ -47,31 +47,45 @@ export class SubscriptionPaymentPage {
this.planDetails = page.locator("#plan-details-product");
this.planDetails3 = page.locator("#product-details-heading");
this.planType = page.locator(".plan-details-description");
this.planType3 = page.getByTestId("total-price");
// this is ugly but someone repeated the datatest-id="total-price" in the component
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

suggestion (non-blocking): bring this up with FXA team too? I'm okay to merge this as-is, but it seems like they should fix this part themselves, and if they do, we might have to revert this particular code?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes i agree!

@vpremamozilla
Copy link
Collaborator Author

question (non-blocking): Why did we seem to remove the isVersion3() check from only a couple of the tests, but we left it in-place for the other one?

  • this is actually a good point, do i have the go-ahead to remove all the version3 logic? it seems that they all use version3 values. i didnt want to remove it since it seemed like you put that in there intentionally a couple of months ago

@vpremamozilla vpremamozilla added this pull request to the merge queue Jul 21, 2025
Merged via the queue into main with commit d4b7db0 Jul 21, 2025
33 checks passed
@vpremamozilla vpremamozilla deleted the e2e-test-fail-sub-flow-failing branch July 21, 2025 18:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@groovecoder groovecoder groovecoder approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@vpremamozilla @groovecoder