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

misc: Close extra tabs/windows between tests #28204

Merged
merged 10 commits into from
Nov 16, 2023
4 changes: 4 additions & 0 deletions cli/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,10 @@ _Released 11/21/2023 (PENDING)_

- When artifacts are uploaded to the Cypress Cloud, the duration of each upload will be displayed in the console. Addresses [#28237](https://github.com/cypress-io/cypress/issues/28237).

**Misc:**

- Browser tabs and windows other than the Cypress tab are now closed between tests in Chromium-based browsers. Addressed in [#28204](https://github.com/cypress-io/cypress/pull/28204).

## 13.5.1

_Released 11/14/2023_
Expand Down
5 changes: 5 additions & 0 deletions packages/driver/src/cypress/cy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -338,6 +338,11 @@ export class $Cy extends EventEmitter2 implements ITimeouts, IStability, IAssert
this.enqueue($Command.create(attrs))
})

// clears out any extra tabs/windows between tests
Cypress.on('test:before:run:async', () => {
return Cypress.backend('close:extra:targets')
})

handleCrossOriginCookies(Cypress)
}

Expand Down
12 changes: 12 additions & 0 deletions packages/server/lib/browsers/browser-cri-client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -585,6 +585,18 @@ export class BrowserCriClient {
this.extraTargetClients.delete(targetId)
}

async closeExtraTargets () {
await Promise.all(Array.from(this.extraTargetClients).map(async ([targetId]) => {
debug('Close extra target (id: %s)', targetId)

try {
await this.browserClient.send('Target.closeTarget', { targetId })
chrisbreiding marked this conversation as resolved.
Show resolved Hide resolved
} catch (err: any) {
debug('Closing extra target errored: %s', err?.stack || err)
}
}))
}

/**
* @returns the websocket debugger URL for the currently connected browser
*/
Expand Down
4 changes: 4 additions & 0 deletions packages/server/lib/browsers/chrome.ts
Original file line number Diff line number Diff line change
Expand Up @@ -651,4 +651,8 @@ export = {
// with additional method to close the remote connection
return launchedBrowser
},

async closeExtraTargets () {
return browserCriClient?.closeExtraTargets()
},
}
4 changes: 4 additions & 0 deletions packages/server/lib/browsers/electron.ts
Original file line number Diff line number Diff line change
Expand Up @@ -545,4 +545,8 @@ export = {

return instance
},

async closeExtraTargets () {
return browserCriClient?.closeExtraTargets()
},
}
7 changes: 7 additions & 0 deletions packages/server/lib/browsers/firefox.ts
Original file line number Diff line number Diff line change
Expand Up @@ -583,3 +583,10 @@ export async function open (browser: Browser, url: string, options: BrowserLaunc

return browserInstance
}

export async function closeExtraTargets () {
chrisbreiding marked this conversation as resolved.
Show resolved Hide resolved
// we're currently holding off on implementing Firefox support in order
// to release Chromium support as soon as possible and may add Firefox
// support in the future
debug('Closing extra targets is not currently supported in Firefox')
}
11 changes: 11 additions & 0 deletions packages/server/lib/browsers/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -273,4 +273,15 @@ export = {

return instance
},

/**
* Closes extra targets that are not the Cypress tab
*/
async closeExtraTargets () {
if (!instance?.browser) return

const browserLauncher = await getBrowserLauncher(instance.browser, [])

await browserLauncher.closeExtraTargets()
},
} as const
4 changes: 4 additions & 0 deletions packages/server/lib/browsers/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,10 @@ export type BrowserLauncher = {
* Used to connect the protocol to an existing browser.
*/
connectProtocolToBrowser: (options: { protocolManager?: ProtocolManagerShape }) => Promise<void>
/**
* Closes any targets that are not the currently-attached Cypress target
*/
closeExtraTargets: () => Promise<void>
}

export type GracefulShutdownOptions = {
Expand Down
7 changes: 7 additions & 0 deletions packages/server/lib/browsers/webkit.ts
Original file line number Diff line number Diff line change
Expand Up @@ -154,3 +154,10 @@ export async function open (browser: Browser, url: string, options: BrowserLaunc

return new WkInstance()
}

export async function closeExtraTargets () {
// we're currently holding off on implementing Webkit support in order
// to release Chromium support as soon as possible and may add Webkit
// support in the future
debug('Closing extra targets is not currently supported in Webkit')
}
5 changes: 5 additions & 0 deletions packages/server/lib/project-base.ts
Original file line number Diff line number Diff line change
Expand Up @@ -349,6 +349,7 @@ export class ProjectBase extends EE {
onFocusTests: options.onFocusTests,
onSpecChanged: options.onSpecChanged,
onSavedStateChanged: (state: any) => this.saveState(state),
closeExtraTargets: this.closeExtraTargets,

onCaptureVideoFrames: (data: any) => {
// TODO: move this to browser automation middleware
Expand Down Expand Up @@ -417,6 +418,10 @@ export class ProjectBase extends EE {
return this.server.socket.resetBrowserState()
}

closeExtraTargets () {
return browsers.closeExtraTargets()
}

isRunnerSocketConnected () {
return this.server.socket.isRunnerSocketConnected()
}
Expand Down
4 changes: 3 additions & 1 deletion packages/server/lib/socket-base.ts
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,7 @@ export class SocketBase {
onSpecChanged () {},
onChromiumRun () {},
onReloadBrowser () {},
checkForAppErrors () {},
closeExtraTargets () {},
onSavedStateChanged () {},
onTestFileChange () {},
onCaptureVideoFrames () {},
Expand Down Expand Up @@ -478,6 +478,8 @@ export class SocketBase {
return (telemetry.exporter() as OTLPTraceExporterCloud)?.send(args[0], () => {}, (err) => {
debug('error exporting telemetry data from browser %s', err)
})
case 'close:extra:targets':
chrisbreiding marked this conversation as resolved.
Show resolved Hide resolved
return options.closeExtraTargets()
default:
throw new Error(`You requested a backend event we cannot handle: ${eventName}`)
}
Expand Down
34 changes: 33 additions & 1 deletion packages/server/test/unit/browsers/browser-cri-client_spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -361,7 +361,7 @@ describe('lib/browsers/cri-client', function () {
BrowserCriClient._onTargetDestroyed(options as any)

expect(options.browserCriClient.removeExtraTargetClient).to.be.calledWith('target-id')
// error is caught or else the test would fail
// error is caught or else the test would fail
})

it('removes the extra target client from the tracker', () => {
Expand Down Expand Up @@ -546,6 +546,38 @@ describe('lib/browsers/cri-client', function () {
})
})

context('#closeExtraTargets', () => {
it('closes any extra tracked targets', async () => {
const browserClient = await getClient() as any

browserClient.browserClient.send = sinon.stub().resolves()

browserClient.addExtraTargetClient({ targetId: 'target-id-1' }, {})
browserClient.addExtraTargetClient({ targetId: 'target-id-2' }, {})

await browserClient.closeExtraTargets()

expect(browserClient.browserClient.send).to.be.calledWith('Target.closeTarget', { targetId: 'target-id-1' })
expect(browserClient.browserClient.send).to.be.calledWith('Target.closeTarget', { targetId: 'target-id-2' })
})

it('ignores errors', async () => {
const browserClient = await getClient() as any

browserClient.browserClient.send = sinon.stub().resolves()
browserClient.browserClient.send.onFirstCall().rejects(new Error('failed to close target'))

browserClient.addExtraTargetClient({ targetId: 'target-id-1' }, {})
browserClient.addExtraTargetClient({ targetId: 'target-id-2' }, {})

await browserClient.closeExtraTargets()

expect(browserClient.browserClient.send).to.be.calledWith('Target.closeTarget', { targetId: 'target-id-1' })
expect(browserClient.browserClient.send).to.be.calledWith('Target.closeTarget', { targetId: 'target-id-2' })
// error is caught or else the test would fail
})
})

context('#close', function () {
it('closes the currently attached target if it exists and the browser client', async function () {
const mockCurrentlyAttachedTarget = {
Expand Down
Loading