From de076e7f7d4cd8f760ed05475c5ac86975e561e3 Mon Sep 17 00:00:00 2001 From: Ravi <7014230+arelra@users.noreply.github.com> Date: Mon, 18 Aug 2025 22:31:08 +0100 Subject: [PATCH 1/9] Playwright load-page utility uses POST endpoints --- dotcom-rendering/playwright.config.ts | 22 ++- dotcom-rendering/playwright/lib/load-page.ts | 194 +++++++++++++------ 2 files changed, 150 insertions(+), 66 deletions(-) diff --git a/dotcom-rendering/playwright.config.ts b/dotcom-rendering/playwright.config.ts index d9e01b4cf88..0f79fc1f3d7 100644 --- a/dotcom-rendering/playwright.config.ts +++ b/dotcom-rendering/playwright.config.ts @@ -1,10 +1,14 @@ import { defineConfig, devices } from '@playwright/test'; const isDev = process.env.NODE_ENV !== 'production'; + /** - * The server port for local development or CI + * Server port + * local development: 3030 + * CI: 9000 */ export const PORT = isDev ? 3030 : 9000; +export const ORIGIN = `http://localhost:9000`; /** * See https://playwright.dev/docs/test-configuration. @@ -38,12 +42,12 @@ export default defineConfig({ use: { ...devices['Desktop Chrome'] }, }, ], - webServer: { - // On CI the server is already started so a no-op - command: isDev ? 'make dev' : ':', - url: `http://localhost:${PORT}`, - reuseExistingServer: true, - stdout: 'pipe', - stderr: 'pipe', - }, + // webServer: { + // // On CI the server is already started so a no-op + // command: ':', + // url: ORIGIN, + // reuseExistingServer: true, + // stdout: 'pipe', + // stderr: 'pipe', + // }, }); diff --git a/dotcom-rendering/playwright/lib/load-page.ts b/dotcom-rendering/playwright/lib/load-page.ts index d28f6aa96bf..05eb7f7285e 100644 --- a/dotcom-rendering/playwright/lib/load-page.ts +++ b/dotcom-rendering/playwright/lib/load-page.ts @@ -1,9 +1,82 @@ import type { Page } from '@playwright/test'; -import { PORT } from 'playwright.config'; +import { ORIGIN } from '../../playwright.config'; import type { FEArticle } from '../../src/frontend/feArticle'; -import { validateAsFEArticle } from '../../src/model/validate'; +import type { FEFront } from '../../src/frontend/feFront'; +import { + validateAsFEArticle, + validateAsFEFront, +} from '../../src/model/validate'; -const BASE_URL = `http://localhost:${PORT}`; +type LoadPageOptions = { + queryParams?: Record; + queryParamsOn?: boolean; + fragment?: `#${string}`; + waitUntil?: 'domcontentloaded' | 'load'; + region?: 'GB' | 'US' | 'AU' | 'INT'; + preventSupportBanner?: boolean; + useSecure?: boolean; + overrides?: { + configOverrides?: Record; + switchOverrides?: Record; + article?: FEArticle; + }; +}; + +type LoadPageParams = { + page: Page; + path: string; +} & LoadPageOptions; + +const getDcrPostUrl = (path: string) => `${ORIGIN}/${path.split('/')[1]}`; + +const getFrontendUrl = (path: string) => { + const secondSlashIndex = path.indexOf('/', 1); + return `${path.substring(secondSlashIndex + 1)}.json?dcr`; +}; + +const getDcrUrl = ({ + path, + queryParamsOn, + queryParams, + fragment, +}: Required< + Pick +> & + Pick): string => { + const paramsString = queryParamsOn + ? `?${new URLSearchParams({ + adtest: 'fixed-puppies-ci', + ...queryParams, + }).toString()}` + : ''; + + return `${ORIGIN}${path}${paramsString}${fragment ?? ''}`; +}; + +const getFrontendArticle = async ( + url: string, +): Promise => { + try { + const response = await fetch(getFrontendUrl(url)); + if (!response.ok) { + throw new Error( + `Failed to fetch article JSON from ${url}: ${response.statusText}`, + ); + } + if (url.startsWith('/Article')) { + return validateAsFEArticle(await response.json()); + } else if (url.startsWith('/Front')) { + return validateAsFEFront(await response.json()); + } + throw new Error(`Unsupported URL for fetching article: ${url}`); + } catch (error) { + throw new Error( + `Error fetching or validating article JSON from ${url}: ${ + error instanceof Error ? error.message : String(error) + }`, + ); + } +}; /** * Loads a page in Playwright and centralises setup @@ -17,16 +90,9 @@ const loadPage = async ({ waitUntil = 'domcontentloaded', region = 'GB', preventSupportBanner = true, -}: { - page: Page; - path: string; - queryParams?: Record; - queryParamsOn?: boolean; - fragment?: `#${string}`; - waitUntil?: 'domcontentloaded' | 'load'; - region?: 'GB' | 'US' | 'AU' | 'INT'; - preventSupportBanner?: boolean; -}): Promise => { + useSecure = false, + overrides = {}, +}: LoadPageParams): Promise => { await page.addInitScript( (args) => { // Set the geo region, defaults to GB @@ -47,19 +113,46 @@ const loadPage = async ({ preventSupportBanner, }, ); - // Add an adtest query param to ensure we get a fixed test ad - const paramsString = queryParamsOn - ? `?${new URLSearchParams({ - adtest: 'fixed-puppies-ci', - ...queryParams, - }).toString()}` - : ''; - // The default Playwright waitUntil: 'load' ensures all requests have completed - // Use 'domcontentloaded' to speed up tests and prevent hanging requests from timing out tests - await page.goto(`${BASE_URL}${path}${paramsString}${fragment ?? ''}`, { - waitUntil, + // If overrides exist, but no article fixture we fetch it from Frontend + const frontendArticle = await (overrides.article + ? Promise.resolve(overrides.article) + : getFrontendArticle(path)); + + // Apply the overrides to the article config and switches and then send the + // modified JSON payload to DCR + const postData = { + ...frontendArticle, + config: { + ...frontendArticle.config, + ...overrides.configOverrides, + switches: { + ...frontendArticle.config.switches, + ...overrides.switchOverrides, + }, + }, + }; + + const dcrUrl = getDcrUrl({ + path, + useSecure, + queryParamsOn, + queryParams, + fragment, + }); + + await page.route(dcrUrl, async (route) => { + await route.continue({ + method: 'POST', + headers: { + 'Content-Type': 'application/json', + }, + postData, + url: getDcrPostUrl(path), + }); }); + + await page.goto(dcrUrl, { waitUntil }); }; /** @@ -73,33 +166,19 @@ const loadPageWithOverrides = async ( configOverrides?: Record; switchOverrides?: Record; }, + options?: LoadPageOptions, ): Promise => { - const path = `/Article`; - await page.route(`${BASE_URL}${path}`, async (route) => { - const postData = { - ...article, - config: { - ...article.config, - ...overrides?.configOverrides, - switches: { - ...article.config.switches, - ...overrides?.switchOverrides, - }, - }, - }; - await route.continue({ - method: 'POST', - headers: { - 'Content-Type': 'application/json', - }, - postData, - }); + await loadPage({ + page, + path: '/Article', + queryParamsOn: false, + ...options, + overrides: { ...overrides, article }, }); - await loadPage({ page, path, queryParamsOn: false }); }; /** - * Fetch the page json from PROD then load it as a POST with overrides + * Fetch the page json from the provided URL then load it locally as a POST with overrides */ const fetchAndloadPageWithOverrides = async ( page: Page, @@ -108,21 +187,22 @@ const fetchAndloadPageWithOverrides = async ( configOverrides?: Record; switchOverrides?: Record; }, + options?: LoadPageOptions, ): Promise => { const article = validateAsFEArticle( await fetch(`${url}.json?dcr`).then((res) => res.json()), ); - await loadPageWithOverrides(page, article, { - configOverrides: overrides?.configOverrides, - switchOverrides: { - ...overrides?.switchOverrides, + await loadPageWithOverrides( + page, + article, + { + configOverrides: overrides?.configOverrides, + switchOverrides: { + ...overrides?.switchOverrides, + }, }, - }); + options, + ); }; -export { - BASE_URL, - fetchAndloadPageWithOverrides, - loadPage, - loadPageWithOverrides, -}; +export { fetchAndloadPageWithOverrides, loadPage, loadPageWithOverrides }; From 5909f8d29a686657a0ed1579661e7cea7e02b414 Mon Sep 17 00:00:00 2001 From: Ravi <7014230+arelra@users.noreply.github.com> Date: Mon, 18 Aug 2025 23:33:38 +0100 Subject: [PATCH 2/9] Fix fragment handling --- dotcom-rendering/playwright/lib/load-page.ts | 23 +++++++++----------- 1 file changed, 10 insertions(+), 13 deletions(-) diff --git a/dotcom-rendering/playwright/lib/load-page.ts b/dotcom-rendering/playwright/lib/load-page.ts index 05eb7f7285e..b088d7bfc6f 100644 --- a/dotcom-rendering/playwright/lib/load-page.ts +++ b/dotcom-rendering/playwright/lib/load-page.ts @@ -14,7 +14,6 @@ type LoadPageOptions = { waitUntil?: 'domcontentloaded' | 'load'; region?: 'GB' | 'US' | 'AU' | 'INT'; preventSupportBanner?: boolean; - useSecure?: boolean; overrides?: { configOverrides?: Record; switchOverrides?: Record; @@ -31,18 +30,17 @@ const getDcrPostUrl = (path: string) => `${ORIGIN}/${path.split('/')[1]}`; const getFrontendUrl = (path: string) => { const secondSlashIndex = path.indexOf('/', 1); - return `${path.substring(secondSlashIndex + 1)}.json?dcr`; + const contentUrl = path.substring(secondSlashIndex + 1); + return `${contentUrl}.json?dcr`; }; const getDcrUrl = ({ path, queryParamsOn, queryParams, - fragment, }: Required< - Pick -> & - Pick): string => { + Pick +>): string => { const paramsString = queryParamsOn ? `?${new URLSearchParams({ adtest: 'fixed-puppies-ci', @@ -50,7 +48,7 @@ const getDcrUrl = ({ }).toString()}` : ''; - return `${ORIGIN}${path}${paramsString}${fragment ?? ''}`; + return `${ORIGIN}${path}${paramsString}`; }; const getFrontendArticle = async ( @@ -90,7 +88,6 @@ const loadPage = async ({ waitUntil = 'domcontentloaded', region = 'GB', preventSupportBanner = true, - useSecure = false, overrides = {}, }: LoadPageParams): Promise => { await page.addInitScript( @@ -119,8 +116,7 @@ const loadPage = async ({ ? Promise.resolve(overrides.article) : getFrontendArticle(path)); - // Apply the overrides to the article config and switches and then send the - // modified JSON payload to DCR + // Apply the overrides to the article config and switches const postData = { ...frontendArticle, config: { @@ -135,16 +131,17 @@ const loadPage = async ({ const dcrUrl = getDcrUrl({ path, - useSecure, queryParamsOn, queryParams, - fragment, }); + // Override the request to the DCR URL to use a POST method + // with the overridden payload await page.route(dcrUrl, async (route) => { await route.continue({ method: 'POST', headers: { + ...route.request().headers(), 'Content-Type': 'application/json', }, postData, @@ -152,7 +149,7 @@ const loadPage = async ({ }); }); - await page.goto(dcrUrl, { waitUntil }); + await page.goto(`${dcrUrl}${fragment ?? ''}`, { waitUntil }); }; /** From 19d6a5c2857c847bb31d9818d1fedbd8ecd1f7b6 Mon Sep 17 00:00:00 2001 From: Ravi <7014230+arelra@users.noreply.github.com> Date: Tue, 19 Aug 2025 00:38:12 +0100 Subject: [PATCH 3/9] Pass query params to the fetch for Frontend json payload. Fixes liveblog tests which passes live=true param --- dotcom-rendering/playwright/lib/load-page.ts | 48 +++++++++++--------- 1 file changed, 26 insertions(+), 22 deletions(-) diff --git a/dotcom-rendering/playwright/lib/load-page.ts b/dotcom-rendering/playwright/lib/load-page.ts index b088d7bfc6f..14add2f5391 100644 --- a/dotcom-rendering/playwright/lib/load-page.ts +++ b/dotcom-rendering/playwright/lib/load-page.ts @@ -26,36 +26,23 @@ type LoadPageParams = { path: string; } & LoadPageOptions; -const getDcrPostUrl = (path: string) => `${ORIGIN}/${path.split('/')[1]}`; - const getFrontendUrl = (path: string) => { const secondSlashIndex = path.indexOf('/', 1); const contentUrl = path.substring(secondSlashIndex + 1); - return `${contentUrl}.json?dcr`; -}; - -const getDcrUrl = ({ - path, - queryParamsOn, - queryParams, -}: Required< - Pick ->): string => { - const paramsString = queryParamsOn - ? `?${new URLSearchParams({ - adtest: 'fixed-puppies-ci', - ...queryParams, - }).toString()}` - : ''; - - return `${ORIGIN}${path}${paramsString}`; + return `${contentUrl}.json`; }; const getFrontendArticle = async ( url: string, + queryParams: LoadPageParams['queryParams'], ): Promise => { try { - const response = await fetch(getFrontendUrl(url)); + const paramsString = `${new URLSearchParams({ + dcr: 'true', + ...queryParams, + }).toString()}`; + const frontendUrl = `${getFrontendUrl(url)}?${paramsString}`; + const response = await fetch(frontendUrl); if (!response.ok) { throw new Error( `Failed to fetch article JSON from ${url}: ${response.statusText}`, @@ -76,6 +63,23 @@ const getFrontendArticle = async ( } }; +const getDcrUrl = ({ + path, + queryParamsOn, + queryParams, +}: Pick): string => { + const paramsString = queryParamsOn + ? `?${new URLSearchParams({ + adtest: 'fixed-puppies-ci', + ...queryParams, + }).toString()}` + : ''; + + return `${ORIGIN}${path}${paramsString}`; +}; + +const getDcrPostUrl = (path: string) => `${ORIGIN}/${path.split('/')[1]}`; + /** * Loads a page in Playwright and centralises setup */ @@ -114,7 +118,7 @@ const loadPage = async ({ // If overrides exist, but no article fixture we fetch it from Frontend const frontendArticle = await (overrides.article ? Promise.resolve(overrides.article) - : getFrontendArticle(path)); + : getFrontendArticle(path, queryParams)); // Apply the overrides to the article config and switches const postData = { From 8a7f58a33d285334f63bf0fc4eb5abb7f44a6315 Mon Sep 17 00:00:00 2001 From: Ravi <7014230+arelra@users.noreply.github.com> Date: Tue, 19 Aug 2025 01:04:37 +0100 Subject: [PATCH 4/9] Fix edition switcher test by passing cookies to Frontend request. Ensures correct value is set for Edition from the forceful setting of the GU_EDITION cookie. --- dotcom-rendering/playwright/lib/load-page.ts | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/dotcom-rendering/playwright/lib/load-page.ts b/dotcom-rendering/playwright/lib/load-page.ts index 14add2f5391..7b2cb7bc925 100644 --- a/dotcom-rendering/playwright/lib/load-page.ts +++ b/dotcom-rendering/playwright/lib/load-page.ts @@ -1,4 +1,4 @@ -import type { Page } from '@playwright/test'; +import type { Cookie, Page } from '@playwright/test'; import { ORIGIN } from '../../playwright.config'; import type { FEArticle } from '../../src/frontend/feArticle'; import type { FEFront } from '../../src/frontend/feFront'; @@ -34,6 +34,7 @@ const getFrontendUrl = (path: string) => { const getFrontendArticle = async ( url: string, + cookies: Cookie[], queryParams: LoadPageParams['queryParams'], ): Promise => { try { @@ -42,7 +43,8 @@ const getFrontendArticle = async ( ...queryParams, }).toString()}`; const frontendUrl = `${getFrontendUrl(url)}?${paramsString}`; - const response = await fetch(frontendUrl); + const cookie = cookies.map((c) => `${c.name}=${c.value}`).join('; '); + const response = await fetch(frontendUrl, { headers: { cookie } }); if (!response.ok) { throw new Error( `Failed to fetch article JSON from ${url}: ${response.statusText}`, @@ -115,10 +117,12 @@ const loadPage = async ({ }, ); + const cookies = await page.context().cookies(); + // If overrides exist, but no article fixture we fetch it from Frontend const frontendArticle = await (overrides.article ? Promise.resolve(overrides.article) - : getFrontendArticle(path, queryParams)); + : getFrontendArticle(path, cookies, queryParams)); // Apply the overrides to the article config and switches const postData = { From 3fac4108c1769a6b2a29a52e3cd6ae112b7de5f9 Mon Sep 17 00:00:00 2001 From: Ravi <7014230+arelra@users.noreply.github.com> Date: Tue, 19 Aug 2025 01:19:31 +0100 Subject: [PATCH 5/9] Replace loadPageWithOverrides and fetchAndloadPageWithOverrides with loadPage --- dotcom-rendering/playwright/lib/load-page.ts | 52 +--------------- .../playwright/tests/atom.video.e2e.spec.ts | 51 +++++++-------- .../playwright/tests/lightbox.e2e.spec.ts | 62 +++++++++++++++---- ...din.e2e.spec.ts => signed-out.e2e.spec.ts} | 10 ++- 4 files changed, 85 insertions(+), 90 deletions(-) rename dotcom-rendering/playwright/tests/{signedin.e2e.spec.ts => signed-out.e2e.spec.ts} (79%) diff --git a/dotcom-rendering/playwright/lib/load-page.ts b/dotcom-rendering/playwright/lib/load-page.ts index 7b2cb7bc925..266048494ac 100644 --- a/dotcom-rendering/playwright/lib/load-page.ts +++ b/dotcom-rendering/playwright/lib/load-page.ts @@ -160,54 +160,4 @@ const loadPage = async ({ await page.goto(`${dcrUrl}${fragment ?? ''}`, { waitUntil }); }; -/** - * Create a POST request to the /Article endpoint so we can override config - * and switches in the json sent to DCR - */ -const loadPageWithOverrides = async ( - page: Page, - article: FEArticle, - overrides?: { - configOverrides?: Record; - switchOverrides?: Record; - }, - options?: LoadPageOptions, -): Promise => { - await loadPage({ - page, - path: '/Article', - queryParamsOn: false, - ...options, - overrides: { ...overrides, article }, - }); -}; - -/** - * Fetch the page json from the provided URL then load it locally as a POST with overrides - */ -const fetchAndloadPageWithOverrides = async ( - page: Page, - url: string, - overrides?: { - configOverrides?: Record; - switchOverrides?: Record; - }, - options?: LoadPageOptions, -): Promise => { - const article = validateAsFEArticle( - await fetch(`${url}.json?dcr`).then((res) => res.json()), - ); - await loadPageWithOverrides( - page, - article, - { - configOverrides: overrides?.configOverrides, - switchOverrides: { - ...overrides?.switchOverrides, - }, - }, - options, - ); -}; - -export { fetchAndloadPageWithOverrides, loadPage, loadPageWithOverrides }; +export { loadPage }; diff --git a/dotcom-rendering/playwright/tests/atom.video.e2e.spec.ts b/dotcom-rendering/playwright/tests/atom.video.e2e.spec.ts index bd32e128d5d..80cb6ed9a2a 100644 --- a/dotcom-rendering/playwright/tests/atom.video.e2e.spec.ts +++ b/dotcom-rendering/playwright/tests/atom.video.e2e.spec.ts @@ -3,7 +3,7 @@ import type { Page } from '@playwright/test'; import { expect, test } from '@playwright/test'; import { allowRejectAll, cmpAcceptAll, cmpRejectAll } from '../lib/cmp'; import { waitForIsland } from '../lib/islands'; -import { fetchAndloadPageWithOverrides } from '../lib/load-page'; +import { loadPage } from '../lib/load-page'; import { expectToBeVisible, expectToNotExist } from '../lib/locators'; type YouTubeEmbedConfig = { @@ -122,11 +122,11 @@ const muteYouTube = async (page: Page, iframeSelector: string) => { test.describe.skip('YouTube Atom', () => { // Skipping because the video in this article has stopped working. Investigation needed! test.skip('plays main media video: skipped', async ({ page }) => { - await fetchAndloadPageWithOverrides( + await loadPage({ page, - 'https://www.theguardian.com/uk-news/2020/dec/04/edinburgh-hit-by-thundersnow-as-sonic-boom-wakes-residents', - { switchOverrides: { youtubeIma: false } }, - ); + path: '/Article/https://www.theguardian.com/uk-news/2020/dec/04/edinburgh-hit-by-thundersnow-as-sonic-boom-wakes-residents', + overrides: { switchOverrides: { youtubeIma: false } }, + }); await cmpAcceptAll(page); await waitForIsland(page, 'YoutubeBlockComponent'); @@ -173,11 +173,12 @@ test.describe.skip('YouTube Atom', () => { }); test.skip('plays main media video', async ({ page }) => { - await fetchAndloadPageWithOverrides( + await loadPage({ page, - 'https://www.theguardian.com/us-news/article/2024/may/30/trump-trial-hush-money-verdict', - { switchOverrides: { youtubeIma: false } }, - ); + path: '/Article/https://www.theguardian.com/us-news/article/2024/may/30/trump-trial-hush-money-verdict', + overrides: { switchOverrides: { youtubeIma: false } }, + }); + await cmpAcceptAll(page); await waitForIsland(page, 'YoutubeBlockComponent'); @@ -224,11 +225,11 @@ test.describe.skip('YouTube Atom', () => { }); test.skip('plays in body video', async ({ page }) => { - await fetchAndloadPageWithOverrides( + await loadPage({ page, - 'https://www.theguardian.com/environment/2021/oct/05/volcanoes-are-life-how-the-ocean-is-enriched-by-eruptions-devastating-on-land', - { switchOverrides: { youtubeIma: false } }, - ); + path: '/Article/https://www.theguardian.com/environment/2021/oct/05/volcanoes-are-life-how-the-ocean-is-enriched-by-eruptions-devastating-on-land', + overrides: { switchOverrides: { youtubeIma: false } }, + }); await cmpAcceptAll(page); await waitForIsland(page, 'YoutubeBlockComponent'); @@ -277,11 +278,11 @@ test.describe.skip('YouTube Atom', () => { test('each video plays when the same video exists both in body and in main media of a blog', async ({ page, }) => { - await fetchAndloadPageWithOverrides( + await loadPage({ page, - 'https://www.theguardian.com/world/live/2022/mar/28/russia-ukraine-war-latest-news-zelenskiy-putin-live-updates', - { switchOverrides: { youtubeIma: false } }, - ); + path: '/Article/https://www.theguardian.com/world/live/2022/mar/28/russia-ukraine-war-latest-news-zelenskiy-putin-live-updates', + overrides: { switchOverrides: { youtubeIma: false } }, + }); await cmpAcceptAll(page); // Wait for hydration of all videos @@ -387,11 +388,11 @@ test.describe.skip('YouTube Atom', () => { }) => { await allowRejectAll(context); - await fetchAndloadPageWithOverrides( + await loadPage({ page, - 'https://www.theguardian.com/environment/2021/oct/05/volcanoes-are-life-how-the-ocean-is-enriched-by-eruptions-devastating-on-land', - { switchOverrides: { youtubeIma: false } }, - ); + path: '/Article/https://www.theguardian.com/environment/2021/oct/05/volcanoes-are-life-how-the-ocean-is-enriched-by-eruptions-devastating-on-land', + overrides: { switchOverrides: { youtubeIma: false } }, + }); await cmpRejectAll(page); @@ -441,11 +442,11 @@ test.describe.skip('YouTube Atom', () => { test('video is sticky when the user plays a video then scrolls the video out of the viewport', async ({ page, }) => { - await fetchAndloadPageWithOverrides( + await loadPage({ page, - 'https://www.theguardian.com/world/live/2022/mar/28/russia-ukraine-war-latest-news-zelenskiy-putin-live-updates', - { switchOverrides: { youtubeIma: false } }, - ); + path: '/Article/https://www.theguardian.com/world/live/2022/mar/28/russia-ukraine-war-latest-news-zelenskiy-putin-live-updates', + overrides: { switchOverrides: { youtubeIma: false } }, + }); await cmpAcceptAll(page); // Wait for hydration of all videos diff --git a/dotcom-rendering/playwright/tests/lightbox.e2e.spec.ts b/dotcom-rendering/playwright/tests/lightbox.e2e.spec.ts index a5fcdc01923..d7bf896cfd2 100644 --- a/dotcom-rendering/playwright/tests/lightbox.e2e.spec.ts +++ b/dotcom-rendering/playwright/tests/lightbox.e2e.spec.ts @@ -3,7 +3,7 @@ import { expect, test } from '@playwright/test'; import { Live as LiveBlog } from '../../fixtures/generated/fe-articles/Live'; import { PhotoEssay as photoEssayArticle } from '../../fixtures/generated/fe-articles/PhotoEssay'; import { disableCMP } from '../lib/cmp'; -import { loadPageWithOverrides } from '../lib/load-page'; +import { loadPage } from '../lib/load-page'; import { expectToBeVisible, expectToNotBeVisible } from '../lib/locators'; // LIGHTBOX RL notes @@ -74,7 +74,11 @@ test.describe('Lightbox', () => { page, }) => { await disableCMP(context); - await loadPageWithOverrides(page, photoEssayArticle); + await loadPage({ + page, + path: '/Article', + overrides: { article: photoEssayArticle }, + }); await expectToNotBeVisible(page, '#gu-lightbox'); @@ -95,7 +99,11 @@ test.describe('Lightbox', () => { page, }) => { await disableCMP(context); - await loadPageWithOverrides(page, photoEssayArticle); + await loadPage({ + page, + path: '/Article', + overrides: { article: photoEssayArticle }, + }); await expectToNotBeVisible(page, '#gu-lightbox'); @@ -113,7 +121,11 @@ test.describe('Lightbox', () => { test('should trap focus', async ({ context, page }) => { await disableCMP(context); - await loadPageWithOverrides(page, photoEssayArticle); + await loadPage({ + page, + path: '/Article', + overrides: { article: photoEssayArticle }, + }); await page.locator('article img').first().click({ force: true }); await expectToBeVisible(page, '#gu-lightbox'); @@ -158,7 +170,11 @@ test.describe('Lightbox', () => { page, }) => { await disableCMP(context); - await loadPageWithOverrides(page, photoEssayArticle); + await loadPage({ + page, + path: '/Article', + overrides: { article: photoEssayArticle }, + }); await expectToNotBeVisible(page, '#gu-lightbox'); @@ -246,7 +262,11 @@ test.describe('Lightbox', () => { } await disableCMP(context); - await loadPageWithOverrides(page, photoEssayArticle); + await loadPage({ + page, + path: '/Article', + overrides: { article: photoEssayArticle }, + }); // eq(6) here means the 7th button is clicked (base zero) await page.locator('button.open-lightbox').nth(6).click(); @@ -285,7 +305,11 @@ test.describe('Lightbox', () => { page, }) => { await disableCMP(context); - await loadPageWithOverrides(page, photoEssayArticle); + await loadPage({ + page, + path: '/Article', + overrides: { article: photoEssayArticle }, + }); await page.locator('button.open-lightbox').nth(1).click(); await expectToBeVisible(page, '#gu-lightbox'); @@ -332,7 +356,11 @@ test.describe('Lightbox', () => { page, }) => { await disableCMP(context); - await loadPageWithOverrides(page, photoEssayArticle); + await loadPage({ + page, + path: '/Article', + overrides: { article: photoEssayArticle }, + }); await page.locator('button.open-lightbox').nth(1).click(); await expectToBeVisible(page, '#gu-lightbox'); @@ -374,7 +402,11 @@ test.describe('Lightbox', () => { page, }) => { await disableCMP(context); - await loadPageWithOverrides(page, LiveBlog); + await loadPage({ + page, + path: '/Article', + overrides: { article: LiveBlog }, + }); await page.locator('button.open-lightbox').nth(1).click(); await expectToBeVisible(page, '#gu-lightbox'); @@ -401,7 +433,11 @@ test.describe('Lightbox', () => { page, }) => { await disableCMP(context); - await loadPageWithOverrides(page, photoEssayArticle); + await loadPage({ + page, + path: '/Article', + overrides: { article: photoEssayArticle }, + }); await page.locator('button.open-lightbox').nth(1).click(); await expectToBeVisible(page, '#gu-lightbox'); @@ -431,7 +467,11 @@ test.describe('Lightbox', () => { page, }) => { await disableCMP(context); - await loadPageWithOverrides(page, photoEssayArticle); + await loadPage({ + page, + path: '/Article', + overrides: { article: photoEssayArticle }, + }); await expectToNotBeVisible(page, '#gu-lightbox'); // Open lightbox using the second button on the page (the first is main media) diff --git a/dotcom-rendering/playwright/tests/signedin.e2e.spec.ts b/dotcom-rendering/playwright/tests/signed-out.e2e.spec.ts similarity index 79% rename from dotcom-rendering/playwright/tests/signedin.e2e.spec.ts rename to dotcom-rendering/playwright/tests/signed-out.e2e.spec.ts index 11fb1b014ac..c1642f7a02f 100644 --- a/dotcom-rendering/playwright/tests/signedin.e2e.spec.ts +++ b/dotcom-rendering/playwright/tests/signed-out.e2e.spec.ts @@ -2,15 +2,19 @@ import { expect, test } from '@playwright/test'; import { Standard as standardArticle } from '../../fixtures/generated/fe-articles/Standard'; import { disableCMP } from '../lib/cmp'; import { waitForIsland } from '../lib/islands'; -import { loadPageWithOverrides } from '../lib/load-page'; +import { loadPage } from '../lib/load-page'; -test.describe('Signed in readers', () => { +test.describe('Signed out readers', () => { test('should not display signed in texts when users are not signed in', async ({ context, page, }) => { await disableCMP(context); - await loadPageWithOverrides(page, standardArticle); + await loadPage({ + page, + path: '/Article', + overrides: { article: standardArticle }, + }); await waitForIsland(page, 'DiscussionWeb'); From b68a5aa3d093506290168ffb776409214970aff2 Mon Sep 17 00:00:00 2001 From: Ravi <7014230+arelra@users.noreply.github.com> Date: Tue, 19 Aug 2025 17:23:53 +0100 Subject: [PATCH 6/9] Split up load-page functions and add docs --- dotcom-rendering/playwright/lib/load-page.ts | 91 +++++++++++++++----- 1 file changed, 71 insertions(+), 20 deletions(-) diff --git a/dotcom-rendering/playwright/lib/load-page.ts b/dotcom-rendering/playwright/lib/load-page.ts index 266048494ac..0527b0193fa 100644 --- a/dotcom-rendering/playwright/lib/load-page.ts +++ b/dotcom-rendering/playwright/lib/load-page.ts @@ -26,45 +26,83 @@ type LoadPageParams = { path: string; } & LoadPageOptions; -const getFrontendUrl = (path: string) => { +/** + * @param path The path for a DCR endpoint path + * e.g. `/Article/https://www.theguardian.com/world/2025/aug/19/the-big-church-move-sweden-kiruna-kyrka` + * @returns The Frontend URL to fetch the JSON payload + * e.g. `https://www.theguardian.com/world/2025/aug/19/the-big-church-move-sweden-kiruna-kyrka.json` + */ +const getFrontendJsonUrl = (path: string) => { const secondSlashIndex = path.indexOf('/', 1); const contentUrl = path.substring(secondSlashIndex + 1); return `${contentUrl}.json`; }; -const getFrontendArticle = async ( - url: string, +/** + * @param path The Frontend URL to fetch the JSON payload + * e.g. `https://www.theguardian.com/world/2025/aug/19/the-big-church-move-sweden-kiruna-kyrka` + * @param cookies Cookies to send with the request + * e.g. `GU_EDITION=US` + * @param queryParams Query parameters to append to the request + * e.g. `live=true` for live blogs + * @returns The JSON response from the Frontend URL + */ +const getFrontendJson = async ( + path: string, cookies: Cookie[], queryParams: LoadPageParams['queryParams'], -): Promise => { +): Promise => { try { const paramsString = `${new URLSearchParams({ dcr: 'true', ...queryParams, }).toString()}`; - const frontendUrl = `${getFrontendUrl(url)}?${paramsString}`; + const frontendUrl = `${getFrontendJsonUrl(path)}?${paramsString}`; const cookie = cookies.map((c) => `${c.name}=${c.value}`).join('; '); const response = await fetch(frontendUrl, { headers: { cookie } }); if (!response.ok) { throw new Error( - `Failed to fetch article JSON from ${url}: ${response.statusText}`, + `Failed to fetch from ${path}: ${response.statusText}`, ); } - if (url.startsWith('/Article')) { - return validateAsFEArticle(await response.json()); - } else if (url.startsWith('/Front')) { - return validateAsFEFront(await response.json()); - } - throw new Error(`Unsupported URL for fetching article: ${url}`); + return response.json(); } catch (error) { throw new Error( - `Error fetching or validating article JSON from ${url}: ${ + `Error fetching from ${path}: ${ error instanceof Error ? error.message : String(error) }`, ); } }; +/** + * Validates the JSON response from the Frontend URL based on the path. + + * Add more validation logic here if additional content types are required. + * + * @param path The path for a DCR endpoint, used to determine the content type. + * e.g. `/Article/https://www.theguardian.com/world/2025/aug/19/the-big-church-move-sweden-kiruna-kyrka` + * @param json The JSON response from the Frontend URL + * @returns The validated `FEArticle` or `FEFront` object + */ +const validateJson = (path: string, json: unknown): FEArticle | FEFront => { + if (path.startsWith('/Article')) { + return validateAsFEArticle(json); + } else if (path.startsWith('/Front')) { + return validateAsFEFront(json); + } + throw new Error(`Unsupported URL for validating article: ${path}`); +}; + +/** + * Constructs a DCR URL for a given path and query parameters. + * @param params The parameters for constructing the DCR URL + * @param params.path The path for a DCR endpoint + * @param params.queryParamsOn Whether to append query parameters to the URL + * @param params.queryParams Query parameters to append to the request + * @returns The DCR URL + * e.g. `http://localhost:9000/Article/https://theguardian.com/sport/live/2022/mar/27/west-indies-v-england-third-test-day-four-live?adtest=fixed-puppies-ci&live=true&force-liveblog-epic=true` + */ const getDcrUrl = ({ path, queryParamsOn, @@ -76,10 +114,17 @@ const getDcrUrl = ({ ...queryParams, }).toString()}` : ''; - return `${ORIGIN}${path}${paramsString}`; }; +/** + * Constructs a DCR POST URL for a given path. + * @param path The path for a DCR endpoint + * e.g. `/Article/https://www.theguardian.com/world/2025/aug/19/the-big-church-move-sweden-kiruna-kyrka` + * @returns The DCR POST URL to send the request to + * e.g. `http://localhost:9000/Article` + * This is used to override the request method to POST in Playwright tests. + */ const getDcrPostUrl = (path: string) => `${ORIGIN}/${path.split('/')[1]}`; /** @@ -120,18 +165,21 @@ const loadPage = async ({ const cookies = await page.context().cookies(); // If overrides exist, but no article fixture we fetch it from Frontend - const frontendArticle = await (overrides.article + const frontendPage = await (overrides.article ? Promise.resolve(overrides.article) - : getFrontendArticle(path, cookies, queryParams)); + : validateJson( + path, + await getFrontendJson(path, cookies, queryParams), + )); // Apply the overrides to the article config and switches const postData = { - ...frontendArticle, + ...frontendPage, config: { - ...frontendArticle.config, + ...frontendPage.config, ...overrides.configOverrides, switches: { - ...frontendArticle.config.switches, + ...frontendPage.config.switches, ...overrides.switchOverrides, }, }, @@ -143,7 +191,7 @@ const loadPage = async ({ queryParams, }); - // Override the request to the DCR URL to use a POST method + // Override any request matching dcrUrl to use a POST method // with the overridden payload await page.route(dcrUrl, async (route) => { await route.continue({ @@ -157,6 +205,9 @@ const loadPage = async ({ }); }); + // Initiate the page load + // Add the fragment here as Playwright has an issue when matching urls + // with fragments in the page.route handler await page.goto(`${dcrUrl}${fragment ?? ''}`, { waitUntil }); }; From 481a46759ba5ba8a37aa45c6c03f20fb2cddcc97 Mon Sep 17 00:00:00 2001 From: Ravi <7014230+arelra@users.noreply.github.com> Date: Tue, 19 Aug 2025 17:41:12 +0100 Subject: [PATCH 7/9] Minor tidy up - rename article to feFixture in load-page - undo playwright.config changes used for testing - set local workers to 2 to improve stability --- dotcom-rendering/playwright.config.ts | 24 +++++++++---------- dotcom-rendering/playwright/lib/load-page.ts | 18 +++++++------- .../playwright/tests/lightbox.e2e.spec.ts | 20 ++++++++-------- .../playwright/tests/signed-out.e2e.spec.ts | 2 +- 4 files changed, 31 insertions(+), 33 deletions(-) diff --git a/dotcom-rendering/playwright.config.ts b/dotcom-rendering/playwright.config.ts index 0f79fc1f3d7..53e35b4851b 100644 --- a/dotcom-rendering/playwright.config.ts +++ b/dotcom-rendering/playwright.config.ts @@ -3,12 +3,10 @@ import { defineConfig, devices } from '@playwright/test'; const isDev = process.env.NODE_ENV !== 'production'; /** - * Server port - * local development: 3030 - * CI: 9000 + * The server port for local development or CI */ export const PORT = isDev ? 3030 : 9000; -export const ORIGIN = `http://localhost:9000`; +export const ORIGIN = `http://localhost:${PORT}`; /** * See https://playwright.dev/docs/test-configuration. @@ -23,7 +21,7 @@ export default defineConfig({ // Retry on CI only retries: process.env.CI ? 3 : 1, // Workers run tests files in parallel - workers: process.env.CI ? 4 : undefined, + workers: process.env.CI ? 4 : 2, // Reporter to use. See https://playwright.dev/docs/test-reporters reporter: [['line'], ['html', { open: 'never' }]], // Shared settings for all the projects below. See https://playwright.dev/docs/api/class-testoptions @@ -42,12 +40,12 @@ export default defineConfig({ use: { ...devices['Desktop Chrome'] }, }, ], - // webServer: { - // // On CI the server is already started so a no-op - // command: ':', - // url: ORIGIN, - // reuseExistingServer: true, - // stdout: 'pipe', - // stderr: 'pipe', - // }, + webServer: { + // On CI the server is already started so a no-op + command: isDev ? 'make dev' : ':', + url: ORIGIN, + reuseExistingServer: true, + stdout: 'pipe', + stderr: 'pipe', + }, }); diff --git a/dotcom-rendering/playwright/lib/load-page.ts b/dotcom-rendering/playwright/lib/load-page.ts index 0527b0193fa..8fdd3767b50 100644 --- a/dotcom-rendering/playwright/lib/load-page.ts +++ b/dotcom-rendering/playwright/lib/load-page.ts @@ -17,7 +17,7 @@ type LoadPageOptions = { overrides?: { configOverrides?: Record; switchOverrides?: Record; - article?: FEArticle; + feFixture?: FEArticle | FEFront; }; }; @@ -91,7 +91,7 @@ const validateJson = (path: string, json: unknown): FEArticle | FEFront => { } else if (path.startsWith('/Front')) { return validateAsFEFront(json); } - throw new Error(`Unsupported URL for validating article: ${path}`); + throw new Error(`Unsupported URL for validating payload for: ${path}`); }; /** @@ -164,22 +164,22 @@ const loadPage = async ({ const cookies = await page.context().cookies(); - // If overrides exist, but no article fixture we fetch it from Frontend - const frontendPage = await (overrides.article - ? Promise.resolve(overrides.article) + // If overrides exist, but no fixture is provided we fetch it from Frontend + const frontendModel = await (overrides.feFixture + ? Promise.resolve(overrides.feFixture) : validateJson( path, await getFrontendJson(path, cookies, queryParams), )); - // Apply the overrides to the article config and switches + // Apply the config and switch overrides const postData = { - ...frontendPage, + ...frontendModel, config: { - ...frontendPage.config, + ...frontendModel.config, ...overrides.configOverrides, switches: { - ...frontendPage.config.switches, + ...frontendModel.config.switches, ...overrides.switchOverrides, }, }, diff --git a/dotcom-rendering/playwright/tests/lightbox.e2e.spec.ts b/dotcom-rendering/playwright/tests/lightbox.e2e.spec.ts index d7bf896cfd2..5b902439b82 100644 --- a/dotcom-rendering/playwright/tests/lightbox.e2e.spec.ts +++ b/dotcom-rendering/playwright/tests/lightbox.e2e.spec.ts @@ -77,7 +77,7 @@ test.describe('Lightbox', () => { await loadPage({ page, path: '/Article', - overrides: { article: photoEssayArticle }, + overrides: { feFixture: photoEssayArticle }, }); await expectToNotBeVisible(page, '#gu-lightbox'); @@ -102,7 +102,7 @@ test.describe('Lightbox', () => { await loadPage({ page, path: '/Article', - overrides: { article: photoEssayArticle }, + overrides: { feFixture: photoEssayArticle }, }); await expectToNotBeVisible(page, '#gu-lightbox'); @@ -124,7 +124,7 @@ test.describe('Lightbox', () => { await loadPage({ page, path: '/Article', - overrides: { article: photoEssayArticle }, + overrides: { feFixture: photoEssayArticle }, }); await page.locator('article img').first().click({ force: true }); @@ -173,7 +173,7 @@ test.describe('Lightbox', () => { await loadPage({ page, path: '/Article', - overrides: { article: photoEssayArticle }, + overrides: { feFixture: photoEssayArticle }, }); await expectToNotBeVisible(page, '#gu-lightbox'); @@ -265,7 +265,7 @@ test.describe('Lightbox', () => { await loadPage({ page, path: '/Article', - overrides: { article: photoEssayArticle }, + overrides: { feFixture: photoEssayArticle }, }); // eq(6) here means the 7th button is clicked (base zero) @@ -308,7 +308,7 @@ test.describe('Lightbox', () => { await loadPage({ page, path: '/Article', - overrides: { article: photoEssayArticle }, + overrides: { feFixture: photoEssayArticle }, }); await page.locator('button.open-lightbox').nth(1).click(); @@ -359,7 +359,7 @@ test.describe('Lightbox', () => { await loadPage({ page, path: '/Article', - overrides: { article: photoEssayArticle }, + overrides: { feFixture: photoEssayArticle }, }); await page.locator('button.open-lightbox').nth(1).click(); @@ -405,7 +405,7 @@ test.describe('Lightbox', () => { await loadPage({ page, path: '/Article', - overrides: { article: LiveBlog }, + overrides: { feFixture: LiveBlog }, }); await page.locator('button.open-lightbox').nth(1).click(); @@ -436,7 +436,7 @@ test.describe('Lightbox', () => { await loadPage({ page, path: '/Article', - overrides: { article: photoEssayArticle }, + overrides: { feFixture: photoEssayArticle }, }); await page.locator('button.open-lightbox').nth(1).click(); @@ -470,7 +470,7 @@ test.describe('Lightbox', () => { await loadPage({ page, path: '/Article', - overrides: { article: photoEssayArticle }, + overrides: { feFixture: photoEssayArticle }, }); await expectToNotBeVisible(page, '#gu-lightbox'); diff --git a/dotcom-rendering/playwright/tests/signed-out.e2e.spec.ts b/dotcom-rendering/playwright/tests/signed-out.e2e.spec.ts index c1642f7a02f..30b0b3748f6 100644 --- a/dotcom-rendering/playwright/tests/signed-out.e2e.spec.ts +++ b/dotcom-rendering/playwright/tests/signed-out.e2e.spec.ts @@ -13,7 +13,7 @@ test.describe('Signed out readers', () => { await loadPage({ page, path: '/Article', - overrides: { article: standardArticle }, + overrides: { feFixture: standardArticle }, }); await waitForIsland(page, 'DiscussionWeb'); From ff579baca7c58e8ff65d5162dfcf86fee358aa03 Mon Sep 17 00:00:00 2001 From: Ravi <7014230+arelra@users.noreply.github.com> Date: Tue, 19 Aug 2025 18:02:48 +0100 Subject: [PATCH 8/9] Playwright only starts the server in dev and not CI --- dotcom-rendering/playwright.config.ts | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/dotcom-rendering/playwright.config.ts b/dotcom-rendering/playwright.config.ts index 53e35b4851b..a7dcaea7d5d 100644 --- a/dotcom-rendering/playwright.config.ts +++ b/dotcom-rendering/playwright.config.ts @@ -40,12 +40,13 @@ export default defineConfig({ use: { ...devices['Desktop Chrome'] }, }, ], - webServer: { - // On CI the server is already started so a no-op - command: isDev ? 'make dev' : ':', - url: ORIGIN, - reuseExistingServer: true, - stdout: 'pipe', - stderr: 'pipe', - }, + webServer: isDev + ? { + command: 'make dev', + url: ORIGIN, + reuseExistingServer: true, + stdout: 'pipe', + stderr: 'pipe', + } + : undefined, }); From 52a02a198f87fbf13198db65d3789b29fdb1eda0 Mon Sep 17 00:00:00 2001 From: Ravi <7014230+arelra@users.noreply.github.com> Date: Wed, 20 Aug 2025 18:04:53 +0100 Subject: [PATCH 9/9] Fix jsdoc --- dotcom-rendering/playwright/lib/load-page.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/dotcom-rendering/playwright/lib/load-page.ts b/dotcom-rendering/playwright/lib/load-page.ts index 8fdd3767b50..e8c9620148f 100644 --- a/dotcom-rendering/playwright/lib/load-page.ts +++ b/dotcom-rendering/playwright/lib/load-page.ts @@ -39,8 +39,8 @@ const getFrontendJsonUrl = (path: string) => { }; /** - * @param path The Frontend URL to fetch the JSON payload - * e.g. `https://www.theguardian.com/world/2025/aug/19/the-big-church-move-sweden-kiruna-kyrka` + * @param path The path for a DCR endpoint path + * e.g. `/Article/https://www.theguardian.com/world/2025/aug/19/the-big-church-move-sweden-kiruna-kyrka` * @param cookies Cookies to send with the request * e.g. `GU_EDITION=US` * @param queryParams Query parameters to append to the request