From 49d699336bc3283353418a357bb715abbf8bf86d Mon Sep 17 00:00:00 2001 From: Mark Noonan Date: Wed, 23 Feb 2022 11:23:11 -0500 Subject: [PATCH] chore(unify): always allow devtools in launchpad (#20298) * no longer check if dev to show devtools * match tests to new behavior * remove 'withDevTools` option * Revert "remove 'withDevTools` option" This reverts commit ff4a8b96e4a446e749475175829e6b3a2f0d51ce. * rename withDevTools to withInteralDevTools * coniditionally show internal dev tools in menu * test nesting --- packages/server/lib/browsers/electron.js | 4 +- packages/server/lib/gui/menu.js | 115 +++++++++--------- packages/server/lib/modes/interactive-e2e.ts | 6 +- .../test/unit/browsers/electron_spec.js | 8 +- packages/server/test/unit/gui/menu_spec.js | 29 +++-- packages/server/test/unit/gui/windows_spec.ts | 6 +- .../test/unit/modes/interactive_spec.js | 16 +-- 7 files changed, 100 insertions(+), 84 deletions(-) diff --git a/packages/server/lib/browsers/electron.js b/packages/server/lib/browsers/electron.js index d9dd46ba52..74b6e4c6bb 100644 --- a/packages/server/lib/browsers/electron.js +++ b/packages/server/lib/browsers/electron.js @@ -146,7 +146,7 @@ module.exports = { }, onFocus () { if (options.show) { - return menu.set({ withDevTools: true }) + return menu.set({ withInternalDevTools: true }) } }, onNewWindow (e, url) { @@ -222,7 +222,7 @@ module.exports = { _launch (win, url, automation, options) { if (options.show) { - menu.set({ withDevTools: true }) + menu.set({ withInternalDevTools: true }) } ELECTRON_DEBUG_EVENTS.forEach((e) => { diff --git a/packages/server/lib/gui/menu.js b/packages/server/lib/gui/menu.js index 7927721749..9f95518656 100644 --- a/packages/server/lib/gui/menu.js +++ b/packages/server/lib/gui/menu.js @@ -167,64 +167,69 @@ module.exports = { }) } - if (options.withDevTools) { - template.push( - { - label: 'Developer Tools', - submenu: [ - { - label: 'Reload', - accelerator: 'CmdOrCtrl+R', - click: (item, focusedWindow) => { - if (focusedWindow) { - return focusedWindow.reload() - } - }, - }, - { - label: 'Toggle Developer Tools', - accelerator: (() => { - if (os.platform() === 'darwin') { - return 'Alt+Command+I' - } - - return 'Ctrl+Shift+I' - })(), - click: (item, focusedWindow) => { - if (focusedWindow) { - return focusedWindow.toggleDevTools() - } - }, - }, - { - label: `GraphQL requests over Fetch (${process.env.CYPRESS_INTERNAL_GQL_NO_SOCKET ? 'on' : 'off'})`, - click: (item, focusedWindow) => { - if (process.env.CYPRESS_INTERNAL_GQL_NO_SOCKET) { - delete process.env.CYPRESS_INTERNAL_GQL_NO_SOCKET - } else { - process.env.CYPRESS_INTERNAL_GQL_NO_SOCKET = '1' - } - - this.set(opts) - }, - }, - { - label: 'GraphiQL', - click () { - return shell.openExternal(`http://localhost:${options.getGraphQLPort()}/__launchpad/graphql`) - }, - }, - { - label: 'View App Data', - click () { - return open.opn(appData.path()) - }, - }, - ], + let devToolsSubmenu = [ + { + label: 'Reload', + accelerator: 'CmdOrCtrl+R', + click: (item, focusedWindow) => { + if (focusedWindow) { + return focusedWindow.reload() + } }, - ) + }, + { + label: 'Toggle Developer Tools', + accelerator: (() => { + if (os.platform() === 'darwin') { + return 'Alt+Command+I' + } + + return 'Ctrl+Shift+I' + })(), + click: (item, focusedWindow) => { + if (focusedWindow) { + return focusedWindow.toggleDevTools() + } + }, + }, + { + label: 'View App Data', + click () { + return open.opn(appData.path()) + }, + }, + ] + + if (options.withInternalDevTools) { + devToolsSubmenu = devToolsSubmenu.concat([ + { + label: `GraphQL requests over Fetch (${process.env.CYPRESS_INTERNAL_GQL_NO_SOCKET ? 'on' : 'off'})`, + click: (item, focusedWindow) => { + if (process.env.CYPRESS_INTERNAL_GQL_NO_SOCKET) { + delete process.env.CYPRESS_INTERNAL_GQL_NO_SOCKET + } else { + process.env.CYPRESS_INTERNAL_GQL_NO_SOCKET = '1' + } + + this.set(opts) + }, + }, + { + label: 'GraphiQL', + click () { + return shell.openExternal(`http://localhost:${options.getGraphQLPort()}/__launchpad/graphql`) + }, + }, + ]) } + template.push( + { + label: 'Developer Tools', + submenu: devToolsSubmenu, + }, + ) + const menu = Menu.buildFromTemplate(template) return Menu.setApplicationMenu(menu) diff --git a/packages/server/lib/modes/interactive-e2e.ts b/packages/server/lib/modes/interactive-e2e.ts index 97fd6e7546..562ce82ebb 100644 --- a/packages/server/lib/modes/interactive-e2e.ts +++ b/packages/server/lib/modes/interactive-e2e.ts @@ -92,9 +92,9 @@ export = { return Windows.hideAllUnlessAnotherWindowIsFocused() }, onFocus () { - // hide dev tools if in production and previously focused + // hide internal dev tools if in production and previously focused // window was the electron browser - menu.set({ withDevTools: isDev() }) + menu.set({ withInternalDevTools: isDev() }) return Windows.showAll() }, @@ -136,7 +136,7 @@ export = { // TODO: potentially just pass an event emitter // instance here instead of callback functions menu.set({ - withDevTools: isDev(), + withInternalDevTools: isDev(), onLogOutClicked () { return globalPubSub.emit('menu:item:clicked', 'log:out') }, diff --git a/packages/server/test/unit/browsers/electron_spec.js b/packages/server/test/unit/browsers/electron_spec.js index c6089d8d70..b54937a375 100644 --- a/packages/server/test/unit/browsers/electron_spec.js +++ b/packages/server/test/unit/browsers/electron_spec.js @@ -190,7 +190,7 @@ describe('lib/browsers/electron', () => { it('sets menu.set whether or not its in headless mode', function () { return electron._launch(this.win, this.url, this.automation, { show: true }) .then(() => { - expect(menu.set).to.be.calledWith({ withDevTools: true }) + expect(menu.set).to.be.calledWith({ withInternalDevTools: true }) }).then(() => { menu.set.reset() @@ -439,7 +439,7 @@ describe('lib/browsers/electron', () => { let opts = electron._defaultOptions('/foo', this.state, { show: true, browser: {} }) opts.onFocus() - expect(menu.set).to.be.calledWith({ withDevTools: true }) + expect(menu.set).to.be.calledWith({ withInternalDevTools: true }) menu.set.reset() @@ -581,7 +581,7 @@ describe('lib/browsers/electron', () => { // once for main window, once for child expect(menu.set).to.be.calledTwice - expect(menu.set).to.be.calledWith({ withDevTools: true }) + expect(menu.set).to.be.calledWith({ withInternalDevTools: true }) }) }) @@ -591,7 +591,7 @@ describe('lib/browsers/electron', () => { // once for main window, once for child, once for focus expect(menu.set).to.be.calledThrice - expect(menu.set).to.be.calledWith({ withDevTools: true }) + expect(menu.set).to.be.calledWith({ withInternalDevTools: true }) }) }) diff --git a/packages/server/test/unit/gui/menu_spec.js b/packages/server/test/unit/gui/menu_spec.js index 9c545de569..2ef15ec567 100644 --- a/packages/server/test/unit/gui/menu_spec.js +++ b/packages/server/test/unit/gui/menu_spec.js @@ -252,19 +252,30 @@ describe('gui/menu', function () { }) context('Developer Tools', () => { - it('does not exist by default', () => { + it('exists by default', () => { menu.set() - expect(getMenuItem('Developer Tools')).to.be.undefined + expect(getMenuItem('Developer Tools')).to.be.defined }) - it('does not exist by when withDevTools is false', () => { - menu.set({ withDevTools: false }) - expect(getMenuItem('Developer Tools')).to.be.undefined + it('exists when withInternalDevTools is false', () => { + menu.set({ withInternalDevTools: false }) + expect(getMenuItem('Developer Tools')).to.be.defined }) - describe('when withDevTools is true', () => { + it('contains only Reload and Toggle Developer Tools items in expected order', () => { + menu.set() + const labels = getLabels(getMenuItem('Developer Tools').submenu) + + expect(labels).to.eql([ + 'Reload', + 'Toggle Developer Tools', + 'View App Data', + ]) + }) + + describe('when withInternalDevTools is true', () => { beforeEach(function () { - menu.set({ withDevTools: true }) + menu.set({ withInternalDevTools: true }) this.devSubmenu = getMenuItem('Developer Tools').submenu }) @@ -274,9 +285,9 @@ describe('gui/menu', function () { expect(labels).to.eql([ 'Reload', 'Toggle Developer Tools', + 'View App Data', 'GraphQL requests over Fetch (off)', 'GraphiQL', - 'View App Data', ]) }) @@ -301,7 +312,7 @@ describe('gui/menu', function () { it('sets shortcut for Toggle Developer Tools when not macOS', () => { os.platform.returns('linux') - menu.set({ withDevTools: true }) + menu.set({ withInternalDevTools: true }) expect(getMenuItem('Developer Tools').submenu[1].accelerator).to.equal('Ctrl+Shift+I') }) diff --git a/packages/server/test/unit/gui/windows_spec.ts b/packages/server/test/unit/gui/windows_spec.ts index c074c00403..ad7dee54b9 100644 --- a/packages/server/test/unit/gui/windows_spec.ts +++ b/packages/server/test/unit/gui/windows_spec.ts @@ -90,7 +90,7 @@ describe('lib/gui/windows', () => { height: 'someHeight', x: 'anX', y: 'aY', - devTools: 'whatsUpWithDevTools', + devTools: 'whatsUpwithInternalDevTools', } }) }) @@ -167,7 +167,7 @@ describe('lib/gui/windows', () => { return Promise .delay(100) .then(() => { - expect(this.state.set).to.be.calledWith({ whatsUpWithDevTools: true }) + expect(this.state.set).to.be.calledWith({ whatsUpwithInternalDevTools: true }) }) }) @@ -178,7 +178,7 @@ describe('lib/gui/windows', () => { return Promise .delay(100) .then(() => { - expect(this.state.set).to.be.calledWith({ whatsUpWithDevTools: false }) + expect(this.state.set).to.be.calledWith({ whatsUpwithInternalDevTools: false }) }) }) }) diff --git a/packages/server/test/unit/modes/interactive_spec.js b/packages/server/test/unit/modes/interactive_spec.js index 2a5fa29d8c..a31c59f971 100644 --- a/packages/server/test/unit/modes/interactive_spec.js +++ b/packages/server/test/unit/modes/interactive_spec.js @@ -101,21 +101,21 @@ describe('gui/interactive', () => { sinon.stub(menu, 'set') }) - it('calls menu.set withDevTools: true when in dev env', () => { + it('calls menu.set withInternalDevTools: true when in dev env', () => { const env = process.env['CYPRESS_INTERNAL_ENV'] process.env['CYPRESS_INTERNAL_ENV'] = 'development' interactiveMode.getWindowArgs({}).onFocus() - expect(menu.set.lastCall.args[0].withDevTools).to.be.true + expect(menu.set.lastCall.args[0].withInternalDevTools).to.be.true process.env['CYPRESS_INTERNAL_ENV'] = env }) - it('calls menu.set withDevTools: false when not in dev env', () => { + it('calls menu.set withInternalDevTools: false when not in dev env', () => { const env = process.env['CYPRESS_INTERNAL_ENV'] process.env['CYPRESS_INTERNAL_ENV'] = 'production' interactiveMode.getWindowArgs({}).onFocus() - expect(menu.set.lastCall.args[0].withDevTools).to.be.false + expect(menu.set.lastCall.args[0].withInternalDevTools).to.be.false process.env['CYPRESS_INTERNAL_ENV'] = env }) }) @@ -155,24 +155,24 @@ describe('gui/interactive', () => { }) }) - it('calls menu.set withDevTools: true when in dev env', () => { + it('calls menu.set withInternalDevTools: true when in dev env', () => { const env = process.env['CYPRESS_INTERNAL_ENV'] process.env['CYPRESS_INTERNAL_ENV'] = 'development' return interactiveMode.ready({}).then(() => { - expect(menu.set.lastCall.args[0].withDevTools).to.be.true + expect(menu.set.lastCall.args[0].withInternalDevTools).to.be.true process.env['CYPRESS_INTERNAL_ENV'] = env }) }) - it('calls menu.set withDevTools: false when not in dev env', () => { + it('calls menu.set withInternalDevTools: false when not in dev env', () => { const env = process.env['CYPRESS_INTERNAL_ENV'] process.env['CYPRESS_INTERNAL_ENV'] = 'production' return interactiveMode.ready({}).then(() => { - expect(menu.set.lastCall.args[0].withDevTools).to.be.false + expect(menu.set.lastCall.args[0].withInternalDevTools).to.be.false process.env['CYPRESS_INTERNAL_ENV'] = env }) })