From 78d2a5e3c094ae4e1ec463ac1e64a1ebe949d83b Mon Sep 17 00:00:00 2001 From: Steve Larson <9larsons@gmail.com> Date: Wed, 27 Mar 2024 13:57:53 -0500 Subject: [PATCH] =?UTF-8?q?=F0=9F=90=9B=20Fixed=20flaky=20browser=20tests?= =?UTF-8?q?=20(#19929)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ref https://linear.app/tryghost/issue/CFR-13 - enabled saving traces on browser test failure; this makes troubleshooting a lot easier - updated handling in offers tests to ensure the tier has fully loaded in the UI (not just `networkidle`) - updated publishing test to examine the publish button reaction to the save action response instead of a 300ms pause In general, our tests use a lot of watching for 'networkidle' - and sometimes just raw timeouts - which do not scale well into running tests on CI. In particular, 'networkidle' does not work if we're expecting to see React components' state updates propagate and re-render. We should always instead look to the content which encapsulates the response and the UI updates. This is something we should tackle on a larger scale. --- ghost/core/playwright.config.js | 3 ++- ghost/core/test/e2e-browser/admin/publishing.spec.js | 6 ++++-- ghost/core/test/e2e-browser/portal/donations.spec.js | 4 ++++ .../core/test/e2e-browser/utils/e2e-browser-utils.js | 12 +++++++++--- 4 files changed, 19 insertions(+), 6 deletions(-) diff --git a/ghost/core/playwright.config.js b/ghost/core/playwright.config.js index 23946f3c04..19b016c6d2 100644 --- a/ghost/core/playwright.config.js +++ b/ghost/core/playwright.config.js @@ -5,11 +5,12 @@ const config = { expect: { timeout: 10000 }, + // save trace on fail retries: process.env.CI ? 2 : 0, workers: process.env.CI ? '100%' : (process.env.PLAYWRIGHT_SLOWMO ? 1 : undefined), reporter: process.env.CI ? [['list', {printSteps: true}], ['html']] : [['list', {printSteps: true}]], use: { - // trace: 'retain-on-failure', + trace: 'retain-on-failure', // Use a single browser since we can't simultaneously test multiple browsers browserName: 'chromium', headless: !process.env.PLAYWRIGHT_DEBUG, diff --git a/ghost/core/test/e2e-browser/admin/publishing.spec.js b/ghost/core/test/e2e-browser/admin/publishing.spec.js index 3325b873e7..c535cbad80 100644 --- a/ghost/core/test/e2e-browser/admin/publishing.spec.js +++ b/ghost/core/test/e2e-browser/admin/publishing.spec.js @@ -314,10 +314,12 @@ test.describe('Publishing', () => { await adminPage.fill('[data-test-field="custom-excerpt"]', 'Short description and meta'); // save - await adminPage.locator('[data-test-button="publish-save"]').first().click(); + const saveButton = await adminPage.locator('[data-test-button="publish-save"]').first(); + await expect(saveButton).toHaveText('Update'); + await saveButton.click(); + await expect(saveButton).toHaveText('Updated'); // check front-end has new text after reloading - await frontendPage.waitForTimeout(300); // let save go through await frontendPage.reload(); await expect(publishedBody).toContainText('This is some updated text.'); await expect(publishedHeader).toContainText('Jan 7, 2022'); diff --git a/ghost/core/test/e2e-browser/portal/donations.spec.js b/ghost/core/test/e2e-browser/portal/donations.spec.js index 7db5441f7b..e772e65367 100644 --- a/ghost/core/test/e2e-browser/portal/donations.spec.js +++ b/ghost/core/test/e2e-browser/portal/donations.spec.js @@ -12,7 +12,9 @@ test.describe('Portal', () => { await sharedPage.locator('#email').fill('member-donation-test-1@ghost.org'); await completeStripeSubscription(sharedPage); + await sharedPage.pause(); // Check success message + await sharedPage.waitForSelector('[data-testid="portal-popup-frame"]', {state: 'visible'}); const portalFrame = sharedPage.frameLocator('[data-testid="portal-popup-frame"]'); await expect(portalFrame.getByText('Thank you!')).toBeVisible(); }); @@ -36,6 +38,7 @@ test.describe('Portal', () => { await completeStripeSubscription(sharedPage); // Check success message + await sharedPage.waitForSelector('[data-testid="portal-popup-frame"]', {state: 'visible'}); const portalFrame = sharedPage.frameLocator('[data-testid="portal-popup-frame"]'); await expect(portalFrame.getByText('Thank you!')).toBeVisible(); }); @@ -64,6 +67,7 @@ test.describe('Portal', () => { await completeStripeSubscription(sharedPage); // Check success message + await sharedPage.waitForSelector('[data-testid="portal-popup-frame"]', {state: 'visible'}); const portalFrame = sharedPage.frameLocator('[data-testid="portal-popup-frame"]'); await expect(portalFrame.getByText('Thank you!')).toBeVisible(); }); diff --git a/ghost/core/test/e2e-browser/utils/e2e-browser-utils.js b/ghost/core/test/e2e-browser/utils/e2e-browser-utils.js index 2d42a93365..2a59b842b3 100644 --- a/ghost/core/test/e2e-browser/utils/e2e-browser-utils.js +++ b/ghost/core/test/e2e-browser/utils/e2e-browser-utils.js @@ -260,9 +260,14 @@ const createOffer = async (page, {name, tierName, offerType, amount, discountTyp offerName = `${name} (${new ObjectID().toHexString().slice(0, 40 - name.length - 3)})`; // Tiers request can take time, so waiting until there is no connections before interacting with them await page.waitForLoadState('networkidle'); + // ... and even so, the component updates can take a bit to trickle down, so we should verify that the Tier is fully loaded before proceeding + await page.getByTestId('tiers').getByText('No active tiers found').waitFor({state: 'hidden'}); + await page.getByTestId('offers').getByRole('button', {name: 'Manage tiers'}).waitFor({state: 'hidden'}); + // only one of these buttons is ever available - either 'Add offer' or 'Manage offers' const hasExistingOffers = await page.getByTestId('offers').getByRole('button', {name: 'Manage offers'}).isVisible(); const isCTA = await page.getByTestId('offers').getByRole('button', {name: 'Add offer'}).isVisible(); + // Archive other offers to keep the list tidy // We only need 1 offer to be active at a time // Either the list of active offers loads, or the CTA when no offers exist @@ -281,12 +286,13 @@ const createOffer = async (page, {name, tierName, offerType, amount, discountTyp if (isCTA) { await page.getByTestId('offers').getByRole('button', {name: 'Add offer'}).click(); } else { + // ensure the modal is open + if (!page.getByTestId('offers-modal').isVisible()) { + await page.getByTestId('offers').getByRole('button', {name: 'Manage offers'}).click(); + } await page.getByText('New offer').click(); } - // const newOfferButton = await page.getByTestId('offers').getByRole('button', {name: 'Add offer'}) || await page.getByTestId('offers').getByRole('button', {name: 'New offer'}); - // await page.getByTestId('offers').getByRole('button', {name: 'Add offer'}).click(); - // await newOfferButton.click(); await page.getByLabel('Offer name').fill(offerName); if (offerType === 'freeTrial') {