Repository: electron/fiddle
Pull request #646 (2021-04-08 19:22:23)

test: improve binary spec coverage
No functional changes, not even a refactor.

Just adds coverage for a couple of pieces in `renderer/binary`!

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

Coverage increased (+0.2%) to 94.154% when pulling **0be331ca8ea03ec0af65581a25d4a2ecf0f4ef39 on test/improve-binary-spec-coverage** into **8b16dff5fd73eda95292e2c6e46ca8aa3f664f66 on master**.


Patch:
diff --git a/src/renderer/fetch-types.ts b/src/renderer/fetch-types.ts
index 28a9fbb6..8f7a8603 100644
--- a/src/renderer/fetch-types.ts
+++ b/src/renderer/fetch-types.ts
@@ -16,7 +16,10 @@ const definitionPath = path.join(USER_DATA_PATH, 'electron-typedef');
  * @returns {Promise<string>}
  */
 export async function fetchTypeDefinitions(version: string): Promise<string> {
-  const url = `https://unpkg.com/electron@${version}/electron.d.ts`;
+  const packageName = version.includes('nightly')
+    ? 'electron-nightly'
+    : 'electron';
+  const url = `https://unpkg.com/${packageName}@${version}/electron.d.ts`;
 
   let text: string;
   try {
diff --git a/src/renderer/npm.ts b/src/renderer/npm.ts
index 49d9c58b..8d8926d9 100644
--- a/src/renderer/npm.ts
+++ b/src/renderer/npm.ts
@@ -138,7 +138,7 @@ export async function installModules(
   let nameArgs: Array<string> = [];
 
   if (packageManager === 'npm') {
-    nameArgs = names.length > 0 ? ['-S', ...names] : ['--dev --prod'];
+    nameArgs = names.length > 0 ? ['-S', ...names] : ['--also=dev --prod'];
   } else {
     nameArgs = [...names];
   }
diff --git a/src/renderer/transforms/forge.ts b/src/renderer/transforms/forge.ts
index 411f2138..6d168cf9 100644
--- a/src/renderer/transforms/forge.ts
+++ b/src/renderer/transforms/forge.ts
@@ -1,5 +1,7 @@
+import { execSync } from 'child_process';
 import { Files } from '../../interfaces';
 import { PACKAGE_NAME } from '../../shared-constants';
+import { getElectronBinaryPath } from '../binary';
 
 /**
  * This transform turns the files into an electron-forge
@@ -35,6 +37,19 @@ export async function forgeTransform(files: Files): Promise<Files> {
       parsed.config.forge = {};
       parsed.config.forge.packagerConfig = {};
 
+      const nightlyVersion = parsed.devDependencies['electron-nightly'];
+      if (nightlyVersion) {
+        // Fetch forced ABI for nightly.
+        const binaryPath = getElectronBinaryPath(nightlyVersion);
+        const abi = execSync(
+          `ELECTRON_RUN_AS_NODE=1 "${binaryPath}" -p process.versions.modules`,
+        );
+
+        parsed.config.forge.electronRebuildConfig = {
+          forceABI: abi.toString().trim(),
+        };
+      }
+
       // electron-forge makers
       parsed.config.forge.makers = [
         {
diff --git a/src/utils/get-package.ts b/src/utils/get-package.ts
index a233b6c4..c2fe5cbd 100644
--- a/src/utils/get-package.ts
+++ b/src/utils/get-package.ts
@@ -35,7 +35,10 @@ export async function getPackageJson(
   const dependencies: Record<string, string> = {};
 
   if (includeElectron) {
-    devDependencies.electron = appState.version;
+    const packageName = appState.version?.includes('nightly')
+      ? 'electron-nightly'
+      : 'electron';
+    devDependencies[packageName] = appState.version;
   }
 
   if (includeDependencies && values) {


Test patch:
diff --git a/tests/renderer/binary-spec.ts b/tests/renderer/binary-spec.ts
index 6591cf50..29cf4fbe 100644
--- a/tests/renderer/binary-spec.ts
+++ b/tests/renderer/binary-spec.ts
@@ -16,9 +16,11 @@ import { waitFor } from '../../src/utils/wait-for';
 import * as path from 'path';
 import * as semver from 'semver';
 import extract from 'extract-zip';
+import { download as electronDownload } from '@electron/get';
 
 jest.mock('fs-extra');
 jest.mock('extract-zip');
+jest.mock('@electron/get');
 jest.mock('../../src/renderer/ipc', () => ({}));
 jest.mock('../../src/renderer/constants', () => ({
   USER_DATA_PATH: 'user/data/',
@@ -26,9 +28,6 @@ jest.mock('../../src/renderer/constants', () => ({
 jest.mock('../../src/renderer/fetch-types', () => ({
   removeTypeDefsForVersion: jest.fn(),
 }));
-jest.mock('@electron/get', () => ({
-  download: jest.fn(),
-}));
 
 describe('binary', () => {
   let mockState: any = {};
@@ -106,8 +105,9 @@ describe('binary', () => {
     it(`returns ${VersionState.downloading} when downloading`, () => {
       // mock a download in progress
       let downloadResolve: any;
-      const { download } = require('@electron/get');
-      download.mockReturnValue(new Promise((r) => (downloadResolve = r)));
+      (electronDownload as jest.Mock).mockReturnValueOnce(
+        new Promise((r) => (downloadResolve = r)),
+      );
 
       setupBinary(mockState, ver.version);
 
@@ -117,8 +117,7 @@ describe('binary', () => {
 
     it(`returns ${VersionState.unzipping} when unzipping`, async () => {
       // mock a completed download + an unzip in progress
-      const { download } = require('@electron/get');
-      download.mockResolvedValueOnce('/fake/path');
+      (electronDownload as jest.Mock).mockResolvedValueOnce('/fake/path');
       let unzipResolve: any;
       const unzipPromise = new Promise((r) => (unzipResolve = r));
       (extract as jest.Mock).mockReturnValueOnce(unzipPromise);
@@ -193,11 +192,9 @@ describe('binary', () => {
   });
 
   describe('setupBinary()', () => {
-    const { download } = require('@electron/get');
-
     it('does not download local versions', async () => {
       const localPath = '/fake/path';
-      download.mockResolvedValue(localPath);
+      (electronDownload as jest.Mock).mockResolvedValue(localPath);
 
       const version = '3.0.0';
       mockState.versions[version] = {
@@ -208,7 +205,7 @@ describe('binary', () => {
 
       await setupBinary(mockState, version);
 
-      expect(download).not.toHaveBeenCalled();
+      expect(electronDownload).not.toHaveBeenCalled();
       expect(mockState.versions[version].state).toBe(VersionState.unknown);
     });
 
@@ -216,7 +213,7 @@ describe('binary', () => {
       (fs.existsSync as jest.Mock).mockReturnValueOnce(false);
 
       const localPath = '/fake/path';
-      download.mockResolvedValue(localPath);
+      (electronDownload as jest.Mock).mockResolvedValue(localPath);
 
       const version = '3.0.0';
       mockState.versions[version] = {
@@ -226,10 +223,45 @@ describe('binary', () => {
       };
       await setupBinary(mockState, version);
 
-      expect(download).toHaveBeenCalled();
+      expect(electronDownload).toHaveBeenCalled();
       expect(mockState.versions[version].state).toBe('ready');
     });
 
+    it('handles progress callbacks from electron/get', async () => {
+      const percent = 50;
+      const version = '9.9.9';
+      const ver: RunnableVersion = {
+        source: VersionSource.remote,
+        state: VersionState.unknown,
+        version,
+      };
+      mockState.versions[version] = ver;
+
+      // mock a download in progress...
+      let downloadResolve: any;
+      let getProgressCallback: any;
+      (electronDownload as jest.Mock).mockImplementation(
+        (_: any, opts: any) => {
+          ({ getProgressCallback } = opts.downloadOptions);
+          return new Promise((r) => (downloadResolve = r));
+        },
+      );
+
+      setupBinary(mockState, version);
+      await waitFor(() => getVersionState(ver) === VersionState.downloading);
+
+      expect(ver.downloadProgress).not.toBe(percent);
+
+      // send a progress update
+      getProgressCallback({ percent });
+      expect(ver.downloadProgress).toBe(percent);
+      // and do it again to trigger a different if-else branch
+      getProgressCallback({ percent });
+      expect(ver.downloadProgress).toBe(percent);
+
+      downloadResolve();
+    });
+
     it(`does not download a version again`, async () => {
       (fs.existsSync as jest.Mock).mockReturnValue(true);
 
@@ -242,11 +274,27 @@ describe('binary', () => {
 
       await setupBinary(mockState, version);
 
-      expect(download).not.toHaveBeenCalled();
+      expect(electronDownload).not.toHaveBeenCalled();
       expect(mockState.versions[version].state).toBe('ready');
     });
 
-    it(`waits if asked for a version is already downloading`, async () => {
+    it(`returns the same promise if called twice for the same version`, async () => {
+      (fs.existsSync as jest.Mock).mockReturnValue(false);
+
+      const version = '123.456.789';
+      mockState.versions[version] = {
+        source: VersionSource.remote,
+        state: VersionState.unknown,
+        version,
+      };
+
+      const prom1 = setupBinary(mockState, version);
+      const prom2 = setupBinary(mockState, version);
+
+      expect(prom1).toBe(prom2);
+    });
+
+    it(`waits if asked for a version that is already downloading`, async () => {
       (fs.existsSync as jest.Mock).mockReturnValue(true);
 
       const version = '3.0.0';
@@ -258,14 +306,12 @@ describe('binary', () => {
 
       await setupBinary(mockState, version);
 
-      expect(download).not.toHaveBeenCalled();
+      expect(electronDownload).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');
+      (electronDownload as jest.Mock).mockReturnValue('/fake/path');
       const version = '3.0.0';
       mockState.versions[version] = {
         source: VersionSource.remote,
diff --git a/tests/renderer/npm-spec.ts b/tests/renderer/npm-spec.ts
index 5081f67c..2cf58ae5 100644
--- a/tests/renderer/npm-spec.ts
+++ b/tests/renderer/npm-spec.ts
@@ -220,7 +220,7 @@ describe('npm', () => {
 
         expect(exec).toHaveBeenCalledWith<any>(
           '/my/directory',
-          'npm install --dev --prod',
+          'npm install --also=dev --prod',
         );
       });
     });
diff --git a/tests/utils/get-package-spec.ts b/tests/utils/get-package-spec.ts
index 45d56fa4..880ffcfb 100644
--- a/tests/utils/get-package-spec.ts
+++ b/tests/utils/get-package-spec.ts
@@ -47,6 +47,51 @@ describe('get-package', () => {
     );
   });
 
+  it('getPackageJson() includes electron-nightly if needed', async () => {
+    const result = await getPackageJson(
+      {
+        getName: () => 'test-app',
+        version: '1.0.0-nightly.123456789',
+      } as any,
+      {
+        main: 'app.goDoTheThing()',
+        renderer: `const say = require('say')`,
+        html: '<html />',
+        preload: 'preload',
+        css: 'body { color: black }',
+      },
+      {
+        includeElectron: true,
+        includeDependencies: true,
+      },
+    );
+
+    expect(result).toEqual(
+      JSON.stringify(
+        {
+          name: 'test-app',
+          productName: 'test-app',
+          description: 'My Electron application description',
+          keywords: [],
+          main: './main.js',
+          version: '1.0.0',
+          author: 'test-user',
+          scripts: {
+            start: 'electron .',
+          },
+          dependencies: {
+            say: '*',
+          },
+          devDependencies: {
+            'electron-nightly': '1.0.0-nightly.123456789',
+          },
+        },
+        undefined,
+        2,
+      ),
+    );
+  });
+
   it('getPackageJson() includes electron if needed', async () => {
     const result = await getPackageJson(
       {