From 5b91b766031c435e44dda912be75f685d77fe073 Mon Sep 17 00:00:00 2001
From: Barry Pollard <barrypollard@google.com>
Date: Sun, 4 Aug 2024 08:54:30 +0100
Subject: [PATCH] Improve test flakiness (#507)

* Remove firefox workarounds for test flakiness

* Add extra wait to hidden test

* Skip prerender tests if unsupported

* Oops, forgot to include the new file

* Urgh I should just test this first

* Refactor

* Skip prerender on all metrics if not supported

* Alternative LCP hidden flakiness fix

* Another anti-flaky test

* Check

* Update to complete

* Fix for Safari

* Add comment

* Another Safari fix

* Try Phil's fix

* Revert fix

* Remove 100ms delay

* Add Safari to navigateTo workaround

* Remove debugging logging

* Revert Safari change

* Try another fix

* Check both

* Fix it

* Restrict to Safari and Firefox

* Remove debugging logging

* Wait for interactive for flakey INP test

* Temporarily disable prerender support check

* Remove the prerender skips in tests

---------

Co-authored-by: Philip Walton <philip@philipwalton.com>
---
 test/e2e/onFCP-test.js   | 20 +-------------------
 test/e2e/onINP-test.js   |  6 +++++-
 test/e2e/onLCP-test.js   | 36 ++++--------------------------------
 test/utils/navigateTo.js | 19 +++++++++++++------
 wdio.conf.cjs            |  2 ++
 5 files changed, 25 insertions(+), 58 deletions(-)

diff --git a/test/e2e/onFCP-test.js b/test/e2e/onFCP-test.js
index ce0de79c..d7748136 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);
@@ -134,7 +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 navigateTo('/test/fcp?hidden=1', {readyState: 'interactive'});
+    await navigateTo('/test/fcp?hidden=1', {readyState: 'complete'});
 
     await stubVisibilityChange('visible');
 
diff --git a/test/e2e/onINP-test.js b/test/e2e/onINP-test.js
index 3bdc691b..4db92e47 100644
--- a/test/e2e/onINP-test.js
+++ b/test/e2e/onINP-test.js
@@ -699,12 +699,16 @@ describe('onINP()', async function () {
     it('includes LoAF entries if the browser supports it', async function () {
       if (!browserSupportsLoAF) this.skip();
 
-      await navigateTo('/test/inp?attribution=1&pointerdown=100');
+      await navigateTo('/test/inp?attribution=1&pointerdown=100', {
+        readyState: 'interactive',
+      });
 
       // Click on the <textarea>.
       const textarea = await $('#textarea');
       await textarea.click();
 
+      await nextFrame();
+
       await stubVisibilityChange('hidden');
       await beaconCountIs(1);
 
diff --git a/test/e2e/onLCP-test.js b/test/e2e/onLCP-test.js
index 4b02bc7e..b1cd7782 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);
@@ -277,7 +259,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));
@@ -716,19 +700,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);
diff --git a/test/utils/navigateTo.js b/test/utils/navigateTo.js
index 5a5fd008..22932dda 100644
--- a/test/utils/navigateTo.js
+++ b/test/utils/navigateTo.js
@@ -24,15 +24,22 @@ import {domReadyState} from './domReadyState.js';
 export async function navigateTo(urlPath, opts) {
   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
+  // In Firefox and Safari, 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);
-    });
+  if (browser.capabilities.browserName !== 'chrome') {
+    await browser.waitUntil(
+      async () => {
+        // Get the URL from the browser and webdriver to ensure the page has
+        // actually started to load.
+        const url = await browser.execute(() => location.href);
+
+        return url.endsWith(urlPath);
+      },
+      {interval: 50},
+    );
   }
 
   if (opts?.readyState) {
diff --git a/wdio.conf.cjs b/wdio.conf.cjs
index 7a0b1d79..c6e14391 100644
--- a/wdio.conf.cjs
+++ b/wdio.conf.cjs
@@ -82,6 +82,8 @@ module.exports.config = {
     if (browserName === 'chrome') {
       capability['goog:chromeOptions'] = {
         excludeSwitches: ['enable-automation'],
+        // Can remove next line after puppeteer 21.2.1 lands
+        args: ['disable-search-engine-choice-screen'],
         // Uncomment to test on Chrome Canary.
         // binary: '/Applications/Google Chrome Canary.app/Contents/MacOS/Google Chrome Canary'
       };