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 schema infrastructure and switch CDP client to be passed into protocol directly #26222

Merged
merged 24 commits into from
Mar 30, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,7 @@
"@percy/cli": "1.2.0",
"@semantic-release/changelog": "5.0.1",
"@semantic-release/git": "9.0.0",
"@types/better-sqlite3": "^7.6.3",
"@types/bluebird": "3.5.29",
"@types/chai-enzyme": "0.6.7",
"@types/classnames": "2.2.9",
Expand Down
15 changes: 2 additions & 13 deletions packages/server/lib/browsers/browser-cri-client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -150,13 +150,8 @@ export class BrowserCriClient {
throw new Error(`Could not find url target in browser ${url}. Targets were ${JSON.stringify(targets)}`)
}

await this.protocolManager?.connectToBrowser({
target: target.targetId,
host: this.host,
port: this.port,
})

this.currentlyAttachedTarget = await create(target.targetId, this.onAsynchronousError, this.host, this.port)
await this.protocolManager?.connectToBrowser(this.currentlyAttachedTarget)

return this.currentlyAttachedTarget
}, this.browserName, this.port)
Expand Down Expand Up @@ -188,21 +183,15 @@ export class BrowserCriClient {
])

if (target) {
await this.protocolManager?.connectToBrowser({
target: target.targetId,
host: this.host,
port: this.port,
})

this.currentlyAttachedTarget = await create(target.targetId, this.onAsynchronousError, this.host, this.port)
await this.protocolManager?.connectToBrowser(this.currentlyAttachedTarget)
}
}

/**
* Closes the browser client socket as well as the socket for the currently attached page target
*/
close = async () => {
this.protocolManager?.close()
if (this.currentlyAttachedTarget) {
await this.currentlyAttachedTarget.close()
}
Expand Down
6 changes: 4 additions & 2 deletions packages/server/lib/browsers/cdp_automation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -138,9 +138,11 @@ export const normalizeResourceType = (resourceType: string | undefined): Resourc
return ffToStandardResourceTypeMap[resourceType] || 'other'
}

type SendDebuggerCommand = <T extends CdpCommand>(message: T, data?: any) => Promise<ProtocolMapping.Commands[T]['returnType']>
export type SendDebuggerCommand = <T extends CdpCommand>(message: T, data?: ProtocolMapping.Commands[T]['paramsType'][0]) => Promise<ProtocolMapping.Commands[T]['returnType']>

export type OnFn = <T extends CdpEvent>(eventName: T, cb: (data: ProtocolMapping.Events[T][0]) => void) => void

type SendCloseCommand = (shouldKeepTabOpen: boolean) => Promise<any> | void
type OnFn = <T extends CdpEvent>(eventName: T, cb: (data: ProtocolMapping.Events[T][0]) => void) => void

// the intersection of what's valid in CDP and what's valid in FFCDP
// Firefox: https://searchfox.org/mozilla-central/rev/98a9257ca2847fad9a19631ac76199474516b31e/remote/cdp/domains/parent/Network.jsm#22
Expand Down
6 changes: 3 additions & 3 deletions packages/server/lib/browsers/cri-client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import debugModule from 'debug'
import _ from 'lodash'
import CRI from 'chrome-remote-interface'
import * as errors from '../errors'
import type { CdpCommand, CdpEvent } from './cdp_automation'
import type { SendDebuggerCommand, OnFn, CdpCommand, CdpEvent } from './cdp_automation'

const debug = debugModule('cypress:server:browsers:cri-client')
// debug using cypress-verbose:server:browsers:cri-client:send:*
Expand All @@ -21,12 +21,12 @@ export interface CriClient {
* Sends a command to the Chrome remote interface.
* @example client.send('Page.navigate', { url })
*/
send (command: CdpCommand, params?: object): Promise<any>
send: SendDebuggerCommand
/**
* Registers callback for particular event.
* @see https://github.com/cyrus-and/chrome-remote-interface#class-cdp
*/
on (eventName: CdpEvent, cb: Function): void
on: OnFn
/**
* Calls underlying remote interface client close
*/
Expand Down
59 changes: 42 additions & 17 deletions packages/server/lib/cloud/protocol.ts
Original file line number Diff line number Diff line change
@@ -1,13 +1,13 @@
import fs from 'fs-extra'
import { NodeVM } from 'vm2'
import Debug from 'debug'
import CDP from 'chrome-remote-interface'
import type { ProtocolManager, AppCaptureProtocolInterface } from '@packages/types'
import Database from 'better-sqlite3'
import path from 'path'
import os from 'os'

const debug = Debug('cypress:server:protocol')
const debugVerbose = Debug('cypress-verbose:server:protocol')

const setupProtocol = async (url?: string): Promise<AppCaptureProtocolInterface | undefined> => {
let script: string | undefined
Expand All @@ -27,12 +27,7 @@ const setupProtocol = async (url?: string): Promise<AppCaptureProtocolInterface
const vm = new NodeVM({
console: 'inherit',
sandbox: {
nodePath: path,
Debug,
CDP,
Database,
CY_PROTOCOL_DIR: cypressProtocolDirectory,
betterSqlite3Binding: path.join(require.resolve('better-sqlite3/build/Release/better_sqlite3.node')),
},
})

Expand All @@ -46,7 +41,6 @@ const setupProtocol = async (url?: string): Promise<AppCaptureProtocolInterface

class ProtocolManagerImpl implements ProtocolManager {
private protocol: AppCaptureProtocolInterface | undefined
private db: Database

protocolEnabled (): boolean {
return !!this.protocol
Expand All @@ -58,39 +52,70 @@ class ProtocolManagerImpl implements ProtocolManager {
this.protocol = await setupProtocol(url)
}

async connectToBrowser (options) {
async connectToBrowser (cdpClient) {
if (!this.protocolEnabled()) {
return
}

debug('connecting to browser for new spec')

return this.protocol?.connectToBrowser(options)
await this.protocol?.connectToBrowser(cdpClient)
}

addRunnables (runnables) {
if (!this.protocolEnabled()) {
return
}

this.protocol?.addRunnables(runnables)
}

beforeSpec (spec) {
debug('initializing new spec %O', spec)
this.protocol?.beforeSpec(spec)
beforeSpec (spec: { instanceId: string }) {
if (!this.protocolEnabled()) {
return
}

const cypressProtocolDirectory = path.join(os.tmpdir(), 'cypress', 'protocol')
const dbPath = path.join(cypressProtocolDirectory, `${spec.instanceId}.db`)

debug('connecting to database at %s', dbPath)

const db = Database(dbPath, {
nativeBinding: path.join(require.resolve('better-sqlite3/build/Release/better_sqlite3.node')),
verbose: debugVerbose,
})

this.protocol?.beforeSpec(db)
}

afterSpec () {
if (!this.protocolEnabled()) {
return
}

debug('after spec')

this.protocol?.afterSpec()
}

beforeTest (test) {
if (!this.protocolEnabled()) {
return
}

debug('before test %O', test)

this.protocol?.beforeTest(test)
}

afterTest (test) {
if (!this.protocolEnabled()) {
return
}

debug('after test %O', test)
this.protocol?.afterTest(test)
}

close () {
debug('closing')
this.protocol?.close()
this.protocol?.afterTest(test)
}
}

Expand Down
Original file line number Diff line number Diff line change
@@ -1,13 +1,8 @@
/* global Debug, CDP, CY_PROTOCOL_DIR, betterSqlite3Binding, nodePath, Database */
/* global Debug */

const AppCaptureProtocol = class {
constructor () {
this.Debug = Debug
this.CDP = CDP
this.CY_PROTOCOL_DIR = CY_PROTOCOL_DIR
this.betterSqlite3Binding = betterSqlite3Binding
this.nodePath = nodePath
this.Database = Database

this.connectToBrowser = this.connectToBrowser.bind(this)
this.addRunnables = this.addRunnables.bind(this)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -166,11 +166,7 @@ describe('lib/browsers/cri-client', function () {
const client = await browserClient.attachToTargetUrl('http://foo.com')

expect(client).to.be.equal(mockPageClient)
expect(protocolManager.connectToBrowser).to.be.calledWith({
host: HOST,
port: PORT,
target: '1',
})
expect(protocolManager.connectToBrowser).to.be.calledWith(client)
})

it('retries when the passed in url is not found', async function () {
Expand Down
28 changes: 14 additions & 14 deletions packages/server/test/unit/cloud/protocol_spec.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,13 @@
import '../../spec_helper'
import ProtocolManager from '../../../lib/cloud/protocol'
import { proxyquire } from '../../spec_helper'
import path from 'path'
import os from 'os'

const mockDb = sinon.stub()
const mockDatabase = sinon.stub().returns(mockDb)

const { default: ProtocolManager } = proxyquire('../lib/cloud/protocol', {
'better-sqlite3': mockDatabase,
})

describe('lib/cloud/protocol', () => {
beforeEach(() => {
Expand All @@ -20,11 +27,6 @@ describe('lib/cloud/protocol', () => {

expect(protocolManager.protocolEnabled()).to.be.true
expect(protocol.Debug).not.to.be.undefined
expect(protocol.CDP).not.to.be.undefined
expect(protocol.CY_PROTOCOL_DIR).not.to.be.undefined
expect(protocol.betterSqlite3Binding).not.to.be.undefined
expect(protocol.nodePath).not.to.be.undefined
expect(protocol.Database).not.to.be.undefined
})

it('should be able to connect to the browser', async () => {
Expand Down Expand Up @@ -59,15 +61,13 @@ describe('lib/cloud/protocol', () => {
sinon.stub(protocol, 'beforeSpec')

protocolManager.beforeSpec({
name: 'spec',
relative: 'relative',
absolute: 'absolute',
instanceId: 'instanceId',
})

expect(protocol.beforeSpec).to.be.calledWith({
name: 'spec',
relative: 'relative',
absolute: 'absolute',
expect(protocol.beforeSpec).to.be.calledWith(mockDb)
expect(mockDatabase).to.be.calledWith(path.join(os.tmpdir(), 'cypress', 'protocol', 'instanceId.db'), {
nativeBinding: path.join(require.resolve('better-sqlite3/build/Release/better_sqlite3.node')),
verbose: sinon.match.func,
})
})

Expand Down
26 changes: 21 additions & 5 deletions packages/types/src/protocol.ts
Original file line number Diff line number Diff line change
@@ -1,18 +1,34 @@
import type { SpecFile } from '.'
import type { Database } from 'better-sqlite3'
import type ProtocolMapping from 'devtools-protocol/types/protocol-mapping'

type Commands = ProtocolMapping.Commands
type Command<T extends keyof Commands> = Commands[T]
type Events = ProtocolMapping.Events
type Event<T extends keyof Events> = Events[T]

interface CDPClient {
send<T extends Extract<keyof Commands, string>> (command: T, params?: Command<T>['paramsType'][0]): Promise<Command<T>['returnType']>
on<T extends Extract<keyof Events, string>> (eventName: T, cb: (event: Event<T>[0]) => void): void
}

// TODO(protocol): This is basic for now but will evolve as we progress with the protocol work

export interface AppCaptureProtocolInterface {
addRunnables (runnables: any): void
connectToBrowser (options: { target: string, host: string, port: number }): Promise<void>
beforeSpec (spec: SpecFile & { instanceId: string }): void
connectToBrowser (cdpClient: CDPClient): Promise<void>
beforeSpec (db: Database): void
afterSpec (): void
beforeTest(test: Record<string, any>): void
afterTest(test: Record<string, any>): void
close(): void
}

export interface ProtocolManager extends AppCaptureProtocolInterface {
export interface ProtocolManager {
setupProtocol(url?: string): Promise<void>
protocolEnabled(): boolean
addRunnables (runnables: any): void
connectToBrowser (cdpClient: CDPClient): Promise<void>
beforeSpec (spec: { instanceId: string}): void
afterSpec (): void
beforeTest(test: Record<string, any>): void
afterTest(test: Record<string, any>): void
}
7 changes: 7 additions & 0 deletions yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -6028,6 +6028,13 @@
dependencies:
"@babel/types" "^7.3.0"

"@types/better-sqlite3@^7.6.3":
version "7.6.3"
resolved "https://registry.yarnpkg.com/@types/better-sqlite3/-/better-sqlite3-7.6.3.tgz#117c3c182e300799b84d1b7e1781c27d8d536505"
integrity sha512-YS64N9SNDT/NAvou3QNdzAu3E2om/W/0dhORimtPGLef+zSK5l1vDzfsWb4xgXOgfhtOI5ZDTRxnvRPb22AIVQ==
dependencies:
"@types/node" "*"

"@types/bluebird@*", "@types/bluebird@3.5.33":
version "3.5.33"
resolved "https://registry.yarnpkg.com/@types/bluebird/-/bluebird-3.5.33.tgz#d79c020f283bd50bd76101d7d300313c107325fc"
Expand Down