@W-17682173 Fixing E2E tests failures #2224
Conversation
jeremy-jung1
changed the title
jeremy-jung1
changed the title
This reverts commit 4f7b9d8.
|
|
||
| const REGISTERED_USER_CREDENTIALS = generateUserCredentials(); | ||
|
|
||
| const checkDntCookie = async (page, expectedValue) => { |
There was a problem hiding this comment.
Thanks for adding more test cases to assert DNT flows. 🥳
|
@jeremy-jung1 Looks like you're missing handling the DNT banner in the wishlist test for mobile and desktop. The flow in that test is:
Here when the login page loaded initially, DNT banner is already up and the login fails. |
| @@ -161,9 +184,10 @@ export const registerShopper = async ({page, userCredentials, isMobile = false}) | |||
| .locator("input#password") | |||
| .fill(userCredentials.password); | |||
|
|
|||
| const tokenResponsePromise=page.waitForResponse('**/shopper/auth/v1/organizations/**/oauth2/token') | |||
There was a problem hiding this comment.
As a best practice, it is always better to await the network call and assert on the network response rather than waiting for pageLoadState(). If an assert after the await for pageLoadState times out, the lock for pageloadstate will be released while the previous network call is still in progress. This will lead to race conditions where if the network call takes longer than the await timeout, the test flow will incorrectly assume the network called failed.
There was a problem hiding this comment.
Do we want this in a comment somewhere so we don't forget to do this in future tests?
| @@ -153,7 +153,7 @@ test("Registered shopper can add item to wishlist", async ({ page }) => { | |||
| if(!isLoggedIn) { | |||
| await registerShopper({ | |||
| page, | |||
| userCredentials: REGISTERED_USER_CREDENTIALS, | |||
| userCredentials: generateUserCredentials(), | |||
There was a problem hiding this comment.
While each individual test runs in isolation from the other tests, we must be mindful that the tests are running against a real deployed instance and the global variables in the test file like REGISTERED_USER_CREDENTIALS will not reset for each test.
When REGISTERED_USER_CREDENTIALS is reused for sign up in multiple tests, all tests after the first one will fail since the user with REGISTERED_USER_CREDENTIALS will already exist on the instance. Hence if you have signup flows in multiple tests, you must always call generateUserCredentials() to avoid conflicting usernames.
…wa-kit into fixing-e2e-dnt
This reverts commit 736303f.
|
Thanks for cleaning up the tests @jeremy-jung1 and @shethj Tests are passing on my machine |
The DNT notification is covering some elements in the e2e tests. Tests were modified to make the E2E tests pass again.
Description
Types of Changes
Changes
How to Test-Drive This PR
npm run test:e2e. This runs the E2E test suite against the deployed E2E scaffold site.node e2e/scripts/generate-project.js --project-key retail-app-extcd ../generated-projects/retail-app-extandnpm startexport RETAIL_APP_HOME=http://localhost:3000npm run test:e2eand see that tests pass for mobile chromium and chromium (desktop)Checklists
General
Accessibility Compliance
You must check off all items in one of the follow two lists:
or...
Localization