Skip to content

Commit

Permalink
Merge branch 'develop' into ryanm/fix/restart-server-on-baseUrl-change
Browse files Browse the repository at this point in the history
  • Loading branch information
mjhenkes authored Jun 10, 2022
2 parents 42b2252 + 203758f commit d226315
Show file tree
Hide file tree
Showing 15 changed files with 102 additions and 72 deletions.
8 changes: 5 additions & 3 deletions packages/extension/app/background.js
Original file line number Diff line number Diff line change
Expand Up @@ -127,8 +127,8 @@ const connect = function (host, path, extraOpts) {
return invoke('takeScreenshot', id)
case 'reset:browser:state':
return invoke('resetBrowserState', id)
case 'close:browser:tabs':
return invoke('closeBrowserTabs', id)
case 'reset:browser:tabs:for:next:test':
return invoke('resetBrowserTabsForNextTest', id)
default:
return fail(id, { message: `No handler registered for: '${msg}'` })
}
Expand Down Expand Up @@ -264,8 +264,10 @@ const automation = {
return browser.browsingData.remove({}, { cache: true, cookies: true, downloads: true, formData: true, history: true, indexedDB: true, localStorage: true, passwords: true, pluginData: true, serviceWorkers: true }).then(fn)
},

closeBrowserTabs (fn) {
resetBrowserTabsForNextTest (fn) {
return Promise.try(() => {
return browser.tabs.create({ url: 'about:blank' })
}).then(() => {
return browser.windows.getCurrent({ populate: true })
}).then((windowInfo) => {
return browser.tabs.remove(windowInfo.tabs.map((tab) => tab.id))
Expand Down
8 changes: 6 additions & 2 deletions packages/extension/test/integration/background_spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ const browser = {
},
runtime: {},
tabs: {
create () {},
query () {},
executeScript () {},
captureVisibleTab () {},
Expand All @@ -48,6 +49,7 @@ const browser = {
mockRequire('webextension-polyfill', browser)

const background = require('../../app/background')
const { expect } = require('chai')

const PORT = 12345

Expand Down Expand Up @@ -846,8 +848,9 @@ describe('app/background', () => {
})
})

describe('close:browser:tabs', () => {
describe('reset:browser:tabs:for:next:test', () => {
beforeEach(() => {
sinon.stub(browser.tabs, 'create').withArgs({ url: 'about:blank' })
sinon.stub(browser.windows, 'getCurrent').withArgs({ populate: true }).resolves({ id: '10', tabs: [{ id: '1' }, { id: '2' }, { id: '3' }] })
sinon.stub(browser.tabs, 'remove').withArgs(['1', '2', '3']).resolves()
})
Expand All @@ -857,13 +860,14 @@ describe('app/background', () => {
expect(id).to.eq(123)
expect(obj.response).to.be.undefined

expect(browser.tabs.create).to.be.called
expect(browser.windows.getCurrent).to.be.called
expect(browser.tabs.remove).to.be.called

done()
})

return this.server.emit('automation:request', 123, 'close:browser:tabs')
return this.server.emit('automation:request', 123, 'reset:browser:tabs:for:next:test')
})
})
})
Expand Down
32 changes: 14 additions & 18 deletions packages/server/lib/browsers/browser-cri-client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ const retryWithIncreasingDelay = async <T>(retryable: () => Promise<T>, browserN
}

export class BrowserCriClient {
private currentlyAttachedTarget: CRIWrapper.Client | undefined
currentlyAttachedTarget: CRIWrapper.Client | undefined
private constructor (private browserClient: CRIWrapper.Client, private versionInfo, private port: number, private browserName: string, private onAsynchronousError: Function) {}

/**
Expand Down Expand Up @@ -132,28 +132,22 @@ export class BrowserCriClient {
}

/**
* Creates a new target with the given url and then attaches to it
* Resets the browser's targets optionally keeping a tab open
*
* @param url the url to create and attach to
* @returns the chrome remote interface wrapper for the target
*/
attachToNewUrl = async (url: string): Promise<CRIWrapper.Client> => {
debug('Attaching to new url %s', url)
const target = await this.browserClient.send('Target.createTarget', { url })

this.currentlyAttachedTarget = await create(target.targetId, this.onAsynchronousError, HOST, this.port)

return this.currentlyAttachedTarget
}

/**
* Closes the currently attached page target
* @param shouldKeepTabOpen whether or not to keep the tab open
*/
closeCurrentTarget = async (): Promise<void> => {
resetBrowserTargets = async (shouldKeepTabOpen: boolean): Promise<void> => {
if (!this.currentlyAttachedTarget) {
throw new Error('Cannot close target because no target is currently attached')
}

let target

// If we are keeping a tab open, we need to first launch a new default tab prior to closing the existing one
if (shouldKeepTabOpen) {
target = await this.browserClient.send('Target.createTarget', { url: 'about:blank' })
}

debug('Closing current target %s', this.currentlyAttachedTarget.targetId)

await Promise.all([
Expand All @@ -162,7 +156,9 @@ export class BrowserCriClient {
this.browserClient.send('Target.closeTarget', { targetId: this.currentlyAttachedTarget.targetId }),
])

this.currentlyAttachedTarget = undefined
if (target) {
this.currentlyAttachedTarget = await create(target.targetId, this.onAsynchronousError, HOST, this.port)
}
}

/**
Expand Down
6 changes: 3 additions & 3 deletions packages/server/lib/browsers/cdp_automation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -164,7 +164,7 @@ const normalizeResourceType = (resourceType: string | undefined): ResourceType =
}

type SendDebuggerCommand = (message: string, data?: any) => Promise<any>
type SendCloseCommand = () => Promise<any>
type SendCloseCommand = (shouldKeepTabOpen: boolean) => Promise<any>
type OnFn = (eventName: string, cb: Function) => void

// the intersection of what's valid in CDP and what's valid in FFCDP
Expand Down Expand Up @@ -355,8 +355,8 @@ export class CdpAutomation {
this.sendDebuggerCommandFn('Storage.clearDataForOrigin', { origin: '*', storageTypes: 'all' }),
this.sendDebuggerCommandFn('Network.clearBrowserCache'),
])
case 'close:browser:tabs':
return this.sendCloseCommandFn()
case 'reset:browser:tabs:for:next:test':
return this.sendCloseCommandFn(data.shouldKeepTabOpen)
case 'focus:browser:window':
return this.sendDebuggerCommandFn('Page.bringToFront')
default:
Expand Down
12 changes: 6 additions & 6 deletions packages/server/lib/browsers/chrome.ts
Original file line number Diff line number Diff line change
Expand Up @@ -433,8 +433,8 @@ const _handlePausedRequests = async (client) => {
})
}

const _setAutomation = async (client: CRIWrapper.Client, automation: Automation, closeCurrentTarget: () => Promise<void>, options: CypressConfiguration = {}) => {
const cdpAutomation = await CdpAutomation.create(client.send, client.on, closeCurrentTarget, automation, options.experimentalSessionAndOrigin)
const _setAutomation = async (client: CRIWrapper.Client, automation: Automation, resetBrowserTargets: (shouldKeepTabOpen: boolean) => Promise<void>, options: CypressConfiguration = {}) => {
const cdpAutomation = await CdpAutomation.create(client.send, client.on, resetBrowserTargets, automation, options.experimentalSessionAndOrigin)

return automation.use(cdpAutomation)
}
Expand Down Expand Up @@ -555,9 +555,9 @@ export = {
debug('connecting to new chrome tab in existing instance with url and debugging port', { url: options.url })

const browserCriClient = this._getBrowserCriClient()
const pageCriClient = await browserCriClient.attachToNewUrl('about:blank')
const pageCriClient = browserCriClient.currentlyAttachedTarget

await this._setAutomation(pageCriClient, automation, browserCriClient.closeCurrentTarget, options)
await this._setAutomation(pageCriClient, automation, browserCriClient.resetBrowserTargets, options)

// make sure page events are re enabled or else frame tree updates will NOT work as well as other items listening for page events
await pageCriClient.send('Page.enable')
Expand All @@ -584,7 +584,7 @@ export = {
const browserCriClient = await BrowserCriClient.create(port, browser.displayName, options.onError, onReconnect)
const pageCriClient = await browserCriClient.attachToTargetUrl(options.url)

await this._setAutomation(pageCriClient, automation, browserCriClient.closeCurrentTarget, options)
await this._setAutomation(pageCriClient, automation, browserCriClient.resetBrowserTargets, options)
},

async open (browser: Browser, url, options: CypressConfiguration = {}, automation: Automation): Promise<LaunchedBrowser> {
Expand Down Expand Up @@ -679,7 +679,7 @@ export = {

const pageCriClient = await browserCriClient.attachToTargetUrl('about:blank')

await this._setAutomation(pageCriClient, automation, browserCriClient.closeCurrentTarget, options)
await this._setAutomation(pageCriClient, automation, browserCriClient.resetBrowserTargets, options)

await pageCriClient.send('Page.enable')

Expand Down
4 changes: 2 additions & 2 deletions packages/server/lib/browsers/firefox-util.ts
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,7 @@ async function connectToNewSpec (options, automation: Automation, browserCriClie

const pageCriClient = await browserCriClient.attachToTargetUrl('about:blank')

await CdpAutomation.create(pageCriClient.send, pageCriClient.on, browserCriClient.closeCurrentTarget, automation, options.experimentalSessionAndOrigin)
await CdpAutomation.create(pageCriClient.send, pageCriClient.on, browserCriClient.resetBrowserTargets, automation, options.experimentalSessionAndOrigin)

await options.onInitializeNewBrowserTab()

Expand All @@ -138,7 +138,7 @@ async function setupRemote (remotePort, automation, onError, options): Promise<B
const browserCriClient = await BrowserCriClient.create(remotePort, 'Firefox', onError)
const pageCriClient = await browserCriClient.attachToTargetUrl('about:blank')

await CdpAutomation.create(pageCriClient.send, pageCriClient.on, browserCriClient.closeCurrentTarget, automation, options.experimentalSessionAndOrigin)
await CdpAutomation.create(pageCriClient.send, pageCriClient.on, browserCriClient.resetBrowserTargets, automation, options.experimentalSessionAndOrigin)

return browserCriClient
}
Expand Down
15 changes: 8 additions & 7 deletions packages/server/lib/modes/run.js
Original file line number Diff line number Diff line change
Expand Up @@ -1169,7 +1169,7 @@ module.exports = {
},

waitForTestsToFinishRunning (options = {}) {
const { project, screenshots, startedVideoCapture, endVideoCapture, videoName, compressedVideoName, videoCompression, videoUploadOnPasses, exit, spec, estimated, quiet, config } = options
const { project, screenshots, startedVideoCapture, endVideoCapture, videoName, compressedVideoName, videoCompression, videoUploadOnPasses, exit, spec, estimated, quiet, config, shouldKeepTabOpen } = options

// https://github.com/cypress-io/cypress/issues/2370
// delay 1 second if we're recording a video to give
Expand Down Expand Up @@ -1251,7 +1251,7 @@ module.exports = {
// await openProject.closeBrowser()
// } else {
debug('attempting to close the browser tab')
await openProject.closeBrowserTabs()
await openProject.resetBrowserTabsForNextTest(shouldKeepTabOpen)
// }

debug('resetting server state')
Expand Down Expand Up @@ -1330,16 +1330,16 @@ module.exports = {
})
}

let firstSpec = true
let isFirstSpec = true

const runEachSpec = (spec, index, length, estimated) => {
if (!options.quiet) {
displaySpecHeader(spec.baseName, index + 1, length, estimated)
}

return this.runSpec(config, spec, options, estimated, firstSpec)
return this.runSpec(config, spec, options, estimated, isFirstSpec, index === length - 1)
.tap(() => {
firstSpec = false
isFirstSpec = false
})
.get('results')
.tap((results) => {
Expand Down Expand Up @@ -1435,7 +1435,7 @@ module.exports = {
})
},

runSpec (config, spec = {}, options = {}, estimated, firstSpec) {
runSpec (config, spec = {}, options = {}, estimated, isFirstSpec, isLastSpec) {
const { project, browser, onError } = options

const { isHeadless } = browser
Expand Down Expand Up @@ -1484,6 +1484,7 @@ module.exports = {
videoUploadOnPasses: options.videoUploadOnPasses,
quiet: options.quiet,
browser,
shouldKeepTabOpen: !isLastSpec,
}),

connection: this.waitForBrowserToConnect({
Expand All @@ -1496,7 +1497,7 @@ module.exports = {
socketId: options.socketId,
webSecurity: options.webSecurity,
projectRoot: options.projectRoot,
shouldLaunchNewTab: !firstSpec, // !process.env.CYPRESS_INTERNAL_FORCE_BROWSER_RELAUNCH && !firstSpec,
shouldLaunchNewTab: !isFirstSpec, // !process.env.CYPRESS_INTERNAL_FORCE_BROWSER_RELAUNCH && !isFirstSpec,
// TODO(tim): investigate the socket disconnect
}),
})
Expand Down
4 changes: 2 additions & 2 deletions packages/server/lib/open_project.ts
Original file line number Diff line number Diff line change
Expand Up @@ -196,8 +196,8 @@ export class OpenProject {
return browsers.close()
}

async closeBrowserTabs () {
return this.projectBase?.closeBrowserTabs()
async resetBrowserTabsForNextTest (shouldKeepTabOpen: boolean) {
return this.projectBase?.resetBrowserTabsForNextTest(shouldKeepTabOpen)
}

async resetBrowserState () {
Expand Down
4 changes: 2 additions & 2 deletions packages/server/lib/project-base.ts
Original file line number Diff line number Diff line change
Expand Up @@ -409,8 +409,8 @@ export class ProjectBase<TServer extends Server> extends EE {
this.ctx.setAppSocketServer(io)
}

async closeBrowserTabs () {
return this.server.socket.closeBrowserTabs()
async resetBrowserTabsForNextTest (shouldKeepTabOpen: boolean) {
return this.server.socket.resetBrowserTabsForNextTest(shouldKeepTabOpen)
}

async resetBrowserState () {
Expand Down
12 changes: 6 additions & 6 deletions packages/server/lib/socket-base.ts
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ const retry = (fn: (res: any) => void) => {
}

export class SocketBase {
private _sendCloseBrowserTabsMessage
private _sendResetBrowserTabsForNextTestMessage
private _sendResetBrowserStateMessage
private _isRunnerSocketConnected
private _sendFocusBrowserMessage
Expand Down Expand Up @@ -294,8 +294,8 @@ export class SocketBase {
})
})

this._sendCloseBrowserTabsMessage = async () => {
await automationRequest('close:browser:tabs', {})
this._sendResetBrowserTabsForNextTestMessage = async (shouldKeepTabOpen: boolean) => {
await automationRequest('reset:browser:tabs:for:next:test', { shouldKeepTabOpen })
}

this._sendResetBrowserStateMessage = async () => {
Expand Down Expand Up @@ -583,9 +583,9 @@ export class SocketBase {
return this._io?.emit('tests:finished')
}

async closeBrowserTabs () {
if (this._sendCloseBrowserTabsMessage) {
await this._sendCloseBrowserTabsMessage()
async resetBrowserTabsForNextTest (shouldKeepTabOpen: boolean) {
if (this._sendResetBrowserTabsForNextTestMessage) {
await this._sendResetBrowserTabsForNextTestMessage(shouldKeepTabOpen)
}
}

Expand Down
Loading

0 comments on commit d226315

Please sign in to comment.