[37, 119, 191, 207, 105] Fix flaky test and remove retries (#5142)

* SALEOR_37 fix * add remove retires comment * cleanup * expect items instead of waiting for DOM * retain traces on first failure * 119 and 191 * changeset * change global timeouts * improve 119 * remove action timeout * 105 fix * SALEOR_207 wait for currency dropdown to load * wait for currency to be visible * keep one number convention * change changeset
saleor · Sep 23, 2024 · a9c9600 · a9c9600
1 parent 86f55c7
commit a9c9600
Show file tree

Hide file tree

Showing 10 changed files with 69 additions and 45 deletions.
diff --git a/.changeset/quiet-camels-raise.md b/.changeset/quiet-camels-raise.md
@@ -0,0 +1,5 @@
+---
+"saleor-dashboard": patch
+---
+
+Some end-to-end Playwright tests now have extended timeouts for UI elements to load. This means that automation tests should fail less. Playwright retires value has been set to 0.
diff --git a/playwright.config.ts b/playwright.config.ts
@@ -4,39 +4,42 @@ import dotenv from "dotenv";
 dotenv.config();
 
 const env = process.env;
-const DEFAULT_RETRIES = "1";
 const DEFAULT_WORKERS = "2";
+// const DEFAULT_RETRIES = "1";
 
 export default defineConfig({
   testDir: "playwright/tests",
   fullyParallel: true,
   forbidOnly: !!env.CI,
-  retries: parseInt(env.RETRIES || DEFAULT_RETRIES),
+  retries: 0,
+  // We are disabling retries for now as it prolongs the test run time
+  // as the test will most likely fail again. We can enable it later if needed.
+  // retries: parseInt(env.RETRIES || DEFAULT_RETRIES),
   workers: parseInt(env.WORKERS || DEFAULT_WORKERS),
   reporter: process.env.CI
     ? [
-      ["blob"],
-      ["github"],
-      [
-        "playwright-testmo-reporter",
-        {
-          outputFile: "testmo/testmo.xml", // Optional: Output file path. Defaults to 'testmo.xml'.
-          embedBrowserType: true, // Optional: Embed browser type in the XML file. Defaults to false.
-          embedTestSteps: true, // Optional: Embed test steps in the XML file. Defaults to true.
-          testStepCategories: ["hook", "expect", "pw:api", "test.step"], // Optional: Test step categories to include in the XML file. Defaults to ["hook","expect","pw:api","test.step"]. Possible options are "hook", "expect", "pw:api", "test.step".
-          testTitleDepth: 1, // Optional: Test case title depth to report in the XML file. Defaults to 1. Increase this to 2 include suite name. Increase this even further to include the path.
-          attachmentBasePathCallback: () =>
-            process.env.URL_TO_RUN ? process.env.URL_TO_RUN : "",
-        },
-      ],
-    ]
+        ["blob"],
+        ["github"],
+        [
+          "playwright-testmo-reporter",
+          {
+            outputFile: "testmo/testmo.xml", // Optional: Output file path. Defaults to 'testmo.xml'.
+            embedBrowserType: true, // Optional: Embed browser type in the XML file. Defaults to false.
+            embedTestSteps: true, // Optional: Embed test steps in the XML file. Defaults to true.
+            testStepCategories: ["hook", "expect", "pw:api", "test.step"], // Optional: Test step categories to include in the XML file. Defaults to ["hook","expect","pw:api","test.step"]. Possible options are "hook", "expect", "pw:api", "test.step".
+            testTitleDepth: 1, // Optional: Test case title depth to report in the XML file. Defaults to 1. Increase this to 2 include suite name. Increase this even further to include the path.
+            attachmentBasePathCallback: () =>
+              process.env.URL_TO_RUN ? process.env.URL_TO_RUN : "",
+          },
+        ],
+      ]
     : [["html"], ["list"]],
-  expect: { timeout: 5000 },
+  expect: { timeout: 10 * 1000 },
   maxFailures: 10,
-  timeout: 30000,
+  timeout: 30 * 1000,
   use: {
     baseURL: env.BASE_URL || "",
-    trace: env.CI ? "on-all-retries" : "on",
+    trace: env.CI ? "retain-on-failure" : "on",
     screenshot: "only-on-failure",
     testIdAttribute: "data-test-id",
     video: env.CI ? "retain-on-failure" : "off",
@@ -51,7 +54,7 @@ export default defineConfig({
       name: "e2e",
       dependencies: ["setup"],
       use: { ...devices["Desktop Chrome"] },
-      testIgnore: "playwright/tests/apps.spec.ts"
+      testIgnore: "playwright/tests/apps.spec.ts",
     },
     {
       name: "apps-e2e",

diff --git a/playwright/pages/customersPage.ts b/playwright/pages/customersPage.ts
@@ -27,6 +27,7 @@ export class CustomersPage extends BasePage {
     readonly issueNewGiftCardButton = page.getByTestId("issue-new-gift-card"),
     readonly emailPageTitleText = page.getByTestId("user-email-title"),
     readonly customerActiveCheckbox = page.getByTestId("customer-active-checkbox").locator("input"),
+    readonly amountDropdown = page.locator('div[name="balanceCurrency"]'),
   ) {
     super(page);
     this.addressForm = new AddressForm(page);

diff --git a/playwright/pages/dialogs/issueGiftCardDialog.ts b/playwright/pages/dialogs/issueGiftCardDialog.ts
@@ -23,6 +23,8 @@ export class IssueGiftCardDialog extends BasePage {
     readonly okButton = page.getByTestId("submit"),
     readonly copyCodeButton = page.getByTestId("copy-code-button"),
     readonly option = page.getByTestId("select-option"),
+    readonly issueGiftCardDialog = page.getByTestId("gift-card-dialog"),
+    readonly amountDropdown = page.locator('div[name="balanceCurrency"]'),
   ) {
     super(page);
   }
@@ -57,6 +59,7 @@ export class IssueGiftCardDialog extends BasePage {
 
   async typeCustomTag(tag: string) {
     await this.tagsInput.fill(tag);
+    await expect(this.issueGiftCardDialog.getByText("Loading...")).not.toBeVisible();
     await this.tagsInputOptions.filter({ hasText: `Add new value: ${tag}` }).click();
   }
 
@@ -95,7 +98,7 @@ export class IssueGiftCardDialog extends BasePage {
     return allTexts[0];
   }
 
-  async blur() {
-    await this.page.click("[data-test-id='gift-card-dialog']");
+  async tagsInputBlur() {
+    await this.tagsInput.blur();
   }
 }
diff --git a/playwright/pages/pageElements/rightSideDetailsSection.ts b/playwright/pages/pageElements/rightSideDetailsSection.ts
@@ -72,7 +72,9 @@ export class RightSideDetailsPage extends BasePage {
 
   async expectOptionsSelected(section: Locator, names: string[]) {
     for (const name of names) {
-      await expect(section.getByText(name)).toBeVisible({ timeout: 30000 });
+      await expect(section.getByText(name)).toBeVisible({
+        timeout: 30 * 1000, // 30 seconds
+      });
     }
   }
 

diff --git a/playwright/tests/apps.spec.ts b/playwright/tests/apps.spec.ts
@@ -18,45 +18,39 @@ test.beforeEach(({ page }) => {
 
 const PRE_INSTALLATION_TIMEOUT = 20 * 1000;
 const INSTALLATION_PENDING_TIMEOUT = 50 * 1000;
-const APP_LISTED_TIMEOUT = 15 * 1000;
-const APP_INSTALLED_TIMEOUT = 50 * 1000;
+const APP_EXPECT_UI_TIMEOUT = 15 * 1000;
 
 test("TC: SALEOR_119 User should be able to install and configure app from manifest @e2e", async ({
   page,
 }) => {
   await appsPage.gotoAppsList();
-  await appsPage.waitForDOMToFullyLoad();
-  await expect(appsPage.installExternalAppButton).toBeVisible();
   await appsPage.installExternalAppButton.click();
   await appsPage.typeManifestUrl("https://klaviyo.saleor.app/api/manifest");
   await appsPage.installAppFromManifestButton.click();
-  // Klaviyo app can take a while to respond with manifest if it's
-  // cold-starting
   await expect(installationPage.appInstallationPageHeader).toHaveText(
     "You are about to install Klaviyo",
     {
+      // Klaviyo app can take a while to respond with manifest if it's
+      // cold-starting
       timeout: PRE_INSTALLATION_TIMEOUT,
     },
   );
   await installationPage.installAppButton.click();
-  await appsPage.expectSuccessBanner();
+
+  await expect(appsPage.successBanner).toBeVisible({ timeout: INSTALLATION_PENDING_TIMEOUT });
   await expect(appsPage.installedAppRow.first()).toBeVisible();
-  await appsPage.installationPendingLabel.waitFor({
-    state: "hidden",
-    timeout: INSTALLATION_PENDING_TIMEOUT,
-  });
-  await expect(appsPage.appKlaviyo).toContainText("Klaviyo", {
-    timeout: APP_LISTED_TIMEOUT,
-  });
-  await appsPage.installedAppRow
-    .filter({ hasText: "Klaviyo" })
-    .first()
-    .waitFor({ state: "visible", timeout: APP_INSTALLED_TIMEOUT });
+  await expect(appsPage.installationPendingLabel).not.toBeVisible();
+
+  await expect(appsPage.appKlaviyo).toContainText("Klaviyo");
+  await expect(appsPage.installedAppRow.filter({ hasText: "Klaviyo" }).first()).toBeVisible();
   await appsPage.appKlaviyo.click();
 
   const iframeLocator = page.frameLocator("iframe");
 
-  await expect(iframeLocator.getByLabel("PUBLIC_TOKEN")).toBeVisible();
+  await expect(iframeLocator.getByLabel("PUBLIC_TOKEN")).toBeVisible({
+    // Klavyio's UI can take a while to load initially
+    timeout: APP_EXPECT_UI_TIMEOUT,
+  });
   await iframeLocator.getByLabel("PUBLIC_TOKEN").fill("test_token");
   await iframeLocator.getByText("Save").click();
   await appsPage.expectSuccessBanner();

diff --git a/playwright/tests/customers.spec.ts b/playwright/tests/customers.spec.ts
@@ -215,6 +215,8 @@ test("TC: SALEOR_207 Issue a new gift card for the customer @e2e @customer", asy
 
   await customersPage.gotoCustomerDetailsPage(CUSTOMERS.editCustomer.id);
   await customersPage.clickIssueNewGiftCard();
+
+  await expect(customersPage.amountDropdown).toBeVisible();
   await customersPage.issueGiftCardDialog.typeAmount(amount);
   await customersPage.issueGiftCardDialog.typeCustomTag(faker.lorem.word());
   await customersPage.issueGiftCardDialog.typeNote(faker.lorem.sentences(3));

diff --git a/playwright/tests/giftCards.spec.ts b/playwright/tests/giftCards.spec.ts
@@ -17,9 +17,10 @@ test.beforeEach(async ({ page, request }) => {
 });
 test("TC: SALEOR_105 Issue gift card @e2e @gift", async () => {
   await giftCardsPage.clickIssueCardButton();
+  await expect(giftCardsPage.issueGiftCardDialog.amountDropdown).toBeVisible();
   await giftCardsPage.issueGiftCardDialog.typeAmount("50");
   await giftCardsPage.issueGiftCardDialog.typeCustomTag("super ultra automation discount");
-  await giftCardsPage.issueGiftCardDialog.blur();
+  await giftCardsPage.issueGiftCardDialog.tagsInputBlur();
   await giftCardsPage.issueGiftCardDialog.clickRequiresActivationCheckbox();
   await giftCardsPage.issueGiftCardDialog.clickIssueButton();
   await expect(giftCardsPage.issueGiftCardDialog.cardCode).toBeVisible();

diff --git a/playwright/tests/orders.spec.ts b/playwright/tests/orders.spec.ts
@@ -257,6 +257,10 @@ test("TC: SALEOR_84 Create draft order @e2e @draft", async () => {
 });
 
 test("TC: SALEOR_191 Refund products from the fully paid order @e2e @refunds", async () => {
+  // All steps of this test pass (including after hooks), but Playwright
+  // marks it as failed because of exceeding 30s timeout
+  test.slow();
+
   const order = ORDERS.fullyPaidOrderWithSingleTransaction;
 
   await ordersPage.goToExistingOrderPage(order.id);
@@ -268,7 +272,7 @@ test("TC: SALEOR_191 Refund products from the fully paid order @e2e @refunds", a
 
   const productRow = await refundPage.getProductRow(order.lineItems[0].name);
 
-  expect(productRow.locator(refundPage.productQuantityInput)).toHaveValue(
+  await expect(productRow.locator(refundPage.productQuantityInput)).toHaveValue(
     order.lineItems[0].quantity,
   );
 

diff --git a/playwright/tests/shippingMethods.spec.ts b/playwright/tests/shippingMethods.spec.ts
@@ -140,8 +140,17 @@ test("TC: SALEOR_37 Update a shipping method @shipping-method @e2e", async () =>
     SHIPPING_METHODS.shippingMethodToBeUpdated.name,
   );
 
+  await shippingMethodsPage.rightSideDetailsPage.expectOptionsSelected(
+    channelSection,
+    alreadyAssignedChannels,
+  );
   await shippingMethodsPage.rightSideDetailsPage.clickChannelsSelectShippingPage();
   await shippingMethodsPage.rightSideDetailsPage.selectSingleChannelShippingPage();
+  await shippingMethodsPage.rightSideDetailsPage.expectOptionsSelected(
+    warehouseSection,
+    alreadyAssignedWarehouses,
+  );
+
   await shippingMethodsPage.rightSideDetailsPage.clickWarehouseSelectShippingPage();
   await shippingMethodsPage.rightSideDetailsPage.typeAndSelectMultipleWarehousesShippingPage(
     warehousesToBeAssigned,