Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve test flakiness #507

Merged
merged 28 commits into from
Aug 4, 2024
Merged
Show file tree
Hide file tree
Changes from 9 commits
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
c23b0f6
Remove firefox workarounds for test flakiness
tunetheweb Aug 1, 2024
c57c1a6
Add extra wait to hidden test
tunetheweb Aug 1, 2024
360db97
Skip prerender tests if unsupported
tunetheweb Aug 1, 2024
8b7fa8b
Oops, forgot to include the new file
tunetheweb Aug 1, 2024
df3ab5e
Urgh I should just test this first
tunetheweb Aug 1, 2024
6e15ba8
Refactor
tunetheweb Aug 1, 2024
4a81735
Skip prerender on all metrics if not supported
tunetheweb Aug 1, 2024
71916de
Alternative LCP hidden flakiness fix
tunetheweb Aug 1, 2024
ee0eb2f
Another anti-flaky test
tunetheweb Aug 1, 2024
1741c04
Check
tunetheweb Aug 1, 2024
267e303
Update to complete
tunetheweb Aug 1, 2024
b115980
Fix for Safari
tunetheweb Aug 1, 2024
4494dca
Add comment
tunetheweb Aug 1, 2024
22a2068
Another Safari fix
tunetheweb Aug 1, 2024
1d5aa87
Try Phil's fix
tunetheweb Aug 2, 2024
94792fe
Revert fix
tunetheweb Aug 2, 2024
526fee6
Remove 100ms delay
tunetheweb Aug 2, 2024
9b2d7f7
Add Safari to navigateTo workaround
tunetheweb Aug 2, 2024
1bd2263
Remove debugging logging
tunetheweb Aug 2, 2024
639134d
Revert Safari change
tunetheweb Aug 2, 2024
46c8e91
Try another fix
tunetheweb Aug 2, 2024
4604f46
Check both
tunetheweb Aug 2, 2024
ad022dd
Fix it
tunetheweb Aug 2, 2024
1b000b8
Restrict to Safari and Firefox
tunetheweb Aug 2, 2024
feed6ab
Remove debugging logging
tunetheweb Aug 2, 2024
aeb6e91
Wait for interactive for flakey INP test
tunetheweb Aug 2, 2024
d162c14
Temporarily disable prerender support check
philipwalton Aug 2, 2024
b474f82
Remove the prerender skips in tests
philipwalton Aug 2, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions test/e2e/onCLS-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand All @@ -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);
Expand Down Expand Up @@ -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');

Expand Down
27 changes: 9 additions & 18 deletions test/e2e/onFCP-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,36 +16,21 @@

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';
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);

let browserSupportsFCP;
let browserSupportsPrerender;
before(async function () {
browserSupportsFCP = await browserSupportsEntry('paint');
browserSupportsPrerender = await browserSupportsActivationStart();
});

beforeEach(async function () {
Expand Down Expand Up @@ -89,6 +74,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');

Expand Down Expand Up @@ -136,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);

tunetheweb marked this conversation as resolved.
Show resolved Hide resolved
await stubVisibilityChange('visible');

// Wait a bit to ensure no beacons were sent.
Expand Down Expand Up @@ -335,6 +325,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',
Expand Down
4 changes: 4 additions & 0 deletions test/e2e/onFID-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand All @@ -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 () {
Expand Down Expand Up @@ -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');

Expand Down
4 changes: 4 additions & 0 deletions test/e2e/onINP-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand All @@ -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 () {
Expand Down Expand Up @@ -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',
Expand Down
41 changes: 9 additions & 32 deletions test/e2e/onLCP-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,37 +16,22 @@

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';
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);

let browserSupportsLCP;
let browserSupportsPrerender;
before(async function () {
browserSupportsLCP = await browserSupportsEntry('largest-contentful-paint');
browserSupportsPrerender = await browserSupportsActivationStart();
});

beforeEach(async function () {
Expand Down Expand Up @@ -161,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');

Expand Down Expand Up @@ -277,7 +263,9 @@ 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));
Expand Down Expand Up @@ -554,6 +542,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');

Expand Down Expand Up @@ -716,19 +705,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);
Expand Down
8 changes: 8 additions & 0 deletions test/e2e/onTTFB-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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';

Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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();
Expand Down
25 changes: 25 additions & 0 deletions test/utils/browserSupportsActivationStart.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
/*
* 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 browserSupportsActivationStart() {
return browser.execute(() => {
return 'activationStart' in self.PerformanceNavigationTiming.prototype;
});
}
1 change: 1 addition & 0 deletions wdio.conf.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,7 @@ module.exports.config = {
if (browserName === 'chrome') {
capability['goog:chromeOptions'] = {
excludeSwitches: ['enable-automation'],
args: ['disable-search-engine-choice-screen'],
philipwalton marked this conversation as resolved.
Show resolved Hide resolved
// Uncomment to test on Chrome Canary.
// binary: '/Applications/Google Chrome Canary.app/Contents/MacOS/Google Chrome Canary'
};
Expand Down