From 4c449fbc75dcf6bc5fb8831f12814a010f517fb4 Mon Sep 17 00:00:00 2001 From: Milan Burda Date: Wed, 1 Jul 2020 21:14:38 +0200 Subject: [PATCH] test: convert more tests to async / await (#24373) --- spec-main/api-app-spec.ts | 45 ++++-------- spec-main/api-browser-window-spec.ts | 100 +++++++++++++-------------- spec-main/api-menu-spec.ts | 34 ++++----- spec-main/api-web-contents-spec.ts | 4 +- spec-main/chromium-spec.ts | 12 ++-- spec-main/webview-spec.ts | 15 ++-- spec/webview-spec.js | 43 ++++++------ 7 files changed, 116 insertions(+), 137 deletions(-) diff --git a/spec-main/api-app-spec.ts b/spec-main/api-app-spec.ts index 05e3822c32d9..ac3005248c0a 100644 --- a/spec-main/api-app-spec.ts +++ b/spec-main/api-app-spec.ts @@ -5,6 +5,7 @@ import * as http from 'http'; import * as net from 'net'; import * as fs from 'fs'; import * as path from 'path'; +import { promisify } from 'util'; import { app, BrowserWindow, Menu, session } from 'electron/main'; import { emittedOnce } from './events-helpers'; import { closeWindow, closeAllWindows } from './window-helpers'; @@ -891,34 +892,24 @@ describe('app module', () => { expect(app.isDefaultProtocolClient(protocol)).to.equal(false); }); - it('creates a registry entry for the protocol class', (done) => { + it('creates a registry entry for the protocol class', async () => { app.setAsDefaultProtocolClient(protocol); - classesKey.keys((error: Error, keys: any[]) => { - if (error) throw error; - - const exists = !!keys.find(key => key.key.includes(protocol)); - expect(exists).to.equal(true); - - done(); - }); + const keys = await promisify(classesKey.keys).call(classesKey) as any[]; + const exists = !!keys.find(key => key.key.includes(protocol)); + expect(exists).to.equal(true); }); - it('completely removes a registry entry for the protocol class', (done) => { + it('completely removes a registry entry for the protocol class', async () => { app.setAsDefaultProtocolClient(protocol); app.removeAsDefaultProtocolClient(protocol); - classesKey.keys((error: Error, keys: any[]) => { - if (error) throw error; - - const exists = !!keys.find(key => key.key.includes(protocol)); - expect(exists).to.equal(false); - - done(); - }); + const keys = await promisify(classesKey.keys).call(classesKey) as any[]; + const exists = !!keys.find(key => key.key.includes(protocol)); + expect(exists).to.equal(false); }); - it('only unsets a class registry key if it contains other data', (done) => { + it('only unsets a class registry key if it contains other data', async () => { app.setAsDefaultProtocolClient(protocol); const protocolKey = new Winreg({ @@ -926,18 +917,12 @@ describe('app module', () => { key: `\\Software\\Classes\\${protocol}` }); - protocolKey.set('test-value', 'REG_BINARY', '123', () => { - app.removeAsDefaultProtocolClient(protocol); + await promisify(protocolKey.set).call(protocolKey, 'test-value', 'REG_BINARY', '123'); + app.removeAsDefaultProtocolClient(protocol); - classesKey.keys((error: Error, keys: any[]) => { - if (error) throw error; - - const exists = !!keys.find(key => key.key.includes(protocol)); - expect(exists).to.equal(true); - - done(); - }); - }); + const keys = await promisify(classesKey.keys).call(classesKey) as any[]; + const exists = !!keys.find(key => key.key.includes(protocol)); + expect(exists).to.equal(true); }); it('sets the default client such that getApplicationNameForProtocol returns Electron', () => { diff --git a/spec-main/api-browser-window-spec.ts b/spec-main/api-browser-window-spec.ts index 57756c70cd2a..5d3f54396bc7 100644 --- a/spec-main/api-browser-window-spec.ts +++ b/spec-main/api-browser-window-spec.ts @@ -311,13 +311,15 @@ describe('BrowserWindow module', () => { server.close(); }); - it('should emit did-start-loading event', (done) => { - w.webContents.on('did-start-loading', () => { done(); }); + it('should emit did-start-loading event', async () => { + const didStartLoading = emittedOnce(w.webContents, 'did-start-loading'); w.loadURL('about:blank'); + await didStartLoading; }); - it('should emit ready-to-show event', (done) => { - w.on('ready-to-show', () => { done(); }); + it('should emit ready-to-show event', async () => { + const readyToShow = emittedOnce(w, 'ready-to-show'); w.loadURL('about:blank'); + await readyToShow; }); // TODO(deepak1556): The error code now seems to be `ERR_FAILED`, verify what // changed and adjust the test. @@ -539,11 +541,10 @@ describe('BrowserWindow module', () => { after(() => { server.close(); }); - it('is emitted on redirects', (done) => { - w.webContents.on('will-redirect', () => { - done(); - }); + it('is emitted on redirects', async () => { + const willRedirect = emittedOnce(w.webContents, 'will-redirect'); w.loadURL(`${url}/302`); + await willRedirect; }); it('is emitted after will-navigate on redirects', async () => { @@ -1986,15 +1987,6 @@ describe('BrowserWindow module', () => { }); describe('"sandbox" option', () => { - function waitForEvents (emitter: { once: Function }, events: string[], callback: () => void) { - let count = events.length; - for (const event of events) { - emitter.once(event, () => { - if (!--count) callback(); - }); - } - } - const preload = path.join(path.resolve(__dirname, 'fixtures'), 'module', 'preload-sandbox.js'); let server: http.Server = null as unknown as http.Server; @@ -2201,7 +2193,7 @@ describe('BrowserWindow module', () => { expect(webPreferences.foo).to.equal('bar'); }); - it('should set ipc event sender correctly', (done) => { + it('should set ipc event sender correctly', async () => { const w = new BrowserWindow({ show: false, webPreferences: { @@ -2224,11 +2216,13 @@ describe('BrowserWindow module', () => { expect(event.sender).to.equal(childWc, 'sender should be the child'); event.sender.send('verified'); }); - waitForEvents(ipcMain, [ + + const done = Promise.all([ 'parent-answer', 'child-answer' - ], done); + ].map(name => emittedOnce(ipcMain, name))); w.loadFile(path.join(__dirname, 'fixtures', 'api', 'sandbox.html'), { search: 'verify-ipc-sender' }); + await done; }); describe('event handling', () => { @@ -2236,24 +2230,24 @@ describe('BrowserWindow module', () => { beforeEach(() => { w = new BrowserWindow({ show: false, webPreferences: { sandbox: true } }); }); - it('works for window events', (done) => { - waitForEvents(w, [ - 'page-title-updated' - ], done); + it('works for window events', async () => { + const pageTitleUpdated = emittedOnce(w, 'page-title-updated'); w.loadURL('data:text/html,'); + await pageTitleUpdated; }); - it('works for stop events', (done) => { - waitForEvents(w.webContents, [ + it('works for stop events', async () => { + const done = Promise.all([ 'did-navigate', 'did-fail-load', 'did-stop-loading' - ], done); + ].map(name => emittedOnce(w.webContents, name))); w.loadURL('data:text/html,'); + await done; }); - it('works for web contents events', (done) => { - waitForEvents(w.webContents, [ + it('works for web contents events', async () => { + const done = Promise.all([ 'did-finish-load', 'did-frame-finish-load', 'did-navigate-in-page', @@ -2262,8 +2256,9 @@ describe('BrowserWindow module', () => { 'did-stop-loading', 'did-frame-finish-load', 'dom-ready' - ], done); + ].map(name => emittedOnce(w.webContents, name))); w.loadFile(path.join(__dirname, 'fixtures', 'api', 'sandbox.html'), { search: 'webcontents-events' }); + await done; }); }); @@ -2929,26 +2924,29 @@ describe('BrowserWindow module', () => { ifdescribe(process.platform !== 'linux')('max/minimize events', () => { afterEach(closeAllWindows); - it('emits an event when window is maximized', (done) => { + it('emits an event when window is maximized', async () => { const w = new BrowserWindow({ show: false }); - w.once('maximize', () => { done(); }); + const maximize = emittedOnce(w, 'maximize'); w.show(); w.maximize(); + await maximize; }); - it('emits an event when window is unmaximized', (done) => { + it('emits an event when window is unmaximized', async () => { const w = new BrowserWindow({ show: false }); - w.once('unmaximize', () => { done(); }); + const unmaximize = emittedOnce(w, 'unmaximize'); w.show(); w.maximize(); w.unmaximize(); + await unmaximize; }); - it('emits an event when window is minimized', (done) => { + it('emits an event when window is minimized', async () => { const w = new BrowserWindow({ show: false }); - w.once('minimize', () => { done(); }); + const minimize = emittedOnce(w, 'minimize'); w.show(); w.minimize(); + await minimize; }); }); @@ -3162,26 +3160,26 @@ describe('BrowserWindow module', () => { describe('parent window', () => { afterEach(closeAllWindows); - ifit(process.platform === 'darwin')('sheet-begin event emits when window opens a sheet', (done) => { + ifit(process.platform === 'darwin')('sheet-begin event emits when window opens a sheet', async () => { const w = new BrowserWindow(); - w.once('sheet-begin', () => { - done(); - }); + const sheetBegin = emittedOnce(w, 'sheet-begin'); // eslint-disable-next-line no-new new BrowserWindow({ modal: true, parent: w }); + await sheetBegin; }); - ifit(process.platform === 'darwin')('sheet-end event emits when window has closed a sheet', (done) => { + ifit(process.platform === 'darwin')('sheet-end event emits when window has closed a sheet', async () => { const w = new BrowserWindow(); const sheet = new BrowserWindow({ modal: true, parent: w }); - w.once('sheet-end', () => { done(); }); + const sheetEnd = emittedOnce(w, 'sheet-end'); sheet.close(); + await sheetEnd; }); describe('parent option', () => { @@ -3872,24 +3870,22 @@ describe('BrowserWindow module', () => { expect(w.isFullScreen()).to.be.false('isFullScreen'); }); - it('does not crash when exiting simpleFullScreen (properties)', (done) => { + it('does not crash when exiting simpleFullScreen (properties)', async () => { const w = new BrowserWindow(); w.setSimpleFullScreen(true); - setTimeout(() => { - w.setFullScreen(!w.isFullScreen()); - done(); - }, 1000); + await delay(1000); + + w.setFullScreen(!w.isFullScreen()); }); - it('does not crash when exiting simpleFullScreen (functions)', (done) => { + it('does not crash when exiting simpleFullScreen (functions)', async () => { const w = new BrowserWindow(); w.simpleFullScreen = true; - setTimeout(() => { - w.setFullScreen(!w.isFullScreen()); - done(); - }, 1000); + await delay(1000); + + w.setFullScreen(!w.isFullScreen()); }); it('should not be changed by setKiosk method', async () => { diff --git a/spec-main/api-menu-spec.ts b/spec-main/api-menu-spec.ts index 59d04be99788..cbce05964ae1 100644 --- a/spec-main/api-menu-spec.ts +++ b/spec-main/api-menu-spec.ts @@ -4,7 +4,7 @@ import { expect } from 'chai'; import { BrowserWindow, Menu, MenuItem } from 'electron/main'; import { sortMenuItems } from '../lib/browser/api/menu-utils'; import { emittedOnce } from './events-helpers'; -import { ifit } from './spec-helpers'; +import { ifit, delay } from './spec-helpers'; import { closeWindow } from './window-helpers'; const fixturesPath = path.resolve(__dirname, 'fixtures'); @@ -836,7 +836,7 @@ describe('Menu module', function () { }); }); - it('prevents menu from getting garbage-collected when popuping', (done) => { + it('prevents menu from getting garbage-collected when popuping', async () => { const menu = Menu.buildFromTemplate([{ role: 'paste' }]); menu.popup({ window: w }); @@ -844,21 +844,21 @@ describe('Menu module', function () { // eslint-disable-next-line no-undef const wr = new (globalThis as any).WeakRef(menu); - setTimeout(() => { - // Do garbage collection, since |menu| is not referenced in this closure - // it would be gone after next call. - const v8Util = process._linkedBinding('electron_common_v8_util'); - v8Util.requestGarbageCollectionForTesting(); - setTimeout(() => { - // Try to receive menu from weak reference. - if (wr.deref()) { - wr.deref().closePopup(); - done(); - } else { - done('Menu is garbage-collected while popuping'); - } - }); - }); + await delay(); + + // Do garbage collection, since |menu| is not referenced in this closure + // it would be gone after next call. + const v8Util = process._linkedBinding('electron_common_v8_util'); + v8Util.requestGarbageCollectionForTesting(); + + await delay(); + + // Try to receive menu from weak reference. + if (wr.deref()) { + wr.deref().closePopup(); + } else { + throw new Error('Menu is garbage-collected while popuping'); + } }); }); diff --git a/spec-main/api-web-contents-spec.ts b/spec-main/api-web-contents-spec.ts index c6be750db0ff..46de9dd6603b 100644 --- a/spec-main/api-web-contents-spec.ts +++ b/spec-main/api-web-contents-spec.ts @@ -1003,6 +1003,8 @@ describe('webContents module', () => { defer(() => { w2.setClosable(true); w2.close(); + + protocol.unregisterProtocol(scheme); }); await w.loadURL(`${scheme}://host3`); @@ -1015,8 +1017,6 @@ describe('webContents module', () => { const zoomLevel2 = w2.webContents.zoomLevel; expect(zoomLevel2).to.equal(0); expect(zoomLevel1).to.not.equal(zoomLevel2); - - protocol.unregisterProtocol(scheme); }); it('can persist when it contains iframe', (done) => { diff --git a/spec-main/chromium-spec.ts b/spec-main/chromium-spec.ts index ae4f4a3e0175..a5da960e282b 100644 --- a/spec-main/chromium-spec.ts +++ b/spec-main/chromium-spec.ts @@ -450,7 +450,7 @@ describe('chromium features', () => { } }); - it('returns error when permission is denied', (done) => { + it('returns error when permission is denied', async () => { const w = new BrowserWindow({ show: false, webPreferences: { @@ -458,13 +458,7 @@ describe('chromium features', () => { partition: 'geolocation-spec' } }); - w.webContents.on('ipc-message', (event, channel) => { - if (channel === 'success') { - done(); - } else { - done('unexpected response from geolocation api'); - } - }); + const message = emittedOnce(w.webContents, 'ipc-message'); w.webContents.session.setPermissionRequestHandler((wc, permission, callback) => { if (permission === 'geolocation') { callback(false); @@ -473,6 +467,8 @@ describe('chromium features', () => { } }); w.loadFile(path.join(fixturesPath, 'pages', 'geolocation', 'index.html')); + const [, channel] = await message; + expect(channel).to.equal('success', 'unexpected response from geolocation api'); }); }); diff --git a/spec-main/webview-spec.ts b/spec-main/webview-spec.ts index bb5dc408a883..91f29969bf12 100644 --- a/spec-main/webview-spec.ts +++ b/spec-main/webview-spec.ts @@ -1,7 +1,7 @@ import * as path from 'path'; import { BrowserWindow, session, ipcMain, app, WebContents } from 'electron/main'; import { closeAllWindows } from './window-helpers'; -import { emittedOnce } from './events-helpers'; +import { emittedOnce, emittedUntil } from './events-helpers'; import { ifdescribe } from './spec-helpers'; import { expect } from 'chai'; @@ -415,18 +415,17 @@ describe(' tag', function () { await emittedOnce(app, 'browser-window-created'); }); - it('emits a web-contents-created event', (done) => { - app.on('web-contents-created', function listener (event, contents) { - if (contents.getType() === 'window') { - app.removeListener('web-contents-created', listener); - done(); - } - }); + it('emits a web-contents-created event', async () => { + const webContentsCreated = emittedUntil(app, 'web-contents-created', + (event: Electron.Event, contents: Electron.WebContents) => contents.getType() === 'window'); + loadWebView(w.webContents, { allowpopups: 'on', webpreferences: 'nativeWindowOpen=1', src: `file://${fixtures}/pages/window-open.html` }); + + await webContentsCreated; }); }); diff --git a/spec/webview-spec.js b/spec/webview-spec.js index d32cc22e681b..53c7638667c5 100644 --- a/spec/webview-spec.js +++ b/spec/webview-spec.js @@ -954,32 +954,35 @@ describe(' tag', function () { }); describe('found-in-page event', () => { - it('emits when a request is made', (done) => { - let requestId = null; - const activeMatchOrdinal = []; - const listener = (e) => { - expect(e.result.requestId).to.equal(requestId); - expect(e.result.matches).to.equal(3); - activeMatchOrdinal.push(e.result.activeMatchOrdinal); - if (e.result.activeMatchOrdinal === e.result.matches) { - expect(activeMatchOrdinal).to.deep.equal([1, 2, 3]); - webview.stopFindInPage('clearSelection'); - done(); - } else { - listener2(); - } - }; - const listener2 = () => { - requestId = webview.findInPage('virtual'); - }; - webview.addEventListener('found-in-page', listener); - webview.addEventListener('did-finish-load', listener2); + it('emits when a request is made', async () => { + const didFinishLoad = waitForEvent(webview, 'did-finish-load'); loadWebView(webview, { src: `file://${fixtures}/pages/content.html` }); // TODO(deepak1556): With https://codereview.chromium.org/2836973002 // focus of the webContents is required when triggering the api. // Remove this workaround after determining the cause for // incorrect focus. webview.focus(); + await didFinishLoad; + + const activeMatchOrdinal = []; + + for (;;) { + const foundInPage = waitForEvent(webview, 'found-in-page'); + const requestId = webview.findInPage('virtual'); + const event = await foundInPage; + + expect(event.result.requestId).to.equal(requestId); + expect(event.result.matches).to.equal(3); + + activeMatchOrdinal.push(event.result.activeMatchOrdinal); + + if (event.result.activeMatchOrdinal === event.result.matches) { + break; + } + } + + expect(activeMatchOrdinal).to.deep.equal([1, 2, 3]); + webview.stopFindInPage('clearSelection'); }); });