Repository: electron/fiddle
Pull request #614 (2021-03-21 16:15:21)

perf: faster sorting in sortedElectronMap()
Refactor sortedElectronMap() to parse the versions strings less often.

Calls to `semver.valid()`, `semver.prerelease()`, `semver.coerce()`, and `semver.gt()` keep causing the version string to be  reparsed. This refactor calls `semver.parse()` once per RunnableVersion and uses that object for the compare function. For our current startup, this drops the number of times `semver.parse()` is called on startup from 50k times to 4k times.


coveralls: 
[![Coverage Status](https://coveralls.io/builds/38116946/badge)](https://coveralls.io/builds/38116946)

Coverage decreased (-0.06%) to 93.08% when pulling **890efd1cc5686358c751cbe52dd066563dd942b7 on perf/faster-sorting** into **adb4488a52df122faab135b6a2ae8f77161492d0 on master**.


Patch:
diff --git a/src/renderer/binary.ts b/src/renderer/binary.ts
index 6d7b875d..9af0bcab 100644
--- a/src/renderer/binary.ts
+++ b/src/renderer/binary.ts
@@ -1,13 +1,13 @@
-import * as fsType from 'fs-extra';
+import * as fs from 'fs-extra';
 import * as path from 'path';
-import * as extractZipType from 'extract-zip';
+import extract from 'extract-zip';
 
 import { VersionState } from '../interfaces';
-import { fancyImport } from '../utils/import';
 import { normalizeVersion } from '../utils/normalize-version';
 import { USER_DATA_PATH } from './constants';
 import { removeTypeDefsForVersion } from './fetch-types';
 import { AppState } from './state';
+import { download as electronDownload } from '@electron/get';
 
 const pendingDownloads: Record<string, Promise<void>> = {};
 
@@ -18,17 +18,17 @@ const pendingDownloads: Record<string, Promise<void>> = {};
  * @param {string} iVersion
  * @returns {Promise<void>}
  */
-export async function setupBinary(
+export function setupBinary(
   appState: AppState,
   iVersion: string,
 ): Promise<void> {
   const version = normalizeVersion(iVersion);
 
   // If we already have it, then we're done
-  if (await getIsDownloaded(version)) {
+  if (getIsDownloaded(version)) {
     console.log(`Binary: Electron ${version} already downloaded.`);
     appState.versions[version].state = VersionState.ready;
-    return;
+    return Promise.resolve();
   }
 
   // Return a promise that resolves when the download completes
@@ -58,7 +58,6 @@ async function downloadBinary(
     appState.versions[version].state = VersionState.unzipping;
 
     // Ensure the target path exists
-    const fs = await fancyImport<typeof fsType>('fs-extra');
     await fs.mkdirp(extractPath);
 
     // Ensure the target path is empty
@@ -85,7 +84,6 @@ async function downloadBinary(
  */
 export async function removeBinary(iVersion: string) {
   const version = normalizeVersion(iVersion);
-  const fs = await fancyImport<typeof fsType>('fs-extra');
   let isDeleted = false;
 
   // utility to re-run removal functions upon failure
@@ -106,7 +104,7 @@ export async function removeBinary(iVersion: string) {
   };
 
   const binaryCleaner = async () => {
-    if (await getIsDownloaded(version)) {
+    if (getIsDownloaded(version)) {
       // This is necessary since we're messing with .asar files inside
       // the Electron binaries. Electron, powering Fiddle, will try to
       // "correct" our calls, but we don't want that right here.
@@ -135,13 +133,8 @@ export async function removeBinary(iVersion: string) {
  * @param {string} dir
  * @returns {boolean}
  */
-export async function getIsDownloaded(
-  version: string,
-  dir?: string,
-): Promise<boolean> {
+export function getIsDownloaded(version: string, dir?: string): boolean {
   const expectedPath = getElectronBinaryPath(version, dir);
-  const fs = await fancyImport<typeof fsType>('fs-extra');
-
   return fs.existsSync(expectedPath);
 }
 
@@ -183,23 +176,14 @@ export function getDownloadingVersions(appState: AppState) {
  * @returns {Promise<Array<string>>}
  */
 export async function getDownloadedVersions(): Promise<Array<string>> {
-  const fs = await fancyImport<typeof fsType>('fs-extra');
   const downloadPath = path.join(USER_DATA_PATH, 'electron-bin');
   console.log(`Binary: Checking for downloaded versions`);
 
   try {
-    const directories = await fs.readdir(downloadPath);
-    const knownVersions: Array<string> = [];
-
-    for (const directory of directories) {
-      if (await getIsDownloaded(directory)) {
-        knownVersions.push(directory);
-      }
-    }
-
-    return knownVersions;
+    const dirs = await fs.readdir(downloadPath);
+    return dirs.filter((dir) => dir && getIsDownloaded(dir));
   } catch (error) {
-    console.warn(`Could not read known Electron versions`);
+    console.warn(`Could not read known Electron versions`, error);
     return [];
   }
 }
@@ -212,7 +196,6 @@ export async function getDownloadedVersions(): Promise<Array<string>> {
  * @returns {Promise<string>}
  */
 async function download(appState: AppState, version: string): Promise<string> {
-  const { download: electronDownload } = await import('@electron/get');
   const getProgressCallback = (progress: Progress) => {
     const roundedProgress = Math.round(progress.percent * 100) / 100;
 
@@ -258,9 +241,6 @@ function getDownloadPath(version: string): string {
  * @returns {Promise<void>}
  */
 async function unzip(zipPath: string, dir: string): Promise<void> {
-  const extract: typeof extractZipType = (await fancyImport<any>('extract-zip'))
-    .default;
-
   process.noAsar = true;
 
   try {
diff --git a/src/renderer/components/dialog-add-version.tsx b/src/renderer/components/dialog-add-version.tsx
index eea2d1d4..3f5776ae 100644
--- a/src/renderer/components/dialog-add-version.tsx
+++ b/src/renderer/components/dialog-add-version.tsx
@@ -70,9 +70,8 @@ export class AddVersionDialog extends React.Component<
    *
    * @param {React.ChangeEvent<HTMLInputElement>} event
    */
-  public async setFolderPath(folderPath: string) {
-    const isValidElectron = !!(await getIsDownloaded('custom', folderPath));
-
+  public setFolderPath(folderPath: string) {
+    const isValidElectron = getIsDownloaded('custom', folderPath);
     this.setState({ folderPath, isValidElectron });
   }
 
diff --git a/src/renderer/components/editors.tsx b/src/renderer/components/editors.tsx
index 3de9173c..88d0d6a8 100644
--- a/src/renderer/components/editors.tsx
+++ b/src/renderer/components/editors.tsx
@@ -82,6 +82,8 @@ export class Editors extends React.Component<EditorsProps, EditorsState> {
    * @memberof Editors
    */
   public async componentDidMount() {
+    this.stopListening();
+
     ipcRendererManager.on(
       IpcEvents.MONACO_EXECUTE_COMMAND,
       (_event, cmd: string) => {
@@ -103,12 +105,12 @@ export class Editors extends React.Component<EditorsProps, EditorsState> {
     );
 
     ipcRendererManager.on(IpcEvents.SELECT_ALL_IN_EDITOR, (_event) => {
-      // programmatically fetch all editor contents and set as selection
       const editor = getFocusedEditor();
-      const range = editor?.getModel()?.getFullModelRange();
-
-      if (!!range) {
-        editor?.setSelection(range);
+      if (editor) {
+        const model = editor.getModel();
+        if (model) {
+          editor.setSelection(model.getFullModelRange());
+        }
       }
     });
 
@@ -119,7 +121,10 @@ export class Editors extends React.Component<EditorsProps, EditorsState> {
 
   public componentWillUnmount() {
     this.disposeLayoutAutorun();
+    this.stopListening();
+  }
 
+  private stopListening() {
     ipcRendererManager.removeAllListeners(IpcEvents.MONACO_EXECUTE_COMMAND);
     ipcRendererManager.removeAllListeners(IpcEvents.FS_NEW_FIDDLE);
     ipcRendererManager.removeAllListeners(IpcEvents.MONACO_TOGGLE_OPTION);
diff --git a/src/renderer/content.ts b/src/renderer/content.ts
index 3c34c1b7..a9dac3bc 100644
--- a/src/renderer/content.ts
+++ b/src/renderer/content.ts
@@ -7,7 +7,7 @@ import { readFiddle } from '../utils/read-fiddle';
 import * as fsType from 'fs-extra';
 import * as path from 'path';
 import * as semver from 'semver';
-import * as decompress from 'decompress';
+import decompress from 'decompress';
 
 // parent directory of all the downloaded template fiddles
 const TEMPLATES_DIR = path.join(USER_DATA_PATH, 'Templates');
diff --git a/src/renderer/runner.ts b/src/renderer/runner.ts
index 6d616349..f0fa0c9f 100644
--- a/src/renderer/runner.ts
+++ b/src/renderer/runner.ts
@@ -71,7 +71,7 @@ export class Runner {
       return false;
     }
 
-    const isReady = await getIsDownloaded(version, localPath);
+    const isReady = getIsDownloaded(version, localPath);
 
     if (!isReady) {
       console.warn(`Runner: Binary ${version} not ready`);
diff --git a/src/utils/editor-mounted.ts b/src/utils/editor-mounted.ts
index 4e9cc34c..1534587a 100644
--- a/src/utils/editor-mounted.ts
+++ b/src/utils/editor-mounted.ts
@@ -1,28 +1,16 @@
 import { MosaicId } from '../interfaces';
 
+import { waitFor } from './wait-for';
+
 /**
  * Waits for editors to mount on a list of Mosaic IDs
  * @param editors
  */
 export function waitForEditorsToMount(editors: Array<MosaicId>) {
-  let time = 0;
-  const maxTime = 3000;
-  const interval = 100;
-  return new Promise<void>((resolve, reject) => {
-    (function checkMountedEditors() {
-      const allMounted = editors.every(
-        (id) => !!window.ElectronFiddle.editors[id],
-      );
-      if (allMounted) {
-        return resolve();
-      }
-      time += interval;
-      if (time > maxTime) {
-        return reject(
-          `Timed out after ${maxTime}ms: can't mount editors onto mosaics.`,
-        );
-      }
-      setTimeout(checkMountedEditors, 100);
-    })();
+  const allMounted = () =>
+    editors.every((id) => !!window.ElectronFiddle.editors[id]);
+  const opts = { timeout: 3000, interval: 100 };
+  return waitFor(allMounted, opts).catch((error) => {
+    throw `Can't mount editors onto mosaics. ${error.toString()}`;
   });
 }
diff --git a/src/utils/octokit.ts b/src/utils/octokit.ts
index 22bc4a62..67859def 100644
--- a/src/utils/octokit.ts
+++ b/src/utils/octokit.ts
@@ -1,7 +1,6 @@
 import { Octokit } from '@octokit/rest';
 import { AppState } from '../renderer/state';
 
-let _Octokit: typeof Octokit;
 let _octo: Octokit;
 
 /**
@@ -12,8 +11,7 @@ let _octo: Octokit;
  * @returns {Promise<typeof GitHubType>}
  */
 export async function getOctokit(appState?: AppState): Promise<Octokit> {
-  _Octokit = _Octokit || ((await import('@octokit/rest')) as any).default;
-  _octo = _octo || new _Octokit();
+  _octo = _octo || new Octokit();
 
   // You can load Gists without being authenticated,
   // but we get better rate limits when authenticated
diff --git a/src/utils/sorted-electron-map.ts b/src/utils/sorted-electron-map.ts
index 53b6b75f..43dd023c 100644
--- a/src/utils/sorted-electron-map.ts
+++ b/src/utils/sorted-electron-map.ts
@@ -2,49 +2,46 @@ import * as semver from 'semver';
 
 import { RunnableVersion } from '../interfaces';
 
+function electronSemVerCompare(a: semver.SemVer, b: semver.SemVer) {
+  const l = a.compareMain(b);
+  if (l) return l;
+  // Electron's approach is nightly -> beta -> stable.
+  // Account for 'beta' coming before 'nightly' lexicographically
+  if (a.prerelease[0] === 'nightly' && b.prerelease[0] === 'beta') return -1;
+  if (a.prerelease[0] === 'beta' && b.prerelease[0] === 'nightly') return 1;
+  return a.comparePre(b);
+}
+
+function compare(a: semver.SemVer | string, b: semver.SemVer | string) {
+  // non-semver goes first, sorted lexicographically
+  const astr = typeof a === 'string';
+  const bstr = typeof b === 'string';
+  if (astr && bstr) return (a as string).localeCompare(b as string);
+  if (astr) return -1;
+  if (bstr) return 1;
+
+  // then semver, sorted newest-to-oldest
+  return -electronSemVerCompare(a as semver.SemVer, b as semver.SemVer);
+}
+
 /**
- * Sorts Electron versions and returns the result of a
- * map function.
+ * Sorts Electron versions and returns the result of a map function.
  *
- * @param {Record<string, RunnableVersion>} versions
- * @param {(key: string, version: RunnableVersion) => void} mapFn
+ * @param {Record<string, RunnableVersion?>} versions
+ * @param {(key: string, version: RunnableVersion?) => T} mapFn
  * @returns {Array<T>}
  */
 export function sortedElectronMap<T>(
   versions: Record<string, RunnableVersion>,
   mapFn: (key: string, version: RunnableVersion) => T,
 ) {
-  return Object.keys(versions)
-    .sort((a, b) => {
-      if (!semver.valid(a) && !semver.valid(b)) {
-        return a.localeCompare(b);
-      }
-
-      if (!semver.valid(a)) return -1;
-      if (!semver.valid(b)) return 1;
-
-      // Handle prerelease tag sorting - semver does not handle this.
-      // See https://www.npmjs.com/package/semver#prerelease-tags for more.
-      const bothPre = semver.prerelease(a) && semver.prerelease(b);
-      const sameCoerced =
-        semver.valid(semver.coerce(a)) === semver.valid(semver.coerce(b));
-      if (bothPre && sameCoerced) {
-        const [tagA, valueA] = semver.prerelease(a)!;
-        const [tagB, valueB] = semver.prerelease(b)!;
-
-        if (tagA === 'beta' && tagA === tagB) {
-          // Both are betas - compare beta number.
-          return valueA > valueB ? -1 : 1;
-        } else if (tagA === 'nightly' && tagA === tagB) {
-          // Both are nightlies - compare nightly dates strings.
-          return -`${valueA}`.localeCompare(`${valueB}`);
-        } else {
-          // beta > nightly for the same major version.
-          return tagA === 'beta' ? -1 : 1;
-        }
-      }
-
-      return semver.gt(a, b, true) ? -1 : 1;
-    })
-    .map((key) => mapFn(key, versions[key])) as Array<T>;
+  type VerSemRun = [
+    ver: string,
+    sem: semver.SemVer | null,
+    run: RunnableVersion,
+  ];
+  return Object.entries(versions)
+    .map(([ver, run]): VerSemRun => [ver, semver.parse(ver), run])
+    .sort(([vera, sema], [verb, semb]) => compare(sema || vera, semb || verb))
+    .map(([ver, _, run]) => mapFn(ver, run));
 }
diff --git a/src/utils/wait-for.ts b/src/utils/wait-for.ts
new file mode 100644
index 00000000..cb7edef4
--- /dev/null
+++ b/src/utils/wait-for.ts
@@ -0,0 +1,32 @@
+/**
+ * Waits up to `timeout` msec for a test to pass.
+ *
+ * @return a promise that returns the test result on success, or rejects on timeout
+ * @param {() => any} test - function to test
+ * @param {Object} options
+ * @param {Number} [options.interval=100] - polling frequency, in msec
+ * @param {Number} [options.timeout=2000] - timeout interval, in msec
+ */
+export async function waitFor(
+  test: () => any,
+  options = {
+    interval: 100,
+    timeout: 2000,
+  },
+): Promise<any> {
+  const { interval, timeout } = options;
+  let elapsed = 0;
+  return new Promise<void>((resolve, reject) => {
+    (function check() {
+      const result = test();
+      if (result) {
+        return resolve(result);
+      }
+      elapsed += interval;
+      if (elapsed >= timeout) {
+        return reject(`Timed out: ${timeout}ms`);
+      }
+      setTimeout(check, interval);
+    })();
+  });
+}
diff --git a/tsconfig.json b/tsconfig.json
index 1d5bd5d6..8c132529 100644
--- a/tsconfig.json
+++ b/tsconfig.json
@@ -1,5 +1,6 @@
 {
   "compilerOptions": {
+    "esModuleInterop": true,
     "outDir": "./dist",
     "allowJs": true,
     "skipLibCheck": true,


Test patch:
diff --git a/tests/main/menu-spec.ts b/tests/main/menu-spec.ts
index 7450b642..1275d724 100644
--- a/tests/main/menu-spec.ts
+++ b/tests/main/menu-spec.ts
@@ -272,6 +272,15 @@ describe('menu', () => {
 
     describe('getFileMenu()', () => {
       let file: any;
+      enum Idx {
+        NEW_FIDDLE = 0,
+        NEW_WINDOW = 1,
+        OPEN = 3,
+        SAVE = 5,
+        SAVE_AS = 6,
+        PUBLISH = 8,
+        FORGE = 9,
+      }
 
       beforeEach(() => {
         const mock = (electron.Menu.buildFromTemplate as any).mock;
@@ -280,50 +289,50 @@ describe('menu', () => {
       });
 
       it('creates a new fiddle', () => {
-        file.submenu[0].click();
+        file.submenu[Idx.NEW_FIDDLE].click();
         expect(ipcMainManager.send).toHaveBeenCalledWith<any>(
           IpcEvents.FS_NEW_FIDDLE,
         );
       });
 
       it('creates a new window', () => {
-        file.submenu[1].click();
-        file.submenu[1].click();
+        file.submenu[Idx.NEW_WINDOW].click();
+        file.submenu[Idx.NEW_WINDOW].click();
         expect(createMainWindow).toHaveBeenCalledTimes(2);
       });
 
       it('opens a Fiddle', () => {
-        file.submenu[3].click();
+        file.submenu[Idx.OPEN].click();
         expect(electron.dialog.showOpenDialog).toHaveBeenCalled();
       });
 
       it('saves a Fiddle', () => {
-        file.submenu[5].click();
+        file.submenu[Idx.SAVE].click();
         expect(ipcMainManager.send).toHaveBeenCalledWith<any>(
           IpcEvents.FS_SAVE_FIDDLE,
         );
       });
 
       it('saves a Fiddle as', () => {
-        file.submenu[6].click();
+        file.submenu[Idx.SAVE_AS].click();
         expect(electron.dialog.showOpenDialogSync).toHaveBeenCalled();
       });
 
       it('saves a Fiddle as a gist', () => {
-        file.submenu[8].click();
+        file.submenu[Idx.PUBLISH].click();
         expect(ipcMainManager.send).toHaveBeenCalledWith<any>(
           IpcEvents.FS_SAVE_FIDDLE_GIST,
         );
       });
 
       it('saves a Fiddle as a forge project', () => {
-        file.submenu[9].click();
+        file.submenu[Idx.FORGE].click();
         expect(electron.dialog.showOpenDialogSync).toHaveBeenCalled();
       });
 
       it('saves a Fiddle with blocked accelerator', () => {
         setupMenu([BlockableAccelerator.save]);
-        file.submenu[5].click();
+        file.submenu[Idx.SAVE].click();
         expect(ipcMainManager.send).toHaveBeenCalledWith<any>(
           IpcEvents.FS_SAVE_FIDDLE,
         );
@@ -331,7 +340,7 @@ describe('menu', () => {
 
       it('saves as a Fiddle with blocked accelerator', () => {
         setupMenu([BlockableAccelerator.saveAs]);
-        file.submenu[6].click();
+        file.submenu[Idx.SAVE_AS].click();
         expect(electron.dialog.showOpenDialogSync).toHaveBeenCalled();
       });
     });
diff --git a/tests/mocks/worker.ts b/tests/mocks/worker.ts
index 9305a742..b14ca9f3 100644
--- a/tests/mocks/worker.ts
+++ b/tests/mocks/worker.ts
@@ -1,4 +1,4 @@
-import * as decomment from 'decomment';
+import decomment from 'decomment';
 
 export default class MockWorker {
   public onmessage: (msg: any) => any;
diff --git a/tests/renderer/binary-spec.ts b/tests/renderer/binary-spec.ts
index 7b666367..c11e45c3 100644
--- a/tests/renderer/binary-spec.ts
+++ b/tests/renderer/binary-spec.ts
@@ -9,23 +9,20 @@ import { removeTypeDefsForVersion } from '../../src/renderer/fetch-types';
 import { overridePlatform, resetPlatform } from '../utils';
 
 import * as path from 'path';
+import extract from 'extract-zip';
 
 jest.mock('fs-extra');
+jest.mock('extract-zip');
 jest.mock('../../src/renderer/ipc', () => ({}));
-jest.mock('extract-zip', () => {
-  return jest.fn((_a, _b) => Promise.resolve());
-});
 jest.mock('../../src/renderer/constants', () => ({
   USER_DATA_PATH: 'user/data/',
 }));
 jest.mock('../../src/utils/import', () => ({
   fancyImport: async (p: string) => {
-    if (p === 'fs-extra') {
-      return require('fs-extra');
-    }
     if (p === 'extract-zip') {
       return { default: require('extract-zip') };
     }
+    throw new Error(`Unexpected module '${p}'`);
   },
 }));
 jest.mock('../../src/renderer/fetch-types', () => ({
@@ -37,6 +34,7 @@ jest.mock('@electron/get', () => ({
 
 describe('binary', () => {
   let mockState: any = {};
+  const fs = require('fs-extra');
 
   beforeEach(() => {
     mockState = {
@@ -54,8 +52,6 @@ describe('binary', () => {
 
   describe('remove()', () => {
     it('removes a version', async () => {
-      const fs = require('fs-extra');
-
       (fs.existsSync as jest.Mock<any>).mockReturnValue(true);
 
       await removeBinary('v3.0.0');
@@ -63,8 +59,6 @@ describe('binary', () => {
     });
 
     it('retries on failure', async () => {
-      const fs = require('fs-extra');
-
       (fs.existsSync as jest.Mock<any>).mockReturnValue(true);
       (fs.remove as jest.Mock<any>).mockImplementation(() => {
         throw new Error('Bwap bwap');
@@ -75,8 +69,6 @@ describe('binary', () => {
     });
 
     it("attempts to clean up the version's associated typedefs", async () => {
-      const fs = require('fs-extra');
-
       (fs.existsSync as jest.Mock<any>).mockReturnValue(true);
 
       await removeBinary('v3.0.0');
@@ -85,7 +77,6 @@ describe('binary', () => {
     });
 
     it('retries typedef cleanup upon failure', async () => {
-      const fs = require('fs-extra');
       (removeTypeDefsForVersion as jest.Mock).mockImplementation(() => {
         throw new Error('Bwap bwap');
       });
@@ -99,30 +90,22 @@ describe('binary', () => {
 
   describe('getDownloadedVersions()', () => {
     it('finds downloaded versions', async () => {
-      const fs = require('fs-extra');
-
-      (fs.readdir as jest.Mock<any>).mockReturnValue(['v3.0.0']);
+      const versions = ['v3.0.0'];
+      (fs.readdir as jest.Mock<any>).mockResolvedValue(versions);
       (fs.existsSync as jest.Mock<any>).mockReturnValue(true);
-
       const result = await getDownloadedVersions();
-
-      expect(result).toEqual(['v3.0.0']);
+      expect(result).toEqual(versions);
     });
 
     it('is okay without versions ', async () => {
-      const fs = require('fs-extra');
-
-      (fs.readdir as jest.Mock<any>).mockReturnValue([]);
+      const versions: string[] = [];
+      (fs.readdir as jest.Mock<any>).mockResolvedValue(versions);
       const result = await getDownloadedVersions();
-      expect(result).toEqual([]);
+      expect(result).toEqual(versions);
     });
 
     it('is okay without versions and with errors', async () => {
-      const fs = require('fs-extra');
-
-      (fs.readdir as jest.Mock<any>).mockImplementationOnce(() => {
-        throw new Error('💩');
-      });
+      (fs.readdir as jest.Mock<any>).mockRejectedValueOnce('💩');
       const result = await getDownloadedVersions();
       expect(result).toEqual([]);
     });
@@ -169,52 +152,56 @@ describe('binary', () => {
   });
 
   describe('setupBinary()', () => {
-    it(`downloads a version it hasn't seen before`, async () => {
-      const { download } = require('@electron/get');
-      download.mockReturnValue('/fake/path');
+    const { download } = require('@electron/get');
 
-      mockState.versions['3.0.0'] = { state: 'unknown' };
-      await setupBinary(mockState, 'v3.0.0');
+    it(`downloads a version it has not seen before`, async () => {
+      (fs.existsSync as jest.Mock).mockReturnValueOnce(false);
+
+      const localPath = '/fake/path';
+      download.mockResolvedValue(localPath);
+
+      const version = '3.0.0';
+      mockState.versions[version] = { state: 'unknown' };
+      await setupBinary(mockState, version);
 
       expect(download).toHaveBeenCalled();
-      expect(mockState.versions['3.0.0'].state).toBe('ready');
+      expect(mockState.versions[version].state).toBe('ready');
     });
 
     it(`does not download a version again`, async () => {
-      const fs = require('fs-extra');
       (fs.existsSync as jest.Mock).mockReturnValue(true);
-      const { download } = require('@electron/get');
 
-      mockState.versions['3.0.0'] = { state: 'unknown' };
-      await setupBinary(mockState, 'v3.0.0');
+      const version = '3.0.0';
+      mockState.versions[version] = { state: 'unknown' };
+
+      await setupBinary(mockState, version);
 
-      expect(download).toHaveBeenCalledTimes(0);
-      expect(mockState.versions['3.0.0'].state).toBe('ready');
+      expect(download).not.toHaveBeenCalled();
+      expect(mockState.versions[version].state).toBe('ready');
     });
 
     it(`waits if asked for a version is already downloading`, async () => {
-      const fs = require('fs-extra');
-      const { download } = require('@electron/get');
-
       (fs.existsSync as jest.Mock).mockReturnValue(true);
-      mockState.versions['3.0.0'].state = 'downloading';
 
-      await setupBinary(mockState, 'v3.0.0');
+      const version = '3.0.0';
+      mockState.versions[version] = { state: 'downloading' };
+      await setupBinary(mockState, version);
 
-      expect(download).toHaveBeenCalledTimes(0);
-      expect(mockState.versions['3.0.0'].state).toBe('ready');
+      expect(download).not.toHaveBeenCalled();
+      expect(mockState.versions[version].state).toBe('ready');
     });
 
     it('handles an error in the zip file', async () => {
       const { download } = require('@electron/get');
+
       download.mockReturnValue('/fake/path');
+      mockState.versions['3.0.0'] = { state: 'unknown' };
 
-      const mockZip = require('extract-zip');
-      mockZip.mockImplementationOnce((_a: any, _b: any, c: any) =>
-        c(new Error('bwap-bwap')),
-      );
+      (extract as jest.Mock).mockRejectedValue(new Error('bwap-bwap'));
 
       await setupBinary(mockState, 'v3.0.0');
+
+      expect(extract as jest.Mock).toHaveBeenCalledTimes(1);
     });
   });
 });
diff --git a/tests/renderer/components/dialog-add-version-spec.tsx b/tests/renderer/components/dialog-add-version-spec.tsx
index c6a7bca0..0fb38acd 100644
--- a/tests/renderer/components/dialog-add-version-spec.tsx
+++ b/tests/renderer/components/dialog-add-version-spec.tsx
@@ -29,9 +29,6 @@ describe('AddVersionDialog component', () => {
     store = {
       isAddVersionDialogShowing: true,
       addLocalVersion: jest.fn(),
-      binaryManager: {
-        getIsDownloaded: jest.fn(),
-      },
     };
   });
 
@@ -70,7 +67,7 @@ describe('AddVersionDialog component', () => {
 
   describe('setFolderPath()', () => {
     it('does something', async () => {
-      (getIsDownloaded as jest.Mock).mockResolvedValue(true);
+      (getIsDownloaded as jest.Mock).mockReturnValue(true);
       const wrapper = shallow(<AddVersionDialog appState={store} />);
       await (wrapper.instance() as any).setFolderPath('/test/');
 
diff --git a/tests/renderer/components/editors-spec.tsx b/tests/renderer/components/editors-spec.tsx
index 6b6462de..804cc123 100644
--- a/tests/renderer/components/editors-spec.tsx
+++ b/tests/renderer/components/editors-spec.tsx
@@ -2,13 +2,19 @@ import { mount, shallow } from 'enzyme';
 import { observable } from 'mobx';
 import * as React from 'react';
 
-import { ALL_MOSAICS, DocsDemoPage, EditorId } from '../../../src/interfaces';
+import {
+  ALL_MOSAICS,
+  DocsDemoPage,
+  EditorId,
+  EditorValues,
+} from '../../../src/interfaces';
 import { IpcEvents } from '../../../src/ipc-events';
 import { Editors, TITLE_MAP } from '../../../src/renderer/components/editors';
 import { ipcRendererManager } from '../../../src/renderer/ipc';
 import { updateEditorLayout } from '../../../src/utils/editor-layout';
 import { createMosaicArrangement } from '../../../src/utils/editors-mosaic-arrangement';
 import { getFocusedEditor } from '../../../src/utils/focused-editor';
+import * as content from '../../../src/renderer/content';
 
 jest.mock('monaco-loader', () =>
   jest.fn(async () => {
@@ -173,18 +179,82 @@ describe('Editors component', () => {
       expect(mockAction.run).toHaveBeenCalled();
     });
 
-    it('handles an FS_NEW_FIDDLE command', async (done) => {
+    const fakeValues: EditorValues = Object.seal({
+      css: '',
+      html: '',
+      main: 'hi',
+      preload: '',
+      renderer: '',
+    });
+
+    it('handles an FS_NEW_FIDDLE command', async () => {
+      const { app } = window.ElectronFiddle;
+
       let resolve: any;
       const replacePromise = new Promise((r) => {
         resolve = r;
       });
-      (window.ElectronFiddle.app
-        .replaceFiddle as jest.Mock<any>).mockImplementation(resolve);
 
+      // setup
+      const getTemplateSpy = jest
+        .spyOn(content, 'getTemplate')
+        .mockImplementation(() => Promise.resolve(fakeValues));
+      const replaceFiddleSpy = jest
+        .spyOn(app, 'replaceFiddle')
+        .mockImplementation(() => resolve());
+
+      // invoke the call
       ipcRendererManager.emit(IpcEvents.FS_NEW_FIDDLE, null);
       await replacePromise;
-      expect(window.ElectronFiddle.app.replaceFiddle).toHaveBeenCalled();
-      done();
+
+      // check the results
+      expect(getTemplateSpy).toHaveBeenCalledTimes(1);
+      expect(replaceFiddleSpy).toHaveBeenCalledTimes(1);
+
+      // cleanup
+      getTemplateSpy.mockRestore();
+      replaceFiddleSpy.mockRestore();
+    });
+
+    describe('SELECT_ALL_IN_EDITOR handler', () => {
+      it('selects all in the focused editor', (done) => {
+        shallow(<Editors appState={store} />);
+        const mockEditor = {
+          getModel: () => ({
+            getFullModelRange: jest.fn().mockReturnValue('range'),
+          }),
+          setSelection: jest.fn(),
+        };
+        (getFocusedEditor as any).mockReturnValueOnce(mockEditor);
+        ipcRendererManager.emit(IpcEvents.SELECT_ALL_IN_EDITOR, null);
+        process.nextTick(() => {
+          expect(mockEditor.setSelection).toHaveBeenCalledWith('range');
+          done();
+        });
+      });
+
+      it('does not change selection if the selected editor has no model', (done) => {
+        shallow(<Editors appState={store} />);
+        const mockEditor = {
+          getModel: jest.fn(() => null),
+          setSelection: jest.fn(),
+        };
+        (getFocusedEditor as any).mockReturnValueOnce(mockEditor);
+
+        ipcRendererManager.emit(IpcEvents.SELECT_ALL_IN_EDITOR, null);
+
+        process.nextTick(() => {
+          expect(mockEditor.getModel).toHaveBeenCalledTimes(1);
+          expect(mockEditor.setSelection).not.toHaveBeenCalled();
+          done();
+        });
+      });
+
+      it('does not crash if there is no selected editor', () => {
+        shallow(<Editors appState={store} />);
+        (getFocusedEditor as any).mockReturnValueOnce(null);
+        ipcRendererManager.emit(IpcEvents.SELECT_ALL_IN_EDITOR, null);
+      });
     });
 
     it('handles a SELECT_ALL_IN_EDITOR command', (done) => {
@@ -233,6 +303,18 @@ describe('Editors component', () => {
     });
   });
 
+  describe('setFocused()', () => {
+    it('sets the "focused" property', () => {
+      const wrapper = shallow(<Editors appState={store} />);
+      const instance: Editors = wrapper.instance() as any;
+      const spy = jest.spyOn(instance, 'setState');
+
+      const id = EditorId.html;
+      instance.setFocused(id);
+      expect(spy).toHaveBeenCalledWith({ focused: id });
+    });
+  });
+
   describe('disposeLayoutAutorun()', () => {
     it('automatically updates the layout when the mosaic arrangement changes', () => {
       class MockStore {
diff --git a/tests/renderer/components/settings-electron-spec.tsx b/tests/renderer/components/settings-electron-spec.tsx
index e2518798..40087d20 100644
--- a/tests/renderer/components/settings-electron-spec.tsx
+++ b/tests/renderer/components/settings-electron-spec.tsx
@@ -4,7 +4,7 @@ import * as React from 'react';
 import { VersionSource, VersionState } from '../../../src/interfaces';
 import { ElectronSettings } from '../../../src/renderer/components/settings-electron';
 import { ElectronReleaseChannel } from '../../../src/renderer/versions';
-import { mockVersions } from '../../mocks/electron-versions';
+import { mockVersions, mockVersionsArray } from '../../mocks/electron-versions';
 
 describe('ElectronSettings component', () => {
   let store: any;
@@ -97,7 +97,9 @@ describe('ElectronSettings component', () => {
     const instance = wrapper.instance() as any;
     await instance.handleDeleteAll();
 
-    expect(store.removeVersion).toHaveBeenCalledTimes(2);
+    expect(store.removeVersion).toHaveBeenCalledTimes(
+      mockVersionsArray.length - 1,
+    );
   });
 
   it('handles the downloadAll()', async () => {
diff --git a/tests/renderer/npm-spec.ts b/tests/renderer/npm-spec.ts
index 933fb394..ec7c4481 100644
--- a/tests/renderer/npm-spec.ts
+++ b/tests/renderer/npm-spec.ts
@@ -1,5 +1,5 @@
 import { mocked } from 'ts-jest/utils';
-import * as decomment from 'decomment';
+import decomment from 'decomment';
 
 import {
   findModules,
diff --git a/tests/renderer/state-spec.ts b/tests/renderer/state-spec.ts
index 3a4fc29c..79791803 100644
--- a/tests/renderer/state-spec.ts
+++ b/tests/renderer/state-spec.ts
@@ -1,5 +1,4 @@
 import * as MonacoType from 'monaco-editor';
-import { when } from 'mobx';
 
 import {
   ALL_MOSAICS,
@@ -27,8 +26,9 @@ import {
   saveLocalVersions,
 } from '../../src/renderer/versions';
 import { createMosaicArrangement } from '../../src/utils/editors-mosaic-arrangement';
+import { waitFor } from '../../src/utils/wait-for';
 import { getName } from '../../src/utils/get-title';
-import { mockVersions } from '../mocks/electron-versions';
+import { mockVersions, mockVersionsArray } from '../mocks/electron-versions';
 import { overridePlatform, resetPlatform } from '../utils';
 
 jest.mock('../../src/renderer/content', () => ({
@@ -144,19 +144,21 @@ describe('AppState', () => {
     });
 
     it('sets the onDidChangeModelContent handler if saved', async (done) => {
+      // confirm that setting appState.isUnsaved to false
+      // causes a new change-model-content callback to be installed
       appState.isUnsaved = false;
-      await when(() => appState.isUnsaved === false);
-
-      expect(window.onbeforeunload).toBe(null);
-
       const fn = window.ElectronFiddle.editors!.renderer!
         .onDidChangeModelContent;
-      const call = (fn as jest.Mock<any>).mock.calls[0];
-      const cb = call[0];
-
-      cb();
+      await waitFor(() => (fn as jest.Mock).mock.calls.length > 0);
+      expect(window.onbeforeunload).toBe(null);
+      expect(fn as jest.Mock).toHaveBeenCalledTimes(1);
 
+      // confirm that invoking the new callback sets appState.isUnsaved to true
+      const call = (fn as jest.Mock).mock.calls[0];
+      const callback = call[0];
+      callback();
       expect(appState.isUnsaved).toBe(true);
+
       done();
     });
   });
@@ -327,14 +329,14 @@ describe('AppState', () => {
       appState.channelsToShow = ['Unsupported' as any];
       expect(appState.versionsToShow.length).toEqual(0);
       appState.channelsToShow = ['Stable' as any];
-      expect(appState.versionsToShow.length).toEqual(3);
+      expect(appState.versionsToShow.length).toEqual(mockVersionsArray.length);
     });
 
     it('excludes states', () => {
       appState.statesToShow = [VersionState.downloading];
       expect(appState.versionsToShow.length).toEqual(0);
       appState.statesToShow = [VersionState.ready];
-      expect(appState.versionsToShow.length).toEqual(3);
+      expect(appState.versionsToShow.length).toEqual(mockVersionsArray.length);
     });
   });
 
@@ -397,10 +399,9 @@ describe('AppState', () => {
   });
 
   describe('setVersion()', () => {
-    it('falls back if a version does not exist', async () => {
+    it('uses the newest version iff the specified version does not exist', async () => {
       await appState.setVersion('v999.99.99');
-
-      expect(appState.version).toBe('2.0.2');
+      expect(appState.version).toBe(mockVersionsArray[0].version);
     });
 
     it('downloads a version if necessary', async () => {
@@ -467,11 +468,9 @@ describe('AppState', () => {
       // We just want to verify that the version state was
       // refreshed - we didn't actually add the local version
       // above, since versions.ts is mocked
-      expect(Object.keys(appState.versions)).toEqual([
-        '2.0.2',
-        '2.0.1',
-        '1.8.7',
-      ]);
+      expect(Object.keys(appState.versions)).toEqual(
+        mockVersionsArray.map((v) => v.version),
+      );
     });
   });
 
diff --git a/tests/renderer/versions-spec.ts b/tests/renderer/versions-spec.ts
index 01c30789..6a9acf18 100644
--- a/tests/renderer/versions-spec.ts
+++ b/tests/renderer/versions-spec.ts
@@ -1,6 +1,5 @@
 import * as fs from 'fs';
 import * as path from 'path';
-import semver from 'semver';
 import { RunnableVersion, VersionSource } from '../../src/interfaces';
 import {
   addLocalVersion,
@@ -24,12 +23,6 @@ const mockVersions: Array<Partial<RunnableVersion>> = [
   { version: 'test-2', localPath: '/test/path/2' },
 ];
 
-jest.mock('semver', () => ({
-  default: {
-    gte: jest.fn(),
-  },
-}));
-
 describe('versions', () => {
   describe('getDefaultVersion()', () => {
     it('handles a stored version', () => {
@@ -181,12 +174,6 @@ describe('versions', () => {
       );
       mockFetchOnce(mockUnpkgResponse.toString());
 
-      // return whether or not version in JSON is >=0.24.0
-      (semver.gte as jest.Mock).mockReturnValueOnce(true);
-      (semver.gte as jest.Mock).mockReturnValueOnce(true);
-      (semver.gte as jest.Mock).mockReturnValueOnce(true);
-      (semver.gte as jest.Mock).mockReturnValueOnce(false);
-
       const result = await fetchVersions();
       const expected = [
         { version: '10.0.0-nightly.20200303' },
diff --git a/tests/utils/fancy-import-spec.ts b/tests/utils/fancy-import-spec.ts
index 01e7c488..8eac0921 100644
--- a/tests/utils/fancy-import-spec.ts
+++ b/tests/utils/fancy-import-spec.ts
@@ -1,10 +1,24 @@
 import { fancyImport } from '../../src/utils/import';
 
 describe('fancyImport()', () => {
-  it('imports modules', async () => {
-    const fancy = await fancyImport('fs');
-    const required = require('fs');
+  it('imports es6 modules', async () => {
+    const name = 'fs';
+    const required = require(name);
+    const imported: any = await fancyImport(name);
 
-    expect(fancy).toEqual(required);
+    // https://www.typescriptlang.org/tsconfig#esModuleInterop
+    // can inject a 'default' for compatibility, so the imports may
+    // differ from require in that respect. Remove that from the equation before comparing.
+    const importedMinusDefault = { ...imported };
+    delete importedMinusDefault.default;
+
+    expect(required).toEqual(importedMinusDefault);
+  });
+
+  it('imports cjs single-function modules', async () => {
+    const name = 'decompress';
+    const required = require(name);
+    const imported: any = await fancyImport(name);
+    expect(required).toEqual(imported.default);
   });
 });
diff --git a/tests/utils/octokit-spec.ts b/tests/utils/octokit-spec.ts
index 687b76c0..df24ee23 100644
--- a/tests/utils/octokit-spec.ts
+++ b/tests/utils/octokit-spec.ts
@@ -1,30 +1,22 @@
 import { getOctokit } from '../../src/utils/octokit';
 
-let mockRequired = 0;
 let mockConstructed = 0;
 
 jest.mock('@octokit/rest', () => ({
-  get default() {
-    mockRequired++;
-
-    return class MockOctokit {
-      public authenticate = jest.fn();
-
-      constructor() {
-        mockConstructed++;
-      }
-    };
+  Octokit: class MockOctokit {
+    authenticate: jest.Mock;
+    constructor() {
+      ++mockConstructed;
+      this.authenticate = jest.fn();
+    }
   },
 }));
 
 describe('octokit', () => {
   describe('getOctokit()', () => {
-    it('requires the Octokit only once', async () => {
-      let octokit = await getOctokit();
-      octokit = await getOctokit();
-
-      expect(octokit).toBeTruthy();
-      expect(mockRequired).toBe(1);
+    it('constructs the Octokit only once', async () => {
+      await getOctokit();
+      await getOctokit();
       expect(mockConstructed).toBe(1);
     });
 
diff --git a/tests/utils/sorted-electron-map-spec.ts b/tests/utils/sorted-electron-map-spec.ts
index d25108e1..fce6b841 100644
--- a/tests/utils/sorted-electron-map-spec.ts
+++ b/tests/utils/sorted-electron-map-spec.ts
@@ -27,23 +27,17 @@ describe('sorted-electron-map', () => {
       '2.0.0-nightly.20200101': {
         version: 'v2.0.0-nightly.20200101',
       },
+      '2.0.0-beta.1': {
+        version: 'v2.0.0-beta.1',
+      },
       '2.0.0-nightly.20200102': {
         version: 'v2.0.0-nightly.20200102',
       },
-      '2.0.0': {
-        version: 'v2.0.0',
-      },
-      '3.0.0-nightly.20200105': {
-        version: 'v3.0.0-nightly.20200105',
-      },
-      '3.0.0': {
-        version: 'v3.0.0',
-      },
       '3.0.0-beta.1': {
         version: 'v3.0.0-beta.1',
       },
-      '2.0.0-beta.1': {
-        version: 'v2.0.0-beta.1',
+      '3.0.0-nightly.20200105': {
+        version: 'v3.0.0-nightly.20200105',
       },
       '2.0.0-beta.2': {
         version: 'v2.0.0-beta.2',
@@ -51,6 +45,12 @@ describe('sorted-electron-map', () => {
       '2.0.0-beta.3': {
         version: 'v2.0.0-beta.3',
       },
+      '2.0.0': {
+        version: 'v2.0.0',
+      },
+      '3.0.0': {
+        version: 'v3.0.0',
+      },
     };
 
     const result = sortedElectronMap(map, (_k, e) => e);