Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore: add after:browser:launch node event #28180

Merged
merged 21 commits into from Nov 6, 2023
Merged
Show file tree
Hide file tree
Changes from 16 commits
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
f292279
chore: extract Target.attachedToTarget and Target.targetDestroyed han…
chrisbreiding Oct 30, 2023
277885e
chore: add after:browser:launch node event
chrisbreiding Oct 30, 2023
7be4103
add to firefox and webkit [run ci]
chrisbreiding Oct 30, 2023
dde0d7a
Merge branch 'develop' into crb/cy-puppeteer-refactors
chrisbreiding Oct 30, 2023
054961d
fix electron spec
chrisbreiding Oct 30, 2023
af92198
Merge branch 'crb/cy-puppeteer-refactors' into crb/cy-puppeteer-after…
chrisbreiding Oct 30, 2023
4ba7fb2
un-nest browser-cri tests
chrisbreiding Oct 30, 2023
bbcce26
Merge branch 'crb/cy-puppeteer-refactors' of github.com:cypress-io/cy…
chrisbreiding Oct 30, 2023
671e509
Merge branch 'crb/cy-puppeteer-refactors' into crb/cy-puppeteer-after…
chrisbreiding Oct 30, 2023
96baf2c
Update cli/types/cypress.d.ts
chrisbreiding Oct 31, 2023
3f304c4
Merge branch 'develop' into crb/cy-puppeteer-after-browser-launch
chrisbreiding Oct 31, 2023
2d862af
add comment
chrisbreiding Nov 3, 2023
03ae6a6
add webSocketDebuggerUrl to webkit's after:browser:launch
chrisbreiding Nov 3, 2023
c688aec
Merge branch 'crb/cy-puppeteer-after-browser-launch' of github.com:cy…
chrisbreiding Nov 3, 2023
5c5c4f1
Merge branch 'develop' into crb/cy-puppeteer-after-browser-launch
chrisbreiding Nov 3, 2023
bd7b832
fix tests
chrisbreiding Nov 6, 2023
ac2ab40
Merge remote-tracking branch 'origin/develop' into crb/cy-puppeteer-a…
chrisbreiding Nov 6, 2023
a8d66a4
Merge remote-tracking branch 'origin/develop' into crb/cy-puppeteer-a…
chrisbreiding Nov 6, 2023
5e93741
re-query target id on reconnect
chrisbreiding Nov 6, 2023
3e16190
Revert "re-query target id on reconnect"
chrisbreiding Nov 6, 2023
d58db53
try not reconnecting if child target
chrisbreiding Nov 6, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
9 changes: 7 additions & 2 deletions cli/types/cypress.d.ts
Expand Up @@ -6009,7 +6009,11 @@ declare namespace Cypress {
(fn: (currentSubject: Subject) => void): Chainable<Subject>
}

interface BrowserLaunchOptions {
interface AfterBrowserLaunchDetails {
webSocketDebuggerUrl: string
}

interface BeforeBrowserLaunchOptions {
extensions: string[]
preferences: { [key: string]: any }
args: string[]
Expand Down Expand Up @@ -6090,12 +6094,13 @@ declare namespace Cypress {
}

interface PluginEvents {
(action: 'after:browser:launch', fn: (browser: Browser, browserLaunchDetails: AfterBrowserLaunchDetails) => void | Promise<void>): void
(action: 'after:run', fn: (results: CypressCommandLine.CypressRunResult | CypressCommandLine.CypressFailedRunResult) => void | Promise<void>): void
(action: 'after:screenshot', fn: (details: ScreenshotDetails) => void | AfterScreenshotReturnObject | Promise<AfterScreenshotReturnObject>): void
(action: 'after:spec', fn: (spec: Spec, results: CypressCommandLine.RunResult) => void | Promise<void>): void
(action: 'before:run', fn: (runDetails: BeforeRunDetails) => void | Promise<void>): void
(action: 'before:spec', fn: (spec: Spec) => void | Promise<void>): void
(action: 'before:browser:launch', fn: (browser: Browser, browserLaunchOptions: BrowserLaunchOptions) => void | BrowserLaunchOptions | Promise<BrowserLaunchOptions>): void
(action: 'before:browser:launch', fn: (browser: Browser, afterBrowserLaunchOptions: BeforeBrowserLaunchOptions) => void | Promise<void> | BeforeBrowserLaunchOptions | Promise<BeforeBrowserLaunchOptions>): void
(action: 'file:preprocessor', fn: (file: FileObject) => string | Promise<string>): void
(action: 'dev-server:start', fn: (file: DevServerConfig) => Promise<ResolvedDevServerConfig>): void
(action: 'task', tasks: Tasks): void
Expand Down
7 changes: 7 additions & 0 deletions packages/server/lib/browsers/browser-cri-client.ts
Expand Up @@ -573,6 +573,13 @@ export class BrowserCriClient {
this.extraTargetClients.delete(targetId)
}

/**
* @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
*/
Expand Down
4 changes: 4 additions & 0 deletions packages/server/lib/browsers/chrome.ts
Expand Up @@ -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
Expand Down
4 changes: 4 additions & 0 deletions packages/server/lib/browsers/electron.ts
Expand Up @@ -533,6 +533,10 @@ export = {
},
}) as BrowserInstance

await utils.executeAfterBrowserLaunch(browser, {
webSocketDebuggerUrl: browserCriClient!.getWebSocketDebuggerUrl(),
})

return instance
},
}
8 changes: 6 additions & 2 deletions packages/server/lib/browsers/firefox.ts
Expand Up @@ -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
Expand Down Expand Up @@ -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!)
chrisbreiding marked this conversation as resolved.
Show resolved Hide resolved
}

export function connectToExisting () {
Expand Down Expand Up @@ -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)
}
Expand Down
24 changes: 24 additions & 0 deletions packages/server/lib/browsers/utils.ts
Expand Up @@ -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 {
Expand Down Expand Up @@ -157,6 +158,27 @@ async function executeBeforeBrowserLaunch (browser, launchOptions: typeof defaul
return launchOptions
}

interface AfterBrowserLaunchDetails {
webSocketDebuggerUrl: string | never
}

async function executeAfterBrowserLaunch (browser: Browser, options: AfterBrowserLaunchDetails) {
if (plugins.has('after:browser:launch')) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

iirc, with chrome if we crash, we will re-launch the browser. does this re-trigger if that happens?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we re-launch the browser? From what I can tell, we detect the crash and report it, which seems like the appropriate behavior.

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
Expand Down Expand Up @@ -423,6 +445,8 @@ export = {

extendLaunchOptionsFromPlugins,

executeAfterBrowserLaunch,

executeBeforeBrowserLaunch,

defaultLaunchOptions,
Expand Down
7 changes: 6 additions & 1 deletion packages/server/lib/browsers/webkit.ts
Expand Up @@ -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,
Expand Down Expand Up @@ -147,5 +148,9 @@ export async function open (browser: Browser, url: string, options: BrowserLaunc
}
}

await utils.executeAfterBrowserLaunch(browser, {
webSocketDebuggerUrl: websocketUrl,
})

return new WkInstance()
}
2 changes: 1 addition & 1 deletion packages/server/lib/plugins/child/browser_launch.js
Expand Up @@ -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
Expand Down
4 changes: 3 additions & 1 deletion packages/server/lib/plugins/child/run_plugins.js
Expand Up @@ -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)

Expand Down
7 changes: 6 additions & 1 deletion packages/server/lib/plugins/child/validate_event.js
Expand Up @@ -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,
Expand All @@ -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}`)

Expand Down
2 changes: 2 additions & 0 deletions packages/server/test/integration/cypress_spec.js
Expand Up @@ -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 = {
Expand Down Expand Up @@ -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)
Expand Down
31 changes: 28 additions & 3 deletions packages/server/test/unit/browsers/chrome_spec.js
Expand Up @@ -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 = {
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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 = {
Expand Down
34 changes: 30 additions & 4 deletions packages/server/test/unit/browsers/electron_spec.js
Expand Up @@ -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)
Expand Down Expand Up @@ -111,8 +112,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 () {
Expand Down Expand Up @@ -152,7 +156,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)

Expand Down Expand Up @@ -207,6 +211,25 @@ describe('lib/browsers/electron', () => {
expect(Windows.removeAllExtensions).to.be.calledTwice
})
})

it('sends after:browser:launch with debugger url', function () {
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', 'electron', {
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', () => {
Expand Down Expand Up @@ -800,7 +823,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
Expand Down