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 c71ada1d52..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 @@ -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/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/CHANGELOG.md b/packages/imperative/CHANGELOG.md index 1e97c1fb58..688c88f7a5 100644 --- a/packages/imperative/CHANGELOG.md +++ b/packages/imperative/CHANGELOG.md @@ -2,6 +2,10 @@ 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 configuration file issues. [#2301](https://github.com/zowe/zowe-cli/pull/2301) + ## `8.7.0` - Enhancement: Added optional `proxy` object to ISession interface for extenders to pass a ProxyVariables object that would override the environment variables if in place. [#2330](https://github.com/zowe/zowe-cli/pull/2330) diff --git a/packages/imperative/src/cmd/__tests__/CommandProcessor.unit.test.ts b/packages/imperative/src/cmd/__tests__/CommandProcessor.unit.test.ts index b48f3c2d23..18737c37a2 100644 --- a/packages/imperative/src/cmd/__tests__/CommandProcessor.unit.test.ts +++ b/packages/imperative/src/cmd/__tests__/CommandProcessor.unit.test.ts @@ -192,6 +192,37 @@ const FAKE_HELP_GENERATOR: IHelpGenerator = { const ENV_VAR_PREFIX: string = "UNIT_TEST"; 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: { + validate: () => ({valid: false}), // Simulate faulty config + } as any, + }); + + jest.spyOn(console, "log").mockImplementation(() => {}); // Prevent console logs in tests + }); + + afterEach(() => { + jest.restoreAllMocks(); + }); + + 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); @@ -203,7 +234,6 @@ describe("Command Processor", () => { process.stderr.write = ORIGINAL_STDERR_WRITE; jest.restoreAllMocks(); }); - it("should allow us to create an instance", () => { let caughtError; 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..d3a47a9d6d 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, Logger } from "../.."; const MY_APP = "my_app"; @@ -264,6 +265,72 @@ 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("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"; }); + + let logMsg: string = "Nothing logged"; + jest.spyOn(Logger, "getConsoleLogger").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(readFileSpy).toHaveBeenCalledTimes(1); + expect(secureLoadSpy).toHaveBeenCalledTimes(1); + expect(jsoncParseSpy).toHaveBeenCalledTimes(1); + expect(caughtError).toBeUndefined(); + expect(logMsg).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__/ProfileInfo.TeamConfig.unit.test.ts b/packages/imperative/src/config/__tests__/ProfileInfo.TeamConfig.unit.test.ts index 2c7330d8a8..a27b8a0a3c 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/src/Config.ts b/packages/imperative/src/config/src/Config.ts index 999f568960..8ea129890b 100644 --- a/packages/imperative/src/config/src/Config.ts +++ b/packages/imperative/src/config/src/Config.ts @@ -147,10 +147,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 @@ -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: opts?.ignoreErrors }); } 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; diff --git a/packages/imperative/src/config/src/api/ConfigLayers.ts b/packages/imperative/src/config/src/api/ConfigLayers.ts index e790c321ce..cb64163478 100644 --- a/packages/imperative/src/config/src/api/ConfigLayers.ts +++ b/packages/imperative/src/config/src/api/ConfigLayers.ts @@ -18,6 +18,7 @@ import { ConfigConstants } from "../ConfigConstants"; import { IConfigLayer } from "../doc/IConfigLayer"; import { ConfigApi } from "./ConfigApi"; import { IConfig } from "../doc/IConfig"; +import { Logger } from "../../../logger"; /** * API Class for manipulating config layers. @@ -31,7 +32,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 +50,16 @@ 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 { + Logger.getConsoleLogger().error(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..0b3ea91262 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; } 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..f247b56acb 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/__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..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 @@ -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; } diff --git a/packages/imperative/src/imperative/src/Imperative.ts b/packages/imperative/src/imperative/src/Imperative.ts index 76eab278a5..56bdaf2545 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.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'); + /** * 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 } ); 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, 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; }