From bf327412b741a0b1255a6eff9316e4eb37df0f2d Mon Sep 17 00:00:00 2001 From: Philip Walton Date: Thu, 28 Dec 2023 15:07:42 -0800 Subject: [PATCH] Fix flaky tests after upgrading dependencies --- test/e2e/onCLS-test.js | 66 ++++++++++++++++++------------ test/e2e/onFCP-test.js | 23 +++++------ test/e2e/onFID-test.js | 8 ++-- test/e2e/onINP-test.js | 1 + test/e2e/onLCP-test.js | 21 ++++------ test/e2e/onTTFB-test.js | 11 +++-- test/utils/firstContentfulPaint.js | 35 ++++++++++++++++ test/utils/navigateWithStrategy.js | 40 ++++++++++++++++++ wdio.conf.cjs | 5 +-- 9 files changed, 148 insertions(+), 62 deletions(-) create mode 100644 test/utils/firstContentfulPaint.js create mode 100644 test/utils/navigateWithStrategy.js diff --git a/test/e2e/onCLS-test.js b/test/e2e/onCLS-test.js index 8a0dd903..07c5e700 100644 --- a/test/e2e/onCLS-test.js +++ b/test/e2e/onCLS-test.js @@ -17,8 +17,9 @@ import assert from 'assert'; import {beaconCountIs, clearBeacons, getBeacons} from '../utils/beacons.js'; import {browserSupportsEntry} from '../utils/browserSupportsEntry.js'; -import {domReadyState} from '../utils/domReadyState.js'; +import {firstContentfulPaint} from '../utils/firstContentfulPaint.js'; import {imagesPainted} from '../utils/imagesPainted.js'; +import {navigateWithStrategy} from '../utils/navigateWithStrategy.js'; import {nextFrame} from '../utils/nextFrame.js'; import {stubForwardBack} from '../utils/stubForwardBack.js'; import {stubVisibilityChange} from '../utils/stubVisibilityChange.js'; @@ -33,6 +34,7 @@ describe('onCLS()', async function () { }); beforeEach(async function () { + await browser.url('about:blank'); await clearBeacons(); }); @@ -81,9 +83,7 @@ describe('onCLS()', async function () { it('reports the correct value even if loaded late (reportAllChanges === false)', async function () { if (!browserSupportsCLS) this.skip(); - await browser.url(`/test/cls?lazyLoad=1`); - - await domReadyState('complete'); + await navigateWithStrategy(`/test/cls?lazyLoad=1`, 'complete'); // Wait until all images are loaded and rendered, then change to hidden. await imagesPainted(); @@ -104,9 +104,10 @@ describe('onCLS()', async function () { it('reports the correct value even if loaded late (reportAllChanges === true)', async function () { if (!browserSupportsCLS) this.skip(); - await browser.url(`/test/cls?lazyLoad=1&reportAllChanges=1`); - - await domReadyState('complete'); + await navigateWithStrategy( + `/test/cls?lazyLoad=1&reportAllChanges=1`, + 'complete', + ); // Wait until all images are loaded and rendered, then change to hidden. await imagesPainted(); @@ -560,14 +561,12 @@ describe('onCLS()', async function () { it('reports zero if no layout shifts occurred on first visibility hidden (reportAllChanges === false)', async function () { if (!browserSupportsCLS) this.skip(); - await browser.url(`/test/cls?noLayoutShifts=1`); + await navigateWithStrategy(`/test/cls?noLayoutShifts=1`, 'complete'); - // Wait until the page is loaded before hiding. - await domReadyState('complete'); + // Wait until the page is loaded and content is visible before hiding. + await firstContentfulPaint(); await stubVisibilityChange('hidden'); - await beaconCountIs(1); - const [cls] = await getBeacons(); assert(cls.id.match(/^v3-\d+-\d+$/)); assert.strictEqual(cls.name, 'CLS'); @@ -581,10 +580,13 @@ describe('onCLS()', async function () { it('reports zero if no layout shifts occurred on first visibility hidden (reportAllChanges === true)', async function () { if (!browserSupportsCLS) this.skip(); - await browser.url(`/test/cls?reportAllChanges=1&noLayoutShifts=1`); + await navigateWithStrategy( + `/test/cls?reportAllChanges=1&noLayoutShifts=1`, + 'complete', + ); - // Wait until the page is loaded before hiding. - await domReadyState('complete'); + // Wait until the page is loaded and content is visible before hiding. + await firstContentfulPaint(); await stubVisibilityChange('hidden'); await beaconCountIs(1); @@ -602,10 +604,9 @@ describe('onCLS()', async function () { it('reports zero if no layout shifts occurred on page unload (reportAllChanges === false)', async function () { if (!browserSupportsCLS) this.skip(); - await browser.url(`/test/cls?noLayoutShifts=1`); - // Wait until the page is loaded before navigating away. - await domReadyState('complete'); + await navigateWithStrategy(`/test/cls?noLayoutShifts=1`, 'complete'); + await browser.url('about:blank'); await beaconCountIs(1); @@ -623,10 +624,14 @@ describe('onCLS()', async function () { it('reports zero if no layout shifts occurred on page unload (reportAllChanges === true)', async function () { if (!browserSupportsCLS) this.skip(); - await browser.url(`/test/cls?noLayoutShifts=1&reportAllChanges=1`); - // Wait until the page is loaded before navigating away. - await domReadyState('complete'); + await navigateWithStrategy( + `/test/cls?noLayoutShifts=1&reportAllChanges=1`, + 'complete', + ); + + // Wait until the page is loaded and content is visible before leaving. + await firstContentfulPaint(); await browser.url('about:blank'); await beaconCountIs(1); @@ -766,9 +771,15 @@ describe('onCLS()', async function () { it('reports whether the largest shift was before or after load', async function () { if (!browserSupportsCLS) this.skip(); - await browser.url('/test/cls?attribution=1&noLayoutShifts=1'); + await navigateWithStrategy( + '/test/cls?attribution=1&noLayoutShifts=1', + 'complete', + ); + + // Wait until the page is loaded and content is visible before triggering + // a layout shift. + await firstContentfulPaint(); - await domReadyState('complete'); await triggerLayoutShift(); await stubVisibilityChange('hidden'); @@ -804,10 +815,13 @@ describe('onCLS()', async function () { it('reports an empty object when no shifts', async function () { if (!browserSupportsCLS) this.skip(); - await browser.url('/test/cls?attribution=1&noLayoutShifts=1'); + await navigateWithStrategy( + '/test/cls?attribution=1&noLayoutShifts=1', + 'complete', + ); - // Wait until the page is loaded before navigating away. - await domReadyState('complete'); + // Wait until the page is loaded and content is visible hiding. + await firstContentfulPaint(); await stubVisibilityChange('hidden'); await beaconCountIs(1); diff --git a/test/e2e/onFCP-test.js b/test/e2e/onFCP-test.js index 1e7454d4..b4460e47 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 {domReadyState} from '../utils/domReadyState.js'; +import {navigateWithStrategy} from '../utils/navigateWithStrategy.js'; import {stubForwardBack} from '../utils/stubForwardBack.js'; import {stubVisibilityChange} from '../utils/stubVisibilityChange.js'; @@ -31,6 +31,7 @@ describe('onFCP()', async function () { }); beforeEach(async function () { + await browser.url('about:blank'); await clearBeacons(); }); @@ -115,8 +116,7 @@ describe('onFCP()', async function () { it('does not report if the document was hidden at page load time', async function () { if (!browserSupportsFCP) this.skip(); - await browser.url('/test/fcp?hidden=1'); - await domReadyState('interactive'); + await navigateWithStrategy('/test/fcp?hidden=1', 'interactive'); await stubVisibilityChange('visible'); @@ -130,7 +130,7 @@ describe('onFCP()', async function () { it('does not report if the document changes to hidden before the first entry', async function () { if (!browserSupportsFCP) this.skip(); - await browser.url('/test/fcp?invisible=1'); + await navigateWithStrategy('/test/fcp?invisible=1', 'interactive'); await stubVisibilityChange('hidden'); await stubVisibilityChange('visible'); @@ -211,8 +211,7 @@ describe('onFCP()', async function () { it('reports if the page is restored from bfcache even when the document was hidden at page load time', async function () { if (!browserSupportsFCP) this.skip(); - await browser.url('/test/fcp?hidden=1'); - await domReadyState('interactive'); + await navigateWithStrategy('/test/fcp?hidden=1', 'interactive'); await stubVisibilityChange('visible'); @@ -272,11 +271,10 @@ describe('onFCP()', async function () { it('includes attribution data on the metric object', async function () { if (!browserSupportsFCP) this.skip(); - await browser.url('/test/fcp?attribution=1'); + await navigateWithStrategy('/test/fcp?attribution=1', 'complete'); await beaconCountIs(1); - await domReadyState('complete'); const navEntry = await browser.execute(() => { return performance.getEntriesByType('navigation')[0].toJSON(); }); @@ -320,11 +318,13 @@ describe('onFCP()', async function () { it('accounts for time prerendering the page', async function () { if (!browserSupportsFCP) this.skip(); - await browser.url('/test/fcp?attribution=1&prerender=1'); + await navigateWithStrategy( + '/test/fcp?attribution=1&prerender=1', + 'complete', + ); await beaconCountIs(1); - await domReadyState('complete'); const navEntry = await browser.execute(() => { return performance.getEntriesByType('navigation')[0].toJSON(); }); @@ -371,13 +371,12 @@ describe('onFCP()', async function () { it('reports after a bfcache restore', async function () { if (!browserSupportsFCP) this.skip(); - await browser.url('/test/fcp?attribution=1'); + await navigateWithStrategy('/test/fcp?attribution=1', 'complete'); await beaconCountIs(1); await clearBeacons(); - await domReadyState('complete'); await stubForwardBack(); await beaconCountIs(1); diff --git a/test/e2e/onFID-test.js b/test/e2e/onFID-test.js index eedba0e4..d4a00475 100644 --- a/test/e2e/onFID-test.js +++ b/test/e2e/onFID-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 {domReadyState} from '../utils/domReadyState.js'; +import {navigateWithStrategy} from '../utils/navigateWithStrategy.js'; import {stubForwardBack} from '../utils/stubForwardBack.js'; import {stubVisibilityChange} from '../utils/stubVisibilityChange.js'; @@ -31,6 +31,7 @@ describe('onFID()', async function () { }); beforeEach(async function () { + await browser.url('about:blank'); await clearBeacons(); }); @@ -138,8 +139,7 @@ describe('onFID()', async function () { // https://bugs.webkit.org/show_bug.cgi?id=211101 if (browser.capabilities.browserName === 'Safari') this.skip(); - await browser.url('/test/fid?hidden=1'); - await domReadyState('interactive'); + await navigateWithStrategy('/test/fid?hidden=1', 'complete'); await stubVisibilityChange('visible'); @@ -159,6 +159,8 @@ describe('onFID()', async function () { // https://bugs.webkit.org/show_bug.cgi?id=211101 if (browser.capabilities.browserName === 'Safari') this.skip(); + await navigateWithStrategy('/test/fid', 'complete'); + await stubVisibilityChange('hidden'); // Returning to visible will also render the . diff --git a/test/e2e/onINP-test.js b/test/e2e/onINP-test.js index b419a34f..cdb4dbcb 100644 --- a/test/e2e/onINP-test.js +++ b/test/e2e/onINP-test.js @@ -33,6 +33,7 @@ describe('onINP()', async function () { }); beforeEach(async function () { + await browser.url('about:blank'); await clearBeacons(); }); diff --git a/test/e2e/onLCP-test.js b/test/e2e/onLCP-test.js index f6afa112..31de1391 100644 --- a/test/e2e/onLCP-test.js +++ b/test/e2e/onLCP-test.js @@ -17,8 +17,8 @@ import assert from 'assert'; import {beaconCountIs, clearBeacons, getBeacons} from '../utils/beacons.js'; import {browserSupportsEntry} from '../utils/browserSupportsEntry.js'; -import {domReadyState} from '../utils/domReadyState.js'; import {imagesPainted} from '../utils/imagesPainted.js'; +import {navigateWithStrategy} from '../utils/navigateWithStrategy.js'; import {stubForwardBack} from '../utils/stubForwardBack.js'; import {stubVisibilityChange} from '../utils/stubVisibilityChange.js'; @@ -32,11 +32,8 @@ describe('onLCP()', async function () { }); beforeEach(async function () { - await clearBeacons(); - - // TODO(philipwalton): not sure why this is needed, but it may be related - // to: https://bugs.chromium.org/p/chromium/issues/detail?id=1034080 await browser.url('about:blank'); + await clearBeacons(); }); it('reports the correct value on hidden (reportAllChanges === false)', async function () { @@ -195,8 +192,7 @@ describe('onLCP()', async function () { it('does not report if the document was hidden at page load time', async function () { if (!browserSupportsLCP) this.skip(); - await browser.url('/test/lcp?hidden=1'); - await domReadyState('interactive'); + await navigateWithStrategy('/test/lcp?hidden=1', 'interactive'); await stubVisibilityChange('visible'); @@ -362,8 +358,7 @@ describe('onLCP()', async function () { it('reports if the page is restored from bfcache even when the document was hidden at page load time', async function () { if (!browserSupportsLCP) this.skip(); - await browser.url('/test/lcp?hidden=1'); - await domReadyState('interactive'); + await navigateWithStrategy('/test/lcp?hidden=1', 'interactive'); await stubVisibilityChange('visible'); @@ -602,10 +597,10 @@ describe('onLCP()', async function () { it('handles cases where there is no LCP resource', async function () { if (!browserSupportsLCP) this.skip(); - await browser.url('/test/lcp?attribution=1&imgHidden=1'); - - // Wait until all images are loaded and fully rendered. - await domReadyState('complete'); + await navigateWithStrategy( + '/test/lcp?attribution=1&imgHidden=1', + 'complete', + ); const navEntry = await browser.execute(() => { return performance.getEntriesByType('navigation')[0].toJSON(); diff --git a/test/e2e/onTTFB-test.js b/test/e2e/onTTFB-test.js index b949278a..9515bdeb 100644 --- a/test/e2e/onTTFB-test.js +++ b/test/e2e/onTTFB-test.js @@ -16,7 +16,7 @@ import assert from 'assert'; import {beaconCountIs, clearBeacons, getBeacons} from '../utils/beacons.js'; -import {domReadyState} from '../utils/domReadyState.js'; +import {navigateWithStrategy} from '../utils/navigateWithStrategy.js'; import {stubForwardBack} from '../utils/stubForwardBack.js'; /** @@ -59,6 +59,11 @@ describe('onTTFB()', async function () { this.retries(2); beforeEach(async function () { + // In Safari when navigating to 'about:blank' between tests the + // Navigation Timing data is consistently negative, so the tests fail. + if (browser.capabilities.browserName !== 'Safari') { + await browser.url('about:blank'); + } await clearBeacons(); }); @@ -196,9 +201,7 @@ describe('onTTFB()', async function () { it('ignores navigations with invalid responseStart timestamps', async function () { for (const rs of [-1, 0, 1e12]) { - await browser.url(`/test/ttfb?responseStart=${rs}`); - - await domReadyState('complete'); + await navigateWithStrategy(`/test/ttfb?responseStart=${rs}`, 'complete'); // Wait a bit to ensure no beacons were sent. await browser.pause(1000); diff --git a/test/utils/firstContentfulPaint.js b/test/utils/firstContentfulPaint.js new file mode 100644 index 00000000..86c8309a --- /dev/null +++ b/test/utils/firstContentfulPaint.js @@ -0,0 +1,35 @@ +/* + * Copyright 2023 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 a promise that resolves once the browser window has painted + * the first frame. + * @return {Promise} + */ +export function firstContentfulPaint() { + return browser.executeAsync(async (done) => { + if (PerformanceObserver.supportedEntryTypes.includes('paint')) { + new PerformanceObserver(() => { + done(); + }).observe({ + type: 'paint', + buffered: true, + }); + } else { + done(); + } + }); +} diff --git a/test/utils/navigateWithStrategy.js b/test/utils/navigateWithStrategy.js new file mode 100644 index 00000000..a8e02e11 --- /dev/null +++ b/test/utils/navigateWithStrategy.js @@ -0,0 +1,40 @@ +/* + * Copyright 2023 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. + */ + +import {domReadyState} from './domReadyState.js'; + +/** + * Returns a promise that resolves once the browser window has loaded, all + * load callbacks have finished executing, and any pending `__readyPromises` + * have settled. + * @return {Promise} + */ +export async function navigateWithStrategy(urlPath, readyState) { + await browser.url(urlPath); + + // In Firefox, if the global PageLoadStrategy is set to "none", then + // it's possible that `browser.url()` will return before the navigation + // has started and the old page will still be around, so we have to + // manually wait until the URL matches the passed URL. Note that this can + // still fail if the prior test navigated to a page with the same URL. + if (browser.capabilities.browserName === 'firefox') { + await browser.waitUntil(async () => { + return (await browser.getUrl()).endsWith(urlPath); + }); + } + + await domReadyState(readyState); +} diff --git a/wdio.conf.cjs b/wdio.conf.cjs index 2c947150..cce6b0a9 100644 --- a/wdio.conf.cjs +++ b/wdio.conf.cjs @@ -36,10 +36,7 @@ module.exports.config = { // then the current working directory is where your `package.json` resides, so `wdio` // will be called from there. // - specs: [ - // 'test/e2e/*-test.js' - 'test/e2e/onFCP-test.js', - ], + specs: ['test/e2e/*-test.js'], // Patterns to exclude. exclude: [ // 'path/to/excluded/files'