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 <[email protected]>
GoogleChrome · Aug 4, 2024 · 5b91b76 · 5b91b76
1 parent dc0ee2b
commit 5b91b76
Show file tree

Hide file tree

Showing 5 changed files with 25 additions and 58 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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'
       };