From 7e1e6a10d1261202495668549fc9cfc7e3c33107 Mon Sep 17 00:00:00 2001 From: Wojciech Mista Date: Mon, 23 Sep 2024 13:24:49 +0200 Subject: [PATCH] [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 --- .changeset/quiet-camels-raise.md | 5 +++ playwright.config.ts | 45 ++++++++++--------- playwright/pages/customersPage.ts | 1 + .../pages/dialogs/issueGiftCardDialog.ts | 7 ++- .../pageElements/rightSideDetailsSection.ts | 4 +- playwright/tests/apps.spec.ts | 32 ++++++------- playwright/tests/customers.spec.ts | 2 + playwright/tests/giftCards.spec.ts | 3 +- playwright/tests/orders.spec.ts | 6 ++- playwright/tests/shippingMethods.spec.ts | 9 ++++ 10 files changed, 69 insertions(+), 45 deletions(-) create mode 100644 .changeset/quiet-camels-raise.md diff --git a/.changeset/quiet-camels-raise.md b/.changeset/quiet-camels-raise.md new file mode 100644 index 00000000000..415e784b1c0 --- /dev/null +++ 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. \ No newline at end of file diff --git a/playwright.config.ts b/playwright.config.ts index 9929871456b..4357dbbae11 100644 --- 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 index 02cbb6fd442..5e661267944 100644 --- 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 index 5993840aa02..f2974e63f19 100644 --- 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 index e00bce293b5..cd625ee48b8 100644 --- 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 index 27786d2f1cc..139cf4ef223 100644 --- 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 index be1011512c1..0f6a70427ff 100644 --- 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 index f705482c457..357be0c43f3 100644 --- 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 index b82fdabdbc0..380bda5e9b9 100644 --- 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 index 0c3c2ac09f7..a95a8bea6c3 100644 --- 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,