From 99e563032970ab4e1a0fdcbb8c18aa154171d9a4 Mon Sep 17 00:00:00 2001 From: ATorrise Date: Wed, 9 Oct 2024 14:29:50 -0400 Subject: [PATCH 01/20] all needed changes to meet AC besides tests Signed-off-by: ATorrise --- packages/imperative/src/config/src/Config.ts | 3 ++- .../src/config/src/api/ConfigLayers.ts | 22 ++++++++++++++----- .../src/config/src/doc/IConfigLayer.ts | 1 + 3 files changed, 19 insertions(+), 7 deletions(-) diff --git a/packages/imperative/src/config/src/Config.ts b/packages/imperative/src/config/src/Config.ts index 999f568960..84a6744fe3 100644 --- a/packages/imperative/src/config/src/Config.ts +++ b/packages/imperative/src/config/src/Config.ts @@ -179,7 +179,8 @@ export class Config { exists: false, properties: Config.empty(), global: layer === Layers.GlobalUser || layer === Layers.GlobalConfig, - user: layer === Layers.ProjectUser || layer === Layers.GlobalUser + user: layer === Layers.ProjectUser || layer === Layers.GlobalUser, + ignoreErrors: process.argv.includes("--help") || process.argv.includes("--version") }); } diff --git a/packages/imperative/src/config/src/api/ConfigLayers.ts b/packages/imperative/src/config/src/api/ConfigLayers.ts index e790c321ce..1ad6624c48 100644 --- a/packages/imperative/src/config/src/api/ConfigLayers.ts +++ b/packages/imperative/src/config/src/api/ConfigLayers.ts @@ -18,6 +18,8 @@ import { ConfigConstants } from "../ConfigConstants"; import { IConfigLayer } from "../doc/IConfigLayer"; import { ConfigApi } from "./ConfigApi"; import { IConfig } from "../doc/IConfig"; +import { TextUtils } from "../../../utilities"; +import { CommandResponse } from "../../../cmd/src/response/CommandResponse"; /** * API Class for manipulating config layers. @@ -31,7 +33,7 @@ export class ConfigLayers extends ConfigApi { * @param opts The user and global flags that indicate which of the four * config files (aka layers) is to be read. */ - public read(opts?: { user: boolean; global: boolean }) { + public read(opts?: { user: boolean; global: boolean; ignoreErrors: boolean}) { // Attempt to populate the layer const layer = opts ? this.mConfig.findLayer(opts.user, opts.global) : this.mConfig.layerActive(); if (fs.existsSync(layer.path)) { @@ -49,11 +51,19 @@ export class ConfigLayers extends ConfigApi { layer.properties = JSONC.parse(fileContents.toString()) as any; layer.exists = true; } catch (e) { - throw new ImperativeError({ - msg: `Error parsing JSON in the file '${layer.path}'.\n` + - `Please check this configuration file for errors.\nError details: ${e.message}\nLine ${e.line}, Column ${e.column}`, - suppressDump: true - }); + const msg = `Error parsing JSON in the file '${layer.path}'.\n` + + `Please check this configuration file for errors.\nError details: ${e.message}\nLine ${e.line}, Column ${e.column}` + if (!opts.ignoreErrors){ + throw new ImperativeError({ + msg:msg, + suppressDump: true + }); + } else { + const cmdResp: CommandResponse = new CommandResponse({ + responseFormat: "default" + }); + cmdResp.console.log(TextUtils.chalk.red(msg)); + } } this.mConfig.api.secure.loadCached(opts); } else if (layer.exists) { diff --git a/packages/imperative/src/config/src/doc/IConfigLayer.ts b/packages/imperative/src/config/src/doc/IConfigLayer.ts index 98b17591e0..f8ce5832aa 100644 --- a/packages/imperative/src/config/src/doc/IConfigLayer.ts +++ b/packages/imperative/src/config/src/doc/IConfigLayer.ts @@ -17,4 +17,5 @@ export interface IConfigLayer { properties: IConfig; global: boolean; user: boolean; + ignoreErrors: boolean; } From 6a39c50c9fbd34394a4344e9b8fd5502be17577a Mon Sep 17 00:00:00 2001 From: ATorrise Date: Wed, 9 Oct 2024 15:26:29 -0400 Subject: [PATCH 02/20] changelog and codeql change Signed-off-by: ATorrise --- packages/imperative/CHANGELOG.md | 1 + packages/imperative/src/config/src/api/ConfigLayers.ts | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/imperative/CHANGELOG.md b/packages/imperative/CHANGELOG.md index 9f78d930cf..db219d4e9e 100644 --- a/packages/imperative/CHANGELOG.md +++ b/packages/imperative/CHANGELOG.md @@ -3,6 +3,7 @@ All notable changes to the Imperative package will be documented in this file. ## Recent Changes +- BugFix: Enabling commands with either the `--help` or `--version` flags to still display their information despite any config file issues [#2301](https://github.com/zowe/zowe-cli/pull/2301) - BugFix: Fixed issues flagged by Coverity [#2291](https://github.com/zowe/zowe-cli/pull/2291) diff --git a/packages/imperative/src/config/src/api/ConfigLayers.ts b/packages/imperative/src/config/src/api/ConfigLayers.ts index 1ad6624c48..4d6e65645c 100644 --- a/packages/imperative/src/config/src/api/ConfigLayers.ts +++ b/packages/imperative/src/config/src/api/ConfigLayers.ts @@ -52,7 +52,7 @@ export class ConfigLayers extends ConfigApi { layer.exists = true; } catch (e) { const msg = `Error parsing JSON in the file '${layer.path}'.\n` + - `Please check this configuration file for errors.\nError details: ${e.message}\nLine ${e.line}, Column ${e.column}` + `Please check this configuration file for errors.\nError details: ${e.message}\nLine ${e.line}, Column ${e.column}`; if (!opts.ignoreErrors){ throw new ImperativeError({ msg:msg, From 1e84a49ef7c82c2eabf0397676381855b003b4e1 Mon Sep 17 00:00:00 2001 From: ATorrise Date: Mon, 14 Oct 2024 11:11:10 -0400 Subject: [PATCH 03/20] still more tests to fix Signed-off-by: ATorrise --- .../src/config/__tests__/ProfileInfo.TeamConfig.unit.test.ts | 1 + .../__tests__/__snapshots__/Config.api.unit.test.ts.snap | 1 + packages/imperative/src/config/src/__mocks__/Config.ts | 3 ++- 3 files changed, 4 insertions(+), 1 deletion(-) diff --git a/packages/imperative/src/config/__tests__/ProfileInfo.TeamConfig.unit.test.ts b/packages/imperative/src/config/__tests__/ProfileInfo.TeamConfig.unit.test.ts index 420972d432..d40aa6fc6a 100644 --- a/packages/imperative/src/config/__tests__/ProfileInfo.TeamConfig.unit.test.ts +++ b/packages/imperative/src/config/__tests__/ProfileInfo.TeamConfig.unit.test.ts @@ -1843,6 +1843,7 @@ describe("TeamConfig ProfileInfo tests", () => { properties: { profiles: {}, defaults: {} }, global: true, user: false, + ignoreErrors: false }); expect(cfgSchemaBuildMock).toHaveBeenCalledWith([{ type: "some-type-with-source", diff --git a/packages/imperative/src/config/__tests__/__snapshots__/Config.api.unit.test.ts.snap b/packages/imperative/src/config/__tests__/__snapshots__/Config.api.unit.test.ts.snap index 5ad9779a20..4870c35e9e 100644 --- a/packages/imperative/src/config/__tests__/__snapshots__/Config.api.unit.test.ts.snap +++ b/packages/imperative/src/config/__tests__/__snapshots__/Config.api.unit.test.ts.snap @@ -4,6 +4,7 @@ exports[`Config API tests layers get should get the active layer 1`] = ` Object { "exists": true, "global": false, + "ignoreErrors": false, "path": "project.config.user.json", "properties": Object { "autoStore": false, diff --git a/packages/imperative/src/config/src/__mocks__/Config.ts b/packages/imperative/src/config/src/__mocks__/Config.ts index b302f9b08c..a441bf88c8 100644 --- a/packages/imperative/src/config/src/__mocks__/Config.ts +++ b/packages/imperative/src/config/src/__mocks__/Config.ts @@ -48,7 +48,8 @@ export class Config { defaults: {} }, global: true, - user: false + user: false, + ignoreErrors: false }]; return config; From 1d51d5ddff9ce813ef6c5796ca22f115e7d04db4 Mon Sep 17 00:00:00 2001 From: ATorrise Date: Tue, 22 Oct 2024 16:51:15 -0400 Subject: [PATCH 04/20] i think i put ttests in the right place Signed-off-by: ATorrise --- .../__tests__/CommandProcessor.unit.test.ts | 55 +++++++++++++++++++ packages/imperative/src/config/src/Config.ts | 1 + .../cmd/import/import.handler.unit.test.ts | 41 +++++++++++++- 3 files changed, 96 insertions(+), 1 deletion(-) diff --git a/packages/imperative/src/cmd/__tests__/CommandProcessor.unit.test.ts b/packages/imperative/src/cmd/__tests__/CommandProcessor.unit.test.ts index b48f3c2d23..0c014e5320 100644 --- a/packages/imperative/src/cmd/__tests__/CommandProcessor.unit.test.ts +++ b/packages/imperative/src/cmd/__tests__/CommandProcessor.unit.test.ts @@ -191,6 +191,61 @@ const FAKE_HELP_GENERATOR: IHelpGenerator = { const ENV_VAR_PREFIX: string = "UNIT_TEST"; +describe("Command Processor with --help and --version flags", () => { + let faultyConfigProcessor: CommandProcessor; + + beforeEach(() => { + faultyConfigProcessor = new CommandProcessor({ + envVariablePrefix: ENV_VAR_PREFIX, + definition: SAMPLE_COMMAND_DEFINITION, + helpGenerator: FAKE_HELP_GENERATOR, + rootCommandName: SAMPLE_ROOT_COMMAND, + commandLine: "", + promptPhrase: "dummyPrompt", + config: { + isValid: () => false, // Simulate faulty config + } as any, + }); + + jest.spyOn(console, "log").mockImplementation(() => {}); // Prevent console logs in tests + }); + + afterEach(() => { + jest.restoreAllMocks(); + }); + + it("should display help text even with faulty config", async () => { + const parms: any = { arguments: { _: [], $0: "", help: true }, silent: true }; + const helpResponse: ICommandResponse = await faultyConfigProcessor.invoke(parms); + + expect(helpResponse).toBeDefined(); + expect(helpResponse.stdout?.toString() || "").toContain("Build help invoked!"); + expect(helpResponse.success).toBe(true); + }); + + it("should display version even with faulty config", async () => { + const parms: any = { arguments: { _: [], $0: "", version: true }, silent: true }; + const versionResponse: ICommandResponse = await faultyConfigProcessor.invoke(parms); + + expect(versionResponse).toBeDefined(); + expect(versionResponse.stdout?.toString() || "").toContain("Version:"); + expect(versionResponse.success).toBe(true); + }); + + it("should fail command execution without --help or --version if config is faulty", async () => { + const parms: any = { arguments: { _: ["some", "command"], $0: "" }, silent: true }; + let error; + try { + await faultyConfigProcessor.invoke(parms); + } catch (e) { + error = e; + } + + expect(error).toBeDefined(); + expect(error.message).toContain("Configuration invalid"); + }); +}); + describe("Command Processor", () => { beforeEach(() => { // Mock read stdin diff --git a/packages/imperative/src/config/src/Config.ts b/packages/imperative/src/config/src/Config.ts index 84a6744fe3..8ce9f133b9 100644 --- a/packages/imperative/src/config/src/Config.ts +++ b/packages/imperative/src/config/src/Config.ts @@ -181,6 +181,7 @@ export class Config { global: layer === Layers.GlobalUser || layer === Layers.GlobalConfig, user: layer === Layers.ProjectUser || layer === Layers.GlobalUser, ignoreErrors: process.argv.includes("--help") || process.argv.includes("--version") + || process.argv[process.argv.length-1] === require.resolve('@zowe/cli') }); } diff --git a/packages/imperative/src/imperative/__tests__/config/cmd/import/import.handler.unit.test.ts b/packages/imperative/src/imperative/__tests__/config/cmd/import/import.handler.unit.test.ts index 2d4af8c553..46fed8e157 100644 --- a/packages/imperative/src/imperative/__tests__/config/cmd/import/import.handler.unit.test.ts +++ b/packages/imperative/src/imperative/__tests__/config/cmd/import/import.handler.unit.test.ts @@ -260,8 +260,9 @@ describe("Configuration Import command handler", () => { it("should throw error when schema file is not valid JSON", async () => { jest.spyOn(RestClient, "getExpectString").mockResolvedValueOnce("invalid JSON"); let error: any; + try { - await downloadSchema(new URL(schemaUrl), schemaDestPath); + await downloadSchema(new URL(schemaUrl), schemaDestPath); // Normal execution } catch (err) { error = err; } @@ -271,6 +272,44 @@ describe("Configuration Import command handler", () => { expect(error.message).toContain("Unexpected token"); }); + it("should display help text even when schema file is not valid JSON if --help flag is used", async () => { + const params: IHandlerParameters = getIHandlerParametersObject(); + params.arguments.help = true; // Simulate --help flag + + let error: any; + + try { + if (params.arguments.help) { + console.log("Help text: Command usage here..."); // Simulate help output + } else { + await downloadSchema(new URL(schemaUrl), schemaDestPath); + } + } catch (err) { + error = err; + } + + expect(error).toBeUndefined(); // Ensure no error when help flag is used + }); + + it("should display version even when schema file is not valid JSON if --version flag is used", async () => { + const params: IHandlerParameters = getIHandlerParametersObject(); + params.arguments.version = true; // Simulate --version flag + + let error: any; + + try { + if (params.arguments.version) { + console.log("Version: 1.0.0"); // Simulate version output + } else { + await downloadSchema(new URL(schemaUrl), schemaDestPath); + } + } catch (err) { + error = err; + } + + expect(error).toBeUndefined(); // Ensure no error when version flag is used + }); + it("should throw error when REST client fails to fetch schema file", async () => { jest.spyOn(RestClient, "getExpectString").mockRejectedValueOnce(new Error("invalid URL")); let error; From d978e0acb735dfd82415280ec5438f7fd70ffc7d Mon Sep 17 00:00:00 2001 From: Amber Torrise Date: Thu, 24 Oct 2024 12:16:41 -0400 Subject: [PATCH 05/20] linting Signed-off-by: Amber Torrise --- packages/imperative/src/config/src/ConfigUtils.ts | 3 ++- packages/imperative/src/config/src/ProfileInfo.ts | 3 ++- packages/imperative/src/config/src/api/ConfigLayers.ts | 4 ++-- .../__tests__/config/cmd/import/import.handler.unit.test.ts | 4 ---- zowe-cli | 1 + 5 files changed, 7 insertions(+), 8 deletions(-) create mode 160000 zowe-cli diff --git a/packages/imperative/src/config/src/ConfigUtils.ts b/packages/imperative/src/config/src/ConfigUtils.ts index dc7d0a5853..87705b1bb5 100644 --- a/packages/imperative/src/config/src/ConfigUtils.ts +++ b/packages/imperative/src/config/src/ConfigUtils.ts @@ -300,7 +300,8 @@ export class ConfigUtils { * Checks if the given token has expired. Supports JSON web tokens only. * * @param {string} token - The JSON web token to check - * @returns {boolean} Whether the token has expired. Returns `false` if the token cannot be decoded or an expire time is not specified in the payload. + * @returns {boolean} Whether the token has expired. Returns `false` if the token cannot be decoded or an expire time is + * not specified in the payload. */ public static hasTokenExpired(token: string): boolean { // JWT format: [header].[payload].[signature] diff --git a/packages/imperative/src/config/src/ProfileInfo.ts b/packages/imperative/src/config/src/ProfileInfo.ts index b5d812260c..48820e3eaa 100644 --- a/packages/imperative/src/config/src/ProfileInfo.ts +++ b/packages/imperative/src/config/src/ProfileInfo.ts @@ -180,7 +180,8 @@ export class ProfileInfo { } /** - * Checks if a JSON web token is used for authenticating the given profile name. If so, it will decode the token to determine whether it has expired. + * Checks if a JSON web token is used for authenticating the given profile name. + * If so, it will decode the token to determine whether it has expired. * * @param {string | IProfileLoaded} profile - The name of the profile or the profile object to check the JSON web token for * @returns {boolean} Whether the token has expired for the given profile. Returns `false` if a token value is not set or the token type is LTPA2. diff --git a/packages/imperative/src/config/src/api/ConfigLayers.ts b/packages/imperative/src/config/src/api/ConfigLayers.ts index 4d6e65645c..d5009f4f50 100644 --- a/packages/imperative/src/config/src/api/ConfigLayers.ts +++ b/packages/imperative/src/config/src/api/ConfigLayers.ts @@ -58,12 +58,12 @@ export class ConfigLayers extends ConfigApi { msg:msg, suppressDump: true }); - } else { + } else { const cmdResp: CommandResponse = new CommandResponse({ responseFormat: "default" }); cmdResp.console.log(TextUtils.chalk.red(msg)); - } + } } this.mConfig.api.secure.loadCached(opts); } else if (layer.exists) { diff --git a/packages/imperative/src/imperative/__tests__/config/cmd/import/import.handler.unit.test.ts b/packages/imperative/src/imperative/__tests__/config/cmd/import/import.handler.unit.test.ts index 46fed8e157..d453f07be3 100644 --- a/packages/imperative/src/imperative/__tests__/config/cmd/import/import.handler.unit.test.ts +++ b/packages/imperative/src/imperative/__tests__/config/cmd/import/import.handler.unit.test.ts @@ -280,8 +280,6 @@ describe("Configuration Import command handler", () => { try { if (params.arguments.help) { - console.log("Help text: Command usage here..."); // Simulate help output - } else { await downloadSchema(new URL(schemaUrl), schemaDestPath); } } catch (err) { @@ -299,8 +297,6 @@ describe("Configuration Import command handler", () => { try { if (params.arguments.version) { - console.log("Version: 1.0.0"); // Simulate version output - } else { await downloadSchema(new URL(schemaUrl), schemaDestPath); } } catch (err) { diff --git a/zowe-cli b/zowe-cli new file mode 160000 index 0000000000..027d9cc151 --- /dev/null +++ b/zowe-cli @@ -0,0 +1 @@ +Subproject commit 027d9cc151dd07e5a5a1d236ed47694f68391ee3 From a307dd51126bd1be747e56aeaf43f80faa7ece7c Mon Sep 17 00:00:00 2001 From: Amber Torrise Date: Thu, 24 Oct 2024 14:05:56 -0400 Subject: [PATCH 06/20] removing tests because tests in command processor seem to be enough? Signed-off-by: Amber Torrise --- .../cmd/import/import.handler.unit.test.ts | 34 ------------------- 1 file changed, 34 deletions(-) diff --git a/packages/imperative/src/imperative/__tests__/config/cmd/import/import.handler.unit.test.ts b/packages/imperative/src/imperative/__tests__/config/cmd/import/import.handler.unit.test.ts index d453f07be3..ea793d83e1 100644 --- a/packages/imperative/src/imperative/__tests__/config/cmd/import/import.handler.unit.test.ts +++ b/packages/imperative/src/imperative/__tests__/config/cmd/import/import.handler.unit.test.ts @@ -272,40 +272,6 @@ describe("Configuration Import command handler", () => { expect(error.message).toContain("Unexpected token"); }); - it("should display help text even when schema file is not valid JSON if --help flag is used", async () => { - const params: IHandlerParameters = getIHandlerParametersObject(); - params.arguments.help = true; // Simulate --help flag - - let error: any; - - try { - if (params.arguments.help) { - await downloadSchema(new URL(schemaUrl), schemaDestPath); - } - } catch (err) { - error = err; - } - - expect(error).toBeUndefined(); // Ensure no error when help flag is used - }); - - it("should display version even when schema file is not valid JSON if --version flag is used", async () => { - const params: IHandlerParameters = getIHandlerParametersObject(); - params.arguments.version = true; // Simulate --version flag - - let error: any; - - try { - if (params.arguments.version) { - await downloadSchema(new URL(schemaUrl), schemaDestPath); - } - } catch (err) { - error = err; - } - - expect(error).toBeUndefined(); // Ensure no error when version flag is used - }); - it("should throw error when REST client fails to fetch schema file", async () => { jest.spyOn(RestClient, "getExpectString").mockRejectedValueOnce(new Error("invalid URL")); let error; From 63872625fc2c7357fdf934710561707ce84f9ec4 Mon Sep 17 00:00:00 2001 From: Amber Torrise <112635587+ATorrise@users.noreply.github.com> Date: Wed, 30 Oct 2024 08:33:43 -0400 Subject: [PATCH 07/20] Update packages/imperative/CHANGELOG.md Co-authored-by: Fernando Rijo Cedeno <37381190+zFernand0@users.noreply.github.com> Signed-off-by: Amber Torrise <112635587+ATorrise@users.noreply.github.com> --- packages/imperative/CHANGELOG.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/imperative/CHANGELOG.md b/packages/imperative/CHANGELOG.md index 43e7c55b94..635a745450 100644 --- a/packages/imperative/CHANGELOG.md +++ b/packages/imperative/CHANGELOG.md @@ -3,7 +3,8 @@ All notable changes to the Imperative package will be documented in this file. ## Recent Changes -- BugFix: Enabling commands with either the `--help` or `--version` flags to still display their information despite any config file issues [#2301](https://github.com/zowe/zowe-cli/pull/2301) + +- BugFix: Enabled commands with either the `--help` or `--version` flags to still display their information despite any config file issues [#2301](https://github.com/zowe/zowe-cli/pull/2301) ## `8.2.0` From 0b3ebc906a04ea9f5d3395c9b588cb67d4430da7 Mon Sep 17 00:00:00 2001 From: Amber Torrise Date: Wed, 30 Oct 2024 10:51:10 -0400 Subject: [PATCH 08/20] attempting to fix unit test Signed-off-by: Amber Torrise --- .../__tests__/CommandProcessor.unit.test.ts | 101 ++++++++++-------- 1 file changed, 54 insertions(+), 47 deletions(-) diff --git a/packages/imperative/src/cmd/__tests__/CommandProcessor.unit.test.ts b/packages/imperative/src/cmd/__tests__/CommandProcessor.unit.test.ts index 0c014e5320..dc0724f408 100644 --- a/packages/imperative/src/cmd/__tests__/CommandProcessor.unit.test.ts +++ b/packages/imperative/src/cmd/__tests__/CommandProcessor.unit.test.ts @@ -191,62 +191,70 @@ const FAKE_HELP_GENERATOR: IHelpGenerator = { const ENV_VAR_PREFIX: string = "UNIT_TEST"; -describe("Command Processor with --help and --version flags", () => { - let faultyConfigProcessor: CommandProcessor; +describe("Command Processor", () => { + describe("Command Processor with --help and --version flags", () => { + let faultyConfigProcessor: CommandProcessor; - beforeEach(() => { - faultyConfigProcessor = new CommandProcessor({ - envVariablePrefix: ENV_VAR_PREFIX, - definition: SAMPLE_COMMAND_DEFINITION, - helpGenerator: FAKE_HELP_GENERATOR, - rootCommandName: SAMPLE_ROOT_COMMAND, - commandLine: "", - promptPhrase: "dummyPrompt", - config: { - isValid: () => false, // Simulate faulty config - } as any, - }); + const helpParms: any = { + arguments: { + _: ["sample", "cmd", "--help"], + $0: "", + valid: true + }, + silent: true + }; - jest.spyOn(console, "log").mockImplementation(() => {}); // Prevent console logs in tests - }); + const versionParms: any = { + arguments: { + _: ["sample", "cmd", "--version"], + $0: "", + valid: true + }, + silent: true + }; - afterEach(() => { - jest.restoreAllMocks(); - }); + beforeEach(() => { + faultyConfigProcessor = new CommandProcessor({ + envVariablePrefix: ENV_VAR_PREFIX, + definition: SAMPLE_COMMAND_DEFINITION, + helpGenerator: FAKE_HELP_GENERATOR, + rootCommandName: SAMPLE_ROOT_COMMAND, + commandLine: "", + promptPhrase: "dummyPrompt", + config: { + isValid: () => false, // Simulate faulty config + } as any, + }); - it("should display help text even with faulty config", async () => { - const parms: any = { arguments: { _: [], $0: "", help: true }, silent: true }; - const helpResponse: ICommandResponse = await faultyConfigProcessor.invoke(parms); + jest.spyOn(console, "log").mockImplementation(() => {}); // Prevent console logs in tests + }); - expect(helpResponse).toBeDefined(); - expect(helpResponse.stdout?.toString() || "").toContain("Build help invoked!"); - expect(helpResponse.success).toBe(true); - }); + afterEach(() => { + jest.restoreAllMocks(); + }); - it("should display version even with faulty config", async () => { - const parms: any = { arguments: { _: [], $0: "", version: true }, silent: true }; - const versionResponse: ICommandResponse = await faultyConfigProcessor.invoke(parms); + it("should display help text even with faulty config", async () => { + const helpResponse: ICommandResponse = await faultyConfigProcessor.invoke(helpParms); - expect(versionResponse).toBeDefined(); - expect(versionResponse.stdout?.toString() || "").toContain("Version:"); - expect(versionResponse.success).toBe(true); - }); + expect(helpResponse).toBeDefined(); + expect(helpResponse.success).toBe(true); + }); - it("should fail command execution without --help or --version if config is faulty", async () => { - const parms: any = { arguments: { _: ["some", "command"], $0: "" }, silent: true }; - let error; - try { - await faultyConfigProcessor.invoke(parms); - } catch (e) { - error = e; - } + it("should display version even with faulty config", async () => { + const versionResponse: ICommandResponse = await faultyConfigProcessor.invoke(versionParms); - expect(error).toBeDefined(); - expect(error.message).toContain("Configuration invalid"); - }); -}); + expect(versionResponse).toBeDefined(); + expect(versionResponse.success).toBe(true); + }); -describe("Command Processor", () => { + it("should fail command execution without --help or --version if config is faulty", async () => { + const parms: any = { arguments: { _: ["some", "command"], $0: "" }, silent: true }; + const response: ICommandResponse = await faultyConfigProcessor.invoke(parms); + + expect(response).toBeDefined(); + expect(response.success).toBe(false); + }); + }); beforeEach(() => { // Mock read stdin jest.spyOn(SharedOptions, "readStdinIfRequested").mockResolvedValueOnce(false); @@ -258,7 +266,6 @@ describe("Command Processor", () => { process.stderr.write = ORIGINAL_STDERR_WRITE; jest.restoreAllMocks(); }); - it("should allow us to create an instance", () => { let caughtError; From 82fd8f2413561c9d83b3fd4ac1750984c0490d9f Mon Sep 17 00:00:00 2001 From: Amber Date: Mon, 4 Nov 2024 14:15:21 -0500 Subject: [PATCH 09/20] removing unneeded snaps Signed-off-by: Amber Signed-off-by: Fernando Rijo Cedeno <37381190+zFernand0@users.noreply.github.com> --- .../ftds/FileToDataSet.handler.unit.test.ts | 5 +++-- .../FileToDataSet.handler.unit.test.ts.snap | 16 ---------------- zowe-cli | 1 - 3 files changed, 3 insertions(+), 19 deletions(-) delete mode 160000 zowe-cli diff --git a/packages/cli/__tests__/zosfiles/__unit__/upload/ftds/FileToDataSet.handler.unit.test.ts b/packages/cli/__tests__/zosfiles/__unit__/upload/ftds/FileToDataSet.handler.unit.test.ts index c71ada1d52..09c5f4e9f7 100644 --- a/packages/cli/__tests__/zosfiles/__unit__/upload/ftds/FileToDataSet.handler.unit.test.ts +++ b/packages/cli/__tests__/zosfiles/__unit__/upload/ftds/FileToDataSet.handler.unit.test.ts @@ -329,8 +329,9 @@ describe("Upload file-to-data-set handler", () => { statusMessage: "Uploading to data set" } }); - expect(jsonObj).toMatchSnapshot(); - expect(apiMessage).toMatchSnapshot(); + + expect(apiMessage).toBe(""); + expect(logMessage).toMatch(/success:.*false/); expect(logMessage).toMatch(/from:.*test-file/); expect(logMessage).toMatch(/file_to_upload:.*1/); diff --git a/packages/cli/__tests__/zosfiles/__unit__/upload/ftds/__snapshots__/FileToDataSet.handler.unit.test.ts.snap b/packages/cli/__tests__/zosfiles/__unit__/upload/ftds/__snapshots__/FileToDataSet.handler.unit.test.ts.snap index 1cd2b1490a..3e87a782b2 100644 --- a/packages/cli/__tests__/zosfiles/__unit__/upload/ftds/__snapshots__/FileToDataSet.handler.unit.test.ts.snap +++ b/packages/cli/__tests__/zosfiles/__unit__/upload/ftds/__snapshots__/FileToDataSet.handler.unit.test.ts.snap @@ -1,21 +1,5 @@ // Jest Snapshot v1, https://goo.gl/fbAQLP -exports[`Upload file-to-data-set handler process method should display error when upload file to data set 1`] = ` -Object { - "apiResponse": Array [ - Object { - "from": "test-file", - "success": false, - "to": "testing", - }, - ], - "commandResponse": "uploaded", - "success": false, -} -`; - -exports[`Upload file-to-data-set handler process method should display error when upload file to data set 2`] = `""`; - exports[`Upload file-to-data-set handler process method should upload a file to a data set if requested 1`] = ` Object { "apiResponse": Array [ diff --git a/zowe-cli b/zowe-cli deleted file mode 160000 index 027d9cc151..0000000000 --- a/zowe-cli +++ /dev/null @@ -1 +0,0 @@ -Subproject commit 027d9cc151dd07e5a5a1d236ed47694f68391ee3 From 1edf977de2df2d85a69352b5d78f1822079b3c59 Mon Sep 17 00:00:00 2001 From: Amber Date: Tue, 5 Nov 2024 09:29:49 -0500 Subject: [PATCH 10/20] adding node 22 support and also removing private config constructor that was missed in imperative cleanup Signed-off-by: Amber Signed-off-by: Fernando Rijo Cedeno <37381190+zFernand0@users.noreply.github.com> --- packages/imperative/src/config/src/Config.ts | 12 ++---------- .../imperative/src/config/cmd/report-env/EnvItems.ts | 4 ++-- 2 files changed, 4 insertions(+), 12 deletions(-) diff --git a/packages/imperative/src/config/src/Config.ts b/packages/imperative/src/config/src/Config.ts index 8ce9f133b9..9dd16b8354 100644 --- a/packages/imperative/src/config/src/Config.ts +++ b/packages/imperative/src/config/src/Config.ts @@ -117,14 +117,6 @@ export class Config { secure: ConfigSecure }; - // _______________________________________________________________________ - /** - * Constructor for Config class. Don't use this directly. Await `Config.load` instead. - * @param opts Options to control how Config class behaves - * @private - */ - private constructor(public opts?: IConfigOpts) { } - // _______________________________________________________________________ /** * Return a Config interface with required fields initialized as empty. @@ -147,10 +139,10 @@ export class Config { opts = opts || {}; // Create the basic empty configuration - const myNewConfig = new Config(opts); + const myNewConfig = new Config(); myNewConfig.mApp = app; myNewConfig.mActive = { user: false, global: false }; - myNewConfig.mVault = opts.vault; + myNewConfig.mVault = opts?.vault; myNewConfig.mSecure = {}; // Populate configuration file layers diff --git a/packages/imperative/src/imperative/src/config/cmd/report-env/EnvItems.ts b/packages/imperative/src/imperative/src/config/cmd/report-env/EnvItems.ts index bf72572e61..dc3a942129 100644 --- a/packages/imperative/src/imperative/src/config/cmd/report-env/EnvItems.ts +++ b/packages/imperative/src/imperative/src/config/cmd/report-env/EnvItems.ts @@ -68,8 +68,8 @@ function formatLogLevelMsg(logTypeName: string) { export const probTests: IProbTest[] = [ { itemId: ItemId.NODEJS_VER, - probExpr: "semver.satisfies('{val}', '<18.x || 19.x || >=21.x')", - probMsg: "Only Node.js versions 18 and 20 are supported." + probExpr: "semver.satisfies('{val}', '<18.x || 19.x || 21.x || >=23.x')", + probMsg: "Only Node.js versions 18, 20, and 22 are supported." }, { itemId: ItemId.NPM_VER, From f8f520de61013757c3b0ea5466bd9003fe84cd33 Mon Sep 17 00:00:00 2001 From: Amber Date: Tue, 5 Nov 2024 09:53:00 -0500 Subject: [PATCH 11/20] trying to fix my 2 added unit tests for terminal output given flags Signed-off-by: Amber Signed-off-by: Fernando Rijo Cedeno <37381190+zFernand0@users.noreply.github.com> --- .../src/cmd/__tests__/CommandProcessor.unit.test.ts | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/packages/imperative/src/cmd/__tests__/CommandProcessor.unit.test.ts b/packages/imperative/src/cmd/__tests__/CommandProcessor.unit.test.ts index dc0724f408..41cbc4ed08 100644 --- a/packages/imperative/src/cmd/__tests__/CommandProcessor.unit.test.ts +++ b/packages/imperative/src/cmd/__tests__/CommandProcessor.unit.test.ts @@ -199,18 +199,16 @@ describe("Command Processor", () => { arguments: { _: ["sample", "cmd", "--help"], $0: "", - valid: true - }, - silent: true + valid: false + } }; const versionParms: any = { arguments: { _: ["sample", "cmd", "--version"], $0: "", - valid: true - }, - silent: true + valid: false + } }; beforeEach(() => { @@ -222,7 +220,7 @@ describe("Command Processor", () => { commandLine: "", promptPhrase: "dummyPrompt", config: { - isValid: () => false, // Simulate faulty config + validate: () => ({valid: false}), // Simulate faulty config } as any, }); From ab791ec724b02dc601d2fdc6aee364060ed473dd Mon Sep 17 00:00:00 2001 From: Fernando Rijo Cedeno <37381190+zFernand0@users.noreply.github.com> Date: Tue, 5 Nov 2024 10:39:43 -0500 Subject: [PATCH 12/20] chore: cherry pick d7ea329742879ca1bca05c5fef503b1a57ec09b9 Signed-off-by: Amber Signed-off-by: Fernando Rijo Cedeno <37381190+zFernand0@users.noreply.github.com> --- packages/imperative/src/config/src/doc/IConfigLayer.ts | 2 +- packages/imperative/src/config/src/doc/IConfigOpts.ts | 5 +++++ packages/imperative/src/constants/src/Constants.ts | 2 ++ packages/imperative/src/imperative/src/Imperative.ts | 10 +++++++++- 4 files changed, 17 insertions(+), 2 deletions(-) diff --git a/packages/imperative/src/config/src/doc/IConfigLayer.ts b/packages/imperative/src/config/src/doc/IConfigLayer.ts index f8ce5832aa..0b3ea91262 100644 --- a/packages/imperative/src/config/src/doc/IConfigLayer.ts +++ b/packages/imperative/src/config/src/doc/IConfigLayer.ts @@ -17,5 +17,5 @@ export interface IConfigLayer { properties: IConfig; global: boolean; user: boolean; - ignoreErrors: boolean; + ignoreErrors?: boolean; } diff --git a/packages/imperative/src/config/src/doc/IConfigOpts.ts b/packages/imperative/src/config/src/doc/IConfigOpts.ts index af7cfeaa0a..86e2a0cd36 100644 --- a/packages/imperative/src/config/src/doc/IConfigOpts.ts +++ b/packages/imperative/src/config/src/doc/IConfigOpts.ts @@ -12,6 +12,11 @@ import { IConfigVault } from "./IConfigVault"; export interface IConfigOpts { + /** + * Continue command execution despite config errors because version or help flag present + */ + ignoreErrors?: boolean; + /** * Directory where global config files are located. Defaults to `~/.appName`. */ diff --git a/packages/imperative/src/constants/src/Constants.ts b/packages/imperative/src/constants/src/Constants.ts index cdd1d527f2..254f086e6b 100644 --- a/packages/imperative/src/constants/src/Constants.ts +++ b/packages/imperative/src/constants/src/Constants.ts @@ -59,6 +59,8 @@ export class Constants { public static readonly JSON_OPTION_ALIAS = "rfj"; public static readonly HELP_OPTION = "help"; public static readonly HELP_OPTION_ALIAS = "h"; + public static readonly VERSION_OPTION: "version"; + public static readonly VERSION_OPTION_ALIAS: "v"; public static readonly HELP_EXAMPLES = "help-examples"; public static readonly HELP_WEB_OPTION = "help-web"; public static readonly HELP_WEB_OPTION_ALIAS = "hw"; diff --git a/packages/imperative/src/imperative/src/Imperative.ts b/packages/imperative/src/imperative/src/Imperative.ts index 76eab278a5..01c2038bd1 100644 --- a/packages/imperative/src/imperative/src/Imperative.ts +++ b/packages/imperative/src/imperative/src/Imperative.ts @@ -140,6 +140,13 @@ export class Imperative { ConfigurationValidator.validate(config); ImperativeConfig.instance.loadedConfig = config; + // Detect CLI arguments to determine if errors should be ignored + const ignoreErrors = process.argv.includes(Constants.HELP_OPTION) || + process.argv.includes(Constants.HELP_OPTION_ALIAS) || + process.argv.includes(Constants.VERSION_OPTION) || + process.argv.includes(Constants.VERSION_OPTION_ALIAS) || + process.argv[process.argv.length - 1] === require.resolve('@zowe/cli'); + /** * Get the command name from the package bin. * If no command name exists, we will instead use the file name invoked @@ -178,7 +185,7 @@ export class Imperative { try { ImperativeConfig.instance.config = await Config.load(configAppName, - { homeDir: ImperativeConfig.instance.cliHome } + { homeDir: ImperativeConfig.instance.cliHome, ignoreErrors } ); } catch (err) { delayedConfigLoadError = err; @@ -226,6 +233,7 @@ export class Imperative { ImperativeConfig.instance.config = await Config.load(configAppName, { homeDir: ImperativeConfig.instance.cliHome, + ignoreErrors, noLoad: true } ); From 1b793ac6a5b73d6c66effa8d3a2c5517ddd764b7 Mon Sep 17 00:00:00 2001 From: Amber Date: Tue, 5 Nov 2024 10:42:28 -0500 Subject: [PATCH 13/20] getting back to normal with changes Signed-off-by: Amber --- .../src/cmd/__tests__/CommandProcessor.unit.test.ts | 4 ++-- packages/imperative/src/config/src/api/ConfigLayers.ts | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/packages/imperative/src/cmd/__tests__/CommandProcessor.unit.test.ts b/packages/imperative/src/cmd/__tests__/CommandProcessor.unit.test.ts index 41cbc4ed08..d12b701266 100644 --- a/packages/imperative/src/cmd/__tests__/CommandProcessor.unit.test.ts +++ b/packages/imperative/src/cmd/__tests__/CommandProcessor.unit.test.ts @@ -199,7 +199,7 @@ describe("Command Processor", () => { arguments: { _: ["sample", "cmd", "--help"], $0: "", - valid: false + valid: true } }; @@ -207,7 +207,7 @@ describe("Command Processor", () => { arguments: { _: ["sample", "cmd", "--version"], $0: "", - valid: false + valid: true } }; diff --git a/packages/imperative/src/config/src/api/ConfigLayers.ts b/packages/imperative/src/config/src/api/ConfigLayers.ts index d5009f4f50..d54db1c8a0 100644 --- a/packages/imperative/src/config/src/api/ConfigLayers.ts +++ b/packages/imperative/src/config/src/api/ConfigLayers.ts @@ -33,7 +33,7 @@ export class ConfigLayers extends ConfigApi { * @param opts The user and global flags that indicate which of the four * config files (aka layers) is to be read. */ - public read(opts?: { user: boolean; global: boolean; ignoreErrors: boolean}) { + public read(opts?: { user: boolean; global: boolean; ignoreErrors?: boolean}) { // Attempt to populate the layer const layer = opts ? this.mConfig.findLayer(opts.user, opts.global) : this.mConfig.layerActive(); if (fs.existsSync(layer.path)) { @@ -53,7 +53,7 @@ export class ConfigLayers extends ConfigApi { } catch (e) { const msg = `Error parsing JSON in the file '${layer.path}'.\n` + `Please check this configuration file for errors.\nError details: ${e.message}\nLine ${e.line}, Column ${e.column}`; - if (!opts.ignoreErrors){ + if (!opts?.ignoreErrors){ throw new ImperativeError({ msg:msg, suppressDump: true From 128f61bff0150677c755493e367c8fac2f439113 Mon Sep 17 00:00:00 2001 From: Fernando Rijo Cedeno <37381190+zFernand0@users.noreply.github.com> Date: Tue, 5 Nov 2024 10:48:06 -0500 Subject: [PATCH 14/20] chore: fix lint warnings Signed-off-by: Fernando Rijo Cedeno <37381190+zFernand0@users.noreply.github.com> --- __tests__/__packages__/cli-test-utils/src/TestUtils.ts | 1 + .../upload/ftds/FileToDataSet.handler.unit.test.ts | 2 +- .../cli.zos-jobs.download.output.system.test.ts | 10 ++++------ packages/imperative/src/imperative/src/Imperative.ts | 2 +- .../__tests__/client/AbstractRestClient.unit.test.ts | 2 +- .../rest/__tests__/client/ProxySettings.unit.test.ts | 2 +- .../src/rest/src/client/AbstractRestClient.ts | 2 +- packages/zosfiles/src/methods/upload/Upload.ts | 2 +- 8 files changed, 11 insertions(+), 12 deletions(-) diff --git a/__tests__/__packages__/cli-test-utils/src/TestUtils.ts b/__tests__/__packages__/cli-test-utils/src/TestUtils.ts index 90cb8be95a..961f7fb2c2 100644 --- a/__tests__/__packages__/cli-test-utils/src/TestUtils.ts +++ b/__tests__/__packages__/cli-test-utils/src/TestUtils.ts @@ -61,6 +61,7 @@ export function runCliScript(scriptPath: string, testEnvironment: ITestEnvironme env: childEnv, encoding: "buffer" }); + // eslint-disable-next-line @typescript-eslint/no-magic-numbers if (process.platform === "darwin" && (response.error as ExecFileException)?.errno === -8) { throw new Error(`The script file ${path.basename(scriptPath)} failed to execute. Check that it starts with a shebang line.`); } diff --git a/packages/cli/__tests__/zosfiles/__unit__/upload/ftds/FileToDataSet.handler.unit.test.ts b/packages/cli/__tests__/zosfiles/__unit__/upload/ftds/FileToDataSet.handler.unit.test.ts index 09c5f4e9f7..fc93d7e2ae 100644 --- a/packages/cli/__tests__/zosfiles/__unit__/upload/ftds/FileToDataSet.handler.unit.test.ts +++ b/packages/cli/__tests__/zosfiles/__unit__/upload/ftds/FileToDataSet.handler.unit.test.ts @@ -330,7 +330,7 @@ describe("Upload file-to-data-set handler", () => { } }); - expect(apiMessage).toBe(""); + expect(apiMessage).toBe(""); expect(logMessage).toMatch(/success:.*false/); expect(logMessage).toMatch(/from:.*test-file/); diff --git a/packages/cli/__tests__/zosjobs/__system__/download/cli.zos-jobs.download.output.system.test.ts b/packages/cli/__tests__/zosjobs/__system__/download/cli.zos-jobs.download.output.system.test.ts index c646971f07..d1be44eee1 100644 --- a/packages/cli/__tests__/zosjobs/__system__/download/cli.zos-jobs.download.output.system.test.ts +++ b/packages/cli/__tests__/zosjobs/__system__/download/cli.zos-jobs.download.output.system.test.ts @@ -14,8 +14,6 @@ import { ITestEnvironment } from "../../../../../../__tests__/__src__/environmen import { runCliScript } from "@zowe/cli-test-utils"; import { ITestPropertiesSchema } from "../../../../../../__tests__/__src__/properties/ITestPropertiesSchema"; import * as fs from "fs"; -import { Session } from "@zowe/imperative"; -import { GetJobs } from "@zowe/zos-jobs-for-zowe-sdk"; // Test Environment populated in the beforeAll(); let TEST_ENVIRONMENT: ITestEnvironment; @@ -49,14 +47,14 @@ describe("zos-jobs download output command", () => { it("should download a job and wait for it to reach output status", async () => { const response = runCliScript(__dirname + "/__scripts__/download-output/download_job_wait_for_output.sh", TEST_ENVIRONMENT, [IEFBR14_JCL]); - expect(response.stderr.toString()).toBe(""); - expect(response.status).toBe(0); + expect(response.stderr.toString()).toBe(""); + expect(response.status).toBe(0); }); it("should download a job and wait for it to reach active status", async () => { const response = runCliScript(__dirname + "/__scripts__/download-output/download_job_wait_for_active.sh", TEST_ENVIRONMENT, [IEFBR14_JCL]); - expect(response.stderr.toString()).toBe(""); - expect(response.status).toBe(0); + expect(response.stderr.toString()).toBe(""); + expect(response.status).toBe(0); }); }); describe("output", () => { diff --git a/packages/imperative/src/imperative/src/Imperative.ts b/packages/imperative/src/imperative/src/Imperative.ts index 01c2038bd1..8689d0e074 100644 --- a/packages/imperative/src/imperative/src/Imperative.ts +++ b/packages/imperative/src/imperative/src/Imperative.ts @@ -140,7 +140,7 @@ export class Imperative { ConfigurationValidator.validate(config); ImperativeConfig.instance.loadedConfig = config; - // Detect CLI arguments to determine if errors should be ignored + // Detect CLI arguments to determine if errors should be ignored const ignoreErrors = process.argv.includes(Constants.HELP_OPTION) || process.argv.includes(Constants.HELP_OPTION_ALIAS) || process.argv.includes(Constants.VERSION_OPTION) || diff --git a/packages/imperative/src/rest/__tests__/client/AbstractRestClient.unit.test.ts b/packages/imperative/src/rest/__tests__/client/AbstractRestClient.unit.test.ts index e5d7a047ea..fcd7669552 100644 --- a/packages/imperative/src/rest/__tests__/client/AbstractRestClient.unit.test.ts +++ b/packages/imperative/src/rest/__tests__/client/AbstractRestClient.unit.test.ts @@ -1517,7 +1517,7 @@ describe("AbstractRestClient tests", () => { const result = privateRestClient.buildOptions(resource, request, reqHeaders); expect(Object.keys(result)).toContain('agent'); expect(headerSpy).toHaveBeenCalledWith([{'Proxy-Authorization': restSession.ISession.proxy.proxy_authorization}]); - }) + }); }); }); }); diff --git a/packages/imperative/src/rest/__tests__/client/ProxySettings.unit.test.ts b/packages/imperative/src/rest/__tests__/client/ProxySettings.unit.test.ts index e6b47f4d86..952b9e298f 100644 --- a/packages/imperative/src/rest/__tests__/client/ProxySettings.unit.test.ts +++ b/packages/imperative/src/rest/__tests__/client/ProxySettings.unit.test.ts @@ -41,7 +41,7 @@ describe("Proxy tests", () => { const expected = { proxyUrl: passedUrl, protocol: HTTPS_PROTOCOL - } + }; beforeEach(() => { jest.clearAllMocks(); diff --git a/packages/imperative/src/rest/src/client/AbstractRestClient.ts b/packages/imperative/src/rest/src/client/AbstractRestClient.ts index 3d337bf125..16b5530612 100644 --- a/packages/imperative/src/rest/src/client/AbstractRestClient.ts +++ b/packages/imperative/src/rest/src/client/AbstractRestClient.ts @@ -478,7 +478,7 @@ export abstract class AbstractRestClient { this.mLogger.info(`Using the following proxy setting for the request: ${proxyUrl.href}`); if (this.session.ISession?.proxy?.proxy_authorization) { reqHeaders.push({ 'Proxy-Authorization': this.session.ISession.proxy.proxy_authorization}); - } + } options.agent = ProxySettings.getProxyAgent(this.session.ISession); } } diff --git a/packages/zosfiles/src/methods/upload/Upload.ts b/packages/zosfiles/src/methods/upload/Upload.ts index 13a53abef4..528150b961 100644 --- a/packages/zosfiles/src/methods/upload/Upload.ts +++ b/packages/zosfiles/src/methods/upload/Upload.ts @@ -877,7 +877,7 @@ export class Upload { } else if(options.filesMap?.fileNames.indexOf(path.basename(localPath)) > -1) { tempOptions.binary = options.filesMap.binary; - // Reset encoding to undefined if binary is true to avoid file tagging issues + // Reset encoding to undefined if binary is true to avoid file tagging issues if(tempOptions.binary) tempOptions.encoding = undefined; } From b4597811122d6d2e2e4a75c13b7d407735361386 Mon Sep 17 00:00:00 2001 From: Fernando Rijo Cedeno <37381190+zFernand0@users.noreply.github.com> Date: Tue, 5 Nov 2024 13:24:49 -0500 Subject: [PATCH 15/20] update tests Signed-off-by: Amber Signed-off-by: Fernando Rijo Cedeno <37381190+zFernand0@users.noreply.github.com> --- .../__tests__/CommandProcessor.unit.test.ts | 14 ----- .../config/__tests__/Config.api.unit.test.ts | 60 +++++++++++++++++++ .../Config.api.unit.test.ts.snap | 1 - packages/imperative/src/config/src/Config.ts | 3 +- 4 files changed, 61 insertions(+), 17 deletions(-) diff --git a/packages/imperative/src/cmd/__tests__/CommandProcessor.unit.test.ts b/packages/imperative/src/cmd/__tests__/CommandProcessor.unit.test.ts index d12b701266..bb40d76160 100644 --- a/packages/imperative/src/cmd/__tests__/CommandProcessor.unit.test.ts +++ b/packages/imperative/src/cmd/__tests__/CommandProcessor.unit.test.ts @@ -231,20 +231,6 @@ describe("Command Processor", () => { jest.restoreAllMocks(); }); - it("should display help text even with faulty config", async () => { - const helpResponse: ICommandResponse = await faultyConfigProcessor.invoke(helpParms); - - expect(helpResponse).toBeDefined(); - expect(helpResponse.success).toBe(true); - }); - - it("should display version even with faulty config", async () => { - const versionResponse: ICommandResponse = await faultyConfigProcessor.invoke(versionParms); - - expect(versionResponse).toBeDefined(); - expect(versionResponse.success).toBe(true); - }); - it("should fail command execution without --help or --version if config is faulty", async () => { const parms: any = { arguments: { _: ["some", "command"], $0: "" }, silent: true }; const response: ICommandResponse = await faultyConfigProcessor.invoke(parms); diff --git a/packages/imperative/src/config/__tests__/Config.api.unit.test.ts b/packages/imperative/src/config/__tests__/Config.api.unit.test.ts index af72547cbb..de73747af8 100644 --- a/packages/imperative/src/config/__tests__/Config.api.unit.test.ts +++ b/packages/imperative/src/config/__tests__/Config.api.unit.test.ts @@ -18,6 +18,7 @@ import { ConfigConstants } from "../src/ConfigConstants"; import { IConfig } from "../src/doc/IConfig"; import { IConfigLayer } from "../src/doc/IConfigLayer"; import { IConfigProfile } from "../src/doc/IConfigProfile"; +import { ImperativeError, TextUtils } from "../.."; const MY_APP = "my_app"; @@ -264,6 +265,65 @@ describe("Config API tests", () => { expect(readFileSpy).toHaveBeenCalledTimes(1); expect(secureLoadSpy).toHaveBeenCalledTimes(1); }); + it("should error when ignoreErrors is implicit false", async () => { + const config = await Config.load(MY_APP); + const existsSpy = jest.spyOn(fs, "existsSync").mockReturnValueOnce(true); + const readFileSpy = jest.spyOn(fs, "readFileSync"); + const secureLoadSpy = jest.spyOn(config.api.secure, "loadCached"); + const jsoncParseSpy = jest.spyOn(JSONC, "parse").mockImplementationOnce(() => { throw "failed to parse"; }); + let caughtError: ImperativeError = {} as any; + try { + config.api.layers.read(); + } catch(err) { + caughtError = err; + } + expect(existsSpy).toHaveBeenCalledTimes(5); // Once for each config layer and one more time for read + expect(readFileSpy).toHaveBeenCalledTimes(1); + expect(secureLoadSpy).toHaveBeenCalledTimes(0); + expect(jsoncParseSpy).toHaveBeenCalledTimes(1); + expect(caughtError.message).toContain("Please check this configuration file for errors"); + }); + + it("should error when ignoreErrors is explicit false", async () => { + const config = await Config.load(MY_APP); + const existsSpy = jest.spyOn(fs, "existsSync").mockReturnValueOnce(true); + const readFileSpy = jest.spyOn(fs, "readFileSync"); + const secureLoadSpy = jest.spyOn(config.api.secure, "loadCached"); + const jsoncParseSpy = jest.spyOn(JSONC, "parse").mockImplementationOnce(() => { throw "failed to parse"; }); + let caughtError: ImperativeError = {} as any; + try { + config.api.layers.read({user: false, global: false, ignoreErrors: false}); + } catch(err) { + caughtError = err; + } + expect(existsSpy).toHaveBeenCalledTimes(5); // Once for each config layer and one more time for read + expect(readFileSpy).toHaveBeenCalledTimes(1); + expect(secureLoadSpy).toHaveBeenCalledTimes(0); + expect(jsoncParseSpy).toHaveBeenCalledTimes(1); + expect(caughtError.message).toContain("Please check this configuration file for errors"); + }); + + it("should error when ignoreErrors is true", async () => { + const config = await Config.load(MY_APP); + const existsSpy = jest.spyOn(fs, "existsSync").mockReturnValueOnce(true); + const readFileSpy = jest.spyOn(fs, "readFileSync"); + const secureLoadSpy = jest.spyOn(config.api.secure, "loadCached"); + const jsoncParseSpy = jest.spyOn(JSONC, "parse").mockImplementationOnce(() => { throw "failed to parse"; }); + const redSpy = jest.fn().mockImplementation((str: string) => str); + jest.spyOn(TextUtils, "chalk", "get").mockReturnValueOnce({red: redSpy}); + let caughtError; + try { + config.api.layers.read({user: false, global: false, ignoreErrors: true}); + } catch(err) { + caughtError = err; + } + expect(existsSpy).toHaveBeenCalledTimes(5); // Once for each config layer and one more time for read + expect(readFileSpy).toHaveBeenCalledTimes(1); + expect(secureLoadSpy).toHaveBeenCalledTimes(1); + expect(jsoncParseSpy).toHaveBeenCalledTimes(1); + expect(caughtError).toBeUndefined(); + expect(redSpy.mock.calls[0][0]).toContain("Please check this configuration file for errors"); + }); }); describe("write", () => { it("should save the active config layer", async () => { diff --git a/packages/imperative/src/config/__tests__/__snapshots__/Config.api.unit.test.ts.snap b/packages/imperative/src/config/__tests__/__snapshots__/Config.api.unit.test.ts.snap index 4870c35e9e..5ad9779a20 100644 --- a/packages/imperative/src/config/__tests__/__snapshots__/Config.api.unit.test.ts.snap +++ b/packages/imperative/src/config/__tests__/__snapshots__/Config.api.unit.test.ts.snap @@ -4,7 +4,6 @@ exports[`Config API tests layers get should get the active layer 1`] = ` Object { "exists": true, "global": false, - "ignoreErrors": false, "path": "project.config.user.json", "properties": Object { "autoStore": false, diff --git a/packages/imperative/src/config/src/Config.ts b/packages/imperative/src/config/src/Config.ts index 9dd16b8354..193d0b09df 100644 --- a/packages/imperative/src/config/src/Config.ts +++ b/packages/imperative/src/config/src/Config.ts @@ -172,8 +172,7 @@ export class Config { properties: Config.empty(), global: layer === Layers.GlobalUser || layer === Layers.GlobalConfig, user: layer === Layers.ProjectUser || layer === Layers.GlobalUser, - ignoreErrors: process.argv.includes("--help") || process.argv.includes("--version") - || process.argv[process.argv.length-1] === require.resolve('@zowe/cli') + ignoreErrors: opts?.ignoreErrors }); } From 0d670f735d842652a5e4b501146b73f97e6c2ff2 Mon Sep 17 00:00:00 2001 From: Amber Date: Tue, 5 Nov 2024 15:13:33 -0500 Subject: [PATCH 16/20] removing unused vars Signed-off-by: Amber --- .../__tests__/CommandProcessor.unit.test.ts | 30 ------------------- 1 file changed, 30 deletions(-) diff --git a/packages/imperative/src/cmd/__tests__/CommandProcessor.unit.test.ts b/packages/imperative/src/cmd/__tests__/CommandProcessor.unit.test.ts index d12b701266..18737c37a2 100644 --- a/packages/imperative/src/cmd/__tests__/CommandProcessor.unit.test.ts +++ b/packages/imperative/src/cmd/__tests__/CommandProcessor.unit.test.ts @@ -195,22 +195,6 @@ describe("Command Processor", () => { describe("Command Processor with --help and --version flags", () => { let faultyConfigProcessor: CommandProcessor; - const helpParms: any = { - arguments: { - _: ["sample", "cmd", "--help"], - $0: "", - valid: true - } - }; - - const versionParms: any = { - arguments: { - _: ["sample", "cmd", "--version"], - $0: "", - valid: true - } - }; - beforeEach(() => { faultyConfigProcessor = new CommandProcessor({ envVariablePrefix: ENV_VAR_PREFIX, @@ -231,20 +215,6 @@ describe("Command Processor", () => { jest.restoreAllMocks(); }); - it("should display help text even with faulty config", async () => { - const helpResponse: ICommandResponse = await faultyConfigProcessor.invoke(helpParms); - - expect(helpResponse).toBeDefined(); - expect(helpResponse.success).toBe(true); - }); - - it("should display version even with faulty config", async () => { - const versionResponse: ICommandResponse = await faultyConfigProcessor.invoke(versionParms); - - expect(versionResponse).toBeDefined(); - expect(versionResponse.success).toBe(true); - }); - it("should fail command execution without --help or --version if config is faulty", async () => { const parms: any = { arguments: { _: ["some", "command"], $0: "" }, silent: true }; const response: ICommandResponse = await faultyConfigProcessor.invoke(parms); From 2a370f0f8ede7dc58b3b4fc07aca56016bc53ceb Mon Sep 17 00:00:00 2001 From: Amber Date: Wed, 6 Nov 2024 13:49:00 -0500 Subject: [PATCH 17/20] addresing timothys batch of suggestions Signed-off-by: Amber --- .../config/__tests__/Config.api.unit.test.ts | 19 +++++++++++++------ packages/imperative/src/config/src/Config.ts | 8 ++++++++ .../src/config/src/api/ConfigLayers.ts | 9 +++------ .../imperative/src/constants/src/Constants.ts | 2 +- 4 files changed, 25 insertions(+), 13 deletions(-) diff --git a/packages/imperative/src/config/__tests__/Config.api.unit.test.ts b/packages/imperative/src/config/__tests__/Config.api.unit.test.ts index de73747af8..4b0cbcdc01 100644 --- a/packages/imperative/src/config/__tests__/Config.api.unit.test.ts +++ b/packages/imperative/src/config/__tests__/Config.api.unit.test.ts @@ -18,7 +18,7 @@ import { ConfigConstants } from "../src/ConfigConstants"; import { IConfig } from "../src/doc/IConfig"; import { IConfigLayer } from "../src/doc/IConfigLayer"; import { IConfigProfile } from "../src/doc/IConfigProfile"; -import { ImperativeError, TextUtils } from "../.."; +import { ImperativeError, Logger } from "../.."; const MY_APP = "my_app"; @@ -303,26 +303,33 @@ describe("Config API tests", () => { expect(caughtError.message).toContain("Please check this configuration file for errors"); }); - it("should error when ignoreErrors is true", async () => { + it("shouldnt error when ignoreErrors is true", async () => { const config = await Config.load(MY_APP); const existsSpy = jest.spyOn(fs, "existsSync").mockReturnValueOnce(true); const readFileSpy = jest.spyOn(fs, "readFileSync"); const secureLoadSpy = jest.spyOn(config.api.secure, "loadCached"); const jsoncParseSpy = jest.spyOn(JSONC, "parse").mockImplementationOnce(() => { throw "failed to parse"; }); - const redSpy = jest.fn().mockImplementation((str: string) => str); - jest.spyOn(TextUtils, "chalk", "get").mockReturnValueOnce({red: redSpy}); + + let logMsg: string = "Nothing logged"; + jest.spyOn(Logger, "getImperativeLogger").mockImplementation(() => { + return { + error: jest.fn((errMsg) => { + logMsg = errMsg; + }) + } as any; + }); let caughtError; try { config.api.layers.read({user: false, global: false, ignoreErrors: true}); } catch(err) { caughtError = err; } - expect(existsSpy).toHaveBeenCalledTimes(5); // Once for each config layer and one more time for read + expect(existsSpy).toHaveBeenCalledTimes(6); // Once for each config layer and one more time for read expect(readFileSpy).toHaveBeenCalledTimes(1); expect(secureLoadSpy).toHaveBeenCalledTimes(1); expect(jsoncParseSpy).toHaveBeenCalledTimes(1); expect(caughtError).toBeUndefined(); - expect(redSpy.mock.calls[0][0]).toContain("Please check this configuration file for errors"); + expect(logMsg).toContain("Nothing logged"); }); }); describe("write", () => { diff --git a/packages/imperative/src/config/src/Config.ts b/packages/imperative/src/config/src/Config.ts index 193d0b09df..8ea129890b 100644 --- a/packages/imperative/src/config/src/Config.ts +++ b/packages/imperative/src/config/src/Config.ts @@ -117,6 +117,14 @@ export class Config { secure: ConfigSecure }; + // _______________________________________________________________________ + /** + * Constructor for Config class. Don't use this directly. Await `Config.load` instead. + * @param opts Options to control how Config class behaves + * @private + */ + private constructor(public opts?: IConfigOpts) { } + // _______________________________________________________________________ /** * Return a Config interface with required fields initialized as empty. diff --git a/packages/imperative/src/config/src/api/ConfigLayers.ts b/packages/imperative/src/config/src/api/ConfigLayers.ts index d54db1c8a0..aadec34728 100644 --- a/packages/imperative/src/config/src/api/ConfigLayers.ts +++ b/packages/imperative/src/config/src/api/ConfigLayers.ts @@ -18,8 +18,7 @@ import { ConfigConstants } from "../ConfigConstants"; import { IConfigLayer } from "../doc/IConfigLayer"; import { ConfigApi } from "./ConfigApi"; import { IConfig } from "../doc/IConfig"; -import { TextUtils } from "../../../utilities"; -import { CommandResponse } from "../../../cmd/src/response/CommandResponse"; +import { Logger } from "../../../logger"; /** * API Class for manipulating config layers. @@ -59,10 +58,8 @@ export class ConfigLayers extends ConfigApi { suppressDump: true }); } else { - const cmdResp: CommandResponse = new CommandResponse({ - responseFormat: "default" - }); - cmdResp.console.log(TextUtils.chalk.red(msg)); + const logger = Logger.getAppLogger(); + logger.error(msg); } } this.mConfig.api.secure.loadCached(opts); diff --git a/packages/imperative/src/constants/src/Constants.ts b/packages/imperative/src/constants/src/Constants.ts index 254f086e6b..1796405552 100644 --- a/packages/imperative/src/constants/src/Constants.ts +++ b/packages/imperative/src/constants/src/Constants.ts @@ -60,7 +60,7 @@ export class Constants { public static readonly HELP_OPTION = "help"; public static readonly HELP_OPTION_ALIAS = "h"; public static readonly VERSION_OPTION: "version"; - public static readonly VERSION_OPTION_ALIAS: "v"; + public static readonly VERSION_OPTION_ALIAS: "V"; public static readonly HELP_EXAMPLES = "help-examples"; public static readonly HELP_WEB_OPTION = "help-web"; public static readonly HELP_WEB_OPTION_ALIAS = "hw"; From a2e928254287e08c7e10f0fe32c9545768558aeb Mon Sep 17 00:00:00 2001 From: Amber Date: Wed, 6 Nov 2024 14:42:17 -0500 Subject: [PATCH 18/20] correcting spy Signed-off-by: Amber --- .../imperative/src/config/__tests__/Config.api.unit.test.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/imperative/src/config/__tests__/Config.api.unit.test.ts b/packages/imperative/src/config/__tests__/Config.api.unit.test.ts index 4b0cbcdc01..a93e77c47f 100644 --- a/packages/imperative/src/config/__tests__/Config.api.unit.test.ts +++ b/packages/imperative/src/config/__tests__/Config.api.unit.test.ts @@ -311,7 +311,7 @@ describe("Config API tests", () => { const jsoncParseSpy = jest.spyOn(JSONC, "parse").mockImplementationOnce(() => { throw "failed to parse"; }); let logMsg: string = "Nothing logged"; - jest.spyOn(Logger, "getImperativeLogger").mockImplementation(() => { + jest.spyOn(Logger, "getAppLogger").mockImplementation(() => { return { error: jest.fn((errMsg) => { logMsg = errMsg; @@ -324,12 +324,12 @@ describe("Config API tests", () => { } catch(err) { caughtError = err; } - expect(existsSpy).toHaveBeenCalledTimes(6); // Once for each config layer and one more time for read + expect(existsSpy).toHaveBeenCalledTimes(5); // Once for each config layer and one more time for read expect(readFileSpy).toHaveBeenCalledTimes(1); expect(secureLoadSpy).toHaveBeenCalledTimes(1); expect(jsoncParseSpy).toHaveBeenCalledTimes(1); expect(caughtError).toBeUndefined(); - expect(logMsg).toContain("Nothing logged"); + expect(logMsg).toContain("Please check this configuration file for errors"); }); }); describe("write", () => { From d332d6c3ccc3436be718c64e60495ed39d9b9190 Mon Sep 17 00:00:00 2001 From: Amber Date: Wed, 6 Nov 2024 15:05:29 -0500 Subject: [PATCH 19/20] writing errors Signed-off-by: Amber --- packages/imperative/CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/imperative/CHANGELOG.md b/packages/imperative/CHANGELOG.md index 8c8b741884..688c88f7a5 100644 --- a/packages/imperative/CHANGELOG.md +++ b/packages/imperative/CHANGELOG.md @@ -4,7 +4,7 @@ All notable changes to the Imperative package will be documented in this file. ## Recent Changes -- BugFix: Enabled commands with either the `--help` or `--version` flags to still display their information despite any config file issues [#2301](https://github.com/zowe/zowe-cli/pull/2301) +- BugFix: Enabled commands with either the `--help` or `--version` flags to still display their information despite any configuration file issues. [#2301](https://github.com/zowe/zowe-cli/pull/2301) ## `8.7.0` From 71b032a5031462b8a5502ee3fc757a4c5e9c795e Mon Sep 17 00:00:00 2001 From: Timothy Johnson Date: Wed, 6 Nov 2024 16:30:53 -0500 Subject: [PATCH 20/20] Fix config errors not being ignored Signed-off-by: Timothy Johnson --- .../src/config/__tests__/Config.api.unit.test.ts | 2 +- packages/imperative/src/config/src/api/ConfigLayers.ts | 3 +-- packages/imperative/src/constants/src/Constants.ts | 4 ++-- packages/imperative/src/imperative/src/Imperative.ts | 8 ++++---- 4 files changed, 8 insertions(+), 9 deletions(-) diff --git a/packages/imperative/src/config/__tests__/Config.api.unit.test.ts b/packages/imperative/src/config/__tests__/Config.api.unit.test.ts index a93e77c47f..d3a47a9d6d 100644 --- a/packages/imperative/src/config/__tests__/Config.api.unit.test.ts +++ b/packages/imperative/src/config/__tests__/Config.api.unit.test.ts @@ -311,7 +311,7 @@ describe("Config API tests", () => { const jsoncParseSpy = jest.spyOn(JSONC, "parse").mockImplementationOnce(() => { throw "failed to parse"; }); let logMsg: string = "Nothing logged"; - jest.spyOn(Logger, "getAppLogger").mockImplementation(() => { + jest.spyOn(Logger, "getConsoleLogger").mockImplementation(() => { return { error: jest.fn((errMsg) => { logMsg = errMsg; diff --git a/packages/imperative/src/config/src/api/ConfigLayers.ts b/packages/imperative/src/config/src/api/ConfigLayers.ts index aadec34728..cb64163478 100644 --- a/packages/imperative/src/config/src/api/ConfigLayers.ts +++ b/packages/imperative/src/config/src/api/ConfigLayers.ts @@ -58,8 +58,7 @@ export class ConfigLayers extends ConfigApi { suppressDump: true }); } else { - const logger = Logger.getAppLogger(); - logger.error(msg); + Logger.getConsoleLogger().error(msg); } } this.mConfig.api.secure.loadCached(opts); diff --git a/packages/imperative/src/constants/src/Constants.ts b/packages/imperative/src/constants/src/Constants.ts index 1796405552..f247b56acb 100644 --- a/packages/imperative/src/constants/src/Constants.ts +++ b/packages/imperative/src/constants/src/Constants.ts @@ -59,8 +59,8 @@ export class Constants { public static readonly JSON_OPTION_ALIAS = "rfj"; public static readonly HELP_OPTION = "help"; public static readonly HELP_OPTION_ALIAS = "h"; - public static readonly VERSION_OPTION: "version"; - public static readonly VERSION_OPTION_ALIAS: "V"; + public static readonly VERSION_OPTION = "version"; + public static readonly VERSION_OPTION_ALIAS = "V"; public static readonly HELP_EXAMPLES = "help-examples"; public static readonly HELP_WEB_OPTION = "help-web"; public static readonly HELP_WEB_OPTION_ALIAS = "hw"; diff --git a/packages/imperative/src/imperative/src/Imperative.ts b/packages/imperative/src/imperative/src/Imperative.ts index 8689d0e074..56bdaf2545 100644 --- a/packages/imperative/src/imperative/src/Imperative.ts +++ b/packages/imperative/src/imperative/src/Imperative.ts @@ -141,10 +141,10 @@ export class Imperative { ImperativeConfig.instance.loadedConfig = config; // Detect CLI arguments to determine if errors should be ignored - const ignoreErrors = process.argv.includes(Constants.HELP_OPTION) || - process.argv.includes(Constants.HELP_OPTION_ALIAS) || - process.argv.includes(Constants.VERSION_OPTION) || - process.argv.includes(Constants.VERSION_OPTION_ALIAS) || + const ignoreErrors = process.argv.includes(Constants.OPT_LONG_DASH + Constants.HELP_OPTION) || + process.argv.includes(Constants.OPT_SHORT_DASH + Constants.HELP_OPTION_ALIAS) || + process.argv.includes(Constants.OPT_LONG_DASH + Constants.VERSION_OPTION) || + process.argv.includes(Constants.OPT_SHORT_DASH + Constants.VERSION_OPTION_ALIAS) || process.argv[process.argv.length - 1] === require.resolve('@zowe/cli'); /**