chore: Replaced es6-promisify with nodejs 8 util.promisify helper (#1552

)

package.json

@@ -57,7 +57,6 @@
     "debounce": "1.2.0",
     "decamelize": "3.0.0",
     "es6-error": "4.1.1",
-    "es6-promisify": "5.0.0",
     "event-to-promise": "0.8.0",
     "firefox-profile": "1.2.0",
     "fx-runner": "1.0.10",

src/firefox/index.js

@@ -1,12 +1,12 @@
 /* @flow */
 import nodeFs from 'fs';
 import path from 'path';
+import {promisify} from 'util';
 
 import {default as defaultFxRunner} from 'fx-runner';
 import FirefoxProfile, {copyFromUserProfile as defaultUserProfileCopier}
   from 'firefox-profile';
 import {fs} from 'mz';
-import promisify from 'es6-promisify';
 import eventToPromise from 'event-to-promise';
 
 import isDirectory from '../util/is-directory';
@@ -238,7 +238,7 @@ export async function isDefaultProfile(
 
   // Check for profile dir path.
   const finder = new ProfileFinder(baseProfileDir);
-  const readProfiles = promisify(finder.readProfiles, finder);
+  const readProfiles = promisify(finder.readProfiles.bind(finder));
 
   await readProfiles();
 
@@ -333,8 +333,8 @@ export function defaultCreateProfileFinder(
   }: CreateProfileFinderParams = {}
 ): getProfileFn {
   const finder = new FxProfile.Finder(userDirectoryPath);
-  const readProfiles = promisify(finder.readProfiles, finder);
-  const getPath = promisify(finder.getPath, finder);
+  const readProfiles = promisify(finder.readProfiles.bind(finder));
+  const getPath = promisify(finder.getPath.bind(finder));
   return async (profileName: string): Promise<string | void> => {
     try {
       await readProfiles();

src/util/artifacts.js

@@ -1,7 +1,8 @@
 /* @flow */
+import {promisify} from 'util';
+
 import {fs} from 'mz';
 import mkdirp from 'mkdirp';
-import promisify from 'es6-promisify';
 
 import {UsageError, isErrorWithCode} from '../errors';
 import {createLogger} from './logger';

src/util/promisify.js

@@ -0,0 +1,26 @@
+/* @flow */
+
+/*
+ * A small promisify helper to make it easier to customize a
+ * function promisified (using the 'util' module available in
+ * nodejs >= 8) to resolve to an array of results:
+ *
+ *    import {promisify} from 'util';
+ *    import {multiArgsPromisedFn} from '../util/promisify';
+ *
+ *    aCallbackBasedFn[promisify.custom] = multiArgsPromisedFn(tmp.dir);
+ *    ...
+ */
+export function multiArgsPromisedFn(fn: Function): Function {
+  return (...callerArgs: Array<any>): Promise<any> => {
+    return new Promise((resolve, reject) => {
+      fn(...callerArgs, (err, ...rest) => {
+        if (err) {
+          reject(err);
+        } else {
+          resolve(rest);
+        }
+      });
+    });
+  };
+}

src/util/temp-dir.js

@@ -1,13 +1,18 @@
 /* @flow */
+import {promisify} from 'util';
+
 import tmp from 'tmp';
-import promisify from 'es6-promisify';
 
 import {createLogger} from './logger';
+import {multiArgsPromisedFn} from './promisify';
 
 const log = createLogger(__filename);
 
 export type MakePromiseCallback = (tmpDir: TempDir) => any;
 
+tmp.dir[promisify.custom] = multiArgsPromisedFn(tmp.dir);
+
+const createTempDir = promisify(tmp.dir);
 
 /*
  * Work with a self-destructing temporary directory in a promise chain.
@@ -63,15 +68,13 @@ export class TempDir {
    * been created.
    */
   create(): Promise<TempDir> {
-    const createTempDir = promisify(tmp.dir, {multiArgs: true});
     return createTempDir(
       {
         prefix: 'tmp-web-ext-',
         // This allows us to remove a non-empty tmp dir.
         unsafeCleanup: true,
       })
-      .then((args) => {
-        const [tmpPath, removeTempDir] = args;
+      .then(([tmpPath, removeTempDir]) => {
         this._path = tmpPath;
         this._removeTempDir = removeTempDir;
         log.debug(`Created temporary directory: ${this.path()}`);

src/util/zip-dir.js

@@ -1,5 +1,6 @@
 /* @flow */
+import {promisify} from 'util';
+
 import zipDirModule from 'zip-dir';
-import promisify from 'es6-promisify';
 
 export const zipDir = promisify(zipDirModule);

tests/functional/common.js

@@ -1,9 +1,9 @@
 /* @flow */
 import path from 'path';
 import {ChildProcess, spawn} from 'child_process';
+import {promisify} from 'util';
 
 import copyDir from 'copy-dir';
-import promisify from 'es6-promisify';
 import prettyjson from 'prettyjson';
 
 import * as tmpDirUtils from '../../src/util/temp-dir';
@@ -36,9 +36,10 @@ export type TempAddonParams = {|
 |};
 
 export type TempAddonCallback =
-  (tmpAddonDir: string, tmpDir: string) => Promise<any>
+  (tmpAddonDir: string, tmpDir: string) => Promise<any>;
 
 const copyDirAsPromised = promisify(copyDir);
+
 export function withTempAddonDir(
   {addonPath}: TempAddonParams,
   makePromise: TempAddonCallback,

tests/unit/helpers.js

@@ -3,11 +3,12 @@ import path from 'path';
 import EventEmitter from 'events';
 import tty from 'tty';
 import stream from 'stream';
+import {promisify} from 'util';
 
+import deepcopy from 'deepcopy';
 import sinon from 'sinon';
 import yauzl from 'yauzl';
 import ExtendableError from 'es6-error';
-import promisify from 'es6-promisify';
 
 import {createLogger} from '../../src/util/logger';
 import * as defaultFirefoxApp from '../../src/firefox';
@@ -280,7 +281,7 @@ export class ErrorWithCode extends Error {
 }
 
 /*
- * A basic manifest fixture using in unit tests.
+ * A basic manifest fixture used in unit tests.
  */
 export const basicManifest = {
   name: 'the extension',
@@ -292,6 +293,12 @@ export const basicManifest = {
   },
 };
 
+/*
+ * A basic manifest fixture without an applications property.
+ */
+export const manifestWithoutApps = deepcopy(basicManifest);
+delete manifestWithoutApps.applications;
+
 /*
  * A class that implements an empty IExtensionRunner interface.
  */

tests/unit/test-cmd/test.build.js

@@ -18,9 +18,9 @@ import {
   basicManifest,
   fixturePath,
   makeSureItFails,
+  manifestWithoutApps,
   ZipFile,
 } from '../helpers';
-import {manifestWithoutApps} from '../test-util/test.manifest';
 import {UsageError} from '../../../src/errors';
 import {createLogger} from '../../../src/util/logger';
 

tests/unit/test-cmd/test.sign.js

@@ -1,23 +1,23 @@
 /* @flow */
 import path from 'path';
+import {promisify} from 'util';
 
 import copyDir from 'copy-dir';
 import {fs} from 'mz';
 import {describe, it} from 'mocha';
 import {assert} from 'chai';
 import sinon from 'sinon';
-import promisify from 'es6-promisify';
 
 import {onlyInstancesOf, UsageError, WebExtError} from '../../../src/errors';
 import {getManifestId} from '../../../src/util/manifest';
 import {withTempDir} from '../../../src/util/temp-dir';
-import {manifestWithoutApps} from '../test-util/test.manifest';
 import completeSignCommand, {
   extensionIdFile, getIdFromSourceDir, saveIdToSourceDir,
 } from '../../../src/cmd/sign';
 import {
   basicManifest,
   makeSureItFails,
+  manifestWithoutApps,
   fixturePath,
 } from '../helpers';
 // Import flow type

tests/unit/test-firefox/test.firefox.js

@@ -16,10 +16,10 @@ import {
   fixturePath,
   fake,
   makeSureItFails,
+  manifestWithoutApps,
   TCPConnectError,
   ErrorWithCode,
 } from '../helpers';
-import {manifestWithoutApps} from '../test-util/test.manifest';
 import {RemoteFirefox} from '../../../src/firefox/remote';
 import type {RemotePortFinderParams} from '../../../src/firefox/index';
 
@@ -616,17 +616,29 @@ describe('firefox', () => {
 
     // Define the params type for the prepareProfileFinderTest helper.
     type PrepareProfileFinderTestParams = {|
-      readProfileReturns?: Promise<void>,
-      getPathReturns?: Promise<string>,
+      readProfiles?: Function,
+      getPath?: Function,
+      profiles?: Array<{Name: string}>,
     |};
 
+    const defaultFakeProfiles = [{Name: 'someName'}];
+
+    function defaultReadProfilesMock(cb) {
+      cb(undefined, defaultFakeProfiles);
+    }
+
+    function defaultGetPathMock(name, cb) {
+      cb(undefined, '/fake/path');
+    }
+
     function prepareProfileFinderTest({
-      readProfileReturns = Promise.resolve(),
-      getPathReturns = Promise.resolve('/fake/path'),
+      readProfiles = defaultReadProfilesMock,
+      getPath = defaultGetPathMock,
+      profiles = defaultFakeProfiles,
     }: PrepareProfileFinderTestParams = {}) {
-      const fakeReadProfiles = sinon.spy(() => readProfileReturns);
-      const fakeGetPath = sinon.spy(() => getPathReturns);
-      const fakeProfiles = [{Name: 'someName'}];
+      const fakeReadProfiles = sinon.spy(readProfiles);
+      const fakeGetPath = sinon.spy(getPath);
+      const fakeProfiles = profiles;
       const userDirectoryPath = '/non/existent/path';
 
       const FxProfile = {};
@@ -647,22 +659,17 @@ describe('firefox', () => {
       };
     }
 
-    it('creates a finder', async () => {
-      const FxProfile = {
-        Finder: sinon.spy(function() {
-          return {};
-        }),
-      };
+    it('creates a finder', () => {
+      const {FxProfile} = prepareProfileFinderTest();
+      FxProfile.Finder = sinon.spy(FxProfile.Finder);
       firefox.defaultCreateProfileFinder({FxProfile});
       sinon.assert.calledWith(FxProfile.Finder, sinon.match(undefined));
     });
 
     it('creates finder based on userDirectoryPath if present', async () => {
-      const FxProfile = {
-        Finder: sinon.spy(function() {
-          return {};
-        }),
-      };
+      const {FxProfile} = prepareProfileFinderTest();
+      FxProfile.Finder = sinon.spy(FxProfile.Finder);
+
       const userDirectoryPath = '/non/existent/path';
       firefox.defaultCreateProfileFinder({userDirectoryPath, FxProfile});
 
@@ -678,7 +685,9 @@ describe('firefox', () => {
         FxProfile,
         userDirectoryPath,
       } = prepareProfileFinderTest({
-        getPathReturns: Promise.resolve(expectedResolvedProfilePath),
+        getPath(name, cb) {
+          cb(undefined, expectedResolvedProfilePath);
+        },
       });
 
       const profileFinder = firefox.defaultCreateProfileFinder({
@@ -702,9 +711,9 @@ describe('firefox', () => {
            FxProfile,
            userDirectoryPath,
          } = prepareProfileFinderTest({
-           readProfileReturns: Promise.reject(
-             new ErrorWithCode('ENOENT', 'fake ENOENT error')
-           ),
+           readProfiles(cb) {
+             cb(new ErrorWithCode('ENOENT', 'fake ENOENT error'));
+           },
          });
 
          const getter = firefox.defaultCreateProfileFinder({
@@ -713,13 +722,14 @@ describe('firefox', () => {
          });
 
          const res = await getter('someName');
+
+         sinon.assert.called(fakeReadProfiles);
+         sinon.assert.notCalled(fakeGetPath);
+
          assert.equal(
            res,
            undefined,
            'Got an undefined result when the profiles.ini file does not exist');
-
-         sinon.assert.called(fakeReadProfiles);
-         sinon.assert.notCalled(fakeGetPath);
        });
 
     it('returns a finder that throws unexpected errors',
@@ -730,9 +740,9 @@ describe('firefox', () => {
            FxProfile,
            userDirectoryPath,
          } = prepareProfileFinderTest({
-           readProfileReturns: Promise.reject(
-             new Error('unspecified error')
-           ),
+           readProfiles(cb) {
+             cb(new Error('unspecified error'));
+           },
          });
 
          const getter = firefox.defaultCreateProfileFinder({
@@ -742,7 +752,7 @@ describe('firefox', () => {
 
          const promise = getter('someName');
 
-         assert.isRejected(
+         await assert.isRejected(
            promise,
            'unspecified error',
            'Throws expected error'
@@ -1025,13 +1035,13 @@ describe('firefox', () => {
         });
     });
 
-    it('throws on unexpected errors', () => {
+    it('throws on unexpected errors', async () => {
       const connectToFirefox = sinon.spy(async () => {
         throw new Error('Unexpected connect error');
       });
 
-      assert.isRejected(findRemotePort({connectToFirefox}),
-                        /Unxpected connect error/);
+      await assert.isRejected(findRemotePort({connectToFirefox}),
+                              /Unexpected connect error/);
 
       sinon.assert.calledOnce(connectToFirefox);
     });

tests/unit/test-util/test.manifest.js

@@ -9,11 +9,11 @@ import {fs} from 'mz';
 import {onlyInstancesOf, InvalidManifest} from '../../../src/errors';
 import getValidatedManifest, {getManifestId} from '../../../src/util/manifest';
 import {withTempDir} from '../../../src/util/temp-dir';
-import {basicManifest, makeSureItFails} from '../helpers';
-
-
-export const manifestWithoutApps = deepcopy(basicManifest);
-delete manifestWithoutApps.applications;
+import {
+  basicManifest,
+  makeSureItFails,
+  manifestWithoutApps,
+} from '../helpers';
 
 
 describe('util/manifest', () => {