Update unit tests for plugin install failure

Signed-off-by: Timothy Johnson <timothy.johnson@broadcom.com>

CHANGELOG.md

@@ -2,8 +2,13 @@
 
 All notable changes to the Imperative package will be documented in this file.
 
+## Recent Changes
+
+- BugFix: Fixed plugin install error not displayed correctly. [#954](https://github.com/zowe/imperative/issues/954)
+
 ## `5.9.1`
-- BugFix: Fix environment file not applying to daemon client environment variables
+
+- BugFix: Fixed environment file not applying to daemon client environment variables
 
 ## `5.9.0`
 

packages/imperative/__tests__/config/cmd/report-env/EnvQuery.unit.test.ts

@@ -391,7 +391,7 @@ describe("Tests for EnvQuery module", () => {
             });
 
             const cmdOutput = await EnvQuery["getCmdOutput"]("bogusCmd", ["bogusArg"]);
-            expect(cmdOutput).toContain("Failed to run commmand = bogusCmd");
+            expect(cmdOutput).toContain("Failed to run command = bogusCmd");
             expect(cmdOutput).toContain("Pretend this was thrown by spawnSync");
         });
     }); // end getCmdOutput function

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

@@ -49,6 +49,7 @@ import { readFileSync, writeFileSync } from "jsonfile";
 import { PMFConstants } from "../../../../src/plugins/utilities/PMFConstants";
 import { TextUtils } from "../../../../../utilities";
 import { getRegistry, npmLogin } from "../../../../src/plugins/utilities/NpmFunctions";
+import * as childProcess from "child_process";
 
 let expectedVal;
 let returnedVal;
@@ -347,7 +348,30 @@ describe("Plugin Management Facility install handler", () => {
             expectedError = e;
         }
 
+        expect(expectedError).toBeInstanceOf(ImperativeError);
         expect(expectedError.message).toBe("Install Failed");
     });
+
+    it("should handle an error in spawned process", async () => {
+        const handler = new InstallHandler();
+
+        const params = getIHandlerParametersObject();
+        params.arguments.plugin = ["sample1"];
+
+        let expectedError: ImperativeError;
+
+        jest.spyOn(childProcess, "spawnSync").mockReturnValueOnce({ status: 1 } as any);
+        mocks.getRegistry.mockImplementationOnce(jest.requireActual("../../../../src/plugins/utilities/NpmFunctions").getRegistry);
+
+        try {
+            await handler.process(params);
+        } catch (e) {
+            expectedError = e;
+        }
+
+        expect(expectedError).toBeInstanceOf(ImperativeError);
+        expect(expectedError.additionalDetails).toContain("Command failed");
+        expect(expectedError.additionalDetails).toContain("npm");
+    });
 });
 

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

@@ -27,7 +27,7 @@ import { IPluginJson } from "../../../../src/plugins/doc/IPluginJson";
 import { Logger } from "../../../../../logger";
 import { PMFConstants } from "../../../../src/plugins/utilities/PMFConstants";
 import { readFileSync, writeFileSync } from "jsonfile";
-import { cmdToRun } from "../../../../src/plugins/utilities/NpmFunctions";
+import { findNpmOnPath } from "../../../../src/plugins/utilities/NpmFunctions";
 import { uninstall } from "../../../../src/plugins/utilities/npm-interface";
 
 
@@ -42,7 +42,7 @@ describe("PMF: Uninstall Interface", () => {
     const samplePackageName = "imperative-sample-plugin";
     const packageName = "a";
     const packageRegistry = "https://registry.npmjs.org/";
-    const npmCmd = cmdToRun();
+    const npmCmd = findNpmOnPath();
 
     beforeEach(() => {
         // Mocks need cleared after every test for clean test runs
@@ -101,7 +101,10 @@ describe("PMF: Uninstall Interface", () => {
 
     describe("Basic uninstall", () => {
         beforeEach(() => {
-            mocks.spawnSync.mockReturnValue(`+ ${packageName}`);
+            mocks.spawnSync.mockReturnValue({
+                status: 0,
+                stdout: Buffer.from(`+ ${packageName}`)
+            } as any);
         });
 
         it("should uninstall", () => {

packages/imperative/__tests__/plugins/utilities/runValidatePlugin.test.ts

@@ -25,7 +25,10 @@ const cmdOutputJson = {
     stderr: "The validate commands's standard error",
     data: {}
 };
-const spawnSyncOutput = { stdout: JSON.stringify(cmdOutputJson) };
+const spawnSyncOutput = {
+    status: 0,
+    stdout: JSON.stringify(cmdOutputJson)
+};
 
 describe("runValidatePlugin", () => {
     const mainModule = process.mainModule;

packages/imperative/src/plugins/utilities/NpmFunctions.ts

@@ -41,23 +41,19 @@ export function findNpmOnPath(): string {
  */
 export function installPackages(prefix: string, registry: string, npmPackage: string): string {
     const pipe: StdioOptions = ["pipe", "pipe", process.stderr];
-    try {
-        const execOutput = ProcessUtils.execAndCheckOutput(npmCmd,
-            [
-                "install", npmPackage,
-                "--prefix", prefix,
-                "-g",
-                "--registry", registry,
-                "--legacy-peer-deps"
-            ], {
-                cwd: PMFConstants.instance.PMF_ROOT,
-                stdio: pipe
-            }
-        );
-        return execOutput.toString();
-    } catch (err) {
-        throw (err.message);
-    }
+    const execOutput = ProcessUtils.execAndCheckOutput(npmCmd,
+        [
+            "install", npmPackage,
+            "--prefix", prefix,
+            "-g",
+            "--registry", registry,
+            "--legacy-peer-deps"
+        ], {
+            cwd: PMFConstants.instance.PMF_ROOT,
+            stdio: pipe
+        }
+    );
+    return execOutput.toString();
 }
 
 /**
@@ -66,33 +62,25 @@ export function installPackages(prefix: string, registry: string, npmPackage: st
  * @return {string}
  */
 export function getRegistry(): string {
-    try {
-        const execOutput = ProcessUtils.execAndCheckOutput(npmCmd, [ "config", "get", "registry" ]);
-        return execOutput.toString();
-    } catch (err) {
-        throw(err.message);
-    }
+    const execOutput = ProcessUtils.execAndCheckOutput(npmCmd, [ "config", "get", "registry" ]);
+    return execOutput.toString();
 }
 
 /**
  * NPM login to be able to install from secure registry
  * @param {string} registry The npm registry to install from.
  */
 export function npmLogin(registry: string) {
-    try {
-        ProcessUtils.execAndCheckOutput(npmCmd,
-            [
-                "login",
-                "--registry", registry,
-                "--always-auth",
-                "--auth-type=legacy"
-            ], {
-                stdio: [0,1,2]
-            }
-        );
-    } catch (err) {
-        throw(err.message);
-    }
+    ProcessUtils.execAndCheckOutput(npmCmd,
+        [
+            "login",
+            "--registry", registry,
+            "--always-auth",
+            "--auth-type=legacy"
+        ], {
+            stdio: [0,1,2]
+        }
+    );
 }
 
 /**

packages/utilities/__tests__/ProcessUtils.test.ts

@@ -10,7 +10,7 @@
 */
 
 import * as childProcess from "child_process";
-import { GuiResult, ImperativeConfig, ProcessUtils } from "../../utilities";
+import { DaemonRequest, GuiResult, ImperativeConfig, ProcessUtils } from "../../utilities";
 
 jest.mock("child_process");
 jest.mock("opener");
@@ -292,4 +292,75 @@ describe("ProcessUtils tests", () => {
             expect(childProcess.spawn).toHaveBeenCalledWith("vi", ["filePath"], { stdio: "inherit" });
         });
     });
+
+    describe("execAndCheckOutput", () => {
+        afterEach(() => {
+            jest.clearAllMocks();
+        });
+
+        it("returns stdout if command succeeds", () => {
+            const message = "Hello world!";
+            const options: any = { cwd: __dirname };
+            const stdoutBuffer = Buffer.from(message + "\n");
+            jest.spyOn(childProcess, "spawnSync").mockReturnValueOnce({
+                status: 0,
+                stdout: stdoutBuffer
+            } as any);
+            const execOutput = ProcessUtils.execAndCheckOutput("echo", [message], options);
+            expect(childProcess.spawnSync).toHaveBeenCalledWith("echo", [message], options);
+            expect(execOutput).toBe(stdoutBuffer);
+        });
+
+        it("throws error if command fails and returns error object", () => {
+            const filename = "invalid.txt";
+            const errMsg = `cat: ${filename}: No such file or directory`;
+            jest.spyOn(childProcess, "spawnSync").mockReturnValueOnce({
+                error: new Error(errMsg)
+            } as any);
+            let caughtError: any;
+            try {
+                ProcessUtils.execAndCheckOutput("cat", [filename]);
+            } catch (error) {
+                caughtError = error;
+            }
+            expect(childProcess.spawnSync).toHaveBeenCalledWith("cat", [filename], undefined);
+            expect(caughtError.message).toBe(errMsg);
+        });
+
+        it("throws error if command fails with non-zero status", () => {
+            const filename = "invalid.txt";
+            const stderrBuffer = Buffer.from(`cat: ${filename}: No such file or directory\n`);
+            jest.spyOn(childProcess, "spawnSync").mockReturnValueOnce({
+                status: 1,
+                stderr: stderrBuffer
+            } as any);
+            let caughtError: any;
+            try {
+                ProcessUtils.execAndCheckOutput("cat", [filename]);
+            } catch (error) {
+                caughtError = error;
+            }
+            expect(childProcess.spawnSync).toHaveBeenCalledWith("cat", [filename], undefined);
+            expect(caughtError.message).toBe(`Command failed: cat ${filename}\n${stderrBuffer.toString()}`);
+        });
+
+        it("logs stderr output to daemon stream", () => {
+            const stderrBuffer = Buffer.from("Task failed successfully\n");
+            const streamWriteMock = jest.fn();
+            jest.spyOn(childProcess, "spawnSync").mockReturnValueOnce({
+                status: 0,
+                stderr: stderrBuffer
+            } as any);
+            jest.spyOn(ImperativeConfig, "instance", "get").mockReturnValueOnce({
+                daemonContext: {
+                    stream: { write: streamWriteMock }
+                }
+            } as any);
+            ProcessUtils.execAndCheckOutput("fail");
+            expect(childProcess.spawnSync).toHaveBeenCalledWith("fail", undefined, undefined);
+            expect(streamWriteMock).toHaveBeenCalledWith(JSON.stringify({
+                stderr: stderrBuffer.toString()
+            }) + DaemonRequest.EOW_DELIMITER);
+        });
+    });
 });

packages/utilities/src/ProcessUtils.ts

@@ -13,6 +13,7 @@ import { spawnSync, SpawnSyncOptions } from "child_process";
 import { Logger } from "../../logger";
 import { ImperativeConfig } from "./ImperativeConfig";
 import { ISystemInfo } from "./doc/ISystemInfo";
+import { DaemonRequest } from "./DaemonRequest";
 
 /**
  * This enum represents the possible results from isGuiAvailable.
@@ -155,8 +156,13 @@ export class ProcessUtils {
         // Implementation based on the child_process module
         // https://github.com/nodejs/node/blob/main/lib/child_process.js
         const result = spawnSync(command, args, options);
-        if (options?.stdio == null && result.stderr != null) {
-            process.stderr.write(result.stderr);
+        if (options?.stdio == null && result.stderr?.length > 0) {
+            const daemonStream = ImperativeConfig.instance.daemonContext?.stream;
+            if (daemonStream != null) {
+                daemonStream.write(DaemonRequest.create({ stderr: result.stderr.toString() }));
+            } else {
+                process.stderr.write(result.stderr);
+            }
         }
         if (result.error != null) {
             throw result.error;