core: reduce DevTools flakiness (#14782)

GoogleChrome · Feb 14, 2023 · 43e332a · 43e332a
1 parent 9c13846
commit 43e332a
Show file tree

Hide file tree

Showing 4 changed files with 14 additions and 19 deletions.
diff --git a/core/gather/driver/network-monitor.js b/core/gather/driver/network-monitor.js
@@ -95,9 +95,7 @@ class NetworkMonitor extends NetworkMonitorEventEmitter {
     const frameNavigations = this._frameNavigations;
     if (!frameNavigations.length) return {};
 
-    const frameTreeResponse = await this._session.sendCommand('Page.getFrameTree');
-    const mainFrameId = frameTreeResponse.frameTree.frame.id;
-    const mainFrameNavigations = frameNavigations.filter(frame => frame.id === mainFrameId);
+    const mainFrameNavigations = frameNavigations.filter(frame => !frame.parentId);
     if (!mainFrameNavigations.length) log.warn('NetworkMonitor', 'No detected navigations');
 
     // The requested URL is the initiator request for the first frame navigation.

diff --git a/core/gather/gatherers/bf-cache-failures.js b/core/gather/gatherers/bf-cache-failures.js
@@ -8,7 +8,7 @@ import FRGatherer from '../base-gatherer.js';
 import {waitForFrameNavigated, waitForLoadEvent} from '../driver/wait-for-condition.js';
 import DevtoolsLog from './devtools-log.js';
 
-const FAILURE_EVENT_TIMEOUT = 100;
+const AFTER_RETURN_TIMEOUT = 100;
 const TEMP_PAGE_PAUSE_TIMEOUT = 100;
 
 class BFCacheFailures extends FRGatherer {
@@ -122,14 +122,13 @@ class BFCacheFailures extends FRGatherer {
 
     // The bfcache failure event is not necessarily emitted by this point.
     // If we are expecting a bfcache failure event but haven't seen one, we should wait for it.
-    if (frameNavigatedEvent.type !== 'BackForwardCacheRestore' && !bfCacheEvent) {
-      await new Promise(resolve => setTimeout(resolve, FAILURE_EVENT_TIMEOUT));
+    // This timeout also allows the environment to "settle" before gathering enters it's cleanup phase.
+    await new Promise(resolve => setTimeout(resolve, AFTER_RETURN_TIMEOUT));
 
-      // If we still can't get the failure reasons after the timeout we should fail loudly,
-      // otherwise this gatherer will return no failures when there should be failures.
-      if (!bfCacheEvent) {
-        throw new Error('bfcache failed but the failure reasons were not emitted in time');
-      }
+    // If we still can't get the failure reasons after the timeout we should fail loudly,
+    // otherwise this gatherer will return no failures when there should be failures.
+    if (frameNavigatedEvent.type !== 'BackForwardCacheRestore' && !bfCacheEvent) {
+      throw new Error('bfcache failed but the failure reasons were not emitted in time');
     }
 
     session.off('Page.backForwardCacheNotUsed', onBfCacheNotUsed);

diff --git a/core/test/gather/driver/navigation-test.js b/core/test/gather/driver/navigation-test.js
@@ -37,8 +37,7 @@ describe('.gotoURL', () => {
       .mockResponse('Page.enable') // gotoURL's Page.enable
       .mockResponse('Page.setLifecycleEventsEnabled')
       .mockResponse('Page.navigate')
-      .mockResponse('Runtime.evaluate')
-      .mockResponse('Page.getFrameTree', {frameTree: {frame: {id: 'ABC'}}});
+      .mockResponse('Runtime.evaluate');
   });
 
   it('will track redirects through gotoURL load with warning', async () => {
@@ -71,10 +70,10 @@ describe('.gotoURL', () => {
     navigate({...baseFrame, url: 'https://example.com'});
     navigate({...baseFrame, url: 'https://www.example.com'});
     navigate({...baseFrame, url: 'https://m.example.com'});
-    navigate({...baseFrame, id: 'ad1', url: 'https://frame-a.example.com'});
+    navigate({...baseFrame, id: 'ad1', url: 'https://frame-a.example.com', parentId: 'ABC'});
     navigate({...baseFrame, url: 'https://m.example.com/client'});
-    navigate({...baseFrame, id: 'ad2', url: 'https://frame-b.example.com'});
-    navigate({...baseFrame, id: 'ad3', url: 'https://frame-c.example.com'});
+    navigate({...baseFrame, id: 'ad2', url: 'https://frame-b.example.com', parentId: 'ABC'});
+    navigate({...baseFrame, id: 'ad3', url: 'https://frame-c.example.com', parentId: 'ABC'});
 
     loadListener(baseFrame);
     await flushAllTimersAndMicrotasks();

diff --git a/core/test/gather/driver/network-monitor-test.js b/core/test/gather/driver/network-monitor-test.js
@@ -217,14 +217,13 @@ describe('NetworkMonitor', () => {
     it('should ignore non-main-frame navigations', async () => {
       rootCdpSessionMock.send
         .mockResponse('Target.setAutoAttach')
-        .mockResponse('Target.setAutoAttach')
-        .mockResponse('Page.getFrameTree', {frameTree: {frame: {id: '1'}}});
+        .mockResponse('Target.setAutoAttach');
       await monitor.enable();
 
       const type = 'Navigation';
       const frame = /** @type {*} */ ({id: '1', url: 'https://page.example.com'});
       rootDispatch({method: 'Page.frameNavigated', params: {frame, type}});
-      const iframe = /** @type {*} */ ({id: '2', url: 'https://iframe.example.com'});
+      const iframe = /** @type {*} */ ({id: '2', url: 'https://iframe.example.com', parentId: '1'});
       rootDispatch({method: 'Page.frameNavigated', params: {frame: iframe, type}});
 
       expect(await monitor.getNavigationUrls()).toEqual({