chore: speed up CI a bit (hopefully) (#9731)

gtm-nayan · Rich Harris · eltigerchino · web-flow · commit f3d7a3a3de47 · 2024-07-14T08:35:27.000-07:00
Co-authored-by: Rich Harris &lt;git@rich-harris.dev&gt;
Co-authored-by: Tee Ming &lt;chewteeming01@gmail.com&gt;
Co-authored-by: Rich Harris &lt;rich.harris@vercel.com&gt;
Co-authored-by: Ben McCann &lt;322311+benmccann@users.noreply.github.com&gt;
diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml
@@ -59,6 +59,8 @@ jobs:
       - run: pnpm playwright install ${{ matrix.e2e-browser }}
       - run: pnpm run sync-all
       - run: pnpm test:kit
+      - name: Print flaky test report
+        run: node scripts/print-flaky-test-report.js
       - name: Archive test results
         if: failure()
         shell: bash
@@ -115,6 +117,8 @@ jobs:
       - run: pnpm playwright install ${{ matrix.e2e-browser }}
       - run: pnpm run sync-all
       - run: pnpm test:cross-platform:${{ matrix.mode }}
+      - name: Print flaky test report
+        run: node scripts/print-flaky-test-report.js
       - name: Archive test results
         if: failure()
         shell: bash
diff --git a/packages/kit/test/apps/basics/src/routes/load/cache-control/bust/+page.svelte b/packages/kit/test/apps/basics/src/routes/load/cache-control/bust/+page.svelte
@@ -11,5 +11,5 @@
 	}
 </script>
 
-<p>Count is {data.count}</p>
+<p class="counter">Count is {data.count}</p>
 <button on:click={update}>update</button>
diff --git a/packages/kit/test/apps/basics/src/routes/load/cache-control/bust/count/+server.js b/packages/kit/test/apps/basics/src/routes/load/cache-control/bust/count/+server.js
@@ -1,8 +1,10 @@
 import { json } from '@sveltejs/kit';
-import { count } from '../state.js';
 
-export function GET({ setHeaders }) {
+export function GET({ setHeaders, cookies }) {
 	setHeaders({ 'cache-control': 'public, max-age=4', age: '2' });
+
+	const count = +(cookies.get('cache-control-bust-count') ?? 0);
+
 	return json({ count });
 }
 
diff --git a/packages/kit/test/apps/basics/src/routes/load/cache-control/bust/increment/+server.js b/packages/kit/test/apps/basics/src/routes/load/cache-control/bust/increment/+server.js
@@ -1,7 +1,11 @@
 import { json } from '@sveltejs/kit';
-import { increment } from '../state.js';
 
-export function GET() {
-	increment();
+export function GET({ cookies }) {
+	cookies.set(
+		'cache-control-bust-count',
+		+(cookies.get('cache-control-bust-count') ?? 0) + 1 + '',
+		{ path: '/' }
+	);
+
 	return json({});
 }
diff --git a/packages/kit/test/apps/basics/src/routes/load/cache-control/bust/state.js b/packages/kit/test/apps/basics/src/routes/load/cache-control/bust/state.js
diff --git a/packages/kit/test/apps/basics/src/routes/load/cache-control/default/+page.svelte b/packages/kit/test/apps/basics/src/routes/load/cache-control/default/+page.svelte
@@ -4,11 +4,14 @@
 	/** @type {import('./$types').PageData} */
 	export let data;
 
+	let ticker = 0;
 	async function update() {
+		ticker += 1;
 		await fetch('/load/cache-control/default/increment');
-		invalidate('/load/cache-control/default/count');
+		await invalidate('/load/cache-control/default/count');
+		ticker += 1;
 	}
 </script>
 
-<p>Count is {data.count}</p>
-<button on:click={update}>update</button>
+<p class="counter">Count is {data.count}</p>
+<button on:click={update} data-ticker={ticker}>update</button>
diff --git a/packages/kit/test/apps/basics/src/routes/load/cache-control/default/count/+server.js b/packages/kit/test/apps/basics/src/routes/load/cache-control/default/count/+server.js
@@ -1,8 +1,10 @@
 import { json } from '@sveltejs/kit';
-import { count } from '../state.js';
 
-export function GET({ setHeaders }) {
+export function GET({ setHeaders, cookies }) {
 	setHeaders({ 'cache-control': 'public, max-age=4', age: '2' });
+
+	const count = +(cookies.get('cache-control-default-count') ?? 0);
+
 	return json({ count });
 }
 
diff --git a/packages/kit/test/apps/basics/src/routes/load/cache-control/default/increment/+server.js b/packages/kit/test/apps/basics/src/routes/load/cache-control/default/increment/+server.js
@@ -1,7 +1,11 @@
 import { json } from '@sveltejs/kit';
-import { increment } from '../state.js';
 
-export function GET() {
-	increment();
+export function GET({ cookies }) {
+	cookies.set(
+		'cache-control-default-count',
+		+(cookies.get('cache-control-default-count') ?? 0) + 1 + '',
+		{ path: '/' }
+	);
+
 	return json({});
 }
diff --git a/packages/kit/test/apps/basics/src/routes/load/cache-control/default/state.js b/packages/kit/test/apps/basics/src/routes/load/cache-control/default/state.js
diff --git a/packages/kit/test/apps/basics/src/routes/load/cache-control/force/+page.svelte b/packages/kit/test/apps/basics/src/routes/load/cache-control/force/+page.svelte
@@ -10,5 +10,5 @@
 	}
 </script>
 
-<p>Count is {data.count}</p>
+<p class="counter">Count is {data.count}</p>
 <button on:click={update}>update</button>
diff --git a/packages/kit/test/apps/basics/src/routes/load/cache-control/force/count/+server.js b/packages/kit/test/apps/basics/src/routes/load/cache-control/force/count/+server.js
@@ -1,8 +1,10 @@
 import { json } from '@sveltejs/kit';
-import { count } from '../state.js';
 
-export function GET({ setHeaders }) {
+export function GET({ setHeaders, cookies }) {
 	setHeaders({ 'cache-control': 'public, max-age=4', age: '2' });
+
+	const count = +(cookies.get('cache-control-force-count') ?? 0);
+
 	return json({ count });
 }
 
diff --git a/packages/kit/test/apps/basics/src/routes/load/cache-control/force/increment/+server.js b/packages/kit/test/apps/basics/src/routes/load/cache-control/force/increment/+server.js
@@ -1,7 +1,11 @@
 import { json } from '@sveltejs/kit';
-import { increment } from '../state.js';
 
-export function GET() {
-	increment();
+export function GET({ cookies }) {
+	cookies.set(
+		'cache-control-force-count',
+		+(cookies.get('cache-control-force-count') ?? 0) + 1 + '',
+		{ path: '/' }
+	);
+
 	return json({});
 }
diff --git a/packages/kit/test/apps/basics/src/routes/load/cache-control/force/state.js b/packages/kit/test/apps/basics/src/routes/load/cache-control/force/state.js
diff --git a/packages/kit/test/apps/basics/test/client.test.js b/packages/kit/test/apps/basics/test/client.test.js
@@ -145,35 +145,41 @@ test.describe('Load', () => {
 	});
 
 	test('load does not call fetch if max-age allows it', async ({ page }) => {
-		page.addInitScript(`
+		await page.addInitScript(`
 			window.now = 0;
 			window.performance.now = () => now;
 		`);
 
 		await page.goto('/load/cache-control/default');
-		await expect(page.getByText('Count is 0')).toBeVisible();
-		await page.locator('button').click();
-		await page.waitForLoadState('networkidle');
-		await expect(page.getByText('Count is 0')).toBeVisible();
 
-		await page.evaluate(() => (window.now = 2500));
+		const button = page.locator('button');
+		const p = page.locator('p.counter');
 
-		await page.locator('button').click();
-		await expect(page.getByText('Count is 2')).toBeVisible();
+		await button.click();
+		await expect(button).toHaveAttribute('data-ticker', '2');
+		await expect(p).toHaveText('Count is 0');
+
+		await page.evaluate('window.now = 2500');
+
+		await button.click();
+		await expect(button).toHaveAttribute('data-ticker', '4');
+		await expect(p).toHaveText('Count is 2');
 	});
 
 	test('load does ignore ttl if fetch cache options says so', async ({ page }) => {
 		await page.goto('/load/cache-control/force');
-		await expect(page.getByText('Count is 0')).toBeVisible();
+		const p = page.locator('p.counter');
+		await expect(p).toHaveText('Count is 0');
 		await page.locator('button').click();
-		await expect(page.getByText('Count is 1')).toBeVisible();
+		await expect(p).toHaveText('Count is 1');
 	});
 
 	test('load busts cache if non-GET request to resource is made', async ({ page }) => {
 		await page.goto('/load/cache-control/bust');
-		await expect(page.getByText('Count is 0')).toBeVisible();
+		const p = page.locator('p.counter');
+		await expect(p).toHaveText('Count is 0');
 		await page.locator('button').click();
-		await expect(page.getByText('Count is 1')).toBeVisible();
+		await expect(p).toHaveText('Count is 1');
 	});
 
 	test('__data.json has cache-control: private, no-store', async ({ page, clicknav }) => {
diff --git a/packages/kit/test/apps/basics/test/cross-platform/client.test.js b/packages/kit/test/apps/basics/test/cross-platform/client.test.js
@@ -76,6 +76,7 @@ test.describe('a11y', () => {
 		).toBe(1);
 
 		await clicknav('[href="/selection/b"]');
+
 		expect(
 			await page.evaluate(() => {
 				const selection = getSelection();
@@ -98,16 +99,21 @@ test.describe('a11y', () => {
 	});
 
 	test('autofocus from previous page is ignored', async ({ page, clicknav }) => {
-		page.addInitScript(`
+		await page.addInitScript(`
 			window.active = null;
 			window.addEventListener('focusin', () => window.active = document.activeElement);
 		`);
 
 		await page.goto('/accessibility/autofocus/a');
 		await clicknav('[href="/"]');
 
-		expect(await page.evaluate(() => (window.active || {}).nodeName)).toBe('BODY');
-		expect(await page.evaluate(() => (document.activeElement || {}).nodeName)).toBe('BODY');
+		expect(
+			await page.evaluate(
+				// @ts-expect-error
+				() => window.active?.nodeName
+			)
+		).toBe('BODY');
+		expect(await page.evaluate(() => document.activeElement?.nodeName)).toBe('BODY');
 	});
 });
 
@@ -319,20 +325,18 @@ test.describe('Scrolling', () => {
 		expect(await page.evaluate(() => scrollY)).toBe(0);
 	});
 
-	test('scroll is restored after hitting the back button', async ({ baseURL, clicknav, page }) => {
+	test('scroll is restored after hitting the back button', async ({ clicknav, page }) => {
 		await page.goto('/anchor');
 		await page.locator('#scroll-anchor').click();
 		const originalScrollY = /** @type {number} */ (await page.evaluate(() => scrollY));
 		await clicknav('#routing-page');
 		await page.goBack();
-		await page.waitForLoadState('networkidle');
-		expect(page.url()).toBe(baseURL + '/anchor#last-anchor-2');
+
+		await expect(page).toHaveURL('/anchor#last-anchor-2');
 		expect(await page.evaluate(() => scrollY)).toEqual(originalScrollY);
 
 		await page.goBack();
-		await page.waitForLoadState('networkidle');
-
-		expect(page.url()).toBe(baseURL + '/anchor');
+		await expect(page).toHaveURL('/anchor');
 		expect(await page.evaluate(() => scrollY)).toEqual(0);
 	});
 
@@ -587,18 +591,15 @@ test.describe('Prefetching', () => {
 			expect(requests.filter((req) => req.endsWith('.js')).length).toBeGreaterThan(0);
 		}
 
-		expect(requests.includes(`${baseURL}/routing/preloading/preloaded.json`)).toBe(true);
+		expect(requests).toContain(`${baseURL}/routing/preloading/preloaded.json`);
 
 		requests = [];
 		await app.goto('/routing/preloading/preloaded');
 		expect(requests).toEqual([]);
 
-		try {
-			await app.preloadData('https://example.com');
-			throw new Error('Error was not thrown');
-		} catch (/** @type {any} */ e) {
-			expect(e.message).toMatch('Attempted to preload a URL that does not belong to this app');
-		}
+		await expect(app.preloadData('https://example.com')).rejects.toThrowError(
+			'Attempted to preload a URL that does not belong to this app'
+		);
 	});
 
 	test('chooses correct route when hash route is preloaded but regular route is clicked', async ({
@@ -693,8 +694,9 @@ test.describe('Routing', () => {
 		await page.locator('[href="#hash-target"]').click();
 		await clicknav('[href="/routing/hashes/b"]');
 
+		await expect(page.locator('h1')).toHaveText('b');
 		await page.goBack();
-		expect(await page.textContent('h1')).toBe('a');
+		await expect(page.locator('h1')).toHaveText('a');
 	});
 
 	test('replaces state if the data-sveltekit-replacestate router option is specified for the hash link', async ({
@@ -746,6 +748,7 @@ test.describe('Routing', () => {
 
 	test('responds to <form method="GET"> submission without reload', async ({ page }) => {
 		await page.goto('/routing/form-get');
+
 		expect(await page.textContent('h1')).toBe('...');
 		expect(await page.textContent('h2')).toBe('enter');
 		expect(await page.textContent('h3')).toBe('...');
diff --git a/packages/kit/test/apps/basics/test/cross-platform/test.js b/packages/kit/test/apps/basics/test/cross-platform/test.js
@@ -747,20 +747,15 @@ test.describe('Routing', () => {
 		await clicknav('[href="/routing/a"]');
 
 		await page.goBack();
-		await page.waitForLoadState('networkidle');
 		expect(await page.textContent('h1')).toBe('Great success!');
 	});
 
 	test('focus works if page load has hash', async ({ page, browserName }) => {
 		await page.goto('/routing/hashes/target#p2');
 
 		await page.keyboard.press(browserName === 'webkit' ? 'Alt+Tab' : 'Tab');
-		await page.waitForTimeout(50); // give browser a bit of time to complete the native behavior of the key press
-		expect(
-			await page.evaluate(
-				() => document.activeElement?.textContent || 'ERROR: document.activeElement not set'
-			)
-		).toBe('next focus element');
+
+		await page.waitForSelector('button:focus');
 	});
 
 	test('focus works when navigating to a hash on the same page', async ({ page, browserName }) => {
diff --git a/packages/kit/test/apps/basics/test/test.js b/packages/kit/test/apps/basics/test/test.js
@@ -548,7 +548,7 @@ test.describe('Load', () => {
 	}) => {
 		if (javaScriptEnabled) {
 			await page.goto('/prerendering/prerendered-endpoint');
-			await page.click('a', { noWaitAfter: true });
+			await page.click('a');
 		} else {
 			await page.goto('/prerendering/prerendered-endpoint/from-handle-hook');
 		}
diff --git a/packages/kit/test/apps/embed/test/test.js b/packages/kit/test/apps/embed/test/test.js
@@ -10,8 +10,8 @@ test.describe('embed', () => {
 		await page.goto('/embed');
 
 		if (javaScriptEnabled) {
-			expect(await page.textContent('[data-testid="a"]')).toBe('a (browser)');
-			expect(await page.textContent('[data-testid="b"]')).toBe('b (browser)');
+			await expect(page.getByTestId('a')).toHaveText('a (browser)');
+			await expect(page.getByTestId('b')).toHaveText('b (browser)');
 		} else {
 			expect(await page.textContent('[data-testid="a"]')).toBe('a (server)');
 			expect(await page.textContent('[data-testid="b"]')).toBe('b (server)');
diff --git a/packages/kit/test/github-flaky-warning-reporter.js b/packages/kit/test/github-flaky-warning-reporter.js
@@ -1,3 +1,5 @@
+import { appendFileSync } from 'node:fs';
+
 /**
  * @class
  * @implements {import('@playwright/test/reporter').Reporter}
@@ -24,12 +26,17 @@ export default class GithubFlakyWarningReporter {
 	}
 
 	onEnd() {
-		this._flaky.forEach(({ file, line, title, message }) => {
-			console.log(`::warning file=${file},line=${line},title=${title}::${message}`);
-		});
+		const output = this._flaky
+			.map(
+				({ file, line, title, message }) =>
+					`::warning file=${file},line=${line},title=${title}::${message}\n`
+			)
+			.join('');
+
+		appendFileSync(new URL('../../../_tmp_flaky_test_output.txt', import.meta.url), output);
 	}
 
 	printsToStdio() {
-		return true;
+		return false;
 	}
 }
diff --git a/packages/kit/test/utils.d.ts b/packages/kit/test/utils.d.ts
diff --git a/packages/kit/test/utils.js b/packages/kit/test/utils.js
diff --git a/packages/package/.gitignore b/packages/package/.gitignore
diff --git a/scripts/print-flaky-test-report.js b/scripts/print-flaky-test-report.js

Original file line number	Diff line number	Diff line change
`@@ -11,5 +11,5 @@`
`11`	`11`	`}`
`12`	`12`	`</script>`
`13`	`13`
`14`		`-<p>Count is {data.count}</p>`
	`14`	`+<p class="counter">Count is {data.count}</p>`
`15`	`15`	`<button on:click={update}>update</button>`
Original file line number	Diff line number	Diff line change
`@@ -10,5 +10,5 @@`
`10`	`10`	`}`
`11`	`11`	`</script>`
`12`	`12`
`13`		`-<p>Count is {data.count}</p>`
	`13`	`+<p class="counter">Count is {data.count}</p>`
`14`	`14`	`<button on:click={update}>update</button>`