From c23b0f6a6f5c3b0a99ea6f7365276dbe52e71062 Mon Sep 17 00:00:00 2001 From: Barry Pollard Date: Thu, 1 Aug 2024 15:46:16 +0100 Subject: [PATCH 01/28] Remove firefox workarounds for test flakiness --- test/e2e/onFCP-test.js | 18 ------------------ test/e2e/onLCP-test.js | 32 +------------------------------- 2 files changed, 1 insertion(+), 49 deletions(-) diff --git a/test/e2e/onFCP-test.js b/test/e2e/onFCP-test.js index ce0de79c..f936bf3a 100644 --- a/test/e2e/onFCP-test.js +++ b/test/e2e/onFCP-test.js @@ -21,24 +21,6 @@ import {navigateTo} from '../utils/navigateTo.js'; import {stubForwardBack} from '../utils/stubForwardBack.js'; import {stubVisibilityChange} from '../utils/stubVisibilityChange.js'; -// Temp fix to address Firefox flakiness. -// See https://github.com/GoogleChrome/web-vitals/issues/472 -const originalStrictEqual = assert.strictEqual; -assert.strictEqual = function (actual, expected, message) { - if ( - process.env.GITHUB_ACTIONS && - browser.capabilities.browserName === 'firefox' && - (expected === 'good' || expected === 'needs-improvement') && - actual !== expected - ) { - console.error( - `Override assert for Firefox (actual: ${actual}, expected: ${expected})`, - ); - return true; - } - return originalStrictEqual(actual, expected, message); -}; - describe('onFCP()', async function () { // Retry all tests in this suite up to 2 times. this.retries(2); diff --git a/test/e2e/onLCP-test.js b/test/e2e/onLCP-test.js index 4b02bc7e..a6b233cc 100644 --- a/test/e2e/onLCP-test.js +++ b/test/e2e/onLCP-test.js @@ -22,24 +22,6 @@ import {navigateTo} from '../utils/navigateTo.js'; import {stubForwardBack} from '../utils/stubForwardBack.js'; import {stubVisibilityChange} from '../utils/stubVisibilityChange.js'; -// Temp fix to address Firefox flakiness. -// See https://github.com/GoogleChrome/web-vitals/issues/472 -const originalStrictEqual = assert.strictEqual; -assert.strictEqual = function (actual, expected, message) { - if ( - process.env.GITHUB_ACTIONS && - browser.capabilities.browserName === 'firefox' && - (expected === 'good' || expected === 'needs-improvement') && - actual !== expected - ) { - console.error( - `Override assert for Firefox (actual: ${actual}, expected: ${expected})`, - ); - return true; - } - return originalStrictEqual(actual, expected, message); -}; - describe('onLCP()', async function () { // Retry all tests in this suite up to 2 times. this.retries(2); @@ -716,19 +698,7 @@ const assertStandardReportsAreCorrect = (beacons) => { const assertFullReportsAreCorrect = (beacons) => { const [lcp1, lcp2] = beacons; - // Temp fix to address Firefox flakiness. - // See https://github.com/GoogleChrome/web-vitals/issues/472 - if ( - process.env.GITHUB_ACTIONS && - browser.capabilities.browserName === 'firefox' && - lcp1.value >= 500 - ) { - console.log( - `Override assert for Firefox (actual: ${lcp1.value}, expected: < 500)`, - ); - } else { - assert(lcp1.value < 500); // Less than the image load delay. - } + assert(lcp1.value < 500); // Less than the image load delay. assert(lcp1.id.match(/^v4-\d+-\d+$/)); assert.strictEqual(lcp1.name, 'LCP'); assert.strictEqual(lcp1.value, lcp1.delta); From c57c1a67b4048c0bd93e4abe8624c366b2a4e7b6 Mon Sep 17 00:00:00 2001 From: Barry Pollard Date: Thu, 1 Aug 2024 16:43:39 +0100 Subject: [PATCH 02/28] Add extra wait to hidden test --- test/e2e/onLCP-test.js | 3 +++ 1 file changed, 3 insertions(+) diff --git a/test/e2e/onLCP-test.js b/test/e2e/onLCP-test.js index a6b233cc..3319e876 100644 --- a/test/e2e/onLCP-test.js +++ b/test/e2e/onLCP-test.js @@ -264,6 +264,9 @@ describe('onLCP()', async function () { // Wait for a frame to be painted. await browser.executeAsync((done) => requestAnimationFrame(done)); + // Wait a bit to ensure beacon gets a chance to be sent + await browser.pause(1000); + await stubVisibilityChange('hidden'); await stubVisibilityChange('visible'); From 360db97578fbe117b6c568fca010e8ba19804499 Mon Sep 17 00:00:00 2001 From: Barry Pollard Date: Thu, 1 Aug 2024 16:52:26 +0100 Subject: [PATCH 03/28] Skip prerender tests if unsupported --- test/e2e/onFCP-test.js | 3 +++ 1 file changed, 3 insertions(+) diff --git a/test/e2e/onFCP-test.js b/test/e2e/onFCP-test.js index f936bf3a..85af533d 100644 --- a/test/e2e/onFCP-test.js +++ b/test/e2e/onFCP-test.js @@ -17,6 +17,7 @@ import assert from 'assert'; import {beaconCountIs, clearBeacons, getBeacons} from '../utils/beacons.js'; import {browserSupportsEntry} from '../utils/browserSupportsEntry.js'; +import {browserSupportsPrerender} from '../utiles/browserSupportsPrerender.js'; import {navigateTo} from '../utils/navigateTo.js'; import {stubForwardBack} from '../utils/stubForwardBack.js'; import {stubVisibilityChange} from '../utils/stubVisibilityChange.js'; @@ -71,6 +72,7 @@ describe('onFCP()', async function () { it('accounts for time prerendering the page', async function () { if (!browserSupportsFCP) this.skip(); + if (!browserSupportsPrerender) this.skip(); await navigateTo('/test/fcp?prerender=1'); @@ -317,6 +319,7 @@ describe('onFCP()', async function () { it('accounts for time prerendering the page', async function () { if (!browserSupportsFCP) this.skip(); + if (!browserSupportsPrerender) this.skip(); await navigateTo(`/test/fcp?attribution=1&prerender=1`, { readyState: 'complete', From 8b7fa8b2e0a2e9ec6bd4af273bff32ca5ec3027f Mon Sep 17 00:00:00 2001 From: Barry Pollard Date: Thu, 1 Aug 2024 16:55:57 +0100 Subject: [PATCH 04/28] Oops, forgot to include the new file --- test/utils/browserSupportsPrerender.js | 32 ++++++++++++++++++++++++++ 1 file changed, 32 insertions(+) create mode 100644 test/utils/browserSupportsPrerender.js diff --git a/test/utils/browserSupportsPrerender.js b/test/utils/browserSupportsPrerender.js new file mode 100644 index 00000000..6c949cf0 --- /dev/null +++ b/test/utils/browserSupportsPrerender.js @@ -0,0 +1,32 @@ +/* + * Copyright 2024 Google LLC + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +/** + * Returns true if the browser supports using Prerendering + * @return {boolean} + */ +export function browserSupportsPrerender() { + return browser.execute(() => { + if ( + self.HTMLScriptElement.supports && + self.HTMLScriptElement.supports('speculationrules') + ) { + return true; + } + + return false; + }); +} From df3ab5eaea90d770e3242b48b1e63a02ca0fe10a Mon Sep 17 00:00:00 2001 From: Barry Pollard Date: Thu, 1 Aug 2024 17:01:36 +0100 Subject: [PATCH 05/28] Urgh I should just test this first --- test/e2e/onFCP-test.js | 2 +- wdio.conf.cjs | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/test/e2e/onFCP-test.js b/test/e2e/onFCP-test.js index 85af533d..d9309ccd 100644 --- a/test/e2e/onFCP-test.js +++ b/test/e2e/onFCP-test.js @@ -17,7 +17,7 @@ import assert from 'assert'; import {beaconCountIs, clearBeacons, getBeacons} from '../utils/beacons.js'; import {browserSupportsEntry} from '../utils/browserSupportsEntry.js'; -import {browserSupportsPrerender} from '../utiles/browserSupportsPrerender.js'; +import {browserSupportsPrerender} from '../utils/browserSupportsPrerender.js'; import {navigateTo} from '../utils/navigateTo.js'; import {stubForwardBack} from '../utils/stubForwardBack.js'; import {stubVisibilityChange} from '../utils/stubVisibilityChange.js'; diff --git a/wdio.conf.cjs b/wdio.conf.cjs index 7a0b1d79..8f67ee1c 100644 --- a/wdio.conf.cjs +++ b/wdio.conf.cjs @@ -82,6 +82,7 @@ module.exports.config = { if (browserName === 'chrome') { capability['goog:chromeOptions'] = { excludeSwitches: ['enable-automation'], + args: ['disable-search-engine-choice-screen'], // Uncomment to test on Chrome Canary. // binary: '/Applications/Google Chrome Canary.app/Contents/MacOS/Google Chrome Canary' }; From 6e15ba85ff70fda5459b3b03f40ee0e3d1ba968c Mon Sep 17 00:00:00 2001 From: Barry Pollard Date: Thu, 1 Aug 2024 17:17:24 +0100 Subject: [PATCH 06/28] Refactor --- test/e2e/onFCP-test.js | 7 ++++++- ...Prerender.js => browserSupportsActivationStart.js} | 11 ++--------- 2 files changed, 8 insertions(+), 10 deletions(-) rename test/utils/{browserSupportsPrerender.js => browserSupportsActivationStart.js} (77%) diff --git a/test/e2e/onFCP-test.js b/test/e2e/onFCP-test.js index d9309ccd..0e6f4328 100644 --- a/test/e2e/onFCP-test.js +++ b/test/e2e/onFCP-test.js @@ -17,7 +17,7 @@ import assert from 'assert'; import {beaconCountIs, clearBeacons, getBeacons} from '../utils/beacons.js'; import {browserSupportsEntry} from '../utils/browserSupportsEntry.js'; -import {browserSupportsPrerender} from '../utils/browserSupportsPrerender.js'; +import {browserSupportsActivationStart} from '../utils/browserSupportsActivationStart.js'; import {navigateTo} from '../utils/navigateTo.js'; import {stubForwardBack} from '../utils/stubForwardBack.js'; import {stubVisibilityChange} from '../utils/stubVisibilityChange.js'; @@ -31,6 +31,11 @@ describe('onFCP()', async function () { browserSupportsFCP = await browserSupportsEntry('paint'); }); + let browserSupportsPrerender; + before(async function () { + browserSupportsPrerender = await browserSupportsActivationStart(); + }); + beforeEach(async function () { await navigateTo('about:blank'); await clearBeacons(); diff --git a/test/utils/browserSupportsPrerender.js b/test/utils/browserSupportsActivationStart.js similarity index 77% rename from test/utils/browserSupportsPrerender.js rename to test/utils/browserSupportsActivationStart.js index 6c949cf0..e627d24b 100644 --- a/test/utils/browserSupportsPrerender.js +++ b/test/utils/browserSupportsActivationStart.js @@ -18,15 +18,8 @@ * Returns true if the browser supports using Prerendering * @return {boolean} */ -export function browserSupportsPrerender() { +export function browserSupportsActivationStart() { return browser.execute(() => { - if ( - self.HTMLScriptElement.supports && - self.HTMLScriptElement.supports('speculationrules') - ) { - return true; - } - - return false; + return 'activationStart' in self.PerformanceNavigationTiming.prototype; }); } From 4a8173581ee8bc1fc19a156872d27bb8368cfa78 Mon Sep 17 00:00:00 2001 From: Barry Pollard Date: Thu, 1 Aug 2024 17:46:22 +0100 Subject: [PATCH 07/28] Skip prerender on all metrics if not supported --- test/e2e/onCLS-test.js | 4 ++++ test/e2e/onFCP-test.js | 7 ++----- test/e2e/onFID-test.js | 4 ++++ test/e2e/onINP-test.js | 4 ++++ test/e2e/onLCP-test.js | 5 +++++ test/e2e/onTTFB-test.js | 8 ++++++++ 6 files changed, 27 insertions(+), 5 deletions(-) diff --git a/test/e2e/onCLS-test.js b/test/e2e/onCLS-test.js index bf74bedb..0f453655 100644 --- a/test/e2e/onCLS-test.js +++ b/test/e2e/onCLS-test.js @@ -16,6 +16,7 @@ import assert from 'assert'; import {beaconCountIs, clearBeacons, getBeacons} from '../utils/beacons.js'; +import {browserSupportsActivationStart} from '../utils/browserSupportsActivationStart.js'; import {browserSupportsEntry} from '../utils/browserSupportsEntry.js'; import {firstContentfulPaint} from '../utils/firstContentfulPaint.js'; import {imagesPainted} from '../utils/imagesPainted.js'; @@ -29,8 +30,10 @@ describe('onCLS()', async function () { this.retries(2); let browserSupportsCLS; + let browserSupportsPrerender; before(async function () { browserSupportsCLS = await browserSupportsEntry('layout-shift'); + browserSupportsPrerender = await browserSupportsActivationStart(); // Set a standard screen size so thresholds are the same browser.setWindowSize(1280, 1024); @@ -686,6 +689,7 @@ describe('onCLS()', async function () { it('reports prerender as nav type for prerender', async function () { if (!browserSupportsCLS) this.skip(); + if (!browserSupportsPrerender) this.skip(); await navigateTo('/test/cls?prerender=1'); diff --git a/test/e2e/onFCP-test.js b/test/e2e/onFCP-test.js index 0e6f4328..eea5f926 100644 --- a/test/e2e/onFCP-test.js +++ b/test/e2e/onFCP-test.js @@ -16,8 +16,8 @@ import assert from 'assert'; import {beaconCountIs, clearBeacons, getBeacons} from '../utils/beacons.js'; -import {browserSupportsEntry} from '../utils/browserSupportsEntry.js'; import {browserSupportsActivationStart} from '../utils/browserSupportsActivationStart.js'; +import {browserSupportsEntry} from '../utils/browserSupportsEntry.js'; import {navigateTo} from '../utils/navigateTo.js'; import {stubForwardBack} from '../utils/stubForwardBack.js'; import {stubVisibilityChange} from '../utils/stubVisibilityChange.js'; @@ -27,12 +27,9 @@ describe('onFCP()', async function () { this.retries(2); let browserSupportsFCP; - before(async function () { - browserSupportsFCP = await browserSupportsEntry('paint'); - }); - let browserSupportsPrerender; before(async function () { + browserSupportsFCP = await browserSupportsEntry('paint'); browserSupportsPrerender = await browserSupportsActivationStart(); }); diff --git a/test/e2e/onFID-test.js b/test/e2e/onFID-test.js index bb11e813..1c326d41 100644 --- a/test/e2e/onFID-test.js +++ b/test/e2e/onFID-test.js @@ -16,6 +16,7 @@ import assert from 'assert'; import {beaconCountIs, clearBeacons, getBeacons} from '../utils/beacons.js'; +import {browserSupportsActivationStart} from '../utils/browserSupportsActivationStart.js'; import {browserSupportsEntry} from '../utils/browserSupportsEntry.js'; import {navigateTo} from '../utils/navigateTo.js'; import {stubForwardBack} from '../utils/stubForwardBack.js'; @@ -26,8 +27,10 @@ describe('onFID()', async function () { this.retries(2); let browserSupportsFID; + let browserSupportsPrerender; before(async function () { browserSupportsFID = await browserSupportsEntry('first-input'); + browserSupportsPrerender = await browserSupportsActivationStart(); }); beforeEach(async function () { @@ -188,6 +191,7 @@ describe('onFID()', async function () { it('reports prerender as nav type for prerender', async function () { if (!browserSupportsFID) this.skip(); + if (!browserSupportsPrerender) this.skip(); await navigateTo('/test/fid?prerender=1'); diff --git a/test/e2e/onINP-test.js b/test/e2e/onINP-test.js index 3bdc691b..fec54659 100644 --- a/test/e2e/onINP-test.js +++ b/test/e2e/onINP-test.js @@ -16,6 +16,7 @@ import assert from 'assert'; import {beaconCountIs, clearBeacons, getBeacons} from '../utils/beacons.js'; +import {browserSupportsActivationStart} from '../utils/browserSupportsActivationStart.js'; import {browserSupportsEntry} from '../utils/browserSupportsEntry.js'; import {navigateTo} from '../utils/navigateTo.js'; import {nextFrame} from '../utils/nextFrame.js'; @@ -30,9 +31,11 @@ describe('onINP()', async function () { let browserSupportsINP; let browserSupportsLoAF; + let browserSupportsPrerender; before(async function () { browserSupportsINP = await browserSupportsEntry('event'); browserSupportsLoAF = await browserSupportsEntry('long-animation-frame'); + browserSupportsPrerender = await browserSupportsActivationStart(); }); beforeEach(async function () { @@ -392,6 +395,7 @@ describe('onINP()', async function () { it('reports prerender as nav type for prerender', async function () { if (!browserSupportsINP) this.skip(); + if (!browserSupportsPrerender) this.skip(); await navigateTo('/test/inp?click=100&prerender=1', { readyState: 'interactive', diff --git a/test/e2e/onLCP-test.js b/test/e2e/onLCP-test.js index 3319e876..d39264f0 100644 --- a/test/e2e/onLCP-test.js +++ b/test/e2e/onLCP-test.js @@ -16,6 +16,7 @@ import assert from 'assert'; import {beaconCountIs, clearBeacons, getBeacons} from '../utils/beacons.js'; +import {browserSupportsActivationStart} from '../utils/browserSupportsActivationStart.js'; import {browserSupportsEntry} from '../utils/browserSupportsEntry.js'; import {imagesPainted} from '../utils/imagesPainted.js'; import {navigateTo} from '../utils/navigateTo.js'; @@ -27,8 +28,10 @@ describe('onLCP()', async function () { this.retries(2); let browserSupportsLCP; + let browserSupportsPrerender; before(async function () { browserSupportsLCP = await browserSupportsEntry('largest-contentful-paint'); + browserSupportsPrerender = await browserSupportsActivationStart(); }); beforeEach(async function () { @@ -143,6 +146,7 @@ describe('onLCP()', async function () { it('accounts for time prerendering the page', async function () { if (!browserSupportsLCP) this.skip(); + if (!browserSupportsPrerender) this.skip(); await navigateTo('/test/lcp?prerender=1'); @@ -539,6 +543,7 @@ describe('onLCP()', async function () { it('accounts for time prerendering the page', async function () { if (!browserSupportsLCP) this.skip(); + if (!browserSupportsPrerender) this.skip(); await navigateTo('/test/lcp?attribution=1&prerender=1'); diff --git a/test/e2e/onTTFB-test.js b/test/e2e/onTTFB-test.js index 01833a78..12a07222 100644 --- a/test/e2e/onTTFB-test.js +++ b/test/e2e/onTTFB-test.js @@ -16,6 +16,7 @@ import assert from 'assert'; import {beaconCountIs, clearBeacons, getBeacons} from '../utils/beacons.js'; +import {browserSupportsActivationStart} from '../utils/browserSupportsActivationStart.js'; import {navigateTo} from '../utils/navigateTo.js'; import {stubForwardBack} from '../utils/stubForwardBack.js'; @@ -58,6 +59,11 @@ describe('onTTFB()', async function () { // Retry all tests in this suite up to 2 times. this.retries(2); + let browserSupportsPrerender; + before(async function () { + browserSupportsPrerender = await browserSupportsActivationStart(); + }); + beforeEach(async function () { // In Safari when navigating to 'about:blank' between tests the // Navigation Timing data is consistently negative, so the tests fail. @@ -122,6 +128,7 @@ describe('onTTFB()', async function () { }); it('accounts for time prerendering the page', async function () { + if (!browserSupportsPrerender) this.skip(); await navigateTo('/test/ttfb?prerender=1'); const ttfb = await getTTFBBeacon(); @@ -286,6 +293,7 @@ describe('onTTFB()', async function () { }); it('accounts for time prerendering the page', async function () { + if (!browserSupportsPrerender) this.skip(); await navigateTo('/test/ttfb?attribution=1&prerender=1'); const ttfb = await getTTFBBeacon(); From 71916de0e7b6ff584e2b285bf73443127802a70a Mon Sep 17 00:00:00 2001 From: Barry Pollard Date: Thu, 1 Aug 2024 18:09:28 +0100 Subject: [PATCH 08/28] Alternative LCP hidden flakiness fix --- test/e2e/onLCP-test.js | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/test/e2e/onLCP-test.js b/test/e2e/onLCP-test.js index d39264f0..a6ffe412 100644 --- a/test/e2e/onLCP-test.js +++ b/test/e2e/onLCP-test.js @@ -263,14 +263,13 @@ describe('onLCP()', async function () { it('stops reporting after the document changes to hidden (reportAllChanges === false)', async function () { if (!browserSupportsLCP) this.skip(); - await navigateTo('/test/lcp?imgDelay=0&imgHidden=1'); + await navigateTo('/test/lcp?imgDelay=0&imgHidden=1', { + readyState: 'interactive', + }); // Wait for a frame to be painted. await browser.executeAsync((done) => requestAnimationFrame(done)); - // Wait a bit to ensure beacon gets a chance to be sent - await browser.pause(1000); - await stubVisibilityChange('hidden'); await stubVisibilityChange('visible'); From ee0eb2f8e4c385959a0670c068ec3aab917bd869 Mon Sep 17 00:00:00 2001 From: Barry Pollard Date: Thu, 1 Aug 2024 18:43:39 +0100 Subject: [PATCH 09/28] Another anti-flaky test --- test/e2e/onFCP-test.js | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/test/e2e/onFCP-test.js b/test/e2e/onFCP-test.js index eea5f926..0a33a068 100644 --- a/test/e2e/onFCP-test.js +++ b/test/e2e/onFCP-test.js @@ -122,6 +122,10 @@ describe('onFCP()', async function () { await navigateTo('/test/fcp?hidden=1', {readyState: 'interactive'}); + // Wait a bit to ensure an initial "frame" happens to avoid Safari + // flakiness with this test if switching visibility as soon as interactive. + await browser.pause(100); + await stubVisibilityChange('visible'); // Wait a bit to ensure no beacons were sent. From 1741c0466a29ad423b55d91f55094f7e5b238f8d Mon Sep 17 00:00:00 2001 From: Barry Pollard Date: Thu, 1 Aug 2024 19:23:42 +0100 Subject: [PATCH 10/28] Check --- test/e2e/onFCP-test.js | 6 +++++- test/views/fcp.njk | 1 + 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/test/e2e/onFCP-test.js b/test/e2e/onFCP-test.js index 0a33a068..3bdeb6b3 100644 --- a/test/e2e/onFCP-test.js +++ b/test/e2e/onFCP-test.js @@ -121,10 +121,14 @@ describe('onFCP()', async function () { if (!browserSupportsFCP) this.skip(); await navigateTo('/test/fcp?hidden=1', {readyState: 'interactive'}); + const WVloaded = await browser.execute(() => { + return window.WVloaded; + }); + assert(WVloaded, true); // Wait a bit to ensure an initial "frame" happens to avoid Safari // flakiness with this test if switching visibility as soon as interactive. - await browser.pause(100); + // await browser.pause(1000); await stubVisibilityChange('visible'); diff --git a/test/views/fcp.njk b/test/views/fcp.njk index 3e1be0b8..e9fed661 100644 --- a/test/views/fcp.njk +++ b/test/views/fcp.njk @@ -29,6 +29,7 @@