Skip to content

Commit

Permalink
Merge pull request #2318 from zowe/fix/install-scoped-plugin
Browse files Browse the repository at this point in the history 
Fix handling of scoped registry on plugin install
  • Loading branch information
@zFernand0
zFernand0 authored Oct 28, 2024
2 parents 2a4f467 + 386792d commit 9ad04c4
Show file tree
Hide file tree
Showing 17 changed files with 442 additions and 475 deletions.
4 changes: 4 additions & 0 deletions packages/imperative/CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,10 @@

All notable changes to the Imperative package will be documented in this file.

## Recent Changes

- BugFix: Fixed an issue where the `plugins install` command could fail when installing a scoped package because scoped registry was used to fetch all dependencies. [#2317](https://github.com/zowe/zowe-cli/issues/2317)

## `8.2.0`

- Enhancement: Use the new SDK method `ConfigUtils.hasTokenExpired` to check whether a given JSON web token has expired. [#2298](https://github.com/zowe/zowe-cli/pull/2298)
Expand Down
10 changes: 5 additions & 5 deletions packages/imperative/__tests__/src/packages/imperative/plugins/suites/InstallingPlugins.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ describe("Installing Plugins", () => {
name: "override-plugin",
usage: "override-plugin"
},
location: {
sample_registry: {
location: "imperative-sample-plugin",
name: "imperative-sample-plugin",
usage: "sample-plugin"
Expand Down Expand Up @@ -667,7 +667,7 @@ describe("Installing Plugins", () => {
const savedPluginJson = readFileSync(pluginJsonLocation);
const expectedContent: IPluginJson = fileContent as IPluginJson;

expectedContent[plugins.normal.name].location = envNpmRegistry;
expectedContent[plugins.normal.name].location = plugins.normal.location;

expect(savedPluginJson).toEqual(expectedContent);
});
Expand Down Expand Up @@ -735,7 +735,7 @@ describe("Installing Plugins", () => {
const actualJson = readFileSync(pluginJsonLocation);

// Add missing registry to expected
expectedJson[plugins.normal.name].location = envNpmRegistry;
expectedJson[plugins.normal.name].location = plugins.normal.location;

// Add missing normal2 plugin not present in before each
expectedJson[plugins.normal3.name] = {
Expand All @@ -751,7 +751,7 @@ describe("Installing Plugins", () => {
it("should error when a package and --file is specified", function () {
expect(
T.stripNewLines(
executeCommandString(this, `${pluginGroup} install ${plugins.location.location} --file ${testFile}`).stderr
executeCommandString(this, `${pluginGroup} install ${plugins.sample_registry.location} --file ${testFile}`).stderr
)
).toContain("Option --file can not be specified if positional package... is as well. They are mutually exclusive.");
});
Expand All @@ -760,7 +760,7 @@ describe("Installing Plugins", () => {
expect(
T.stripNewLines(
executeCommandString(this,
`${pluginGroup} install ${plugins.location.location} --file ${testFile} --registry ${TEST_REGISTRY}`).stderr
`${pluginGroup} install ${plugins.sample_registry.location} --file ${testFile} --registry ${TEST_REGISTRY}`).stderr
)
).toContain("The following options conflict (mutually exclusive)");
});
Expand Down
133 changes: 67 additions & 66 deletions ...ages/imperative/src/imperative/__tests__/plugins/cmd/install/install.handler.unit.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,31 +10,22 @@
*/

/* eslint-disable jest/expect-expect */
import Mock = jest.Mock;

let expectedVal: unknown;
let returnedVal: unknown;

jest.mock("cross-spawn");
jest.mock("jsonfile");
jest.mock("../../../../src/plugins/utilities/npm-interface/install");
jest.mock("../../../../src/plugins/utilities/runValidatePlugin");
jest.mock("../../../../src/plugins/utilities/PMFConstants");
jest.mock("../../../../../cmd/src/response/CommandResponse");
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/NpmFunctions");
jest.doMock("path", () => {
const originalPath = jest.requireActual("path");
return {
...originalPath,
resolve: (...path: string[]) => {
if (path[0] == expectedVal) {
resolve: (...paths: string[]) => {
if (paths[0] === expectedVal) {
return returnedVal ? returnedVal : expectedVal;
} else {
return originalPath.resolve(...path);
return originalPath.resolve(...paths);
}
}
};
Expand All @@ -43,29 +34,29 @@ jest.doMock("path", () => {
import { HandlerResponse, IHandlerParameters } from "../../../../../cmd";
import { Console } from "../../../../../console";
import { ImperativeError } from "../../../../../error";
import { install } from "../../../../src/plugins/utilities/npm-interface";
import { runValidatePlugin } from "../../../../src/plugins/utilities/runValidatePlugin";
import * as npmInterface from "../../../../src/plugins/utilities/npm-interface";
import * as validatePlugin from "../../../../src/plugins/utilities/runValidatePlugin";
import InstallHandler from "../../../../src/plugins/cmd/install/install.handler";
import { IPluginJson } from "../../../../src/plugins/doc/IPluginJson";
import { Logger } from "../../../../../logger";
import { readFileSync, writeFileSync } from "jsonfile";
import * as jsonfile from "jsonfile";
import { PMFConstants } from "../../../../src/plugins/utilities/PMFConstants";
import { TextUtils } from "../../../../../utilities";
import { getRegistry, getScopeRegistry, npmLogin } from "../../../../src/plugins/utilities/NpmFunctions";
import { NpmRegistryUtils } from "../../../../src/plugins/utilities/NpmFunctions";
import * as spawn from "cross-spawn";
import { IO } from "../../../../../io";

describe("Plugin Management Facility install handler", () => {

// Objects created so types are correct.
const origGetRegistry = jest.requireActual("../../../../src/plugins/utilities/NpmFunctions").NpmRegistryUtils.getRegistry;
const mocks = {
npmLogin: npmLogin as Mock<typeof npmLogin>,
getRegistry: getRegistry as unknown as Mock<typeof getRegistry>,
getScopeRegistry: getScopeRegistry as unknown as Mock<typeof getScopeRegistry>,
readFileSync: readFileSync as Mock<typeof readFileSync>,
writeFileSync: writeFileSync as Mock<typeof writeFileSync>,
install: install as unknown as Mock<typeof install>,
runValidatePlugin: runValidatePlugin as unknown as Mock<typeof runValidatePlugin>,
npmLogin: jest.spyOn(NpmRegistryUtils, "npmLogin"),
getRegistry: jest.spyOn(NpmRegistryUtils, "getRegistry"),
getScopeRegistry: jest.spyOn(NpmRegistryUtils as any, "getScopeRegistry"),
readFileSync: jest.spyOn(jsonfile, "readFileSync"),
writeFileSync: jest.spyOn(jsonfile, "writeFileSync"),
install: jest.spyOn(npmInterface, "install"),
runValidatePlugin: jest.spyOn(validatePlugin, "runValidatePlugin"),
};

// two plugin set of values
Expand All @@ -84,11 +75,11 @@ describe("Plugin Management Facility install handler", () => {
jest.clearAllMocks();

// This needs to be mocked before running process function of uninstall handler
(Logger.getImperativeLogger as unknown as Mock<typeof Logger.getImperativeLogger>).mockReturnValue(new Logger(new Console()) as any);
mocks.getRegistry.mockReturnValue(packageRegistry as any);
mocks.readFileSync.mockReturnValue({} as any);
npmLogin(packageRegistry);
mocks.runValidatePlugin.mockReturnValue(finalValidationMsg as any);
jest.spyOn(Logger, "getImperativeLogger").mockReturnValue(new Logger(new Console()));
mocks.getRegistry.mockReturnValue(packageRegistry);
mocks.readFileSync.mockReturnValue({});
mocks.runValidatePlugin.mockReturnValue(finalValidationMsg);
mocks.install.mockImplementation();
expectedVal = undefined;
returnedVal = undefined;
});
Expand Down Expand Up @@ -141,15 +132,16 @@ describe("Plugin Management Facility install handler", () => {
const wasInstallCallValid = (
packageLocation: string,
registry: string,
installFromFile = false
installFromFile = false,
extraNpmArgs = {}
) => {
if (installFromFile) {
expect(mocks.install).toHaveBeenCalledWith(
packageLocation, registry, true
packageLocation, { location: registry, npmArgs: { registry, ...extraNpmArgs } }, true
);
} else {
expect(mocks.install).toHaveBeenCalledWith(
packageLocation, registry
packageLocation, { location: registry, npmArgs: { registry, ...extraNpmArgs } }
);
}
};
Expand All @@ -162,8 +154,9 @@ describe("Plugin Management Facility install handler", () => {
*/
const wasInstallSuccessful = (params: IHandlerParameters) => {
// get the text of the last message that was displayed
const outputMsg = (params.response.console.log as Mock).mock.calls[(params.response.console.log as Mock).mock.calls.length - 1][0];
expect(outputMsg).toContain(finalValidationMsg);
expect(params.response.console.log).toHaveBeenLastCalledWith(
expect.stringContaining(finalValidationMsg)
);
};

/**
Expand Down Expand Up @@ -193,10 +186,10 @@ describe("Plugin Management Facility install handler", () => {
};

// Override the return value for this test only
mocks.readFileSync.mockReturnValueOnce(fileJson as any);
mocks.readFileSync.mockReturnValueOnce(fileJson);
mocks.install
.mockReturnValueOnce("a" as any)
.mockReturnValueOnce("plugin2" as any);
.mockResolvedValueOnce("a")
.mockResolvedValueOnce("plugin2");

const handler = new InstallHandler();

Expand All @@ -213,9 +206,6 @@ describe("Plugin Management Facility install handler", () => {
// Validate the call to get the registry value
wasGetRegistryCalled();

// Validate the call to login
wasNpmLoginCallValid(packageRegistry);

expect(mocks.install).toHaveBeenCalledTimes(2);
wasInstallCallValid(`${fileJson.a.package}@${fileJson.a.version}`, packageRegistry, true);
wasInstallCallValid(fileJson.plugin2.package, packageRegistry2, true);
Expand All @@ -224,7 +214,6 @@ describe("Plugin Management Facility install handler", () => {
expect(mocks.runValidatePlugin).toHaveBeenCalledWith("a");
expect(mocks.runValidatePlugin).toHaveBeenCalledWith("plugin2");


// Validate that the read was correct
wasReadFileSyncCallValid(resolveVal);

Expand Down Expand Up @@ -265,9 +254,6 @@ describe("Plugin Management Facility install handler", () => {
// Validate the call to get the registry value
wasGetRegistryCalled();

// Validate the call to login
wasNpmLoginCallValid(packageRegistry);

// Check that install worked as expected
wasInstallCallValid(params.arguments.plugin[0], packageRegistry);

Expand All @@ -290,6 +276,27 @@ describe("Plugin Management Facility install handler", () => {
wasInstallSuccessful(params);
});

it("should install single package with registry and login specified", async () => {
mocks.npmLogin.mockReturnValueOnce();
const handler = new InstallHandler();

const params = getIHandlerParametersObject();
params.arguments.plugin = ["sample1"];
params.arguments.registry = "http://localhost:4873/";
params.arguments.login = true;

await handler.process(params as IHandlerParameters);

// Validate the call to login
wasNpmLoginCallValid(packageRegistry);

// Check that install worked as expected
wasInstallCallValid(params.arguments.plugin[0], params.arguments.registry);

// Check that success is output
wasInstallSuccessful(params);
});

it("should install multiple packages", async () => {
const handler = new InstallHandler();

Expand All @@ -301,9 +308,6 @@ describe("Plugin Management Facility install handler", () => {
// Validate the install
wasGetRegistryCalled();

// Validate the call to login
wasNpmLoginCallValid(packageRegistry);

// Validate that install was called with each of these values
expect(mocks.install).toHaveBeenCalledTimes(params.arguments.plugin.length);
wasInstallCallValid(params.arguments.plugin[0], packageRegistry);
Expand All @@ -324,8 +328,7 @@ describe("Plugin Management Facility install handler", () => {
// Validate the call to get the registry value
wasGetRegistryCalled();

// Validate the call to login
wasNpmLoginCallValid(packageRegistry);
// Validate that the read was correct
wasReadFileSyncCallValid(PMFConstants.instance.PLUGIN_JSON);

expect(params.response.console.log).toHaveBeenCalledWith("No packages were found in " +
Expand Down Expand Up @@ -369,7 +372,7 @@ describe("Plugin Management Facility install handler", () => {
});

jest.spyOn(spawn, "sync").mockReturnValueOnce({ status: 1 } as any);
mocks.getRegistry.mockImplementationOnce(jest.requireActual("../../../../src/plugins/utilities/NpmFunctions").getRegistry);
mocks.getRegistry.mockImplementationOnce(origGetRegistry);

try {
await handler.process(params);
Expand All @@ -381,28 +384,27 @@ describe("Plugin Management Facility install handler", () => {
expect(expectedError.additionalDetails).toContain("Command failed");
expect(expectedError.additionalDetails).toContain("npm");
});
it("should handle installed plugins via public scope", async () => {
it("should handle installed plugins via package name", async () => {
const handler = new InstallHandler();

const params = getIHandlerParametersObject();
params.arguments.plugin = ["@public/sample1"];

mocks.getScopeRegistry.mockReturnValueOnce("publicRegistryUrl" as any);
mocks.getScopeRegistry.mockReturnValueOnce("publicRegistryUrl");

try {
await handler.process(params);
} catch (e) {
expect(e).toBeUndefined();
}

expect(mocks.install).toHaveBeenCalledWith("@public/sample1","publicRegistryUrl");
wasInstallCallValid("@public/sample1", packageRegistry, false, { "@public:registry": "publicRegistryUrl" });
});
it("should handle installed plugins via project/directory", async () => {
const handler = new InstallHandler();

const params = getIHandlerParametersObject();
params.arguments.plugin = ["path/to/dir"];
jest.spyOn(IO, 'isDir').mockReturnValue(true);

try{
await handler.process(params);
Expand All @@ -411,7 +413,7 @@ describe("Plugin Management Facility install handler", () => {
expect(e).toBeUndefined();
}

expect(mocks.install).toHaveBeenCalledWith("path/to/dir","path/to/dir");
wasInstallCallValid("path/to/dir", "path/to/dir", false, { registry: packageRegistry });
});
it("should handle installed plugins via tarball file", async () => {
const handler = new InstallHandler();
Expand All @@ -426,16 +428,15 @@ describe("Plugin Management Facility install handler", () => {
expect(e).toBeUndefined();
}

expect(mocks.install).toHaveBeenCalledWith("path/to/dir/file.tgz","path/to/dir/file.tgz");
wasInstallCallValid("path/to/dir/file.tgz", "path/to/dir/file.tgz", false, { registry: packageRegistry });
});
it("should handle multiple installed plugins via tarball, director, public registry, and private registry", async () => {
it("should handle multiple installed plugins via tarball, directory, and registry", async () => {
const handler = new InstallHandler();

const params = getIHandlerParametersObject();
params.arguments.plugin = ["@public/sample1","@private/sample1","path/to/dir","path/to/dir/file.tgz"];
mocks.getScopeRegistry.mockReturnValueOnce("publicRegistryUrl" as any);
mocks.getScopeRegistry.mockReturnValueOnce("privateRegistryUrl" as any);
jest.spyOn(IO, 'isDir').mockReturnValue(true);
params.arguments.plugin = ["@public/sample1", "@private/sample1", "path/to/dir", "path/to/dir/file.tgz"];
mocks.getScopeRegistry.mockReturnValueOnce("publicRegistryUrl");
mocks.getScopeRegistry.mockReturnValueOnce("privateRegistryUrl");

try{
await handler.process(params);
Expand All @@ -444,10 +445,10 @@ describe("Plugin Management Facility install handler", () => {
expect(e).toBeUndefined();
}

expect(mocks.install).toHaveBeenCalledWith("@public/sample1","publicRegistryUrl");
expect(mocks.install).toHaveBeenCalledWith("@private/sample1","privateRegistryUrl");
expect(mocks.install).toHaveBeenCalledWith("path/to/dir","path/to/dir");
expect(mocks.install).toHaveBeenCalledWith("path/to/dir/file.tgz","path/to/dir/file.tgz");
expect(mocks.install).toHaveBeenCalledTimes(params.arguments.plugin.length);
wasInstallCallValid("@public/sample1", packageRegistry, false, { "@public:registry": "publicRegistryUrl" });
wasInstallCallValid("@private/sample1", packageRegistry, false, { "@private:registry": "privateRegistryUrl" });
wasInstallCallValid("path/to/dir", "path/to/dir", false, { registry: packageRegistry });
wasInstallCallValid("path/to/dir/file.tgz", "path/to/dir/file.tgz", false, { registry: packageRegistry });
});
});

5 changes: 2 additions & 3 deletions packages/imperative/src/imperative/__tests__/plugins/cmd/list/list.handler.unit.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ describe("Plugin Management Facility list handler", () => {
mocks.readFileSync.mockReturnValue({} as any);
});

test("list packages", async () => {
it("list packages", async () => {

// plugin definitions mocking unsorted file contents
const fileJson: IPluginJson = {
Expand Down Expand Up @@ -113,7 +113,7 @@ describe("Plugin Management Facility list handler", () => {

});

test("list packages short", async () => {
it("list packages short", async () => {

// plugin definitions mocking unsorted file contents
const fileJson: IPluginJson = {
Expand Down Expand Up @@ -143,4 +143,3 @@ describe("Plugin Management Facility list handler", () => {

});
});

2 changes: 1 addition & 1 deletion .../imperative/src/imperative/__tests__/plugins/cmd/uninstall/uninstall.handler.unit.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ describe("Plugin Management Facility uninstall handler", () => {
expect(params.response.console.log).toHaveBeenCalledWith("Removal of the npm package(s) was successful.\n");
};

test("uninstall specified package", async () => {
it("uninstall specified package", async () => {
// plugin definitions mocking file contents
const fileJson: IPluginJson = {
a: {
Expand Down
Loading

0 comments on commit 9ad04c4

Please sign in to comment.