From a863b4e88a35709503985eed3f46823badd7102a Mon Sep 17 00:00:00 2001 From: Alex Rudenko Date: Thu, 8 Dec 2022 17:47:20 +0100 Subject: [PATCH] chore: remove debug logs --- packages/puppeteer-core/src/common/Frame.ts | 2 +- .../puppeteer-core/src/common/FrameManager.ts | 2 +- .../src/common/IsolatedWorld.ts | 1 - .../src/common/LifecycleWatcher.ts | 22 ++++++++++++------- test/src/launcher.spec.ts | 3 --- test/src/mocha-utils.ts | 5 +++++ test/src/oopif.spec.ts | 20 ++--------------- 7 files changed, 23 insertions(+), 32 deletions(-) diff --git a/packages/puppeteer-core/src/common/Frame.ts b/packages/puppeteer-core/src/common/Frame.ts index 487d7e6263ffa..7f0310e1d6044 100644 --- a/packages/puppeteer-core/src/common/Frame.ts +++ b/packages/puppeteer-core/src/common/Frame.ts @@ -37,7 +37,7 @@ import {importFS} from './util.js'; import {debug} from './Debug.js'; -const log = debug('puppeteer:frame '); +const log = debug('puppeteer:Frame'); /** * @public diff --git a/packages/puppeteer-core/src/common/FrameManager.ts b/packages/puppeteer-core/src/common/FrameManager.ts index 4e261b526f71e..33ff1f52aed04 100644 --- a/packages/puppeteer-core/src/common/FrameManager.ts +++ b/packages/puppeteer-core/src/common/FrameManager.ts @@ -32,7 +32,7 @@ import {debugError} from './util.js'; import {debug} from './Debug.js'; -const log = debug('puppeteer:frameManager '); +const log = debug('puppeteer:FrameManager'); const UTILITY_WORLD_NAME = '__puppeteer_utility_world__'; diff --git a/packages/puppeteer-core/src/common/IsolatedWorld.ts b/packages/puppeteer-core/src/common/IsolatedWorld.ts index 7cad59b1dcc47..24806e5c5c100 100644 --- a/packages/puppeteer-core/src/common/IsolatedWorld.ts +++ b/packages/puppeteer-core/src/common/IsolatedWorld.ts @@ -296,7 +296,6 @@ export class IsolatedWorld { waitUntil = ['load'], timeout = this.#timeoutSettings.navigationTimeout(), } = options; - // We rely upon the fact that document.open() will reset frame lifecycle with "init" // lifecycle event. @see https://crrev.com/608658 await this.evaluate(html => { diff --git a/packages/puppeteer-core/src/common/LifecycleWatcher.ts b/packages/puppeteer-core/src/common/LifecycleWatcher.ts index f3b932eae8e09..2ae286b50119c 100644 --- a/packages/puppeteer-core/src/common/LifecycleWatcher.ts +++ b/packages/puppeteer-core/src/common/LifecycleWatcher.ts @@ -33,7 +33,7 @@ import {NetworkManagerEmittedEvents} from './NetworkManager.js'; import {CDPSessionEmittedEvents} from './Connection.js'; import {debug} from './Debug.js'; -const log = debug('puppeteer:lifecyleWatcher '); +const log = debug('puppeteer:LifecycleWatcher'); /** * @public @@ -113,7 +113,11 @@ export class LifecycleWatcher { waitUntil: PuppeteerLifeCycleEvent | PuppeteerLifeCycleEvent[], timeout: number ) { - log('LifeCycleWacher created for frame ', frame._id); + log('instantiated for frame', { + frameId: frame._id, + waitUntil, + timeout, + }); if (Array.isArray(waitUntil)) { waitUntil = waitUntil.slice(); } else if (typeof waitUntil === 'string') { @@ -286,12 +290,14 @@ export class LifecycleWatcher { } #checkLifecycleComplete(): void { - log( - 'LifeCycleWacher checkLifecycleComplete', - Array.from(this.#frame._lifecycleEvents), - this.#expectedLifecycle, - this.#swapped - ); + log('#checkLifecycleComplete', { + frameEvents: Array.from(this.#frame._lifecycleEvents), + expectedEvents: this.#expectedLifecycle, + frameSwapped: this.#swapped, + hasSameDocumentNavigation: this.#hasSameDocumentNavigation, + frameLoaderId: this.#frame._loaderId, + initialLoaderId: this.#initialLoaderId, + }); // We expect navigation to commit. if (!checkLifecycle(this.#frame, this.#expectedLifecycle)) { return; diff --git a/test/src/launcher.spec.ts b/test/src/launcher.spec.ts index d67aceed1a232..477d0236e053f 100644 --- a/test/src/launcher.spec.ts +++ b/test/src/launcher.spec.ts @@ -797,9 +797,6 @@ describe('Launcher specs', function () { const restoredPage = pages.find(page => { return page.url() === server.PREFIX + '/frames/nested-frames.html'; })!; - await new Promise(resolve => { - return setTimeout(resolve, 1000); - }); expect(utils.dumpFrames(restoredPage.mainFrame())).toEqual([ 'http://localhost:/frames/nested-frames.html', ' http://localhost:/frames/two-frames.html (2frames)', diff --git a/test/src/mocha-utils.ts b/test/src/mocha-utils.ts index ca678c4778ad2..b4e66721f31b6 100644 --- a/test/src/mocha-utils.ts +++ b/test/src/mocha-utils.ts @@ -282,6 +282,11 @@ export const expectCookieEquals = ( } }; +/** + * Use it if you want to capture debug logs for a specitic test suite in CI. + * This describe function enables capturing of debug logs and would print them + * only if a test fails to reduce the amount of output. + */ export const describeWithDebugLogs = ( description: string, body: (this: Mocha.Suite) => void diff --git a/test/src/oopif.spec.ts b/test/src/oopif.spec.ts index 6ab22c969b5ac..eb2eb5e0861bb 100644 --- a/test/src/oopif.spec.ts +++ b/test/src/oopif.spec.ts @@ -20,9 +20,6 @@ import {describeWithDebugLogs, getTestState} from './mocha-utils.js'; import {Browser} from 'puppeteer-core/internal/api/Browser.js'; import {BrowserContext} from 'puppeteer-core/internal/api/BrowserContext.js'; import {Page} from 'puppeteer-core/internal/api/Page.js'; -import {debug} from 'puppeteer-core/internal/common/Debug.js'; - -const log = debug('puppeteer:test'); describeWithDebugLogs('OOPIF', function () { /* We use a special browser for this test as we need the --site-per-process flag */ @@ -49,11 +46,11 @@ describeWithDebugLogs('OOPIF', function () { }); afterEach(async () => { - await context?.close(); + await context.close(); }); after(async () => { - await browser?.close(); + await browser.close(); }); it('should treat OOP iframes and normal iframes the same', async () => { @@ -132,13 +129,11 @@ describeWithDebugLogs('OOPIF', function () { const [frame1, frame2] = await Promise.all([frame1Promise, frame2Promise]); - log('FRAMES RESOLVED'); expect( await frame1.evaluate(() => { return document.location.href; }) ).toMatch(/one-frame\.html$/); - log('AFTER EVALUATE1'); expect( await frame2.evaluate(() => { return document.location.href; @@ -176,7 +171,6 @@ describeWithDebugLogs('OOPIF', function () { await utils.attachFrame(page, 'frame1', server.EMPTY_PAGE); const frame = await framePromise; - log('FRAME FOUND'); expect(frame.isOOPFrame()).toBe(false); const nav = frame.waitForNavigation(); await utils.navigateFrame( @@ -184,12 +178,9 @@ describeWithDebugLogs('OOPIF', function () { 'frame1', server.CROSS_PROCESS_PREFIX + '/empty.html' ); - log('WAITING FOR NAV TO CROSS_PROCESS_PREFIX '); await nav; - log('NAVIGATED TO CROSS_PROCESS_PREFIX '); expect(frame.isOOPFrame()).toBe(true); await utils.detachFrame(page, 'frame1'); - log('NAVIGATED DETACHED'); expect(page.frames()).toHaveLength(1); }); @@ -223,21 +214,17 @@ describeWithDebugLogs('OOPIF', function () { 'frame1', server.CROSS_PROCESS_PREFIX + '/empty.html' ); - log('AFTER ATTACH'); const frame = await framePromise; - log('BEFORE EVALUATE1'); await frame.evaluate(() => { // eslint-disable-next-line @typescript-eslint/ban-ts-comment // @ts-ignore _test = 'Test 123!'; }); - log('BEFORE EVALUATE2'); const result = await frame.evaluate(() => { // eslint-disable-next-line @typescript-eslint/ban-ts-comment // @ts-ignore return window._test; }); - log('AFTER EVALUATE2'); expect(result).toBe('Test 123!'); }); it('should provide access to elements', async () => { @@ -300,13 +287,10 @@ describeWithDebugLogs('OOPIF', function () { it('should wait for inner OOPIFs', async () => { const {server} = getTestState(); - log('BEFORE NAV'); await page.goto(`http://mainframe:${server.PORT}/main-frame.html`); - log('AFTER NAV'); const frame2 = await page.waitForFrame(frame => { return frame.url().endsWith('inner-frame2.html'); }); - log('AFTER WAIT'); expect(oopifs(context).length).toBe(2); expect( page.frames().filter(frame => {