diff --git a/cli/CHANGELOG.md b/cli/CHANGELOG.md index 31684f940332..0c07abd2a7b2 100644 --- a/cli/CHANGELOG.md +++ b/cli/CHANGELOG.md @@ -11,6 +11,7 @@ _Released 5/7/2024 (PENDING)_ - Fixed a bug where promises rejected with `undefined` were failing inside `cy.origin()`. Addresses [#23937](https://github.com/cypress-io/cypress/issues/23937). - We now pass the same default Chromium flags to Electron as we do to Chrome. As a result of this change, the application under test's `navigator.webdriver` property will now correctly be `true` when testing in Electron. Fixes [#27939](https://github.com/cypress-io/cypress/issues/27939). +- Fixed an issue where extra windows weren't being closed between specs in Firefox causing potential issues in subsequent specs. Fixes [#29473](https://github.com/cypress-io/cypress/issues/29473). **Misc:** diff --git a/packages/data-context/src/actions/ProjectActions.ts b/packages/data-context/src/actions/ProjectActions.ts index 167ee68cb433..744e1f4a132c 100644 --- a/packages/data-context/src/actions/ProjectActions.ts +++ b/packages/data-context/src/actions/ProjectActions.ts @@ -52,7 +52,7 @@ export interface ProjectApiShape { emitter: EventEmitter } isListening: (url: string) => Promise - resetBrowserTabsForNextTest(shouldKeepTabOpen: boolean): Promise + resetBrowserTabsForNextSpec(shouldKeepTabOpen: boolean): Promise resetServer(): void runSpec(spec: Cypress.Spec): Promise routeToDebug(runNumber: number): void @@ -279,7 +279,7 @@ export class ProjectActions { // Used for run-all-specs feature if (options?.shouldLaunchNewTab) { - await this.api.resetBrowserTabsForNextTest(true) + await this.api.resetBrowserTabsForNextSpec(true) this.api.resetServer() } diff --git a/packages/extension/app/v2/background.js b/packages/extension/app/v2/background.js index e64e5a4e377a..e3203174ee08 100644 --- a/packages/extension/app/v2/background.js +++ b/packages/extension/app/v2/background.js @@ -136,8 +136,8 @@ const connect = function (host, path, extraOpts) { return invoke('takeScreenshot', id) case 'reset:browser:state': return invoke('resetBrowserState', id) - case 'reset:browser:tabs:for:next:test': - return invoke('resetBrowserTabsForNextTest', id) + case 'reset:browser:tabs:for:next:spec': + return invoke('resetBrowserTabsForNextSpec', id) default: return fail(id, { message: `No handler registered for: '${msg}'` }) } @@ -284,10 +284,21 @@ 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) }, - resetBrowserTabsForNextTest (fn) { + resetBrowserTabsForNextSpec (callback) { return Promise.try(() => { return browser.windows.getCurrent({ populate: true }) - }).then(async (windowInfo) => { + }).then(async (currentWindowInfo) => { + const windows = await browser.windows.getAll().catch(() => []) + + for (const window of windows) { + // remove/close the window if it's not the current window + if (window.id !== currentWindowInfo.id) { + await browser.windows.remove(window.id).catch(() => {}) + } + } + + return currentWindowInfo + }).then(async (currentWindowInfo) => { let newTabId = null try { @@ -300,8 +311,8 @@ const automation = { // eslint-disable-next-line no-empty } catch (e) {} - return browser.tabs.remove(windowInfo.tabs.map((tab) => tab.id).filter((tab) => tab.id !== newTabId)) - }).then(fn) + return browser.tabs.remove(currentWindowInfo.tabs.map((tab) => tab.id).filter((tab) => tab.id !== newTabId)) + }).then(callback) }, query (data) { diff --git a/packages/extension/test/integration/v2/background_spec.js b/packages/extension/test/integration/v2/background_spec.js index 2e2a6ce8cae7..9fdf21900cd4 100644 --- a/packages/extension/test/integration/v2/background_spec.js +++ b/packages/extension/test/integration/v2/background_spec.js @@ -24,8 +24,10 @@ const browser = { }, }, windows: { - getLastFocused () {}, + getAll () {}, getCurrent () {}, + getLastFocused () {}, + remove () {}, update () {}, }, runtime: {}, @@ -844,7 +846,7 @@ describe('app/background', () => { }) }) - describe('reset:browser:tabs:for:next:test', () => { + describe('reset:browser:tabs:for:next:spec', () => { beforeEach(() => { 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() @@ -855,6 +857,8 @@ describe('app/background', () => { // @see https://github.com/cypress-io/cypress/issues/29172 for Firefox versions 124 and up it('closes the tabs in the current browser window and creates a new "about:blank" tab', function (done) { + sinon.stub(browser.windows, 'getAll').resolves([{ id: '10' }]) + this.socket.on('automation:response', (id, obj) => { expect(id).to.eq(123) expect(obj.response).to.be.undefined @@ -866,7 +870,58 @@ describe('app/background', () => { done() }) - this.server.emit('automation:request', 123, 'reset:browser:tabs:for:next:test') + this.server.emit('automation:request', 123, 'reset:browser:tabs:for:next:spec') + }) + + it('closes any extra windows', function (done) { + sinon.stub(browser.windows, 'getAll').resolves([{ id: '9' }, { id: '10' }, { id: '11' }]) + sinon.stub(browser.windows, 'remove').resolves() + + this.socket.on('automation:response', (id, obj) => { + expect(id).to.eq(123) + expect(obj.response).to.be.undefined + + expect(browser.windows.remove).to.be.calledWith('9') + expect(browser.windows.remove).to.be.calledWith('11') + expect(browser.windows.remove).not.to.be.calledWith('10') + + done() + }) + + this.server.emit('automation:request', 123, 'reset:browser:tabs:for:next:spec') + }) + + it('does not fail if we are unable to close the window', function (done) { + sinon.stub(browser.windows, 'getAll').resolves([{ id: '9' }, { id: '10' }, { id: '11' }]) + sinon.stub(browser.windows, 'remove').rejects() + + this.socket.on('automation:response', (id, obj) => { + expect(id).to.eq(123) + expect(obj.response).to.be.undefined + + expect(browser.windows.remove).to.be.calledWith('9') + expect(browser.windows.remove).to.be.calledWith('11') + + expect(browser.windows.remove).not.to.be.calledWith('10') + done() + }) + + this.server.emit('automation:request', 123, 'reset:browser:tabs:for:next:spec') + }) + + it('does not fail if we are unable to retrieve the windows', function (done) { + sinon.stub(browser.windows, 'getAll').rejects() + sinon.stub(browser.windows, 'remove') + + this.socket.on('automation:response', (id, obj) => { + expect(id).to.eq(123) + expect(obj.response).to.be.undefined + + expect(browser.windows.remove).not.to.be.called + done() + }) + + this.server.emit('automation:request', 123, 'reset:browser:tabs:for:next:spec') }) }) }) diff --git a/packages/server/lib/browsers/cdp_automation.ts b/packages/server/lib/browsers/cdp_automation.ts index 448bf867bfea..29ca5ec411be 100644 --- a/packages/server/lib/browsers/cdp_automation.ts +++ b/packages/server/lib/browsers/cdp_automation.ts @@ -578,7 +578,7 @@ export class CdpAutomation implements CDPClient { this.sendDebuggerCommandFn('Storage.clearDataForOrigin', { origin: '*', storageTypes: 'all' }), this.sendDebuggerCommandFn('Network.clearBrowserCache'), ]) - case 'reset:browser:tabs:for:next:test': + case 'reset:browser:tabs:for:next:spec': return this.sendCloseCommandFn(data.shouldKeepTabOpen) case 'focus:browser:window': return this.sendDebuggerCommandFn('Page.bringToFront') diff --git a/packages/server/lib/browsers/webkit-automation.ts b/packages/server/lib/browsers/webkit-automation.ts index da30566384ae..d9c16798987b 100644 --- a/packages/server/lib/browsers/webkit-automation.ts +++ b/packages/server/lib/browsers/webkit-automation.ts @@ -373,7 +373,7 @@ export class WebKitAutomation { debug('stubbed reset:browser:state') return - case 'reset:browser:tabs:for:next:test': + case 'reset:browser:tabs:for:next:spec': if (data.shouldKeepTabOpen) return await this.reset({}) return await this.context.browser()?.close() diff --git a/packages/server/lib/makeDataContext.ts b/packages/server/lib/makeDataContext.ts index 411f6e14cb48..159d09ae72c2 100644 --- a/packages/server/lib/makeDataContext.ts +++ b/packages/server/lib/makeDataContext.ts @@ -155,8 +155,8 @@ export function makeDataContext (options: MakeDataContextOptions): DataContext { return devServer }, isListening, - resetBrowserTabsForNextTest (shouldKeepTabOpen: boolean) { - return openProject.resetBrowserTabsForNextTest(shouldKeepTabOpen) + resetBrowserTabsForNextSpec (shouldKeepTabOpen: boolean) { + return openProject.resetBrowserTabsForNextSpec(shouldKeepTabOpen) }, resetServer () { return openProject.getProject()?.server.reset() diff --git a/packages/server/lib/modes/run.ts b/packages/server/lib/modes/run.ts index 746bdac2c29f..5dec728b9563 100644 --- a/packages/server/lib/modes/run.ts +++ b/packages/server/lib/modes/run.ts @@ -652,7 +652,7 @@ async function waitForTestsToFinishRunning (options: { project: Project, screens if (!usingExperimentalSingleTabMode || isLastSpec) { debug('attempting to close the browser tab') - await openProject.resetBrowserTabsForNextTest(shouldKeepTabOpen) + await openProject.resetBrowserTabsForNextSpec(shouldKeepTabOpen) debug('resetting server state') diff --git a/packages/server/lib/open_project.ts b/packages/server/lib/open_project.ts index 7242d742c921..76e5ced9399b 100644 --- a/packages/server/lib/open_project.ts +++ b/packages/server/lib/open_project.ts @@ -199,9 +199,9 @@ export class OpenProject { return browsers.close() } - async resetBrowserTabsForNextTest (shouldKeepTabOpen: boolean) { + async resetBrowserTabsForNextSpec (shouldKeepTabOpen: boolean) { try { - await this.projectBase?.resetBrowserTabsForNextTest(shouldKeepTabOpen) + await this.projectBase?.resetBrowserTabsForNextSpec(shouldKeepTabOpen) } catch (e) { // If the CRI client disconnected or crashed, we want to no-op here so that anything // depending on resetting the browser tabs can continue with further operations diff --git a/packages/server/lib/project-base.ts b/packages/server/lib/project-base.ts index c373438e3c06..39403ade4b81 100644 --- a/packages/server/lib/project-base.ts +++ b/packages/server/lib/project-base.ts @@ -440,8 +440,8 @@ export class ProjectBase extends EE { this.ctx.actions.servers.setAppSocketServer(ios) } - async resetBrowserTabsForNextTest (shouldKeepTabOpen: boolean) { - return this.server.socket.resetBrowserTabsForNextTest(shouldKeepTabOpen) + async resetBrowserTabsForNextSpec (shouldKeepTabOpen: boolean) { + return this.server.socket.resetBrowserTabsForNextSpec(shouldKeepTabOpen) } async resetBrowserState () { diff --git a/packages/server/lib/socket-base.ts b/packages/server/lib/socket-base.ts index 4b165f445ab9..142c1b30a6a4 100644 --- a/packages/server/lib/socket-base.ts +++ b/packages/server/lib/socket-base.ts @@ -41,7 +41,7 @@ const retry = (fn: (res: any) => void) => { } export class SocketBase { - private _sendResetBrowserTabsForNextTestMessage + private _sendResetBrowserTabsForNextSpecMessage private _sendResetBrowserStateMessage private _isRunnerSocketConnected private _sendFocusBrowserMessage @@ -281,8 +281,8 @@ export class SocketBase { }) }) - this._sendResetBrowserTabsForNextTestMessage = async (shouldKeepTabOpen: boolean) => { - await automationRequest('reset:browser:tabs:for:next:test', { shouldKeepTabOpen }) + this._sendResetBrowserTabsForNextSpecMessage = async (shouldKeepTabOpen: boolean) => { + await automationRequest('reset:browser:tabs:for:next:spec', { shouldKeepTabOpen }) } this._sendResetBrowserStateMessage = async () => { @@ -616,9 +616,9 @@ export class SocketBase { }) } - async resetBrowserTabsForNextTest (shouldKeepTabOpen: boolean) { - if (this._sendResetBrowserTabsForNextTestMessage) { - await this._sendResetBrowserTabsForNextTestMessage(shouldKeepTabOpen) + async resetBrowserTabsForNextSpec (shouldKeepTabOpen: boolean) { + if (this._sendResetBrowserTabsForNextSpecMessage) { + await this._sendResetBrowserTabsForNextSpecMessage(shouldKeepTabOpen) } } diff --git a/packages/server/test/unit/browsers/cdp_automation_spec.ts b/packages/server/test/unit/browsers/cdp_automation_spec.ts index 65eef466f314..db3f9e85e6d6 100644 --- a/packages/server/test/unit/browsers/cdp_automation_spec.ts +++ b/packages/server/test/unit/browsers/cdp_automation_spec.ts @@ -570,11 +570,11 @@ context('lib/browsers/cdp_automation', () => { }) }) - describe('reset:browser:tabs:for:next:test', function () { + describe('reset:browser:tabs:for:next:spec', function () { it('sends the close target message for the attached target tabs', async function () { this.sendCloseTargetCommand.resolves() - await this.onRequest('reset:browser:tabs:for:next:test', { shouldKeepTabOpen: true }) + await this.onRequest('reset:browser:tabs:for:next:spec', { shouldKeepTabOpen: true }) expect(this.sendCloseTargetCommand).to.be.calledWith(true) }) diff --git a/packages/server/test/unit/browsers/electron_spec.js b/packages/server/test/unit/browsers/electron_spec.js index 45edec21283d..2cde219306b8 100644 --- a/packages/server/test/unit/browsers/electron_spec.js +++ b/packages/server/test/unit/browsers/electron_spec.js @@ -435,7 +435,7 @@ describe('lib/browsers/electron', () => { expect(this.automation.use).to.be.called expect(this.automation.use.lastCall.args[0].onRequest).to.be.a('function') - await this.automation.use.lastCall.args[0].onRequest('reset:browser:tabs:for:next:test', { shouldKeepTabOpen: true }) + await this.automation.use.lastCall.args[0].onRequest('reset:browser:tabs:for:next:spec', { shouldKeepTabOpen: true }) expect(this.win.destroy).to.be.called })