Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

--help fail on config json error #2301

Open
wants to merge 23 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
99e5630
all needed changes to meet AC besides tests
ATorrise Oct 9, 2024
6a39c50
changelog and codeql change
ATorrise Oct 9, 2024
1e84a49
still more tests to fix
ATorrise Oct 14, 2024
1d51d5d
i think i put ttests in the right place
ATorrise Oct 22, 2024
150cd03
Merge remote-tracking branch 'origin/master' into error-msg-help-version
ATorrise Oct 22, 2024
d978e0a
linting
ATorrise Oct 24, 2024
a307dd5
removing tests because tests in command processor seem to be enough?
ATorrise Oct 24, 2024
6387262
Update packages/imperative/CHANGELOG.md
ATorrise Oct 30, 2024
0b3ebc9
attempting to fix unit test
ATorrise Oct 30, 2024
82fd8f2
removing unneeded snaps
ATorrise Nov 4, 2024
1edf977
adding node 22 support and also removing private config constructor t…
ATorrise Nov 5, 2024
f8f520d
trying to fix my 2 added unit tests for terminal output given flags
ATorrise Nov 5, 2024
6545a33
Merge branch 'master' of https://github.com/zowe/zowe-cli into error-…
zFernand0 Nov 5, 2024
ab791ec
chore: cherry pick d7ea329742879ca1bca05c5fef503b1a57ec09b9
zFernand0 Nov 5, 2024
1b793ac
getting back to normal with changes
ATorrise Nov 5, 2024
128f61b
chore: fix lint warnings
zFernand0 Nov 5, 2024
b459781
update tests
zFernand0 Nov 5, 2024
0d670f7
removing unused vars
ATorrise Nov 5, 2024
db09359
Merge branch 'error-msg-help-version' of https://github.com/zowe/zowe…
ATorrise Nov 5, 2024
2a370f0
addresing timothys batch of suggestions
ATorrise Nov 6, 2024
a2e9282
correcting spy
ATorrise Nov 6, 2024
d332d6c
writing errors
ATorrise Nov 6, 2024
71b032a
Fix config errors not being ignored
t1m0thyj Nov 6, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions packages/imperative/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,9 @@

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)
ATorrise marked this conversation as resolved.
Show resolved Hide resolved

## `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
Original file line number Diff line number Diff line change
Expand Up @@ -191,6 +191,61 @@

const ENV_VAR_PREFIX: string = "UNIT_TEST";

describe("Command Processor with --help and --version flags", () => {
ATorrise marked this conversation as resolved.
Show resolved Hide resolved
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!");

Check failure on line 222 in packages/imperative/src/cmd/__tests__/CommandProcessor.unit.test.ts

View workflow job for this annotation

GitHub Actions / test (18.x, macos-14)

Command Processor with --help and --version flags › should display help text even with faulty config

expect(received).toContain(expected) // indexOf Expected substring: "Build help invoked!" Received string: "" at packages/imperative/src/cmd/__tests__/CommandProcessor.unit.test.ts:222:55 at fulfilled (packages/imperative/src/cmd/__tests__/CommandProcessor.unit.test.ts:15:58)

Check failure on line 222 in packages/imperative/src/cmd/__tests__/CommandProcessor.unit.test.ts

View workflow job for this annotation

GitHub Actions / test (18.x, ubuntu-22.04)

Command Processor with --help and --version flags › should display help text even with faulty config

expect(received).toContain(expected) // indexOf Expected substring: "Build help invoked!" Received string: "" at packages/imperative/src/cmd/__tests__/CommandProcessor.unit.test.ts:222:55 at fulfilled (packages/imperative/src/cmd/__tests__/CommandProcessor.unit.test.ts:15:58)

Check failure on line 222 in packages/imperative/src/cmd/__tests__/CommandProcessor.unit.test.ts

View workflow job for this annotation

GitHub Actions / test (18.x, windows-latest)

Command Processor with --help and --version flags › should display help text even with faulty config

expect(received).toContain(expected) // indexOf Expected substring: "Build help invoked!" Received string: "" at packages/imperative/src/cmd/__tests__/CommandProcessor.unit.test.ts:222:55 at fulfilled (packages/imperative/src/cmd/__tests__/CommandProcessor.unit.test.ts:15:58)

Check failure on line 222 in packages/imperative/src/cmd/__tests__/CommandProcessor.unit.test.ts

View workflow job for this annotation

GitHub Actions / test (20.x, macos-14)

Command Processor with --help and --version flags › should display help text even with faulty config

expect(received).toContain(expected) // indexOf Expected substring: "Build help invoked!" Received string: "" at packages/imperative/src/cmd/__tests__/CommandProcessor.unit.test.ts:222:55 at fulfilled (packages/imperative/src/cmd/__tests__/CommandProcessor.unit.test.ts:15:58)

Check failure on line 222 in packages/imperative/src/cmd/__tests__/CommandProcessor.unit.test.ts

View workflow job for this annotation

GitHub Actions / test (20.x, ubuntu-22.04)

Command Processor with --help and --version flags › should display help text even with faulty config

expect(received).toContain(expected) // indexOf Expected substring: "Build help invoked!" Received string: "" at packages/imperative/src/cmd/__tests__/CommandProcessor.unit.test.ts:222:55 at fulfilled (packages/imperative/src/cmd/__tests__/CommandProcessor.unit.test.ts:15:58)

Check failure on line 222 in packages/imperative/src/cmd/__tests__/CommandProcessor.unit.test.ts

View workflow job for this annotation

GitHub Actions / test (20.x, windows-latest)

Command Processor with --help and --version flags › should display help text even with faulty config

expect(received).toContain(expected) // indexOf Expected substring: "Build help invoked!" Received string: "" at packages/imperative/src/cmd/__tests__/CommandProcessor.unit.test.ts:222:55 at fulfilled (packages/imperative/src/cmd/__tests__/CommandProcessor.unit.test.ts:15:58)
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:");

Check failure on line 231 in packages/imperative/src/cmd/__tests__/CommandProcessor.unit.test.ts

View workflow job for this annotation

GitHub Actions / test (18.x, macos-14)

Command Processor with --help and --version flags › should display version even with faulty config

expect(received).toContain(expected) // indexOf Expected substring: "Version:" Received string: "" at packages/imperative/src/cmd/__tests__/CommandProcessor.unit.test.ts:231:58 at fulfilled (packages/imperative/src/cmd/__tests__/CommandProcessor.unit.test.ts:15:58)

Check failure on line 231 in packages/imperative/src/cmd/__tests__/CommandProcessor.unit.test.ts

View workflow job for this annotation

GitHub Actions / test (18.x, ubuntu-22.04)

Command Processor with --help and --version flags › should display version even with faulty config

expect(received).toContain(expected) // indexOf Expected substring: "Version:" Received string: "" at packages/imperative/src/cmd/__tests__/CommandProcessor.unit.test.ts:231:58 at fulfilled (packages/imperative/src/cmd/__tests__/CommandProcessor.unit.test.ts:15:58)

Check failure on line 231 in packages/imperative/src/cmd/__tests__/CommandProcessor.unit.test.ts

View workflow job for this annotation

GitHub Actions / test (18.x, windows-latest)

Command Processor with --help and --version flags › should display version even with faulty config

expect(received).toContain(expected) // indexOf Expected substring: "Version:" Received string: "" at packages/imperative/src/cmd/__tests__/CommandProcessor.unit.test.ts:231:58 at fulfilled (packages/imperative/src/cmd/__tests__/CommandProcessor.unit.test.ts:15:58)

Check failure on line 231 in packages/imperative/src/cmd/__tests__/CommandProcessor.unit.test.ts

View workflow job for this annotation

GitHub Actions / test (20.x, macos-14)

Command Processor with --help and --version flags › should display version even with faulty config

expect(received).toContain(expected) // indexOf Expected substring: "Version:" Received string: "" at packages/imperative/src/cmd/__tests__/CommandProcessor.unit.test.ts:231:58 at fulfilled (packages/imperative/src/cmd/__tests__/CommandProcessor.unit.test.ts:15:58)

Check failure on line 231 in packages/imperative/src/cmd/__tests__/CommandProcessor.unit.test.ts

View workflow job for this annotation

GitHub Actions / test (20.x, ubuntu-22.04)

Command Processor with --help and --version flags › should display version even with faulty config

expect(received).toContain(expected) // indexOf Expected substring: "Version:" Received string: "" at packages/imperative/src/cmd/__tests__/CommandProcessor.unit.test.ts:231:58 at fulfilled (packages/imperative/src/cmd/__tests__/CommandProcessor.unit.test.ts:15:58)

Check failure on line 231 in packages/imperative/src/cmd/__tests__/CommandProcessor.unit.test.ts

View workflow job for this annotation

GitHub Actions / test (20.x, windows-latest)

Command Processor with --help and --version flags › should display version even with faulty config

expect(received).toContain(expected) // indexOf Expected substring: "Version:" Received string: "" at packages/imperative/src/cmd/__tests__/CommandProcessor.unit.test.ts:231:58 at fulfilled (packages/imperative/src/cmd/__tests__/CommandProcessor.unit.test.ts:15:58)
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();

Check failure on line 244 in packages/imperative/src/cmd/__tests__/CommandProcessor.unit.test.ts

View workflow job for this annotation

GitHub Actions / test (18.x, macos-14)

Command Processor with --help and --version flags › should fail command execution without --help or --version if config is faulty

expect(received).toBeDefined() Received: undefined at packages/imperative/src/cmd/__tests__/CommandProcessor.unit.test.ts:244:23 at fulfilled (packages/imperative/src/cmd/__tests__/CommandProcessor.unit.test.ts:15:58)

Check failure on line 244 in packages/imperative/src/cmd/__tests__/CommandProcessor.unit.test.ts

View workflow job for this annotation

GitHub Actions / test (18.x, ubuntu-22.04)

Command Processor with --help and --version flags › should fail command execution without --help or --version if config is faulty

expect(received).toBeDefined() Received: undefined at packages/imperative/src/cmd/__tests__/CommandProcessor.unit.test.ts:244:23 at fulfilled (packages/imperative/src/cmd/__tests__/CommandProcessor.unit.test.ts:15:58)

Check failure on line 244 in packages/imperative/src/cmd/__tests__/CommandProcessor.unit.test.ts

View workflow job for this annotation

GitHub Actions / test (18.x, windows-latest)

Command Processor with --help and --version flags › should fail command execution without --help or --version if config is faulty

expect(received).toBeDefined() Received: undefined at packages/imperative/src/cmd/__tests__/CommandProcessor.unit.test.ts:244:23 at fulfilled (packages/imperative/src/cmd/__tests__/CommandProcessor.unit.test.ts:15:58)

Check failure on line 244 in packages/imperative/src/cmd/__tests__/CommandProcessor.unit.test.ts

View workflow job for this annotation

GitHub Actions / test (20.x, macos-14)

Command Processor with --help and --version flags › should fail command execution without --help or --version if config is faulty

expect(received).toBeDefined() Received: undefined at packages/imperative/src/cmd/__tests__/CommandProcessor.unit.test.ts:244:23 at fulfilled (packages/imperative/src/cmd/__tests__/CommandProcessor.unit.test.ts:15:58)

Check failure on line 244 in packages/imperative/src/cmd/__tests__/CommandProcessor.unit.test.ts

View workflow job for this annotation

GitHub Actions / test (20.x, ubuntu-22.04)

Command Processor with --help and --version flags › should fail command execution without --help or --version if config is faulty

expect(received).toBeDefined() Received: undefined at packages/imperative/src/cmd/__tests__/CommandProcessor.unit.test.ts:244:23 at fulfilled (packages/imperative/src/cmd/__tests__/CommandProcessor.unit.test.ts:15:58)

Check failure on line 244 in packages/imperative/src/cmd/__tests__/CommandProcessor.unit.test.ts

View workflow job for this annotation

GitHub Actions / test (20.x, windows-latest)

Command Processor with --help and --version flags › should fail command execution without --help or --version if config is faulty

expect(received).toBeDefined() Received: undefined at packages/imperative/src/cmd/__tests__/CommandProcessor.unit.test.ts:244:23 at fulfilled (packages/imperative/src/cmd/__tests__/CommandProcessor.unit.test.ts:15:58)
expect(error.message).toContain("Configuration invalid");
});
});

describe("Command Processor", () => {
beforeEach(() => {
// Mock read stdin
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
4 changes: 3 additions & 1 deletion packages/imperative/src/config/src/Config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -179,7 +179,9 @@ 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")
ATorrise marked this conversation as resolved.
Show resolved Hide resolved
ATorrise marked this conversation as resolved.
Show resolved Hide resolved
|| process.argv[process.argv.length-1] === require.resolve('@zowe/cli')
ATorrise marked this conversation as resolved.
Show resolved Hide resolved
});
}

Expand Down
3 changes: 2 additions & 1 deletion packages/imperative/src/config/src/ConfigUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand Down
3 changes: 2 additions & 1 deletion packages/imperative/src/config/src/ProfileInfo.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
3 changes: 2 additions & 1 deletion packages/imperative/src/config/src/__mocks__/Config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,8 @@ export class Config {
defaults: {}
},
global: true,
user: false
user: false,
ignoreErrors: false
}];

return config;
Expand Down
22 changes: 16 additions & 6 deletions packages/imperative/src/config/src/api/ConfigLayers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@
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.
Expand All @@ -31,7 +33,7 @@
* @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}) {
ATorrise marked this conversation as resolved.
Show resolved Hide resolved
// Attempt to populate the layer
const layer = opts ? this.mConfig.findLayer(opts.user, opts.global) : this.mConfig.layerActive();
if (fs.existsSync(layer.path)) {
Expand All @@ -49,11 +51,19 @@
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){
zFernand0 marked this conversation as resolved.
Show resolved Hide resolved
throw new ImperativeError({
msg:msg,
suppressDump: true
});
} else {
const cmdResp: CommandResponse = new CommandResponse({

Check warning on line 62 in packages/imperative/src/config/src/api/ConfigLayers.ts

View check run for this annotation

Codecov / codecov/patch

packages/imperative/src/config/src/api/ConfigLayers.ts#L61-L62

Added lines #L61 - L62 were not covered by tests
responseFormat: "default"
});
cmdResp.console.log(TextUtils.chalk.red(msg));

Check warning on line 65 in packages/imperative/src/config/src/api/ConfigLayers.ts

View check run for this annotation

Codecov / codecov/patch

packages/imperative/src/config/src/api/ConfigLayers.ts#L65

Added line #L65 was not covered by tests
ATorrise marked this conversation as resolved.
Show resolved Hide resolved
}
}
this.mConfig.api.secure.loadCached(opts);
} else if (layer.exists) {
Expand Down
1 change: 1 addition & 0 deletions packages/imperative/src/config/src/doc/IConfigLayer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,4 +17,5 @@ export interface IConfigLayer {
properties: IConfig;
global: boolean;
user: boolean;
ignoreErrors: boolean;
ATorrise marked this conversation as resolved.
Show resolved Hide resolved
}
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down
1 change: 1 addition & 0 deletions zowe-cli
ATorrise marked this conversation as resolved.
Show resolved Hide resolved
Submodule zowe-cli added at 027d9c
Loading