Fix flaky tests after upgrading dependencies

GoogleChrome · Dec 30, 2023 · bf32741 · bf32741
1 parent 73c9f0d
commit bf32741
Show file tree

Hide file tree

Showing 9 changed files with 148 additions and 62 deletions.
diff --git 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
@@ -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
@@ -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 <body>.

diff --git 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
@@ -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
@@ -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);