fix: avoid waitForNavigation as it's flaky

global-setup.ts

@@ -1,13 +1,17 @@
-import { chromium, FullConfig } from '@playwright/test';
+import { chromium } from '@playwright/test';
 
-async function globalSetup(config: FullConfig) {
+async function globalSetup() {
   // If you need to hit an endpoint to populate data before running your test suite,
   // you can pass in the URL through the E2E_DATA_SETUP_URL environment variable
   const dataSetupUrl = process.env.E2E_DATA_SETUP_URL;
   if (dataSetupUrl) {
     const browser = await chromium.launch();
     const page = await browser.newPage();
+    const mainBody = page.locator('body');
+
+    // Make sure the page loads and we have some HTML
     await page.goto(dataSetupUrl);
+    await mainBody.waitFor();
     await browser.close();
   } else {
     console.log('No URL for data setup provided - skipping this step.');

helpers/login.ts

@@ -20,22 +20,29 @@ const logIn = async (
   loginUrl: string,
   firstLoggedInPageUrl: string,
 ): Promise<void> => {
+  const loginButton = page.locator(LOGIN_BUTTON_SELECTOR);
+  const profileDropdown = page.locator('button:has-text("e2e_tester")');
+
   if (process.env.APP_TEST_PASSWORD) {
     // Make sure browser is logged out before attempting to go through login flow
-    await logout(context);
+    await logout(page, context);
 
     // Navigate to login page and wait for login button to load
-    await page.goto(loginUrl);
-    const locator = page.locator(LOGIN_BUTTON_SELECTOR);
-    await locator.waitFor();
+    await page.goto(loginUrl, { waitUntil: 'networkidle' });
+    await loginButton.waitFor();
 
     // Fill out email and password
     await page.fill(LOGIN_EMAIL_INPUT_SELECTOR, E2E_ACCOUNT_LOGIN);
     await page.fill(LOGIN_PASSWORD_INPUT_SELECTOR, process.env.APP_TEST_PASSWORD || '');
 
-    // Then click the login button
-    await locator.click();
-    await page.waitForURL(firstLoggedInPageUrl);
+    // Then click the login button and make sure we navigate
+    await loginButton.click();
+
+    // Make sure the profile dropdown shows, as this appears on both sides once logged in
+    await profileDropdown.waitFor();
+
+    // Make sure this is the page we want
+    expect(page.url().includes(firstLoggedInPageUrl)).toBeTruthy();
   } else {
     console.warn('Could not log in because no APP_TEST_PASSWORD was provided. Failing test.');
     expect(true).toBe(false);
@@ -55,9 +62,8 @@ export const logIntoSupplier = async (page: Page, context: BrowserContext): Prom
   logIn(page, context, SUPPLIER_ROUTES.LOGIN, SUPPLIER_ROUTES.OVERVIEW);
 
 /**
- * Simulates a log out by clearing cookies. With a refresh, this should be enough
- * to fully log out.
+ * Simulates a log out by clearing cookies. On next page nav, we'll refresh.
  */
-export const logout = async (context: BrowserContext): Promise<void> => {
+export const logout = async (page: Page, context: BrowserContext): Promise<void> => {
   await context.clearCookies();
 };

tests/install.spec.ts

@@ -29,6 +29,5 @@ test.describe('Visit to the Shopify App Url', () => {
 
   test('sends users to Shopify login page', async ({ page }) => {
     await expect(page).toHaveURL(SHOPIFY_AUTH_URL_RE);
-    await page.close();
   });
 });

tests/login.spec.ts

@@ -1,6 +1,7 @@
 import { expect, test } from '@playwright/test';
 import {
   logIntoShopkeep,
+  logIntoSupplier,
   LOGIN_BUTTON_SELECTOR,
   LOGIN_EMAIL_INPUT_SELECTOR,
   LOGIN_PASSWORD_INPUT_SELECTOR,
@@ -19,21 +20,25 @@ test.describe('Login', () => {
    * do that in a beforeEach instead of doing it in each test
    */
   test.beforeEach(async ({ page }) => {
-    await page.goto(SHOPKEEP_ROUTES.LOGIN);
-    await page.waitForLoadState('networkidle');
+    await page.goto(SHOPKEEP_ROUTES.LOGIN, { waitUntil: 'networkidle' });
   });
 
   /**
    * Tests that the user can fill out the email and password inputs, then
    * click login and be redirected to the app
    */
-  test('can fill out valid email and password and successfully log in', async ({
-    context,
-    page,
-  }) => {
+  test('valid email and password logs us into Shopkeep', async ({ context, page }) => {
     await logIntoShopkeep(page, context);
   });
 
+  /**
+   * Tests that the user can fill out the email and password inputs, then
+   * click login and be redirected to the app
+   */
+  test('valid email and password logs us into Supplier', async ({ context, page }) => {
+    await logIntoSupplier(page, context);
+  });
+
   /**
    * Tests that we display an error and keep the user on the login page if
    * invalid credentials are provided

tests/shopkeep/discover.spec.ts

@@ -1,4 +1,4 @@
-import { expect, test } from '@playwright/test';
+import { test } from '@playwright/test';
 import { logIntoShopkeep } from '../../helpers/login';
 import { SHOPKEEP_ROUTES } from '../../helpers/routes';
 
@@ -15,11 +15,7 @@ test.describe('Shopkeep Discover', () => {
    */
   test.beforeEach(async ({ context, page }) => {
     await logIntoShopkeep(page, context);
-    await page.goto(SHOPKEEP_ROUTES.DISCOVER);
-
-    expect(page.url().includes(SHOPKEEP_ROUTES.DISCOVER)).toBeTruthy();
-
-    await page.waitForLoadState('networkidle');
+    await page.goto(SHOPKEEP_ROUTES.DISCOVER, { waitUntil: 'networkidle' });
   });
 
   // TODO: add tests once this page is ready to be launched

tests/shopkeep/inventory-management.spec.ts

@@ -13,7 +13,6 @@ test.describe('Shopkeep Inventory Management', () => {
    */
   test.beforeEach(async ({ context, page }) => {
     await logIntoShopkeep(page, context);
-    await page.waitForLoadState('networkidle');
   });
 
   /**

tests/shopkeep/navigation.spec.ts

@@ -1,5 +1,5 @@
 import { expect, test } from '@playwright/test';
-import { logIntoShopkeep, logout } from '../../helpers/login';
+import { logIntoShopkeep } from '../../helpers/login';
 import { SHOPKEEP_ROUTES } from '../../helpers/routes';
 
 test.describe.configure({ mode: 'parallel' });
@@ -15,24 +15,24 @@ test.describe('Shopkeep Navigation', () => {
    */
   test.beforeEach(async ({ context, page }) => {
     await logIntoShopkeep(page, context);
-    await page.waitForLoadState('networkidle');
-  });
-
-  test.afterEach(async ({ context }) => {
-    await logout(context);
   });
 
   test('can navigate to Inventory from Discover', async ({ page }) => {
-    const discoverTab = page.locator('button >> text=Discover');
-    const inventoryTab = page.locator('button >> text=Inventory');
+    const tab = page.locator('button >> text=Inventory');
+
+    // Look for the Products text below the profile dropdown, not next to it
+    const result = page.locator(
+      ':below(button:has-text("e2e_tester")) >> span:has-text("Products")',
+    );
 
     // We start on the Inventory page so navigate to Discover to make sure we can get back
-    expect(page.url().includes(SHOPKEEP_ROUTES.INVENTORY)).toBeTruthy();
-    await Promise.all([discoverTab.click(), page.waitForNavigation()]);
+    await page.goto(SHOPKEEP_ROUTES.DISCOVER);
+    await tab.waitFor();
     expect(page.url().includes(SHOPKEEP_ROUTES.DISCOVER)).toBeTruthy();
 
     // Navigate back to Inventory
-    await Promise.all([inventoryTab.click(), page.waitForNavigation()]);
+    await tab.click();
+    await result.waitFor();
 
     // Ensure that the URL is for the SK inventory page
     expect(page.url().includes(SHOPKEEP_ROUTES.INVENTORY)).toBeTruthy();
@@ -41,42 +41,54 @@ test.describe('Shopkeep Navigation', () => {
   test('can navigate to Discover from Inventory', async ({ page }) => {
     const tab = page.locator('button >> text=Discover');
 
+    // Look for the Suppliers text below the profile dropdown, not next to it
+    const result = page.locator(':below(button:has-text("e2e_tester")):text("Suppliers")');
+
     // Navigate via clicking the tab in the nav
     expect(page.url().includes(SHOPKEEP_ROUTES.INVENTORY)).toBeTruthy();
-    await Promise.all([tab.click(), page.waitForNavigation()]);
+    await tab.click();
+    await result.waitFor();
 
     // Ensure that the URL is for the SK discover page
     expect(page.url().includes(SHOPKEEP_ROUTES.DISCOVER)).toBeTruthy();
   });
 
   test('can navigate to Suppliers from Inventory', async ({ page }) => {
     const tab = page.locator('button >> text="My Suppliers"');
+    const result = page.locator('text="1 Supplier"');
 
     // Navigate via clicking the tab in the nav
     expect(page.url().includes(SHOPKEEP_ROUTES.INVENTORY)).toBeTruthy();
-    await Promise.all([tab.click(), page.waitForNavigation()]);
+    await tab.click();
+    await result.waitFor();
 
     // Ensure that the URL is for the SK suppliers page
     expect(page.url().includes(SHOPKEEP_ROUTES.SUPPLIERS)).toBeTruthy();
   });
 
   test('can navigate to Proposals from Inventory', async ({ page }) => {
     const tab = page.locator('button >> text="Proposals"');
+    const result = page.locator('text="Proposal to e2e_tester"');
 
     // Navigate via clicking the tab in the nav
     expect(page.url().includes(SHOPKEEP_ROUTES.INVENTORY)).toBeTruthy();
-    await Promise.all([tab.click(), page.waitForNavigation()]);
+    await tab.click();
+    await result.waitFor();
 
     // Ensure that the URL is for the SK proposals page
     expect(page.url().includes(SHOPKEEP_ROUTES.PROPOSALS)).toBeTruthy();
   });
 
   test('can navigate to Invite from Inventory', async ({ page }) => {
     const tab = page.locator('button >> text="Invite a Brand"');
+    const result = page.locator(
+      'text="Get started by inviting brands you trust. You\'ll be selling together in no time."',
+    );
 
     // Navigate via clicking the tab in the nav
     expect(page.url().includes(SHOPKEEP_ROUTES.INVENTORY)).toBeTruthy();
-    await Promise.all([tab.click(), page.waitForNavigation()]);
+    await tab.click();
+    await result.waitFor();
 
     // Ensure that the URL is for the SK invite page
     expect(page.url().includes(SHOPKEEP_ROUTES.INVITE)).toBeTruthy();
@@ -86,12 +98,14 @@ test.describe('Shopkeep Navigation', () => {
     // The button that has the user's name is the dropdown in the upper right to log out
     const profileDropdown = page.locator('button:has-text("e2e_tester")');
     const button = page.locator('button:has-text("Log Out")');
+    const result = page.locator('button#login');
 
     expect(page.url().includes(SHOPKEEP_ROUTES.INVENTORY)).toBeTruthy();
 
     // Open the profile dropdown then click the button
     await profileDropdown.click();
-    await Promise.all([button.click(), page.waitForNavigation()]);
+    await button.click();
+    await result.waitFor();
 
     // Ensure that the URL is for the login page
     expect(page.url().includes(SHOPKEEP_ROUTES.LOGIN)).toBeTruthy();
@@ -101,12 +115,14 @@ test.describe('Shopkeep Navigation', () => {
     // The button that has the user's name is the dropdown in the upper right to log out
     const profileDropdown = page.locator('button:has-text("e2e_tester")');
     const button = page.locator('button:has-text("Settings") >> nth=0');
+    const result = page.locator('text="Deactivate account"');
 
     expect(page.url().includes(SHOPKEEP_ROUTES.INVENTORY)).toBeTruthy();
 
     // Open the profile dropdown then click the button
     await profileDropdown.click();
-    await Promise.all([button.click(), page.waitForNavigation()]);
+    await button.click();
+    await result.waitFor();
 
     // Ensure that the URL is for the settings page
     expect(page.url().includes(SHOPKEEP_ROUTES.SETTINGS)).toBeTruthy();

tests/shopkeep/proposals.spec.ts

@@ -1,5 +1,5 @@
 import { expect, test } from '@playwright/test';
-import { logIntoShopkeep, logout } from '../../helpers/login';
+import { logIntoShopkeep } from '../../helpers/login';
 import { SHOPKEEP_ROUTES } from '../../helpers/routes';
 
 test.describe.configure({ mode: 'parallel' });
@@ -20,17 +20,13 @@ test.describe('Shopkeep Proposals', () => {
    * and then navigate to the Discover page
    */
   test.beforeEach(async ({ context, page }) => {
+    // Look for the proposals text below the profile dropdown, not next to it
+    const locator = page.locator(':below(button:has-text("e2e_tester")):text("Proposals")');
+
     await logIntoShopkeep(page, context);
     await page.goto(SHOPKEEP_ROUTES.PROPOSALS);
-
-    expect(page.url().includes(SHOPKEEP_ROUTES.PROPOSALS)).toBeTruthy();
-
-    // Wait for the request data to load before continuing
-    await page.waitForLoadState('networkidle');
-  });
-
-  test.afterEach(async ({ context }) => {
-    await logout(context);
+    await locator.waitFor();
+    expect(page.url()).toEqual(SHOPKEEP_ROUTES.PROPOSALS);
   });
 
   test('display a proposal and buttons for filtering by status', async ({ page }) => {
@@ -46,28 +42,18 @@ test.describe('Shopkeep Proposals', () => {
     await Promise.all(filters.map((filter) => filter.waitFor()));
   });
 
-  test('clicking the Pending filter shows one pending request', async ({ page }) => {
+  test('clicking the Pending filter still shows one pending request', async ({ page }) => {
     const tab = page.locator(PENDING_FILTER);
     const item = page.locator(ITEM_SELECTOR);
+    await item.waitFor();
 
-    // Click the filter and make sure we navigate
-    await Promise.all([tab.click(), page.waitForLoadState('networkidle')]);
+    // Click the filter
+    await tab.click();
 
     // Check that there is one list item still
     await item.waitFor();
   });
 
-  test('clicking the Approved filter shows no requests', async ({ page }) => {
-    const tab = page.locator(APPROVED_FILTER);
-    const item = page.locator(ITEM_SELECTOR);
-
-    // Click the filter and make sure we navigate
-    await Promise.all([tab.click(), page.waitForLoadState('networkidle')]);
-
-    // Make sure there's no list item
-    await item.waitFor({ state: 'detached' });
-  });
-
   test('clicking the Approved filter shows no requests, then clicking All shows one request', async ({
     page,
   }) => {
@@ -76,13 +62,12 @@ test.describe('Shopkeep Proposals', () => {
     const item = page.locator(ITEM_SELECTOR);
 
     // Click the filter and make sure we navigate and there's no item
-    await Promise.all([tab.click(), page.waitForLoadState('networkidle')]);
+    await item.waitFor();
+    await tab.click();
     await item.waitFor({ state: 'detached' });
 
     // Navigate back to all and make sure there's an item back
-    await Promise.all([allTab.click(), page.waitForLoadState('networkidle')]);
+    await allTab.click();
     await item.waitFor();
   });
-
-  // TODO: click on the line item action buttons and test the request response pages
 });