Tweaks to benchmarking

.github/workflows/test-ad-load-time.yml

@@ -6,13 +6,13 @@ on:
   workflow_dispatch:
   pull_request:
 
+permissions: write-all
+
 jobs:
-  test:
+  benchmark:
     name: Test time to load top-above-nav
     timeout-minutes: 30
     runs-on: ubuntu-latest
-    strategy:
-      fail-fast: false
     steps:
       # Commercial
       - name: Checkout
@@ -21,38 +21,56 @@ jobs:
       - name: Set up Node
         uses: ./.github/actions/setup-node-env
 
-      # We always run our commercial code against the latest main of DCR
-      # This does make our tests sensitive to changes in DCR
-      # (e.g. imagine someone removes the top-above-nav slot from DCR)
-      # This is something we accept in order to easily test our own code
-      #
-      # Note we use the containerised version of DCR, published from:
-      # https://github.com/guardian/dotcom-rendering/blob/6a6df272/.github/workflows/container.yml
-      #
-      # The argument `--network host` is crucial here, as it means the container shares the networking stack of the host
-      # This makes the commercial dev server available from inside the container
-      # Note that GHA provides a service container feature, but it does not support this argument
-      - name: Start DCR in a container
-        run: |
-          /usr/bin/docker run -d \
-            --network host \
-            -p 3030:3030 \
-            -e "PORT=3030" \
-            -e "COMMERCIAL_BUNDLE_URL=http://localhost:3031/graun.standalone.commercial.js" \
-            ghcr.io/guardian/dotcom-rendering:main
-
       - name: Install Playwright Browsers
         run: pnpm playwright install --with-deps chromium
 
-      - name: Start Commercial server
-        run: pnpm serve & npx wait-on -v -i 1000 http://localhost:3031/graun.standalone.commercial.js
+      - name: Build Prod (used for rewriting by playwright)
+        run: pnpm build:prod
+        env:
+          BUNDLE_PREFIX: ''
 
       - name: Run Playwright
-        run: pnpm playwright test test-ad-load-time.spec.ts
+        run: pnpm benchmark
 
-      - uses: actions/upload-artifact@v4
-        if: always()
+      - uses: actions/github-script@v7
         with:
-          name: ad-load-time-report
-          path: ./commercial/playwright-report
-          retention-days: 5
+          github-token: ${{secrets.GITHUB_TOKEN}}
+          script: |
+            const { readFileSync, resolve } = require('fs');
+            const consented = readFileSync('./benchmark-results/consented/average.txt', 'utf8');
+            const consentless = readFileSync('./benchmark-results/consentless/average.txt', 'utf8');
+
+            const body = `### Ad load time test results
+
+            For \`consented\`, \`top-above-nav\` took on average ${consented}ms to load.
+            For \`consentless\`, \`top-above-nav\` took on average ${consentless}ms to load.
+
+            Test conditions:
+            - 5mpbs download speed
+            - 1.5mpbs upload speed
+            - 150ms latency`;
+
+            const comments = await github.rest.issues.listComments({
+              issue_number: context.issue.number,
+              owner: context.repo.owner,
+              repo: context.repo.repo,
+            });
+
+            const existingComment = comments.data.find(comment => comment.body.includes('Ad load time test results'));
+
+            if (existingComment) {
+              await github.rest.issues.updateComment({
+                comment_id: existingComment.id,
+                owner: context.repo.owner,
+                repo: context.repo.repo,
+                body
+              });
+              return;
+            } else {
+              const { data } = await github.rest.issues.createComment({
+                issue_number: context.issue.number,
+                owner: context.repo.owner,
+                repo: context.repo.repo,
+                body
+              });
+            }

.gitignore

@@ -26,5 +26,7 @@ wip
 
 # Playwright
 /test-results/
+/benchmark-results/
 /playwright-report/
 /playwright/.cache/
+/playwright/.auth/

package.json

@@ -33,6 +33,7 @@
 		"prettier:check": "prettier . --check --cache",
 		"prettier:fix": "prettier . --write --cache",
 		"serve": "OVERRIDE_BUNDLE=true pnpm webpack-dev-server -c ./webpack.config.dev.js",
+		"benchmark": "STAGE=prod playwright test -c playwright.benchmark.config.ts --project=consented-average --project=consentless-average",
 		"test": "jest",
 		"tsc": "tsc --noEmit",
 		"validate": "npm-run-all tsc lint test build"

playwright.benchmark.config.ts

@@ -0,0 +1,73 @@
+import { defineConfig, devices } from '@playwright/test';
+
+/**
+ * See https://playwright.dev/docs/test-configuration.
+ */
+// eslint-disable-next-line import/no-default-export -- default expected
+export default defineConfig({
+	testDir: './benchmark/tests',
+	// Don't run tests _within_ files in parallel as this causes flakiness locally - investigating
+	// Test files still run in parallel as per the number of workers set below
+	fullyParallel: true,
+	// Fail the build on CI if you accidentally left test.only in the source code.
+	forbidOnly: !!process.env.CI,
+	// Retry
+	retries: 0,
+	// Workers run tests files in parallel
+	workers: process.env.CI ? 4 : undefined,
+	// Reporter to use. See https://playwright.dev/docs/test-reporters
+	// Configure projects for major browsers
+	projects: [
+		{
+			name: 'accept-all-cmp',
+			use: { ...devices['Desktop Chrome'] },
+			testMatch: 'cmp-accept-all.setup.spec.ts',
+			testDir: 'playwright/benchmark',
+		},
+		{
+			name: 'reject-all-cmp',
+			use: { ...devices['Desktop Chrome'] },
+			testMatch: 'cmp-reject-all.setup.spec.ts',
+			testDir: 'playwright/benchmark',
+		},
+		{
+			name: 'consented',
+			use: {
+				...devices['Desktop Chrome'],
+				storageState: 'playwright/.auth/accept-all.json',
+			},
+			testMatch: 'test-ad-load-time.spec.ts',
+			testDir: 'playwright/benchmark',
+			dependencies: ['accept-all-cmp'],
+		},
+		{
+			name: 'consentless',
+			use: {
+				...devices['Desktop Chrome'],
+				storageState: 'playwright/.auth/reject-all.json',
+			},
+			testMatch: 'test-ad-load-time.spec.ts',
+			testDir: './playwright/benchmark',
+
+			dependencies: ['reject-all-cmp'],
+		},
+		{
+			name: 'consented-average',
+			use: {
+				...devices['Desktop Chrome'],
+			},
+			testMatch: 'average.spec.ts',
+			testDir: 'playwright/benchmark',
+			dependencies: ['consented'],
+		},
+		{
+			name: 'consentless-average',
+			use: {
+				...devices['Desktop Chrome'],
+			},
+			testMatch: 'average.spec.ts',
+			testDir: 'playwright/benchmark',
+			dependencies: ['consentless'],
+		},
+	],
+});

playwright/benchmark/average.spec.ts

@@ -0,0 +1,30 @@
+import { readdirSync, readFileSync, writeFileSync } from 'fs';
+import { resolve } from 'path';
+import { test } from '@playwright/test';
+
+// eslint-disable-next-line @typescript-eslint/no-unused-vars -- have to use first arg but don't need it
+test('average', ({ page }, testInfo) => {
+	const name = testInfo.project.name.split('-')[0] as string; // consented-average -> consented
+
+	const path = resolve(__dirname, `../../benchmark-results/${name}`);
+	const files = readdirSync(path).map((file) => {
+		const content = readFileSync(resolve(path, file), 'utf-8');
+
+		const lines = content.split('\n');
+
+		return lines;
+	});
+
+	// flatten the array
+	const lines = files.flat().filter((line) => line !== '');
+
+	// sum of all the lines in all the files
+	const testSum = lines.reduce((acc, line) => acc + parseInt(line), 0);
+
+	// average of all the lines in all the files
+	const average = testSum / lines.length;
+
+	writeFileSync(resolve(path, `average.txt`), String(average));
+
+	console.log(`${name} ad rendering time: ${average}ms`);
+});

playwright/benchmark/cmp-accept-all.setup.spec.ts

@@ -0,0 +1,21 @@
+import { test } from '@playwright/test';
+import { articles } from '../fixtures/pages';
+import { type GuPage } from '../fixtures/pages/Page';
+import { cmpAcceptAll } from '../lib/cmp';
+import { loadPage } from '../lib/load-page';
+
+const viewport = { width: 1400, height: 800 };
+
+const authFile = 'playwright/.auth/accept-all.json';
+
+test('setup', async ({ page }) => {
+	const loadPath = articles[0]?.path as GuPage['path'];
+
+	await page.setViewportSize(viewport);
+
+	await loadPage(page, loadPath);
+
+	await cmpAcceptAll(page);
+
+	await page.context().storageState({ path: authFile });
+});

playwright/benchmark/cmp-reject-all.setup.spec.ts

@@ -0,0 +1,21 @@
+import { test } from '@playwright/test';
+import { articles } from '../fixtures/pages';
+import { type GuPage } from '../fixtures/pages/Page';
+import { cmpAcceptAll } from '../lib/cmp';
+import { loadPage } from '../lib/load-page';
+
+const viewport = { width: 1400, height: 800 };
+
+const authFile = 'playwright/.auth/reject-all.json';
+
+test('setup', async ({ page }) => {
+	const loadPath = articles[0]?.path as GuPage['path'];
+
+	await page.setViewportSize(viewport);
+
+	await loadPage(page, loadPath);
+
+	await cmpAcceptAll(page);
+
+	await page.context().storageState({ path: authFile });
+});

playwright/benchmark/test-ad-load-time.spec.ts

@@ -0,0 +1,87 @@
+import { existsSync, mkdirSync } from 'fs';
+import { appendFile, readFile } from 'fs/promises';
+import { resolve } from 'path';
+import type { Page } from '@playwright/test';
+import { test } from '@playwright/test';
+import { loadTimePages } from '../fixtures/pages/load-time-pages';
+import { loadPage } from '../lib/load-page';
+import { waitForSlot } from '../lib/util';
+
+const networkConditions = {
+	offline: false,
+	downloadThroughput: 5000 * (1024 / 8),
+	uploadThroughput: 1500 * (1024 / 8),
+	latency: 150,
+};
+
+const viewport = { width: 1400, height: 800 };
+
+const interceptCommercial = (page: Page) =>
+	page.route('**/commercial/**/*.js', async (route) => {
+		const path = route.request().url().split('/').pop();
+		if (!path) {
+			return await route.fulfill({
+				status: 404,
+				body: 'Not found',
+			});
+		}
+		const body = await readFile(
+			resolve(`dist/bundle/prod/${path}`),
+			'utf-8',
+		);
+		await route.fulfill({
+			status: 200,
+			body,
+			headers: {
+				'Cache-Control': 'no-cache, no-store, must-revalidate',
+			},
+		});
+	});
+
+test.describe.configure({ mode: 'parallel' });
+
+test.describe('Test how long top-above-nav takes to load', () => {
+	for (const article of loadTimePages) {
+		test(`${article.path}`, async ({ page }, testInfo) => {
+			await interceptCommercial(page);
+
+			const client = await page.context().newCDPSession(page);
+
+			await client.send(
+				'Network.emulateNetworkConditions',
+				networkConditions,
+			);
+
+			await page.setViewportSize(viewport);
+
+			await loadPage(page, article.path);
+
+			const startRenderingTime = Date.now();
+
+			await waitForSlot(page, 'top-above-nav');
+
+			const renderingTime = Date.now() - startRenderingTime;
+
+			console.log(`Ad rendered in ${renderingTime} ms`);
+
+			const file = resolve(
+				__dirname,
+				`../../benchmark-results/${testInfo.project.name}/ad-rendering-time-${testInfo.workerIndex}.txt`,
+			);
+
+			if (!existsSync(file)) {
+				mkdirSync(
+					resolve(
+						__dirname,
+						`../../benchmark-results/${testInfo.project.name}`,
+					),
+					{
+						recursive: true,
+					},
+				);
+			}
+
+			await appendFile(file, String(renderingTime) + '\n');
+		});
+	}
+});