From f2922797c5cc24a4c51970ec8d6183af1e9a1127 Mon Sep 17 00:00:00 2001 From: Chris Breiding Date: Mon, 30 Oct 2023 09:39:30 -0400 Subject: [PATCH 01/12] chore: extract Target.attachedToTarget and Target.targetDestroyed handlers into own methods --- .../server/lib/browsers/browser-cri-client.ts | 251 +++++++++++------- 1 file changed, 152 insertions(+), 99 deletions(-) diff --git a/packages/server/lib/browsers/browser-cri-client.ts b/packages/server/lib/browsers/browser-cri-client.ts index b1b90363a72c..3723cbfa71fb 100644 --- a/packages/server/lib/browsers/browser-cri-client.ts +++ b/packages/server/lib/browsers/browser-cri-client.ts @@ -1,6 +1,7 @@ import Bluebird from 'bluebird' import CRI from 'chrome-remote-interface' import Debug from 'debug' +import type { Protocol } from 'devtools-protocol' import { _connectAsync, _getDelayMsForRetry } from './protocol' import * as errors from '../errors' import { create, CriClient, DEFAULT_NETWORK_ENABLE_OPTIONS } from './cri-client' @@ -25,13 +26,37 @@ type BrowserCriClientOptions = { } type BrowserCriClientCreateOptions = { - hosts: string[] - port: number browserName: string + fullyManageTabs?: boolean + hosts: string[] onAsynchronousError: Function onReconnect?: (client: CriClient) => void + port: number + protocolManager?: ProtocolManagerShape +} + +interface ManageTabsOptions { + browserClient: CriClient + browserCriClient: BrowserCriClient + browserName + host: string + onAsynchronousError: Function + port: number + protocolManager?: ProtocolManagerShape +} + +interface AttachedToTargetOptions { + browserClient: CriClient + event: Protocol.Target.AttachedToTargetEvent protocolManager?: ProtocolManagerShape - fullyManageTabs?: boolean +} + +interface TargetDestroyedOptions { + browserName: string + browserClient: CriClient + browserCriClient: BrowserCriClient + event: Protocol.Target.TargetDestroyedEvent + onAsynchronousError: Function } const isVersionGte = (a: Version, b: Version) => { @@ -169,16 +194,26 @@ export class BrowserCriClient { * Factory method for the browser cri client. Connects to the browser and then returns a chrome remote interface wrapper around the * browser target * - * @param hosts the hosts to which to attempt to connect - * @param port the port to which to connect * @param browserName the display name of the browser being launched + * @param fullyManageTabs whether or not to fully manage tabs. This is useful for firefox where some work is done with marionette and some with CDP. We don't want to handle disconnections in this class in those scenarios + * @param hosts the hosts to which to attempt to connect * @param onAsynchronousError callback for any cdp fatal errors * @param onReconnect callback for when the browser cri client reconnects to the browser + * @param port the port to which to connect * @param protocolManager the protocol manager to use with the browser cri client - * @param fullyManageTabs whether or not to fully manage tabs. This is useful for firefox where some work is done with marionette and some with CDP. We don't want to handle disconnections in this class in those scenarios * @returns a wrapper around the chrome remote interface that is connected to the browser target */ - static async create ({ hosts, port, browserName, onAsynchronousError, onReconnect, protocolManager, fullyManageTabs }: BrowserCriClientCreateOptions): Promise { + static async create (options: BrowserCriClientCreateOptions): Promise { + const { + browserName, + fullyManageTabs, + hosts, + onAsynchronousError, + onReconnect, + port, + protocolManager, + } = options + const host = await ensureLiveBrowser(hosts, port, browserName) return retryWithIncreasingDelay(async () => { @@ -195,104 +230,122 @@ export class BrowserCriClient { const browserCriClient = new BrowserCriClient({ browserClient, versionInfo, host, port, browserName, onAsynchronousError, protocolManager, fullyManageTabs }) if (fullyManageTabs) { - // The basic approach here is we attach to targets and enable network traffic - // We must attach in a paused state so that we can enable network traffic before the target starts running. - browserClient.on('Target.attachedToTarget', async (event) => { - try { - if (event.targetInfo.type !== 'page') { - await browserClient.send('Network.enable', protocolManager?.networkEnableOptions ?? DEFAULT_NETWORK_ENABLE_OPTIONS, event.sessionId) - } - - if (event.waitingForDebugger) { - await browserClient.send('Runtime.runIfWaitingForDebugger', undefined, event.sessionId) - } - } catch (error) { - // it's possible that the target was closed before we could enable network and continue, in that case, just ignore - debug('error attaching to target browser', error) - } - }) - - // Ideally we could use filter rather than checking the type above, but that was added relatively recently - await browserClient.send('Target.setAutoAttach', { autoAttach: true, waitForDebuggerOnStart: true, flatten: true }) - await browserClient.send('Target.setDiscoverTargets', { discover: true }) - browserClient.on('Target.targetDestroyed', (event) => { - debug('Target.targetDestroyed %o', { - event, - closing: browserCriClient.closing, - closed: browserCriClient.closed, - resettingBrowserTargets: browserCriClient.resettingBrowserTargets, - }) - - // we may have gotten a delayed "Target.targetDestroyed" even for a page that we - // have already closed/disposed, so unless this matches our current target then bail - if (event.targetId !== browserCriClient.currentlyAttachedTarget?.targetId) { - return - } - - // otherwise... - // the page or browser closed in an unexpected manner and we need to bubble up this error - // by calling onError() with either browser or page was closed - // - // we detect this by waiting up to 500ms for either the browser's websocket connection to be closed - // OR from process.exit(...) firing - // if the browser's websocket connection has been closed then that means the page was closed - // - // otherwise it means the the browser itself was closed - - // always close the connection to the page target because it was destroyed - browserCriClient.currentlyAttachedTarget.close().catch(() => { }), - - new Bluebird((resolve) => { - // this event could fire either expectedly or unexpectedly - // it's not a problem if we're expected to be closing the browser naturally - // and not as a result of an unexpected page or browser closure - if (browserCriClient.resettingBrowserTargets) { - // do nothing, we're good - return resolve(true) - } - - if (typeof browserCriClient.gracefulShutdown !== 'undefined') { - return resolve(browserCriClient.gracefulShutdown) - } - - // when process.on('exit') is called, we call onClose - browserCriClient.onClose = resolve - - // or when the browser's CDP ws connection is closed - browserClient.ws.once('close', () => { - resolve(false) - }) - }) - .timeout(500) - .then((expectedDestroyedEvent) => { - if (expectedDestroyedEvent === true) { - return - } - - // browserClient websocket was disconnected - // or we've been closed due to process.on('exit') - // meaning the browser was closed and not just the page - errors.throwErr('BROWSER_PROCESS_CLOSED_UNEXPECTEDLY', browserName) - }) - .catch(Bluebird.TimeoutError, () => { - debug('browser websocket did not close, page was closed %o', { targetId: event.targetId }) - // the browser websocket didn't close meaning - // only the page was closed, not the browser - errors.throwErr('BROWSER_PAGE_CLOSED_UNEXPECTEDLY', browserName) - }) - .catch((err) => { - // stop the run instead of moving to the next spec - err.isFatalApiErr = true - - onAsynchronousError(err) - }) - }) + await this._manageTabs({ browserClient, browserCriClient, browserName, host, onAsynchronousError, port, protocolManager }) } return browserCriClient }, browserName, port) } + static async _manageTabs ({ browserClient, browserCriClient, browserName, host, onAsynchronousError, port, protocolManager }: ManageTabsOptions) { + const promises = [ + browserClient.send('Target.setDiscoverTargets', { discover: true }), + browserClient.send('Target.setAutoAttach', { autoAttach: true, waitForDebuggerOnStart: true, flatten: true }), + ] + + browserClient.on('Target.attachedToTarget', async (event: Protocol.Target.AttachedToTargetEvent) => { + await this._onAttachToTarget({ browserClient, event, protocolManager }) + }) + + browserClient.on('Target.targetDestroyed', (event: Protocol.Target.TargetDestroyedEvent) => { + this._onTargetDestroyed({ browserClient, browserCriClient, browserName, event, onAsynchronousError }) + }) + + await Promise.all(promises) + } + + static async _onAttachToTarget (options: AttachedToTargetOptions) { + const { browserClient, event, protocolManager } = options + + debug('Target.attachedToTarget %o', event.targetInfo) + + try { + if (event.targetInfo.type !== 'page') { + await browserClient.send('Network.enable', protocolManager?.networkEnableOptions ?? DEFAULT_NETWORK_ENABLE_OPTIONS, event.sessionId) + } + + if (event.waitingForDebugger) { + await browserClient.send('Runtime.runIfWaitingForDebugger', undefined, event.sessionId) + } + } catch (error) { + // it's possible that the target was closed before we could enable network and continue, in that case, just ignore + debug('error attaching to target browser', error) + } + } + + static _onTargetDestroyed ({ browserClient, browserCriClient, browserName, event, onAsynchronousError }: TargetDestroyedOptions) { + debug('Target.targetDestroyed %o', { + event, + closing: browserCriClient.closing, + closed: browserCriClient.closed, + resettingBrowserTargets: browserCriClient.resettingBrowserTargets, + }) + + // we may have gotten a delayed "Target.targetDestroyed" even for a page that we + // have already closed/disposed, so unless this matches our current target then bail + if (event.targetId !== browserCriClient.currentlyAttachedTarget?.targetId) { + return + } + + // otherwise... + // the page or browser closed in an unexpected manner and we need to bubble up this error + // by calling onError() with either browser or page was closed + // + // we detect this by waiting up to 500ms for either the browser's websocket connection to be closed + // OR from process.exit(...) firing + // if the browser's websocket connection has been closed then that means the page was closed + // + // otherwise it means the the browser itself was closed + + // always close the connection to the page target because it was destroyed + browserCriClient.currentlyAttachedTarget.close().catch(() => { }), + + new Bluebird((resolve) => { + // this event could fire either expectedly or unexpectedly + // it's not a problem if we're expected to be closing the browser naturally + // and not as a result of an unexpected page or browser closure + if (browserCriClient.resettingBrowserTargets) { + // do nothing, we're good + return resolve(true) + } + + if (typeof browserCriClient.gracefulShutdown !== 'undefined') { + return resolve(browserCriClient.gracefulShutdown) + } + + // when process.on('exit') is called, we call onClose + browserCriClient.onClose = resolve + + // or when the browser's CDP ws connection is closed + browserClient.ws.once('close', () => { + resolve(false) + }) + }) + .timeout(500) + .then((expectedDestroyedEvent) => { + if (expectedDestroyedEvent === true) { + return + } + + // browserClient websocket was disconnected + // or we've been closed due to process.on('exit') + // meaning the browser was closed and not just the page + errors.throwErr('BROWSER_PROCESS_CLOSED_UNEXPECTEDLY', browserName) + }) + .catch(Bluebird.TimeoutError, () => { + debug('browser websocket did not close, page was closed %o', { targetId: event.targetId }) + // the browser websocket didn't close meaning + // only the page was closed, not the browser + errors.throwErr('BROWSER_PAGE_CLOSED_UNEXPECTEDLY', browserName) + }) + .catch((err) => { + // stop the run instead of moving to the next spec + err.isFatalApiErr = true + + onAsynchronousError(err) + }) + } + /** * Ensures that the minimum protocol version for the browser is met * From 277885e70008b377a3d27a54b221480168de8793 Mon Sep 17 00:00:00 2001 From: Chris Breiding Date: Mon, 30 Oct 2023 10:15:03 -0400 Subject: [PATCH 02/12] chore: add after:browser:launch node event --- cli/types/cypress.d.ts | 10 ++++-- .../server/lib/browsers/browser-cri-client.ts | 7 ++++ packages/server/lib/browsers/chrome.ts | 4 +++ packages/server/lib/browsers/electron.ts | 4 +++ packages/server/lib/browsers/utils.ts | 24 +++++++++++++ .../lib/plugins/child/browser_launch.js | 2 +- .../server/lib/plugins/child/run_plugins.js | 4 ++- .../lib/plugins/child/validate_event.js | 7 +++- .../server/test/integration/cypress_spec.js | 2 ++ .../server/test/unit/browsers/chrome_spec.js | 31 ++++++++++++++-- .../test/unit/browsers/electron_spec.js | 35 ++++++++++++++++--- .../unit/plugins/child/run_plugins_spec.js | 34 +++++++++++++++++- .../unit/plugins/child/validate_event_spec.js | 1 + 13 files changed, 151 insertions(+), 14 deletions(-) diff --git a/cli/types/cypress.d.ts b/cli/types/cypress.d.ts index cf0604f27902..b4324fdc9d70 100644 --- a/cli/types/cypress.d.ts +++ b/cli/types/cypress.d.ts @@ -6009,8 +6009,11 @@ declare namespace Cypress { (fn: (currentSubject: Subject) => void): Chainable } - interface BrowserLaunchOptions { - extensions: string[] + interface AfterBrowserLaunchDetails { + webSocketDebuggerUrl: string + } + + interface BeforeBrowserLaunchOptions { extensions: string[] preferences: { [key: string]: any } args: string[] env: { [key: string]: any } @@ -6090,12 +6093,13 @@ declare namespace Cypress { } interface PluginEvents { + (action: 'after:browser:launch', fn: (browser: Browser, browserLaunchDetails: AfterBrowserLaunchDetails) => void | Promise): void (action: 'after:run', fn: (results: CypressCommandLine.CypressRunResult | CypressCommandLine.CypressFailedRunResult) => void | Promise): void (action: 'after:screenshot', fn: (details: ScreenshotDetails) => void | AfterScreenshotReturnObject | Promise): void (action: 'after:spec', fn: (spec: Spec, results: CypressCommandLine.RunResult) => void | Promise): void (action: 'before:run', fn: (runDetails: BeforeRunDetails) => void | Promise): void (action: 'before:spec', fn: (spec: Spec) => void | Promise): void - (action: 'before:browser:launch', fn: (browser: Browser, browserLaunchOptions: BrowserLaunchOptions) => void | BrowserLaunchOptions | Promise): void + (action: 'before:browser:launch', fn: (browser: Browser, afterBrowserLaunchOptions: BeforeBrowserLaunchOptions) => void | Promise | BeforeBrowserLaunchOptions | Promise): void (action: 'file:preprocessor', fn: (file: FileObject) => string | Promise): void (action: 'dev-server:start', fn: (file: DevServerConfig) => Promise): void (action: 'task', tasks: Tasks): void diff --git a/packages/server/lib/browsers/browser-cri-client.ts b/packages/server/lib/browsers/browser-cri-client.ts index 3723cbfa71fb..1f6379791cbd 100644 --- a/packages/server/lib/browsers/browser-cri-client.ts +++ b/packages/server/lib/browsers/browser-cri-client.ts @@ -444,6 +444,13 @@ export class BrowserCriClient { this.resettingBrowserTargets = false } + /** + * @returns the websocket debugger URL for the currently connected browser + */ + getWebSocketDebuggerUrl () { + return this.versionInfo.webSocketDebuggerUrl + } + /** * Closes the browser client socket as well as the socket for the currently attached page target */ diff --git a/packages/server/lib/browsers/chrome.ts b/packages/server/lib/browsers/chrome.ts index ea6b1e20d705..09fac3c7c290 100644 --- a/packages/server/lib/browsers/chrome.ts +++ b/packages/server/lib/browsers/chrome.ts @@ -633,6 +633,10 @@ export = { await this.attachListeners(url, pageCriClient, automation, options, browser) + await utils.executeAfterBrowserLaunch(browser, { + webSocketDebuggerUrl: browserCriClient.getWebSocketDebuggerUrl(), + }) + // return the launched browser process // with additional method to close the remote connection return launchedBrowser diff --git a/packages/server/lib/browsers/electron.ts b/packages/server/lib/browsers/electron.ts index 52a7b1e33c73..e0c42309ca7b 100644 --- a/packages/server/lib/browsers/electron.ts +++ b/packages/server/lib/browsers/electron.ts @@ -533,6 +533,10 @@ export = { }, }) as BrowserInstance + await utils.executeAfterBrowserLaunch(browser, { + webSocketDebuggerUrl: browserCriClient!.getWebSocketDebuggerUrl(), + }) + return instance }, } diff --git a/packages/server/lib/browsers/utils.ts b/packages/server/lib/browsers/utils.ts index b6f246b28ff9..457d6f1b943b 100644 --- a/packages/server/lib/browsers/utils.ts +++ b/packages/server/lib/browsers/utils.ts @@ -7,6 +7,7 @@ import * as plugins from '../plugins' import { getError } from '@packages/errors' import * as launcher from '@packages/launcher' import type { Automation } from '../automation' +import type { Browser } from './types' import type { CriClient } from './cri-client' declare global { @@ -157,6 +158,27 @@ async function executeBeforeBrowserLaunch (browser, launchOptions: typeof defaul return launchOptions } +interface AfterBrowserLaunchDetails { + webSocketDebuggerUrl: string +} + +async function executeAfterBrowserLaunch (browser: Browser, options: AfterBrowserLaunchDetails) { + if (plugins.has('after:browser:launch')) { + const span = telemetry.startSpan({ name: 'lifecycle:after:browser:launch' }) + + span?.setAttribute({ + name: browser.name, + channel: browser.channel, + version: browser.version, + isHeadless: browser.isHeadless, + }) + + await plugins.execute('after:browser:launch', browser, options) + + span?.end() + } +} + function extendLaunchOptionsFromPlugins (launchOptions, pluginConfigResult, options: BrowserLaunchOpts) { // if we returned an array from the plugin // then we know the user is using the deprecated @@ -423,6 +445,8 @@ export = { extendLaunchOptionsFromPlugins, + executeAfterBrowserLaunch, + executeBeforeBrowserLaunch, defaultLaunchOptions, diff --git a/packages/server/lib/plugins/child/browser_launch.js b/packages/server/lib/plugins/child/browser_launch.js index c5680961e3d8..54b777d384e3 100644 --- a/packages/server/lib/plugins/child/browser_launch.js +++ b/packages/server/lib/plugins/child/browser_launch.js @@ -3,7 +3,7 @@ const util = require('../util') const ARRAY_METHODS = ['concat', 'push', 'unshift', 'slice', 'pop', 'shift', 'slice', 'splice', 'filter', 'map', 'forEach', 'reduce', 'reverse', 'splice', 'includes'] module.exports = { - wrap (ipc, invoke, ids, args) { + wrapBefore (ipc, invoke, ids, args) { // TODO: remove in next breaking release // This will send a warning message when a deprecated API is used // define array-like functions on this object so we can warn about using deprecated array API diff --git a/packages/server/lib/plugins/child/run_plugins.js b/packages/server/lib/plugins/child/run_plugins.js index f86a35d5cdf9..0e439519860b 100644 --- a/packages/server/lib/plugins/child/run_plugins.js +++ b/packages/server/lib/plugins/child/run_plugins.js @@ -169,7 +169,9 @@ class RunPlugins { case '_get:task:body': return this.taskGetBody(ids, args) case 'before:browser:launch': - return browserLaunch.wrap(this.ipc, this.invoke, ids, args) + return browserLaunch.wrapBefore(this.ipc, this.invoke, ids, args) + case 'after:browser:launch': + return util.wrapChildPromise(this.ipc, this.invoke, ids, args) default: debug('unexpected execute message:', event, args) diff --git a/packages/server/lib/plugins/child/validate_event.js b/packages/server/lib/plugins/child/validate_event.js index 0a82f902d7d8..2c8832682552 100644 --- a/packages/server/lib/plugins/child/validate_event.js +++ b/packages/server/lib/plugins/child/validate_event.js @@ -27,6 +27,7 @@ const eventValidators = { '_get:task:body': isFunction, '_get:task:keys': isFunction, '_process:cross:origin:callback': isFunction, + 'after:browser:launch': isFunction, 'after:run': isFunction, 'after:screenshot': isFunction, 'after:spec': isFunction, @@ -42,7 +43,11 @@ const validateEvent = (event, handler, config, errConstructorFn) => { const validator = eventValidators[event] if (!validator) { - const userEvents = _.reject(_.keys(eventValidators), (event) => event.startsWith('_')) + const userEvents = _.reject(_.keys(eventValidators), (event) => { + // we're currently not documenting after:browser:launch, so it shouldn't + // appear in the list of valid events + return event.startsWith('_') || event === 'after:browser:launch' + }) const error = new Error(`invalid event name registered: ${event}`) diff --git a/packages/server/test/integration/cypress_spec.js b/packages/server/test/integration/cypress_spec.js index ca48af7aae03..f7957c35a0cf 100644 --- a/packages/server/test/integration/cypress_spec.js +++ b/packages/server/test/integration/cypress_spec.js @@ -1006,6 +1006,7 @@ describe('lib/cypress', () => { ensureMinimumProtocolVersion: sinon.stub().resolves(), attachToTargetUrl: sinon.stub().resolves(criClient), close: sinon.stub().resolves(), + getWebSocketDebuggerUrl: sinon.stub().returns('ws://debugger'), } const cdpAutomation = { @@ -1076,6 +1077,7 @@ describe('lib/cypress', () => { attachToTargetUrl: sinon.stub().resolves(criClient), currentlyAttachedTarget: criClient, close: sinon.stub().resolves(), + getWebSocketDebuggerUrl: sinon.stub().returns('ws://debugger'), } sinon.stub(BrowserCriClient, 'create').resolves(browserCriClient) diff --git a/packages/server/test/unit/browsers/chrome_spec.js b/packages/server/test/unit/browsers/chrome_spec.js index 987afc4c8313..f2e314b3781f 100644 --- a/packages/server/test/unit/browsers/chrome_spec.js +++ b/packages/server/test/unit/browsers/chrome_spec.js @@ -33,6 +33,7 @@ describe('lib/browsers/chrome', () => { attachToTargetUrl: sinon.stub().resolves(this.pageCriClient), close: sinon.stub().resolves(), ensureMinimumProtocolVersion: sinon.stub().withArgs('1.3').resolves(), + getWebSocketDebuggerUrl: sinon.stub().returns('ws://debugger'), } this.automation = { @@ -93,14 +94,14 @@ describe('lib/browsers/chrome', () => { }) }) - it('is noop without before:browser:launch', function () { + it('executeBeforeBrowserLaunch is noop if before:browser:launch is not registered', function () { return chrome.open({ isHeadless: true }, 'http://', openOpts, this.automation) .then(() => { - expect(plugins.execute).not.to.be.called + expect(plugins.execute).not.to.be.calledWith('before:browser:launch') }) }) - it('is noop if newArgs are not returned', function () { + it('uses default args if new args are not returned from before:browser:launch', function () { const args = [] sinon.stub(chrome, '_getArgs').returns(args) @@ -304,6 +305,30 @@ describe('lib/browsers/chrome', () => { return expect(chrome.open({ isHeadless: true }, 'http://', openOpts, this.automation)).to.be.rejectedWith('Cypress requires at least Chrome 64.') }) + it('sends after:browser:launch with debugger url', function () { + const args = [] + const browser = { isHeadless: true } + + sinon.stub(chrome, '_getArgs').returns(args) + sinon.stub(plugins, 'has').returns(true) + + plugins.execute.resolves(null) + + return chrome.open(browser, 'http://', openOpts, this.automation) + .then(() => { + expect(plugins.execute).to.be.calledWith('after:browser:launch', browser, { + webSocketDebuggerUrl: 'ws://debugger', + }) + }) + }) + + it('executeAfterBrowserLaunch is noop if after:browser:launch is not registered', function () { + return chrome.open({ isHeadless: true }, 'http://', openOpts, this.automation) + .then(() => { + expect(plugins.execute).not.to.be.calledWith('after:browser:launch') + }) + }) + describe('downloads', function () { it('pushes create:download after download begins', function () { const downloadData = { diff --git a/packages/server/test/unit/browsers/electron_spec.js b/packages/server/test/unit/browsers/electron_spec.js index 1fe7fc2d5c76..3a81c476398c 100644 --- a/packages/server/test/unit/browsers/electron_spec.js +++ b/packages/server/test/unit/browsers/electron_spec.js @@ -111,8 +111,11 @@ describe('lib/browsers/electron', () => { }) context('.open', () => { - beforeEach(function () { - return this.stubForOpen() + beforeEach(async function () { + // shortcut to set the browserCriClient singleton variable + await electron._getAutomation({}, { onError: () => {} }, {}) + + await this.stubForOpen() }) it('calls render with url, state, and options', function () { @@ -152,7 +155,7 @@ describe('lib/browsers/electron', () => { }) }) - it('is noop when before:browser:launch yields null', function () { + it('executeBeforeBrowserLaunch is noop when before:browser:launch yields null', function () { plugins.has.returns(true) plugins.execute.resolves(null) @@ -207,6 +210,27 @@ describe('lib/browsers/electron', () => { expect(Windows.removeAllExtensions).to.be.calledTwice }) }) + + it('sends after:browser:launch with debugger url', function () { + const browser = { isHeadless: true } + + plugins.has.returns(true) + plugins.execute.resolves(null) + + return electron.open('electron', this.url, this.options, this.automation) + .then(() => { + expect(plugins.execute).to.be.calledWith('after:browser:launch', browser, { + webSocketDebuggerUrl: 'ws://debugger', + }) + }) + }) + + it('executeAfterBrowserLaunch is noop if after:browser:launch is not registered', function () { + return electron.open('electron', this.url, this.options, this.automation) + .then(() => { + expect(plugins.execute).not.to.be.calledWith('after:browser:launch') + }) + }) }) context('.connectProtocolToBrowser', () => { @@ -800,7 +824,10 @@ describe('lib/browsers/electron', () => { expect(electron._launchChild).to.be.calledWith(this.url, parentWindow, this.options.projectRoot, this.state, this.options, this.automation) }) - it('adds pid of new BrowserWindow to allPids list', function () { + it('adds pid of new BrowserWindow to allPids list', async function () { + // shortcut to set the browserCriClient singleton variable + await electron._getAutomation({}, { onError: () => {} }, {}) + const opts = electron._defaultOptions(this.options.projectRoot, this.state, this.options) const NEW_WINDOW_PID = ELECTRON_PID * 2 diff --git a/packages/server/test/unit/plugins/child/run_plugins_spec.js b/packages/server/test/unit/plugins/child/run_plugins_spec.js index f87555f27c0f..7b89e2ec7fa6 100644 --- a/packages/server/test/unit/plugins/child/run_plugins_spec.js +++ b/packages/server/test/unit/plugins/child/run_plugins_spec.js @@ -141,6 +141,7 @@ describe('lib/plugins/child/run_plugins', () => { describe(`on 'execute:plugins' message`, () => { let onFilePreprocessor + let afterBrowserLaunch let beforeBrowserLaunch let taskRequested let setupNodeEventsFn @@ -149,11 +150,13 @@ describe('lib/plugins/child/run_plugins', () => { sinon.stub(preprocessor, 'wrap') onFilePreprocessor = sinon.stub().resolves() + afterBrowserLaunch = sinon.stub().resolves() beforeBrowserLaunch = sinon.stub().resolves() taskRequested = sinon.stub().resolves('foo') setupNodeEventsFn = (on) => { on('file:preprocessor', onFilePreprocessor) + on('after:browser:launch', afterBrowserLaunch) on('before:browser:launch', beforeBrowserLaunch) on('task', taskRequested) } @@ -201,6 +204,35 @@ describe('lib/plugins/child/run_plugins', () => { ipc.on.withArgs('execute:plugins').yield('before:browser:launch', ids, args) }) + it('wraps child promise', () => { + expect(util.wrapChildPromise).to.be.calledWith(ipc, sinon.match.func, ids, args) + }) + + it('invokes registered function when invoked by handler', () => { + // console.log(util.wrapChildPromise.withArgs(ipc, sinon.match.func, ids, args).args) + util.wrapChildPromise.withArgs(ipc, sinon.match.func, ids, args).args[0][1](5, args) + + expect(beforeBrowserLaunch).to.be.calledWith(...args) + }) + }) + + context('after:browser:launch', () => { + let args + const ids = { eventId: 2, invocationId: '00' } + + beforeEach(async () => { + sinon.stub(util, 'wrapChildPromise') + + await runPlugins.runSetupNodeEvents(config, setupNodeEventsFn) + + const browser = {} + const launchOptions = browserUtils.getDefaultLaunchOptions({}) + + args = [browser, launchOptions] + + ipc.on.withArgs('execute:plugins').yield('after:browser:launch', ids, args) + }) + it('wraps child promise', () => { expect(util.wrapChildPromise).to.be.called expect(util.wrapChildPromise.lastCall.args[0]).to.equal(ipc) @@ -212,7 +244,7 @@ describe('lib/plugins/child/run_plugins', () => { it('invokes registered function when invoked by handler', () => { util.wrapChildPromise.lastCall.args[1](4, args) - expect(beforeBrowserLaunch).to.be.calledWith(...args) + expect(afterBrowserLaunch).to.be.calledWith(...args) }) }) diff --git a/packages/server/test/unit/plugins/child/validate_event_spec.js b/packages/server/test/unit/plugins/child/validate_event_spec.js index dee6e85a0727..f024ee79cbbb 100644 --- a/packages/server/test/unit/plugins/child/validate_event_spec.js +++ b/packages/server/test/unit/plugins/child/validate_event_spec.js @@ -4,6 +4,7 @@ const _ = require('lodash') const validateEvent = require('../../../../lib/plugins/child/validate_event') const events = [ + ['after:browser:launch', 'a function', () => {}], ['after:run', 'a function', () => {}], ['after:screenshot', 'a function', () => {}], ['after:spec', 'a function', () => {}], From 7be41033188385f36358fed86f6d8f60ccbf3207 Mon Sep 17 00:00:00 2001 From: Chris Breiding Date: Mon, 30 Oct 2023 10:51:14 -0400 Subject: [PATCH 03/12] add to firefox and webkit [run ci] --- packages/server/lib/browsers/firefox.ts | 8 ++- packages/server/lib/browsers/utils.ts | 2 +- packages/server/lib/browsers/webkit.ts | 6 +++ .../server/test/unit/browsers/firefox_spec.ts | 53 ++++++++++++++----- 4 files changed, 54 insertions(+), 15 deletions(-) diff --git a/packages/server/lib/browsers/firefox.ts b/packages/server/lib/browsers/firefox.ts index 4fbbc2c9297d..e7ae8e63ecd7 100644 --- a/packages/server/lib/browsers/firefox.ts +++ b/packages/server/lib/browsers/firefox.ts @@ -347,7 +347,7 @@ toolbar { ` -let browserCriClient +let browserCriClient: BrowserCriClient | undefined export function _createDetachedInstance (browserInstance: BrowserInstance, browserCriClient?: BrowserCriClient): BrowserInstance { const detachedInstance: BrowserInstance = new EventEmitter() as BrowserInstance @@ -382,7 +382,7 @@ export function clearInstanceState (options: GracefulShutdownOptions = {}) { } export async function connectToNewSpec (browser: Browser, options: BrowserNewTabOpts, automation: Automation) { - await firefoxUtil.connectToNewSpec(options, automation, browserCriClient) + await firefoxUtil.connectToNewSpec(options, automation, browserCriClient!) } export function connectToExisting () { @@ -573,6 +573,10 @@ export async function open (browser: Browser, url: string, options: BrowserLaunc return originalBrowserKill.apply(browserInstance, args) } + + await utils.executeAfterBrowserLaunch(browser, { + webSocketDebuggerUrl: browserCriClient.getWebSocketDebuggerUrl(), + }) } catch (err) { errors.throwErr('FIREFOX_COULD_NOT_CONNECT', err) } diff --git a/packages/server/lib/browsers/utils.ts b/packages/server/lib/browsers/utils.ts index 457d6f1b943b..deed00e225cc 100644 --- a/packages/server/lib/browsers/utils.ts +++ b/packages/server/lib/browsers/utils.ts @@ -159,7 +159,7 @@ async function executeBeforeBrowserLaunch (browser, launchOptions: typeof defaul } interface AfterBrowserLaunchDetails { - webSocketDebuggerUrl: string + webSocketDebuggerUrl: string | never } async function executeAfterBrowserLaunch (browser: Browser, options: AfterBrowserLaunchDetails) { diff --git a/packages/server/lib/browsers/webkit.ts b/packages/server/lib/browsers/webkit.ts index a02540daba39..9cbbee478da2 100644 --- a/packages/server/lib/browsers/webkit.ts +++ b/packages/server/lib/browsers/webkit.ts @@ -147,5 +147,11 @@ export async function open (browser: Browser, url: string, options: BrowserLaunc } } + await utils.executeAfterBrowserLaunch(browser, { + get webSocketDebuggerUrl (): never { + throw new Error('The `webSocketDebuggerUrl` property is not currently supported in the `after:browser:launch` event handler details argument') + }, + }) + return new WkInstance() } diff --git a/packages/server/test/unit/browsers/firefox_spec.ts b/packages/server/test/unit/browsers/firefox_spec.ts index b0ab02ffacd3..31ee1bbad0c0 100644 --- a/packages/server/test/unit/browsers/firefox_spec.ts +++ b/packages/server/test/unit/browsers/firefox_spec.ts @@ -126,7 +126,7 @@ describe('lib/browsers/firefox', () => { context('#open', () => { beforeEach(function () { - this.browser = { name: 'firefox', channel: 'stable' } + this.browser = { name: 'firefox', channel: 'stable', majorVersion: 100 } this.automation = { use: sinon.stub().returns({}), } @@ -150,31 +150,40 @@ describe('lib/browsers/firefox', () => { sinon.stub(plugins, 'execute') sinon.stub(launch, 'launch').returns(this.browserInstance) sinon.stub(utils, 'writeExtension').resolves('/path/to/ext') + sinon.stub(utils, 'getPort').resolves(1234) sinon.spy(FirefoxProfile.prototype, 'setPreference') sinon.spy(FirefoxProfile.prototype, 'updatePreferences') + sinon.spy(FirefoxProfile.prototype, 'path') + + const browserCriClient: BrowserCriClient = sinon.createStubInstance(BrowserCriClient) - return sinon.spy(FirefoxProfile.prototype, 'path') + browserCriClient.attachToTargetUrl = sinon.stub().resolves({}) + browserCriClient.getWebSocketDebuggerUrl = sinon.stub().returns('ws://debugger') + browserCriClient.close = sinon.stub().resolves() + + sinon.stub(BrowserCriClient, 'create').resolves(browserCriClient) + sinon.stub(CdpAutomation, 'create').resolves() }) it('executes before:browser:launch if registered', function () { - plugins.has.returns(true) + plugins.has.withArgs('before:browser:launch').returns(true) plugins.execute.resolves(null) return firefox.open(this.browser, 'http://', this.options, this.automation).then(() => { - expect(plugins.execute).to.be.called + expect(plugins.execute).to.be.calledWith('before:browser:launch') }) }) it('does not execute before:browser:launch if not registered', function () { - plugins.has.returns(false) + plugins.has.withArgs('before:browser:launch').returns(false) return firefox.open(this.browser, 'http://', this.options, this.automation).then(() => { - expect(plugins.execute).not.to.be.called + expect(plugins.execute).not.to.be.calledWith('before:browser:launch') }) }) it('uses default preferences if before:browser:launch returns falsy value', function () { - plugins.has.returns(true) + plugins.has.withArgs('before:browser:launch').returns(true) plugins.execute.resolves(null) return firefox.open(this.browser, 'http://', this.options, this.automation).then(() => { @@ -183,7 +192,7 @@ describe('lib/browsers/firefox', () => { }) it('uses default preferences if before:browser:launch returns object with non-object preferences', function () { - plugins.has.returns(true) + plugins.has.withArgs('before:browser:launch').returns(true) plugins.execute.resolves({ preferences: [], }) @@ -194,7 +203,7 @@ describe('lib/browsers/firefox', () => { }) it('sets preferences if returned by before:browser:launch', function () { - plugins.has.returns(true) + plugins.has.withArgs('before:browser:launch').returns(true) plugins.execute.resolves({ preferences: { 'foo': 'bar' }, }) @@ -205,7 +214,7 @@ describe('lib/browsers/firefox', () => { }) it('adds extensions returned by before:browser:launch, along with cypress extension', function () { - plugins.has.returns(true) + plugins.has.withArgs('before:browser:launch').returns(true) plugins.execute.resolves({ extensions: ['/path/to/user/ext'], }) @@ -218,7 +227,7 @@ describe('lib/browsers/firefox', () => { }) it('adds only cypress extension if before:browser:launch returns object with non-array extensions', function () { - plugins.has.returns(true) + plugins.has.withArgs('before:browser:launch').returns(true) plugins.execute.resolves({ extensions: 'not-an-array', }) @@ -331,12 +340,13 @@ describe('lib/browsers/firefox', () => { it('launches with the url and args', function () { return firefox.open(this.browser, 'http://', this.options, this.automation).then(() => { - expect(launch.launch).to.be.calledWith(this.browser, 'about:blank', undefined, [ + expect(launch.launch).to.be.calledWith(this.browser, 'about:blank', 1234, [ '-marionette', '-new-instance', '-foreground', '-start-debugger-server', '-no-remote', + '--remote-debugging-port=1234', '-profile', '/path/to/appData/firefox-stable/interactive', ]) @@ -410,6 +420,25 @@ describe('lib/browsers/firefox', () => { }) }) + it('executes after:browser:launch if registered', function () { + plugins.has.withArgs('after:browser:launch').returns(true) + plugins.execute.resolves(null) + + return firefox.open(this.browser, 'http://', this.options, this.automation).then(() => { + expect(plugins.execute).to.be.calledWith('after:browser:launch', this.browser, { + webSocketDebuggerUrl: 'ws://debugger', + }) + }) + }) + + it('does not execute after:browser:launch if not registered', function () { + plugins.has.withArgs('after:browser:launch').returns(false) + + return firefox.open(this.browser, 'http://', this.options, this.automation).then(() => { + expect(plugins.execute).not.to.be.calledWith('after:browser:launch') + }) + }) + context('returns BrowserInstance', function () { it('from browsers.launch', async function () { const instance = await firefox.open(this.browser, 'http://', this.options, this.automation) From 054961d6eef6e7cb369a35414d83146912649bf5 Mon Sep 17 00:00:00 2001 From: Chris Breiding Date: Mon, 30 Oct 2023 11:34:12 -0400 Subject: [PATCH 04/12] fix electron spec --- packages/server/test/unit/browsers/electron_spec.js | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/packages/server/test/unit/browsers/electron_spec.js b/packages/server/test/unit/browsers/electron_spec.js index 3a81c476398c..ea5dfaf003a0 100644 --- a/packages/server/test/unit/browsers/electron_spec.js +++ b/packages/server/test/unit/browsers/electron_spec.js @@ -80,6 +80,7 @@ describe('lib/browsers/electron', () => { attachToTargetUrl: sinon.stub().resolves(this.pageCriClient), currentlyAttachedTarget: this.pageCriClient, close: sinon.stub().resolves(), + getWebSocketDebuggerUrl: sinon.stub().returns('ws://debugger'), } sinon.stub(BrowserCriClient, 'create').resolves(this.browserCriClient) @@ -212,14 +213,12 @@ describe('lib/browsers/electron', () => { }) it('sends after:browser:launch with debugger url', function () { - const browser = { isHeadless: true } - plugins.has.returns(true) plugins.execute.resolves(null) return electron.open('electron', this.url, this.options, this.automation) .then(() => { - expect(plugins.execute).to.be.calledWith('after:browser:launch', browser, { + expect(plugins.execute).to.be.calledWith('after:browser:launch', 'electron', { webSocketDebuggerUrl: 'ws://debugger', }) }) From 4ba7fb2d9e02115b29c85e89a843336fc83f9220 Mon Sep 17 00:00:00 2001 From: Chris Breiding Date: Mon, 30 Oct 2023 15:03:30 -0400 Subject: [PATCH 05/12] un-nest browser-cri tests --- .../unit/browsers/browser-cri-client_spec.ts | 266 +++++++++--------- 1 file changed, 133 insertions(+), 133 deletions(-) diff --git a/packages/server/test/unit/browsers/browser-cri-client_spec.ts b/packages/server/test/unit/browsers/browser-cri-client_spec.ts index 4beed34046f2..40fd09e09fac 100644 --- a/packages/server/test/unit/browsers/browser-cri-client_spec.ts +++ b/packages/server/test/unit/browsers/browser-cri-client_spec.ts @@ -127,200 +127,200 @@ describe('lib/browsers/cri-client', function () { expect(criImport.Version).to.be.calledTwice }) + }) - context('#ensureMinimumProtocolVersion', function () { - function withProtocolVersion (actual, test) { - return getClient() - .then((client: any) => { - client.versionInfo = { 'Protocol-Version': actual } - - return client.ensureMinimumProtocolVersion(test) - }) - } + context('#ensureMinimumProtocolVersion', function () { + function withProtocolVersion (actual, test) { + return getClient() + .then((client: any) => { + client.versionInfo = { 'Protocol-Version': actual } - it('resolves if protocolVersion = current', function () { - return expect(withProtocolVersion('1.3', '1.3')).to.be.fulfilled + return client.ensureMinimumProtocolVersion(test) }) + } - it('resolves if protocolVersion > current', function () { - return expect(withProtocolVersion('1.4', '1.3')).to.be.fulfilled - }) + it('resolves if protocolVersion = current', function () { + return expect(withProtocolVersion('1.3', '1.3')).to.be.fulfilled + }) - it('rejects if protocolVersion < current', function () { - return expect(withProtocolVersion('1.2', '1.3')).to.be - .rejected.then((err) => { - expect(stripAnsi(err.message)).to.eq(`A minimum CDP version of 1.3 is required, but the current browser has 1.2.`) - }) + it('resolves if protocolVersion > current', function () { + return expect(withProtocolVersion('1.4', '1.3')).to.be.fulfilled + }) + + it('rejects if protocolVersion < current', function () { + return expect(withProtocolVersion('1.2', '1.3')).to.be + .rejected.then((err) => { + expect(stripAnsi(err.message)).to.eq(`A minimum CDP version of 1.3 is required, but the current browser has 1.2.`) }) }) + }) - context('#attachToTargetUrl', function () { - it('creates a page client when the passed in url is found', async function () { - const mockPageClient = {} + context('#attachToTargetUrl', function () { + it('creates a page client when the passed in url is found', async function () { + const mockPageClient = {} - send.withArgs('Target.getTargets').resolves({ targetInfos: [{ targetId: '1', url: 'http://foo.com' }, { targetId: '2', url: 'http://bar.com' }] }) - criClientCreateStub.withArgs({ target: '1', onAsynchronousError: onError, host: HOST, port: PORT, protocolManager: undefined, fullyManageTabs: undefined, browserClient: { on, send, close } }).resolves(mockPageClient) + send.withArgs('Target.getTargets').resolves({ targetInfos: [{ targetId: '1', url: 'http://foo.com' }, { targetId: '2', url: 'http://bar.com' }] }) + criClientCreateStub.withArgs({ target: '1', onAsynchronousError: onError, host: HOST, port: PORT, protocolManager: undefined, fullyManageTabs: undefined, browserClient: { on, send, close } }).resolves(mockPageClient) - const browserClient = await getClient() + const browserClient = await getClient() - const client = await browserClient.attachToTargetUrl('http://foo.com') + const client = await browserClient.attachToTargetUrl('http://foo.com') - expect(client).to.be.equal(mockPageClient) - }) + expect(client).to.be.equal(mockPageClient) + }) - it('creates a page client when the passed in url is found and notifies the protocol manager and fully managed tabs', async function () { - const mockPageClient = {} - const protocolManager: any = { - connectToBrowser: sinon.stub().resolves(), - } + it('creates a page client when the passed in url is found and notifies the protocol manager and fully managed tabs', async function () { + const mockPageClient = {} + const protocolManager: any = { + connectToBrowser: sinon.stub().resolves(), + } - send.withArgs('Target.getTargets').resolves({ targetInfos: [{ targetId: '1', url: 'http://foo.com' }, { targetId: '2', url: 'http://bar.com' }] }) - send.withArgs('Target.setDiscoverTargets', { discover: true }) - on.withArgs('Target.targetDestroyed', sinon.match.func) - criClientCreateStub.withArgs({ target: '1', onAsynchronousError: onError, host: HOST, port: PORT, protocolManager, fullyManageTabs: true, browserClient: { on, send, close } }).resolves(mockPageClient) + send.withArgs('Target.getTargets').resolves({ targetInfos: [{ targetId: '1', url: 'http://foo.com' }, { targetId: '2', url: 'http://bar.com' }] }) + send.withArgs('Target.setDiscoverTargets', { discover: true }) + on.withArgs('Target.targetDestroyed', sinon.match.func) + criClientCreateStub.withArgs({ target: '1', onAsynchronousError: onError, host: HOST, port: PORT, protocolManager, fullyManageTabs: true, browserClient: { on, send, close } }).resolves(mockPageClient) - const browserClient = await getClient({ protocolManager, fullyManageTabs: true }) + const browserClient = await getClient({ protocolManager, fullyManageTabs: true }) - const client = await browserClient.attachToTargetUrl('http://foo.com') + const client = await browserClient.attachToTargetUrl('http://foo.com') - expect(client).to.be.equal(mockPageClient) - expect(protocolManager.connectToBrowser).to.be.calledWith(client) - }) + expect(client).to.be.equal(mockPageClient) + expect(protocolManager.connectToBrowser).to.be.calledWith(client) + }) - it('creates a page client when the passed in url is found and notifies the protocol manager and fully managed tabs and attaching to target throws', async function () { - const mockPageClient = {} - const protocolManager: any = { - connectToBrowser: sinon.stub().resolves(), - } + it('creates a page client when the passed in url is found and notifies the protocol manager and fully managed tabs and attaching to target throws', async function () { + const mockPageClient = {} + const protocolManager: any = { + connectToBrowser: sinon.stub().resolves(), + } - send.withArgs('Target.getTargets').resolves({ targetInfos: [{ targetId: '1', url: 'http://foo.com' }, { targetId: '2', url: 'http://bar.com' }] }) - send.withArgs('Target.setDiscoverTargets', { discover: true }) - on.withArgs('Target.targetDestroyed', sinon.match.func) + send.withArgs('Target.getTargets').resolves({ targetInfos: [{ targetId: '1', url: 'http://foo.com' }, { targetId: '2', url: 'http://bar.com' }] }) + send.withArgs('Target.setDiscoverTargets', { discover: true }) + on.withArgs('Target.targetDestroyed', sinon.match.func) - send.withArgs('Network.enable').throws(new Error('ProtocolError: Inspected target navigated or closed')) + send.withArgs('Network.enable').throws(new Error('ProtocolError: Inspected target navigated or closed')) - criClientCreateStub.withArgs({ target: '1', onAsynchronousError: onError, host: HOST, port: PORT, protocolManager, fullyManageTabs: true, browserClient: { on, send, close } }).resolves(mockPageClient) + criClientCreateStub.withArgs({ target: '1', onAsynchronousError: onError, host: HOST, port: PORT, protocolManager, fullyManageTabs: true, browserClient: { on, send, close } }).resolves(mockPageClient) - const browserClient = await getClient({ protocolManager, fullyManageTabs: true }) + const browserClient = await getClient({ protocolManager, fullyManageTabs: true }) - const client = await browserClient.attachToTargetUrl('http://foo.com') + const client = await browserClient.attachToTargetUrl('http://foo.com') - expect(client).to.be.equal(mockPageClient) - expect(protocolManager.connectToBrowser).to.be.calledWith(client) + expect(client).to.be.equal(mockPageClient) + expect(protocolManager.connectToBrowser).to.be.calledWith(client) - // This would throw if the error was not caught - await on.withArgs('Target.attachedToTarget').args[0][1]({ targetInfo: { type: 'worker' } }) - }) + // This would throw if the error was not caught + await on.withArgs('Target.attachedToTarget').args[0][1]({ targetInfo: { type: 'worker' } }) + }) - it('retries when the passed in url is not found', async function () { - sinon.stub(protocol, '_getDelayMsForRetry') - .onFirstCall().returns(100) - .onSecondCall().returns(100) - .onThirdCall().returns(100) + it('retries when the passed in url is not found', async function () { + sinon.stub(protocol, '_getDelayMsForRetry') + .onFirstCall().returns(100) + .onSecondCall().returns(100) + .onThirdCall().returns(100) - const mockPageClient = {} + const mockPageClient = {} - send.withArgs('Target.getTargets').resolves({ targetInfos: [{ targetId: '1', url: 'http://foo.com' }, { targetId: '2', url: 'http://bar.com' }] }) - send.withArgs('Target.getTargets').resolves({ targetInfos: [{ targetId: '1', url: 'http://foo.com' }, { targetId: '2', url: 'http://bar.com' }] }) - send.withArgs('Target.getTargets').resolves({ targetInfos: [{ targetId: '1', url: 'http://foo.com' }, { targetId: '2', url: 'http://bar.com' }, { targetId: '3', url: 'http://baz.com' }] }) - criClientCreateStub.withArgs({ target: '1', onAsynchronousError: onError, host: HOST, port: PORT, protocolManager: undefined, fullyManageTabs: undefined, browserClient: { on, send, close } }).resolves(mockPageClient) + send.withArgs('Target.getTargets').resolves({ targetInfos: [{ targetId: '1', url: 'http://foo.com' }, { targetId: '2', url: 'http://bar.com' }] }) + send.withArgs('Target.getTargets').resolves({ targetInfos: [{ targetId: '1', url: 'http://foo.com' }, { targetId: '2', url: 'http://bar.com' }] }) + send.withArgs('Target.getTargets').resolves({ targetInfos: [{ targetId: '1', url: 'http://foo.com' }, { targetId: '2', url: 'http://bar.com' }, { targetId: '3', url: 'http://baz.com' }] }) + criClientCreateStub.withArgs({ target: '1', onAsynchronousError: onError, host: HOST, port: PORT, protocolManager: undefined, fullyManageTabs: undefined, browserClient: { on, send, close } }).resolves(mockPageClient) - const browserClient = await getClient() + const browserClient = await getClient() - const client = await browserClient.attachToTargetUrl('http://foo.com') + const client = await browserClient.attachToTargetUrl('http://foo.com') - expect(client).to.be.equal(mockPageClient) - }) + expect(client).to.be.equal(mockPageClient) + }) - it('throws when the passed in url is not found after retrying', async function () { - sinon.stub(protocol, '_getDelayMsForRetry') - .onFirstCall().returns(100) - .onSecondCall().returns(undefined) + it('throws when the passed in url is not found after retrying', async function () { + sinon.stub(protocol, '_getDelayMsForRetry') + .onFirstCall().returns(100) + .onSecondCall().returns(undefined) - const mockPageClient = {} + const mockPageClient = {} - send.withArgs('Target.getTargets').resolves({ targetInfos: [{ targetId: '1', url: 'http://foo.com' }, { targetId: '2', url: 'http://bar.com' }] }) - send.withArgs('Target.getTargets').resolves({ targetInfos: [{ targetId: '1', url: 'http://foo.com' }, { targetId: '2', url: 'http://bar.com' }] }) - criClientCreateStub.withArgs({ target: '1', onAsynchronousError: onError, host: HOST, port: PORT, protocolManager: undefined, fullyManageTabs: undefined, browserClient: { on, send, close } }).resolves(mockPageClient) + send.withArgs('Target.getTargets').resolves({ targetInfos: [{ targetId: '1', url: 'http://foo.com' }, { targetId: '2', url: 'http://bar.com' }] }) + send.withArgs('Target.getTargets').resolves({ targetInfos: [{ targetId: '1', url: 'http://foo.com' }, { targetId: '2', url: 'http://bar.com' }] }) + criClientCreateStub.withArgs({ target: '1', onAsynchronousError: onError, host: HOST, port: PORT, protocolManager: undefined, fullyManageTabs: undefined, browserClient: { on, send, close } }).resolves(mockPageClient) - const browserClient = await getClient() + const browserClient = await getClient() - await expect(browserClient.attachToTargetUrl('http://baz.com')).to.be.rejected - }) + await expect(browserClient.attachToTargetUrl('http://baz.com')).to.be.rejected }) + }) - context('#resetBrowserTargets', function () { - it('closes the currently attached target while keeping a tab open', async function () { - const mockCurrentlyAttachedTarget = { - targetId: '100', - close: sinon.stub().resolves(sinon.stub().resolves()), - } + context('#resetBrowserTargets', function () { + it('closes the currently attached target while keeping a tab open', async function () { + const mockCurrentlyAttachedTarget = { + targetId: '100', + close: sinon.stub().resolves(sinon.stub().resolves()), + } - const mockUpdatedCurrentlyAttachedTarget = { - targetId: '101', - } + const mockUpdatedCurrentlyAttachedTarget = { + targetId: '101', + } - send.withArgs('Target.createTarget', { url: 'about:blank' }).resolves(mockUpdatedCurrentlyAttachedTarget) - send.withArgs('Target.closeTarget', { targetId: '100' }).resolves() - criClientCreateStub.withArgs({ target: '101', onAsynchronousError: onError, host: HOST, port: PORT, protocolManager: undefined, fullyManageTabs: undefined }).resolves(mockUpdatedCurrentlyAttachedTarget) + send.withArgs('Target.createTarget', { url: 'about:blank' }).resolves(mockUpdatedCurrentlyAttachedTarget) + send.withArgs('Target.closeTarget', { targetId: '100' }).resolves() + criClientCreateStub.withArgs({ target: '101', onAsynchronousError: onError, host: HOST, port: PORT, protocolManager: undefined, fullyManageTabs: undefined }).resolves(mockUpdatedCurrentlyAttachedTarget) - const browserClient = await getClient() as any + const browserClient = await getClient() as any - browserClient.currentlyAttachedTarget = mockCurrentlyAttachedTarget + browserClient.currentlyAttachedTarget = mockCurrentlyAttachedTarget - await browserClient.resetBrowserTargets(true) + await browserClient.resetBrowserTargets(true) - expect(mockCurrentlyAttachedTarget.close).to.be.called - expect(browserClient.currentlyAttachedTarget).to.eql(mockUpdatedCurrentlyAttachedTarget) - }) + expect(mockCurrentlyAttachedTarget.close).to.be.called + expect(browserClient.currentlyAttachedTarget).to.eql(mockUpdatedCurrentlyAttachedTarget) + }) - it('closes the currently attached target without keeping a tab open', async function () { - const mockCurrentlyAttachedTarget = { - targetId: '100', - close: sinon.stub().resolves(sinon.stub().resolves()), - } + it('closes the currently attached target without keeping a tab open', async function () { + const mockCurrentlyAttachedTarget = { + targetId: '100', + close: sinon.stub().resolves(sinon.stub().resolves()), + } - send.withArgs('Target.closeTarget', { targetId: '100' }).resolves() + send.withArgs('Target.closeTarget', { targetId: '100' }).resolves() - const browserClient = await getClient() as any + const browserClient = await getClient() as any - browserClient.currentlyAttachedTarget = mockCurrentlyAttachedTarget + browserClient.currentlyAttachedTarget = mockCurrentlyAttachedTarget - await browserClient.resetBrowserTargets(false) + await browserClient.resetBrowserTargets(false) - expect(mockCurrentlyAttachedTarget.close).to.be.called - }) + expect(mockCurrentlyAttachedTarget.close).to.be.called + }) - it('throws when there is no currently attached target', async function () { - const browserClient = await getClient() as any + it('throws when there is no currently attached target', async function () { + const browserClient = await getClient() as any - await expect(browserClient.resetBrowserTargets()).to.be.rejected - }) + await expect(browserClient.resetBrowserTargets()).to.be.rejected }) + }) - context('#close', function () { - it('closes the currently attached target if it exists and the browser client', async function () { - const mockCurrentlyAttachedTarget = { - close: sinon.stub().resolves(), - } + context('#close', function () { + it('closes the currently attached target if it exists and the browser client', async function () { + const mockCurrentlyAttachedTarget = { + close: sinon.stub().resolves(), + } - const browserClient = await getClient() as any + const browserClient = await getClient() as any - browserClient.currentlyAttachedTarget = mockCurrentlyAttachedTarget + browserClient.currentlyAttachedTarget = mockCurrentlyAttachedTarget - await browserClient.close() + await browserClient.close() - expect(mockCurrentlyAttachedTarget.close).to.be.called - expect(close).to.be.called - }) + expect(mockCurrentlyAttachedTarget.close).to.be.called + expect(close).to.be.called + }) - it('just the browser client with no currently attached target', async function () { - const browserClient = await getClient() as any + it('just the browser client with no currently attached target', async function () { + const browserClient = await getClient() as any - await browserClient.close() + await browserClient.close() - expect(close).to.be.called - }) + expect(close).to.be.called }) }) }) From 96baf2c8598fa018456d9ed675918c2f40b7b0b9 Mon Sep 17 00:00:00 2001 From: Chris Breiding Date: Tue, 31 Oct 2023 10:49:40 -0400 Subject: [PATCH 06/12] Update cli/types/cypress.d.ts --- cli/types/cypress.d.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/cli/types/cypress.d.ts b/cli/types/cypress.d.ts index b4324fdc9d70..f4c69ec165cf 100644 --- a/cli/types/cypress.d.ts +++ b/cli/types/cypress.d.ts @@ -6013,7 +6013,8 @@ declare namespace Cypress { webSocketDebuggerUrl: string } - interface BeforeBrowserLaunchOptions { extensions: string[] + interface BeforeBrowserLaunchOptions { + extensions: string[] preferences: { [key: string]: any } args: string[] env: { [key: string]: any } From 2d862affcacd5f810dbcc2c4e888e10173e1ddff Mon Sep 17 00:00:00 2001 From: Chris Breiding Date: Fri, 3 Nov 2023 09:58:02 -0400 Subject: [PATCH 07/12] add comment --- packages/server/test/unit/browsers/firefox_spec.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/packages/server/test/unit/browsers/firefox_spec.ts b/packages/server/test/unit/browsers/firefox_spec.ts index 31ee1bbad0c0..6827d10f0fca 100644 --- a/packages/server/test/unit/browsers/firefox_spec.ts +++ b/packages/server/test/unit/browsers/firefox_spec.ts @@ -126,6 +126,8 @@ describe('lib/browsers/firefox', () => { context('#open', () => { beforeEach(function () { + // majorVersion >= 86 indicates CDP support for Firefox, which provides + // the CDP debugger URL for the after:browser:launch tests this.browser = { name: 'firefox', channel: 'stable', majorVersion: 100 } this.automation = { use: sinon.stub().returns({}), From 03ae6a62777b6a1a8863aee7ce7bd2f5a5e59fdf Mon Sep 17 00:00:00 2001 From: Chris Breiding Date: Fri, 3 Nov 2023 10:51:10 -0400 Subject: [PATCH 08/12] add webSocketDebuggerUrl to webkit's after:browser:launch --- packages/server/lib/browsers/webkit.ts | 7 +- .../server/test/unit/browsers/webkit_spec.ts | 71 ++++++++++++++++++- 2 files changed, 73 insertions(+), 5 deletions(-) diff --git a/packages/server/lib/browsers/webkit.ts b/packages/server/lib/browsers/webkit.ts index 9cbbee478da2..26955261f4e5 100644 --- a/packages/server/lib/browsers/webkit.ts +++ b/packages/server/lib/browsers/webkit.ts @@ -101,7 +101,8 @@ export async function open (browser: Browser, url: string, options: BrowserLaunc removeBadExitListener() - const pwBrowser = await pw.webkit.connect(pwServer.wsEndpoint()) + const websocketUrl = pwServer.wsEndpoint() + const pwBrowser = await pw.webkit.connect(websocketUrl) wkAutomation = await WebKitAutomation.create({ automation, @@ -148,9 +149,7 @@ export async function open (browser: Browser, url: string, options: BrowserLaunc } await utils.executeAfterBrowserLaunch(browser, { - get webSocketDebuggerUrl (): never { - throw new Error('The `webSocketDebuggerUrl` property is not currently supported in the `after:browser:launch` event handler details argument') - }, + webSocketDebuggerUrl: websocketUrl, }) return new WkInstance() diff --git a/packages/server/test/unit/browsers/webkit_spec.ts b/packages/server/test/unit/browsers/webkit_spec.ts index 9906936c98a8..52a15226939a 100644 --- a/packages/server/test/unit/browsers/webkit_spec.ts +++ b/packages/server/test/unit/browsers/webkit_spec.ts @@ -1,12 +1,81 @@ require('../../spec_helper') import { expect } from 'chai' +import proxyquire from 'proxyquire' +import utils from '../../../lib/browsers/utils' +import * as plugins from '../../../lib/plugins' -import * as webkit from '../../../lib/browsers/webkit' +function getWebkit (dependencies = {}) { + return proxyquire('../../../lib/browsers/webkit', dependencies) as typeof import('../../../lib/browsers/webkit') +} describe('lib/browsers/webkit', () => { + context('#open', () => { + let browser + let options + let automation + let webkit + + beforeEach(async () => { + browser = {} + options = { experimentalWebKitSupport: true } + automation = { use: sinon.stub() } + + const launchOptions = { + extensions: [], + args: [], + preferences: { }, + } + const pwWebkit = { + webkit: { + connect: sinon.stub().resolves({ + on: sinon.stub(), + }), + launchServer: sinon.stub().resolves({ + wsEndpoint: sinon.stub().returns('ws://debugger'), + process: sinon.stub().returns({ pid: 'pid' }), + }), + }, + } + const wkAutomation = { + WebKitAutomation: { + create: sinon.stub().resolves({}), + }, + } + + sinon.stub(utils, 'executeBeforeBrowserLaunch').resolves(launchOptions as any) + sinon.stub(plugins, 'execute').resolves() + sinon.stub(plugins, 'has') + + webkit = getWebkit({ + 'playwright-webkit': pwWebkit, + './webkit-automation': wkAutomation, + }) + }) + + it('sends after:browser:launch with debugger url', async () => { + (plugins.has as any).returns(true) + + await webkit.open(browser as any, 'http://the.url', options as any, automation as any) + + expect(plugins.execute).to.be.calledWith('after:browser:launch', browser, { + webSocketDebuggerUrl: 'ws://debugger', + }) + }) + + it('executeAfterBrowserLaunch is noop if after:browser:launch is not registered', async () => { + (plugins.has as any).returns(false) + + await webkit.open(browser as any, 'http://the.url', options as any, automation as any) + + expect(plugins.execute).not.to.be.calledWith('after:browser:launch') + }) + }) + context('#connectProtocolToBrowser', () => { it('throws error', () => { + const webkit = getWebkit() + expect(webkit.connectProtocolToBrowser).to.throw('Protocol is not yet supported in WebKit.') }) }) From bd7b832111ab181dbddc5b423399ed38bf1944ff Mon Sep 17 00:00:00 2001 From: Chris Breiding Date: Mon, 6 Nov 2023 10:23:58 -0500 Subject: [PATCH 09/12] fix tests --- .../server/test/unit/browsers/webkit_spec.ts | 6 +- packages/server/test/unit/util/args_spec.js | 68 +++++++++---------- 2 files changed, 36 insertions(+), 38 deletions(-) diff --git a/packages/server/test/unit/browsers/webkit_spec.ts b/packages/server/test/unit/browsers/webkit_spec.ts index 52a15226939a..708224b6ecd7 100644 --- a/packages/server/test/unit/browsers/webkit_spec.ts +++ b/packages/server/test/unit/browsers/webkit_spec.ts @@ -1,12 +1,10 @@ -require('../../spec_helper') - +import { proxyquire } from '../../spec_helper' import { expect } from 'chai' -import proxyquire from 'proxyquire' import utils from '../../../lib/browsers/utils' import * as plugins from '../../../lib/plugins' function getWebkit (dependencies = {}) { - return proxyquire('../../../lib/browsers/webkit', dependencies) as typeof import('../../../lib/browsers/webkit') + return proxyquire('../lib/browsers/webkit', dependencies) as typeof import('../../../lib/browsers/webkit') } describe('lib/browsers/webkit', () => { diff --git a/packages/server/test/unit/util/args_spec.js b/packages/server/test/unit/util/args_spec.js index 05ddb06210ec..ff7e1ba4af00 100644 --- a/packages/server/test/unit/util/args_spec.js +++ b/packages/server/test/unit/util/args_spec.js @@ -8,7 +8,7 @@ const minimist = require('minimist') const argsUtil = require(`../../../lib/util/args`) const getWindowsProxyUtil = require(`../../../lib/util/get-windows-proxy`) -const cwd = process.cwd() +const getCwd = () => process.cwd() describe('lib/util/args', () => { beforeEach(function () { @@ -92,7 +92,7 @@ describe('lib/util/args', () => { context('--project', () => { it('sets projectRoot', function () { - const projectRoot = path.resolve(cwd, './foo/bar') + const projectRoot = path.resolve(getCwd(), './foo/bar') const options = this.setup('--project', './foo/bar') expect(options.projectRoot).to.eq(projectRoot) @@ -113,7 +113,7 @@ describe('lib/util/args', () => { context('--run-project', () => { it('sets projectRoot', function () { - const projectRoot = path.resolve(cwd, '/baz') + const projectRoot = path.resolve(getCwd(), '/baz') const options = this.setup('--run-project', '/baz') expect(options.projectRoot).to.eq(projectRoot) @@ -138,16 +138,16 @@ describe('lib/util/args', () => { it('converts to array', function () { const options = this.setup('--run-project', 'foo', '--spec', 'cypress/integration/a.js,cypress/integration/b.js,cypress/integration/c.js') - expect(options.spec[0]).to.eq(`${cwd}/cypress/integration/a.js`) - expect(options.spec[1]).to.eq(`${cwd}/cypress/integration/b.js`) + expect(options.spec[0]).to.eq(`${getCwd()}/cypress/integration/a.js`) + expect(options.spec[1]).to.eq(`${getCwd()}/cypress/integration/b.js`) - expect(options.spec[2]).to.eq(`${cwd}/cypress/integration/c.js`) + expect(options.spec[2]).to.eq(`${getCwd()}/cypress/integration/c.js`) }) it('discards wrapping single quotes', function () { const options = this.setup('--run-project', 'foo', '--spec', '\'cypress/integration/foo_spec.js\'') - expect(options.spec[0]).to.eq(`${cwd}/cypress/integration/foo_spec.js`) + expect(options.spec[0]).to.eq(`${getCwd()}/cypress/integration/foo_spec.js`) }) it('throws if argument cannot be parsed', function () { @@ -165,56 +165,56 @@ describe('lib/util/args', () => { it('should be correctly parsing globs with lists & ranges', function () { const options = this.setup('--spec', 'cypress/integration/{[!a]*.spec.js,sub1,{sub2,sub3/sub4}}/*.js') - expect(options.spec[0]).to.eq(`${cwd}/cypress/integration/{[!a]*.spec.js,sub1,{sub2,sub3/sub4}}/*.js`) + expect(options.spec[0]).to.eq(`${getCwd()}/cypress/integration/{[!a]*.spec.js,sub1,{sub2,sub3/sub4}}/*.js`) }) it('should be correctly parsing globs with a mix of lists, ranges & regular paths', function () { const options = this.setup('--spec', 'cypress/integration/{[!a]*.spec.js,sub1,{sub2,sub3/sub4}}/*.js,cypress/integration/foo.spec.js') - expect(options.spec[0]).to.eq(`${cwd}/cypress/integration/{[!a]*.spec.js,sub1,{sub2,sub3/sub4}}/*.js`) - expect(options.spec[1]).to.eq(`${cwd}/cypress/integration/foo.spec.js`) + expect(options.spec[0]).to.eq(`${getCwd()}/cypress/integration/{[!a]*.spec.js,sub1,{sub2,sub3/sub4}}/*.js`) + expect(options.spec[1]).to.eq(`${getCwd()}/cypress/integration/foo.spec.js`) }) it('should be correctly parsing single glob with range', function () { const options = this.setup('--spec', 'cypress/integration/[a-c]*/**') - expect(options.spec[0]).to.eq(`${cwd}/cypress/integration/[a-c]*/**`) + expect(options.spec[0]).to.eq(`${getCwd()}/cypress/integration/[a-c]*/**`) }) it('should be correctly parsing single glob with list', function () { const options = this.setup('--spec', 'cypress/integration/{a,b,c}/*.js') - expect(options.spec[0]).to.eq(`${cwd}/cypress/integration/{a,b,c}/*.js`) + expect(options.spec[0]).to.eq(`${getCwd()}/cypress/integration/{a,b,c}/*.js`) }) // https://github.com/cypress-io/cypress/issues/20794 it('does not split at filename with glob pattern', function () { const options = this.setup('--spec', 'cypress/integration/foo/bar/[baz]/test.ts,cypress/integration/foo1/bar/[baz]/test.ts,cypress/integration/foo2/bar/baz/test.ts,cypress/integration/foo3/bar/baz/foo4.ts') - expect(options.spec[0]).to.eq(`${cwd}/cypress/integration/foo/bar/[baz]/test.ts`) - expect(options.spec[1]).to.eq(`${cwd}/cypress/integration/foo1/bar/[baz]/test.ts`) - expect(options.spec[2]).to.eq(`${cwd}/cypress/integration/foo2/bar/baz/test.ts`) - expect(options.spec[3]).to.eq(`${cwd}/cypress/integration/foo3/bar/baz/foo4.ts`) + expect(options.spec[0]).to.eq(`${getCwd()}/cypress/integration/foo/bar/[baz]/test.ts`) + expect(options.spec[1]).to.eq(`${getCwd()}/cypress/integration/foo1/bar/[baz]/test.ts`) + expect(options.spec[2]).to.eq(`${getCwd()}/cypress/integration/foo2/bar/baz/test.ts`) + expect(options.spec[3]).to.eq(`${getCwd()}/cypress/integration/foo3/bar/baz/foo4.ts`) }) // https://github.com/cypress-io/cypress/issues/20794 it('correctly splits at comma with glob pattern', function () { const options = this.setup('--spec', 'cypress/integration/foo/bar/baz/test.ts,cypress/integration/foo1/bar/[baz]/test.ts,cypress/integration/foo2/bar/baz/test.ts,cypress/integration/foo3/bar/baz/foo4.ts') - expect(options.spec[0]).to.eq(`${cwd}/cypress/integration/foo/bar/baz/test.ts`) - expect(options.spec[1]).to.eq(`${cwd}/cypress/integration/foo1/bar/[baz]/test.ts`) - expect(options.spec[2]).to.eq(`${cwd}/cypress/integration/foo2/bar/baz/test.ts`) - expect(options.spec[3]).to.eq(`${cwd}/cypress/integration/foo3/bar/baz/foo4.ts`) + expect(options.spec[0]).to.eq(`${getCwd()}/cypress/integration/foo/bar/baz/test.ts`) + expect(options.spec[1]).to.eq(`${getCwd()}/cypress/integration/foo1/bar/[baz]/test.ts`) + expect(options.spec[2]).to.eq(`${getCwd()}/cypress/integration/foo2/bar/baz/test.ts`) + expect(options.spec[3]).to.eq(`${getCwd()}/cypress/integration/foo3/bar/baz/foo4.ts`) }) // https://github.com/cypress-io/cypress/issues/20794 it('correctly splits at comma with escaped glob pattern', function () { const options = this.setup('--spec', 'cypress/integration/foo/bar/\[baz\]/test.ts,cypress/integration/foo1/bar/\[baz1\]/test.ts,cypress/integration/foo2/bar/baz/test.ts,cypress/integration/foo3/bar/baz/foo4.ts') - expect(options.spec[0]).to.eq(`${cwd}/cypress/integration/foo/bar/\[baz\]/test.ts`) - expect(options.spec[1]).to.eq(`${cwd}/cypress/integration/foo1/bar/\[baz1\]/test.ts`) - expect(options.spec[2]).to.eq(`${cwd}/cypress/integration/foo2/bar/baz/test.ts`) - expect(options.spec[3]).to.eq(`${cwd}/cypress/integration/foo3/bar/baz/foo4.ts`) + expect(options.spec[0]).to.eq(`${getCwd()}/cypress/integration/foo/bar/\[baz\]/test.ts`) + expect(options.spec[1]).to.eq(`${getCwd()}/cypress/integration/foo1/bar/\[baz1\]/test.ts`) + expect(options.spec[2]).to.eq(`${getCwd()}/cypress/integration/foo2/bar/baz/test.ts`) + expect(options.spec[3]).to.eq(`${getCwd()}/cypress/integration/foo3/bar/baz/foo4.ts`) }) }) @@ -600,9 +600,9 @@ describe('lib/util/args', () => { this.hosts = { a: 'b', b: 'c' } this.blockHosts = ['a.com', 'b.com'] this.specs = [ - path.join(cwd, 'foo'), - path.join(cwd, 'bar'), - path.join(cwd, 'baz'), + path.join(getCwd(), 'foo'), + path.join(getCwd(), 'bar'), + path.join(getCwd(), 'baz'), ] this.env = { @@ -643,7 +643,7 @@ describe('lib/util/args', () => { it('backs up env, config, reporterOptions, spec', function () { expect(this.obj).to.deep.eq({ - cwd, + cwd: getCwd(), _: [], config: this.config, invokedFromCli: false, @@ -666,12 +666,12 @@ describe('lib/util/args', () => { expect(args).to.deep.eq([ `--config=${mergedConfig}`, - `--cwd=${cwd}`, + `--cwd=${getCwd()}`, `--spec=${JSON.stringify(this.specs)}`, ]) expect(argsUtil.toObject(args)).to.deep.eq({ - cwd, + cwd: getCwd(), _: [], invokedFromCli: true, config: this.config, @@ -684,7 +684,7 @@ describe('lib/util/args', () => { expect(result).to.deep.equal({ ciBuildId: '1e100', - cwd, + cwd: getCwd(), _: [], invokedFromCli: false, config: {}, @@ -695,7 +695,7 @@ describe('lib/util/args', () => { const result = argsUtil.toObject(['--config', '{"baseUrl": "http://foobar.com", "specPattern":"**/*.test.js"}']) expect(result).to.deep.equal({ - cwd, + cwd: getCwd(), _: [], invokedFromCli: false, config: { @@ -718,7 +718,7 @@ describe('lib/util/args', () => { ] expect(argsUtil.toObject(argv)).to.deep.eq({ - cwd, + cwd: getCwd(), _: [ '/private/var/folders/wr/3xdzqnq16lz5r1j_xtl443580000gn/T/cypress/Cypress.app/Contents/MacOS/Cypress', '/Applications/Cypress.app', @@ -743,7 +743,7 @@ describe('lib/util/args', () => { ] expect(argsUtil.toObject(argv)).to.deep.eq({ - cwd, + cwd: getCwd(), _: [ '/private/var/folders/wr/3xdzqnq16lz5r1j_xtl443580000gn/T/cypress/Cypress.app/Contents/MacOS/Cypress', '/Applications/Cypress.app1', From 5e93741fd496c18a0564c8dc068591e175bab76a Mon Sep 17 00:00:00 2001 From: Chris Breiding Date: Mon, 6 Nov 2023 14:38:08 -0500 Subject: [PATCH 10/12] re-query target id on reconnect --- .../server/lib/browsers/browser-cri-client.ts | 12 +++--- packages/server/lib/browsers/cri-client.ts | 38 +++++++++++++------ 2 files changed, 32 insertions(+), 18 deletions(-) diff --git a/packages/server/lib/browsers/browser-cri-client.ts b/packages/server/lib/browsers/browser-cri-client.ts index 8d11b913afed..d7c1e38fca48 100644 --- a/packages/server/lib/browsers/browser-cri-client.ts +++ b/packages/server/lib/browsers/browser-cri-client.ts @@ -16,7 +16,6 @@ interface Version { type BrowserCriClientOptions = { browserClient: CriClient - versionInfo: CRI.VersionResult host: string port: number browserName: string @@ -191,9 +190,9 @@ export class BrowserCriClient { extraTargetClients: Map = new Map() onClose: Function | null = null - private constructor ({ browserClient, versionInfo, host, port, browserName, onAsynchronousError, protocolManager, fullyManageTabs }: BrowserCriClientOptions) { + private constructor ({ browserClient, host, port, browserName, onAsynchronousError, protocolManager, fullyManageTabs }: BrowserCriClientOptions) { this.browserClient = browserClient - this.versionInfo = versionInfo + this.versionInfo = browserClient.versionInfo this.host = host this.port = port this.browserName = browserName @@ -229,17 +228,16 @@ export class BrowserCriClient { const host = await ensureLiveBrowser(hosts, port, browserName) return retryWithIncreasingDelay(async () => { - const versionInfo = await CRI.Version({ host, port, useHostName: true }) - const browserClient = await create({ - target: versionInfo.webSocketDebuggerUrl, + host, + port, onAsynchronousError, onReconnect, protocolManager, fullyManageTabs, }) - const browserCriClient = new BrowserCriClient({ browserClient, versionInfo, host, port, browserName, onAsynchronousError, protocolManager, fullyManageTabs }) + const browserCriClient = new BrowserCriClient({ browserClient, host, port, browserName, onAsynchronousError, protocolManager, fullyManageTabs }) if (fullyManageTabs) { await this._manageTabs({ browserClient, browserCriClient, browserName, host, onAsynchronousError, port, protocolManager }) diff --git a/packages/server/lib/browsers/cri-client.ts b/packages/server/lib/browsers/cri-client.ts index 97325b755cac..fb5ea6e85d29 100644 --- a/packages/server/lib/browsers/cri-client.ts +++ b/packages/server/lib/browsers/cri-client.ts @@ -58,6 +58,10 @@ export interface CriClient { * The target id attached to by this client */ targetId: string + /** + * The version info for the client + */ + versionInfo: CRI.VersionResult /** * The underlying websocket connection */ @@ -140,10 +144,10 @@ const maybeDebugCdpMessages = (cri: CDPClient) => { type DeferredPromise = { resolve: Function, reject: Function } type CreateParams = { - target: string + target?: string onAsynchronousError: Function - host?: string - port?: number + host: string + port: number onReconnect?: (client: CriClient) => void protocolManager?: ProtocolManagerShape fullyManageTabs?: boolean @@ -171,7 +175,7 @@ export const create = async ({ let cri: CDPClient let client: CriClient - const reconnect = async (retryIndex) => { + const reconnect = async (retryIndex, target) => { connected = false if (closed) { @@ -210,14 +214,14 @@ export const create = async ({ } } - const retryReconnect = async () => { + const retryReconnect = async (target) => { debug('disconnected, starting retries to reconnect... %o', { closed, target }) const retry = async (retryIndex = 0) => { retryIndex++ try { - return await reconnect(retryIndex) + return await reconnect(retryIndex, target) } catch (err) { if (closed) { debug('could not reconnect because client is closed %o', { closed, target }) @@ -249,7 +253,13 @@ export const create = async ({ const connect = async () => { await cri?.close() - debug('connecting %o', { connected, target }) + debug('connecting %o', { connected }) + + const versionInfo = await CRI.Version({ host, port, useHostName: true }) + + // if target isn't specified by callee, use the freshly queried + // version info debugger url + target ||= versionInfo.webSocketDebuggerUrl cri = await CRI({ host, @@ -267,7 +277,7 @@ export const create = async ({ // Only reconnect when we're not running cypress in cypress. There are a lot of disconnects that happen that we don't want to reconnect on if (!process.env.CYPRESS_INTERNAL_E2E_TESTING_SELF) { - cri.on('disconnect', retryReconnect) + cri.on('disconnect', () => retryReconnect(target)) } // We only want to try and add child target traffic if we have a host set. This indicates that this is the child cri client. @@ -306,12 +316,18 @@ export const create = async ({ await cri.send('Target.setDiscoverTargets', { discover: true }) } } + + return { + target, + versionInfo, + } } - await connect() + const { target: targetId, versionInfo } = await connect() client = { - targetId: target, + targetId, + versionInfo, async send (command: CdpCommand, params?: object, sessionId?: string) { if (crashed) { @@ -373,7 +389,7 @@ export const create = async ({ const p = enqueue() as Promise - await retryReconnect() + await retryReconnect(target) // if enqueued commands were wiped out from the reconnect and the socket is already closed, reject the command as it will never be run if (enqueuedCommands.length === 0 && closed) { From 3e1619057e63eac565f15568984b3d61ceb091e2 Mon Sep 17 00:00:00 2001 From: Chris Breiding Date: Mon, 6 Nov 2023 15:09:12 -0500 Subject: [PATCH 11/12] Revert "re-query target id on reconnect" This reverts commit 5e93741fd496c18a0564c8dc068591e175bab76a. --- .../server/lib/browsers/browser-cri-client.ts | 12 +++--- packages/server/lib/browsers/cri-client.ts | 38 ++++++------------- 2 files changed, 18 insertions(+), 32 deletions(-) diff --git a/packages/server/lib/browsers/browser-cri-client.ts b/packages/server/lib/browsers/browser-cri-client.ts index d7c1e38fca48..8d11b913afed 100644 --- a/packages/server/lib/browsers/browser-cri-client.ts +++ b/packages/server/lib/browsers/browser-cri-client.ts @@ -16,6 +16,7 @@ interface Version { type BrowserCriClientOptions = { browserClient: CriClient + versionInfo: CRI.VersionResult host: string port: number browserName: string @@ -190,9 +191,9 @@ export class BrowserCriClient { extraTargetClients: Map = new Map() onClose: Function | null = null - private constructor ({ browserClient, host, port, browserName, onAsynchronousError, protocolManager, fullyManageTabs }: BrowserCriClientOptions) { + private constructor ({ browserClient, versionInfo, host, port, browserName, onAsynchronousError, protocolManager, fullyManageTabs }: BrowserCriClientOptions) { this.browserClient = browserClient - this.versionInfo = browserClient.versionInfo + this.versionInfo = versionInfo this.host = host this.port = port this.browserName = browserName @@ -228,16 +229,17 @@ export class BrowserCriClient { const host = await ensureLiveBrowser(hosts, port, browserName) return retryWithIncreasingDelay(async () => { + const versionInfo = await CRI.Version({ host, port, useHostName: true }) + const browserClient = await create({ - host, - port, + target: versionInfo.webSocketDebuggerUrl, onAsynchronousError, onReconnect, protocolManager, fullyManageTabs, }) - const browserCriClient = new BrowserCriClient({ browserClient, host, port, browserName, onAsynchronousError, protocolManager, fullyManageTabs }) + const browserCriClient = new BrowserCriClient({ browserClient, versionInfo, host, port, browserName, onAsynchronousError, protocolManager, fullyManageTabs }) if (fullyManageTabs) { await this._manageTabs({ browserClient, browserCriClient, browserName, host, onAsynchronousError, port, protocolManager }) diff --git a/packages/server/lib/browsers/cri-client.ts b/packages/server/lib/browsers/cri-client.ts index fb5ea6e85d29..97325b755cac 100644 --- a/packages/server/lib/browsers/cri-client.ts +++ b/packages/server/lib/browsers/cri-client.ts @@ -58,10 +58,6 @@ export interface CriClient { * The target id attached to by this client */ targetId: string - /** - * The version info for the client - */ - versionInfo: CRI.VersionResult /** * The underlying websocket connection */ @@ -144,10 +140,10 @@ const maybeDebugCdpMessages = (cri: CDPClient) => { type DeferredPromise = { resolve: Function, reject: Function } type CreateParams = { - target?: string + target: string onAsynchronousError: Function - host: string - port: number + host?: string + port?: number onReconnect?: (client: CriClient) => void protocolManager?: ProtocolManagerShape fullyManageTabs?: boolean @@ -175,7 +171,7 @@ export const create = async ({ let cri: CDPClient let client: CriClient - const reconnect = async (retryIndex, target) => { + const reconnect = async (retryIndex) => { connected = false if (closed) { @@ -214,14 +210,14 @@ export const create = async ({ } } - const retryReconnect = async (target) => { + const retryReconnect = async () => { debug('disconnected, starting retries to reconnect... %o', { closed, target }) const retry = async (retryIndex = 0) => { retryIndex++ try { - return await reconnect(retryIndex, target) + return await reconnect(retryIndex) } catch (err) { if (closed) { debug('could not reconnect because client is closed %o', { closed, target }) @@ -253,13 +249,7 @@ export const create = async ({ const connect = async () => { await cri?.close() - debug('connecting %o', { connected }) - - const versionInfo = await CRI.Version({ host, port, useHostName: true }) - - // if target isn't specified by callee, use the freshly queried - // version info debugger url - target ||= versionInfo.webSocketDebuggerUrl + debug('connecting %o', { connected, target }) cri = await CRI({ host, @@ -277,7 +267,7 @@ export const create = async ({ // Only reconnect when we're not running cypress in cypress. There are a lot of disconnects that happen that we don't want to reconnect on if (!process.env.CYPRESS_INTERNAL_E2E_TESTING_SELF) { - cri.on('disconnect', () => retryReconnect(target)) + cri.on('disconnect', retryReconnect) } // We only want to try and add child target traffic if we have a host set. This indicates that this is the child cri client. @@ -316,18 +306,12 @@ export const create = async ({ await cri.send('Target.setDiscoverTargets', { discover: true }) } } - - return { - target, - versionInfo, - } } - const { target: targetId, versionInfo } = await connect() + await connect() client = { - targetId, - versionInfo, + targetId: target, async send (command: CdpCommand, params?: object, sessionId?: string) { if (crashed) { @@ -389,7 +373,7 @@ export const create = async ({ const p = enqueue() as Promise - await retryReconnect(target) + await retryReconnect() // if enqueued commands were wiped out from the reconnect and the socket is already closed, reject the command as it will never be run if (enqueuedCommands.length === 0 && closed) { From d58db5394de9c062c2cb190e15255633760f72bf Mon Sep 17 00:00:00 2001 From: Chris Breiding Date: Mon, 6 Nov 2023 16:02:56 -0500 Subject: [PATCH 12/12] try not reconnecting if child target --- packages/server/lib/browsers/cri-client.ts | 23 ++++++++++++++++------ 1 file changed, 17 insertions(+), 6 deletions(-) diff --git a/packages/server/lib/browsers/cri-client.ts b/packages/server/lib/browsers/cri-client.ts index 97325b755cac..a578592a4a53 100644 --- a/packages/server/lib/browsers/cri-client.ts +++ b/packages/server/lib/browsers/cri-client.ts @@ -265,15 +265,26 @@ export const create = async ({ maybeDebugCdpMessages(cri) - // Only reconnect when we're not running cypress in cypress. There are a lot of disconnects that happen that we don't want to reconnect on - if (!process.env.CYPRESS_INTERNAL_E2E_TESTING_SELF) { + // Having a host set indicates that this is the child cri target, a.k.a. + // the main Cypress tab (as opposed to the root browser cri target) + const isChildTarget = !!host + + // don't reconnect in these circumstances + if ( + // is a child target. we only need to reconnect the root browser target + !isChildTarget + // running cypress in cypress - there are a lot of disconnects that happen + // that we don't want to reconnect on + && !process.env.CYPRESS_INTERNAL_E2E_TESTING_SELF + ) { cri.on('disconnect', retryReconnect) } - // We only want to try and add child target traffic if we have a host set. This indicates that this is the child cri client. - // Browser cri traffic is handled in browser-cri-client.ts. The basic approach here is we attach to targets and enable network traffic - // We must attach in a paused state so that we can enable network traffic before the target starts running. - if (host) { + // We're only interested in child target traffic. Browser cri traffic is + // handled in browser-cri-client.ts. The basic approach here is we attach + // to targets and enable network traffic. We must attach in a paused state + // so that we can enable network traffic before the target starts running. + if (isChildTarget) { cri.on('Target.targetCrashed', async (event) => { if (event.targetId !== target) { return