Fix E2E shopper tests around 3DS and UPE settings #8123
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Test the buildOption 1. Jetpack Beta
Option 2. Jurassic Ninja - available for logged-in A12s🚀 Launch a JN site with this branch 🚀 ℹ️ Install this Tampermonkey script to get more options. Build info:
Note: the build is updated when a new commit is pushed to this PR. |
|
Size Change: 0 B Total Size: 1.27 MB ℹ️ View Unchanged
|
bborman22
requested review from
a team and
lovo-h
and removed request for
a team
achyuthajoy
approved these changes
Feb 4, 2024
There was a problem hiding this comment.
Ran the full E2E test suite against this branch(https://github.com/Automattic/woocommerce-payments/actions/runs/7775675801) and can confirm that the changes are working as expected and wcpay shopper tests are passing except for nightly tests.
Noticed multi currency test failures on nightly, but not related to this PR.
Thanks for working on this @bborman22. LGTM 🚢
tests/e2e/utils/payments.js
Outdated
| @@ -197,7 +197,7 @@ export async function confirmCardAuthentication( | |||
| ); | |||
| let challengeFrame = await challengeFrameHandle.contentFrame(); | |||
| // 3DS 1 cards have another iframe enclosing the authorize form | |||
| if ( cardType.toUpperCase() === '3DS' ) { | |||
| if ( cardType.toUpperCase() === '3DS1' ) { | |||
There was a problem hiding this comment.
@bborman22 I haven't checked Stripe test cards, if there are any specific ones which are triggering 3DS1 check, but AFAIK there is no code which would trigger 3DS1 check here.
For the context: So this might a dead code now. I did so in #8121 to trick linter, otherwise it would complain on unused cardType argument :)
There was a problem hiding this comment.
Good call, I tested all the cards we have defined and none triggered the 3DS1 flow, so I removed card type for this function entirely as it doesn't seem we need to look for 3DS1 any more.
6 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR should fix the E2E shopper tests on pull requests.
Changes proposed in this Pull Request
There were two particular failures that were occurring in previous runs:
The solution's I'm applying in this PR for the 3DS issue is to address the fact that the additional
'iframe[name="acsFrame"]'is no longer present with the retirement of 3DS1. I hadn't seen any particular announcement around this change from Stripe, but it is not unexpected as our current conditional for looking for that iframe states it was a 3DS1 specific feature. I haven't been able to determine when / how this was introduced and could have been just a change from Stripe, but would be nice to narrow this down.The solution to the missing
#usernamefield on the saved cards tests was actually caused by a previous tests issues. In theshopper-checkout-purchase-with-upe-methods.specthemerchantWCP.disablePaymentMethodfunction was not completing, causing the merchant to not be logged out, causing the next test to fail when trying to log in. This was due to the use of Xpath when the function was expecting a selector. There was already some prior art inmerchantWCP.enablePaymentMethodwhich I applied here as well. This was introduced in #7954 when converting selectors into Xpath.The solution to the iterable issue was to add a
postcodeto the newly addedupe-customersdata. This was introduced in #8082 but was likely unnoticed as the E2E tests were failing for a while by this point.One thing I considered was whether we should look to make those tests more resilient by first confirming we're logged out. I leaned away from that just because it seemed that it may cover up issues in other tests not getting "cleaned up" properly, but wanted to open that up for some discussion (and perhaps open an issue to handle in a seperate PR?).
Testing instructions
E2E_GROUP='wcpay' E2E_BRANCH='shopper' npm run test:e2e[WC - latest | wcpay - shopper]npm run changelogto add a changelog file, choosepatchto leave it empty if the change is not significant. You can add multiple changelog files in one PR by running this command a few times.Post merge