install plugin with npm api

package.json

@@ -54,7 +54,6 @@
     "yargs": "8.0.2"
   },
   "devDependencies": {
-    "keytar": "^4.1.0",
     "@types/body-parser": "1.16.4",
     "@types/chai": "4.0.1",
     "@types/chai-string": "1.1.30",
@@ -71,6 +70,7 @@
     "@types/progress": "2.0.0",
     "@types/stack-trace": "0.0.28",
     "@types/tmp": "0.0.33",
+    "keytar": "^4.1.0",
     "chai": "4.1.2",
     "chai-string": "1.4.0",
     "clear-require": "2.0.0",
@@ -96,6 +96,7 @@
     "jest-junit": "3.6.0",
     "jsdoc": "3.5.5",
     "madge": "3.0.0",
+    "npm": "^6.4.1",
     "prompt": "1.0.0",
     "shebang-regex": "2.0.0",
     "through2": "2.0.3",

packages/imperative/__tests__/plugins/cmd/install/install.handler.test.ts

@@ -1,4 +1,4 @@
-/*
+/*
 * This program and the accompanying materials are made available under the terms of the
 * Eclipse Public License v2.0 which accompanies this distribution, and is available at
 * https://www.eclipse.org/legal/epl-v20.html
@@ -22,11 +22,10 @@ jest.mock("../../../../../cmd/src/response/HandlerResponse");
 jest.mock("../../../../../cmd/src/doc/handler/IHandlerParameters");
 jest.mock("../../../../../logger");
 jest.mock("../../../../src/Imperative");
+jest.mock("../../../../src/plugins/utilities/NpmApiFunctions");
 
 import {CommandResponse, IHandlerParameters} from "../../../../../cmd";
 import {Console} from "../../../../../console";
-import {execSync} from "child_process";
-import {Imperative} from "../../../../src/Imperative";
 import {ImperativeError} from "../../../../../error";
 import {install} from "../../../../src/plugins/utilities/npm-interface";
 import {runValidatePlugin} from "../../../../src/plugins/utilities/runValidatePlugin";
@@ -37,12 +36,13 @@ import {readFileSync, writeFileSync} from "jsonfile";
 import {PMFConstants} from "../../../../src/plugins/utilities/PMFConstants";
 import {resolve} from "path";
 import {TextUtils} from "../../../../../utilities";
+import {getRegistry} from "../../../../src/plugins/utilities/NpmApiFunctions";
 
 describe("Plugin Management Facility install handler", () => {
 
   // Objects created so types are correct.
   const mocks = {
-    execSync: execSync as Mock<typeof execSync>,
+    getRegistry: getRegistry as Mock<typeof getRegistry>,
     readFileSync: readFileSync as Mock<typeof readFileSync>,
     writeFileSync: writeFileSync as Mock<typeof writeFileSync>,
     install: install as Mock<typeof install>,
@@ -86,20 +86,18 @@ describe("Plugin Management Facility install handler", () => {
   };
 
   beforeEach(() => {
-    mocks.execSync.mockReturnValue(packageRegistry);
+    mocks.getRegistry.mockReturnValue(packageRegistry);
     mocks.readFileSync.mockReturnValue({});
 
     mocks.runValidatePlugin.mockReturnValue(finalValidationMsg);
   });
 
   /**
-   * Validates that an execSync 'npm config get registry' call was valid
+   * Validates that an getRegistry was called
    * when registry needed based on the parameters passed.
    */
-  const wasExecSyncGetRegistryCallValid = () => {
-    expect(mocks.execSync).toHaveBeenCalledWith(
-      `npm config get registry`
-    );
+  const wasGetRegistryCalled = () => {
+    expect(mocks.getRegistry).toHaveBeenCalled();
   };
 
   /**
@@ -183,7 +181,7 @@ describe("Plugin Management Facility install handler", () => {
     await handler.process(params as IHandlerParameters);
 
     // Validate the call to get the registry value
-    wasExecSyncGetRegistryCallValid();
+    wasGetRegistryCalled();
 
     expect(mocks.install).toHaveBeenCalledTimes(2);
     wasInstallCallValid(`${fileJson.a.package}@${fileJson.a.version}`, packageRegistry, true);
@@ -231,7 +229,7 @@ describe("Plugin Management Facility install handler", () => {
     await handler.process(params as IHandlerParameters);
 
     // Validate the call to get the registry value
-    wasExecSyncGetRegistryCallValid();
+    wasGetRegistryCalled();
 
     // Check that install worked as expected
     wasInstallCallValid(params.arguments.plugin[0], packageRegistry);
@@ -264,7 +262,7 @@ describe("Plugin Management Facility install handler", () => {
     await handler.process(params as IHandlerParameters);
 
     // Validate the install
-    wasExecSyncGetRegistryCallValid();
+    wasGetRegistryCalled();
 
     // Validate that install was called with each of these values
     expect(mocks.install).toHaveBeenCalledTimes(params.arguments.plugin.length);
@@ -284,7 +282,7 @@ describe("Plugin Management Facility install handler", () => {
     await handler.process(params as IHandlerParameters);
 
     // Validate the call to get the registry value
-    wasExecSyncGetRegistryCallValid();
+    wasGetRegistryCalled();
     wasReadFileSyncCallValid(PMFConstants.instance.PLUGIN_JSON);
 
     expect(params.response.console.log).toHaveBeenCalledWith("No packages were found in " +

packages/imperative/__tests__/plugins/utilities/npm-interface/install.test.ts

@@ -20,10 +20,9 @@ jest.mock("../../../../src/plugins/utilities/PMFConstants");
 jest.mock("../../../../../logger");
 jest.mock("../../../../../cmd/src/response/CommandResponse");
 jest.mock("../../../../../cmd/src/response/HandlerResponse");
+jest.mock("../../../../src/plugins/utilities/NpmApiFunctions");
 
-import {CommandResponse} from "../../../../../cmd";
 import {Console} from "../../../../../console";
-import {execSync} from "child_process";
 import {existsSync, lstatSync} from "fs";
 import {ImperativeError} from "../../../../../error";
 import {install} from "../../../../src/plugins/utilities/npm-interface";
@@ -34,12 +33,13 @@ import {Logger} from "../../../../../logger";
 import {PMFConstants} from "../../../../src/plugins/utilities/PMFConstants";
 import {readFileSync, writeFileSync} from "jsonfile";
 import {sync} from "find-up";
+import {installPackages} from "../../../../src/plugins/utilities/NpmApiFunctions";
 
 describe("PMF: Install Interface", () => {
   // Objects created so types are correct.
   const mocks = {
     dirname: dirname as Mock<typeof dirname>,
-    execSync: execSync as Mock<typeof execSync>,
+    installPackages: installPackages as Mock<typeof installPackages>,
     existsSync: existsSync as Mock<typeof existsSync>,
     isAbsolute: isAbsolute as Mock<typeof isAbsolute>,
     join: join as Mock<typeof join>,
@@ -73,21 +73,14 @@ describe("PMF: Install Interface", () => {
   });
 
   /**
-   * Validates that an execSync npm install call was valid based on the parameters passed.
+   * Validates that an npm install call was valid based on the parameters passed.
    *
    * @param {string} expectedPackage The package that should be sent to npm install
    * @param {string} expectedRegistry The registry that should be sent to npm install
-   * @param {boolean} [installFromFile=false] was the install from a file. This affects
-   *                                          the pipe sent to execSync stdio option.
    */
-  const wasExecSyncCallValid = (expectedPackage: string, expectedRegistry: string) => {
-    expect(mocks.execSync).toHaveBeenCalledWith(
-      `npm install "${expectedPackage}" --prefix "${PMFConstants.instance.PLUGIN_INSTALL_LOCATION}" -g --registry "${expectedRegistry}"`,
-      {
-        cwd  : PMFConstants.instance.PMF_ROOT,
-        stdio: ["pipe", "pipe", process.stderr]
-      }
-    );
+  const wasNpmInstallCallValid = (expectedPackage: string, expectedRegistry: string) => {
+      expect(mocks.installPackages).toHaveBeenCalledWith(PMFConstants.instance.PLUGIN_INSTALL_LOCATION,
+          expectedRegistry, true, expectedPackage);
   };
 
   /**
@@ -117,35 +110,35 @@ describe("PMF: Install Interface", () => {
 
   describe("Basic install", () => {
     beforeEach(() => {
-      mocks.execSync.mockReturnValue(`+ ${packageName}@${packageVersion}`);
+      mocks.installPackages.mockReturnValue(`+ ${packageName}@${packageVersion}`);
       mocks.existsSync.mockReturnValue(true);
       mocks.normalize.mockReturnValue("testing");
       mocks.lstatSync.mockReturnValue({
         isSymbolicLink: jest.fn().mockReturnValue(true)
       });
     });
 
-    it("should install from the npm registry", () => {
-      install(packageName, packageRegistry);
+    it("should install from the npm registry", async () => {
+      await install(packageName, packageRegistry);
 
       // Validate the install
-      wasExecSyncCallValid(packageName, packageRegistry);
+      wasNpmInstallCallValid(packageName, packageRegistry);
       wasWriteFileSyncCallValid({}, packageName, {
         package: packageName,
         registry: packageRegistry,
         version: packageVersion
       });
     });
 
-    it("should install an absolute file path", () => {
+    it("should install an absolute file path", async () => {
       const rootFile = "/root/a";
 
       mocks.isAbsolute.mockReturnValue(true);
 
-      install(rootFile, packageRegistry);
+      await install(rootFile, packageRegistry);
 
       // Validate the install
-      wasExecSyncCallValid(rootFile, packageRegistry);
+      wasNpmInstallCallValid(rootFile, packageRegistry);
       wasWriteFileSyncCallValid({}, packageName, {
         package: rootFile,
         registry: packageRegistry,
@@ -163,15 +156,15 @@ describe("PMF: Install Interface", () => {
         mocks.resolve.mockReturnValue(absolutePath);
       });
 
-      it("should install a relative file path", () => {
+      it("should install a relative file path", async () => {
         // Setup mocks for install function
         mocks.existsSync.mockReturnValue(true);
 
         // Call the install function
-        install(relativePath, packageRegistry);
+        await install(relativePath, packageRegistry);
 
         // Validate results
-        wasExecSyncCallValid(absolutePath, packageRegistry);
+        wasNpmInstallCallValid(absolutePath, packageRegistry);
         wasWriteFileSyncCallValid({}, packageName, {
           package: absolutePath,
           registry: packageRegistry,
@@ -180,15 +173,15 @@ describe("PMF: Install Interface", () => {
       });
     });
 
-    it("should install from a url", () => {
+    it("should install from a url", async () => {
       const installUrl = "http://www.example.com";
       mocks.resolve.mockReturnValue(installUrl);
 
       // mocks.isUrl.mockReturnValue(true);
 
-      install(installUrl, packageRegistry);
+      await install(installUrl, packageRegistry);
 
-      wasExecSyncCallValid(installUrl, packageRegistry);
+      wasNpmInstallCallValid(installUrl, packageRegistry);
       wasWriteFileSyncCallValid({}, packageName, {
         package: installUrl,
         registry: packageRegistry,
@@ -198,26 +191,26 @@ describe("PMF: Install Interface", () => {
   });
 
   describe("Advanced install", () => {
-    it("should write even when install from file is true", () => {
+    it("should write even when install from file is true", async () => {
       // This test is constructed in such a way that all if conditions with installFromFile
       // are validated to have been called or not.
       const location = "/this/should/not/change";
 
       mocks.isAbsolute.mockReturnValue(false);
       mocks.existsSync.mockReturnValue(true);
-      mocks.execSync.mockReturnValue(`+ ${packageName}@${packageVersion}`);
+      mocks.installPackages.mockReturnValue(`+ ${packageName}@${packageVersion}`);
       mocks.normalize.mockReturnValue("testing");
       mocks.lstatSync.mockReturnValue({
         isSymbolicLink: jest.fn().mockReturnValue(true)
       });
 
-      install(location, packageRegistry, true);
+      await install(location, packageRegistry, true);
 
-      wasExecSyncCallValid(location, packageRegistry);
+      wasNpmInstallCallValid(location, packageRegistry);
       expect(mocks.writeFileSync).toHaveBeenCalled();
     });
 
-    it("should accept semver properly", () => {
+    it("should accept semver properly", async () => {
       const semverVersion = "^1.5.2";
       const semverPackage = `${packageName}@${semverVersion}`;
 
@@ -232,21 +225,21 @@ describe("PMF: Install Interface", () => {
       mocks.isAbsolute.mockReturnValue(true);
 
       // This is valid under semver ^1.5.2
-      mocks.execSync.mockReturnValue(`+ ${packageName}@1.5.16`);
+      mocks.installPackages.mockReturnValue(`+ ${packageName}@1.5.16`);
 
       // Call the install
-      install(semverPackage, packageRegistry);
+      await install(semverPackage, packageRegistry);
 
       // Test that shit happened
-      wasExecSyncCallValid(semverPackage, packageRegistry);
+      wasNpmInstallCallValid(semverPackage, packageRegistry);
       wasWriteFileSyncCallValid({}, packageName, {
         package: packageName,
         registry: packageRegistry,
         version: semverVersion
       });
     });
 
-    it("should merge contents of previous json file", () => {
+    it("should merge contents of previous json file", async () => {
       // value for our previous plugins.json
       const oneOldPlugin: IPluginJson = {
         plugin1: {
@@ -256,36 +249,36 @@ describe("PMF: Install Interface", () => {
         }
       };
 
-      mocks.execSync.mockReturnValue(`+ ${packageName}@${packageVersion}`);
+      mocks.installPackages.mockReturnValue(`+ ${packageName}@${packageVersion}`);
       mocks.existsSync.mockReturnValue(true);
       mocks.normalize.mockReturnValue("testing");
       mocks.lstatSync.mockReturnValue({
         isSymbolicLink: jest.fn().mockReturnValue(true)
       });
       mocks.readFileSync.mockReturnValue(oneOldPlugin);
 
-      install(packageName, packageRegistry);
+      await install(packageName, packageRegistry);
 
-      wasExecSyncCallValid(packageName, packageRegistry);
+      wasNpmInstallCallValid(packageName, packageRegistry);
       wasWriteFileSyncCallValid(oneOldPlugin, packageName, {
         package: packageName,
         registry: packageRegistry,
         version: packageVersion
       });
     });
 
-    it("should throw errors", () => {
+    it("should throw errors", async () => {
       const error = new Error("This should be caught");
 
-      mocks.execSync.mockImplementation(() => {
+      mocks.installPackages.mockImplementation(() => {
         throw error;
       });
 
       // Create a placeholder error object that should be set after the call to install
       let expectedError: ImperativeError;
 
       try {
-        install("test", "http://www.example.com");
+        await install("test", "http://www.example.com");
       } catch (e) {
         expectedError = e;
       }