Skip to content

Commit

Permalink
Fix code smells and update changelog
Browse files Browse the repository at this point in the history 
Signed-off-by: Timothy Johnson <[email protected]>
  • Loading branch information
@t1m0thyj
t1m0thyj committed Aug 23, 2024
1 parent 96723b6 commit 48fa3da
Show file tree
Hide file tree
Showing 7 changed files with 67 additions and 51 deletions.
4 changes: 4 additions & 0 deletions __tests__/__packages__/cli-test-utils/CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,10 @@

All notable changes to the Zowe CLI test utils package will be documented in this file.

## Recent Changes

- BugFix: Removed obsolete V1 `profiles` property from the parameters object returned by `mockHandlerParameters` method.

## `8.0.0-next.202408131445`

- Update: See `7.28.3` for details
Expand Down
11 changes: 11 additions & 0 deletions packages/imperative/CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,17 @@

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

## Recent Changes

- LTS Breaking: [#2231](https://github.com/zowe/zowe-cli/issues/2231)
- Removed the obsolete V1 `profiles` property from `IHandlerParameters` interface
- Deprecated the following obsolete V1 profile interfaces:
- `IProfileTypeConfiguration.dependencies` - For team config, use nested profiles instead
- `IProfileTypeConfiguration.validationPlanModule` - For team config, validate with JSON schema instead
- Deprecated the following obsolete V1 profile classes/functions:
- `CliUtils.getOptValueFromProfiles` - Use `getOptValuesFromConfig` instead to load from team config
- `CommandProfiles` - Use the `V1ProfileRead` class if you still need to read V1 profiles

## `8.0.0-next.202408191401`

- Update: See `5.26.3` and `5.27.0` for details
Expand Down
2 changes: 0 additions & 2 deletions .../src/example_clis/with_bin_package/__integration__/ExampleDefinitions.integration.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,14 +9,12 @@
*
*/

import { IImperativeConfig } from "../../../../../src/imperative";
import * as T from "../../../TestUtil";

describe("We should provide the ability to define commands through Javascript objects passed through the config " +
"or globs that match modules locally, " +
"tested through an example CLI", function () {
const cliBin = __dirname + "/../ProfileBinExampleCLI.ts";
const config: IImperativeConfig = require(__dirname + "/../ProfileBinExampleConfiguration");

it("All commands defined through module globs should be accurately defined, " +
"and a definition module in the same directory that does not ",
Expand Down
76 changes: 36 additions & 40 deletions packages/imperative/src/cmd/src/CommandProcessor.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -497,24 +497,22 @@ export class CommandProcessor {
{ hideText: true, secToWait: 0 });
}
// array processing
else {
if (preparedArgs[positionalName] != null &&
Array.isArray(preparedArgs[positionalName]) &&
preparedArgs[positionalName][0] != null &&
typeof preparedArgs[positionalName][0] === "string" &&
preparedArgs[positionalName][0].toUpperCase() === this.promptPhrase.toUpperCase()) {
// prompt has been requested for a positional
this.log.debug("Prompting for positional %s which was requested by passing the value %s",
preparedArgs[positionalName][0], this.promptPhrase);
preparedArgs[positionalName][0] =
await response.console.prompt(`"${positionalName}" Description: ` +
`${positional.description}\nPlease enter "${positionalName}":`,
{ hideText: true, secToWait: 0 });
// prompting enters as string but need to place it in array

const array = preparedArgs[positionalName][0].split(" ");
preparedArgs[positionalName] = array;
}
else if (preparedArgs[positionalName] != null &&
Array.isArray(preparedArgs[positionalName]) &&
preparedArgs[positionalName][0] != null &&
typeof preparedArgs[positionalName][0] === "string" &&
preparedArgs[positionalName][0].toUpperCase() === this.promptPhrase.toUpperCase()) {
// prompt has been requested for a positional
this.log.debug("Prompting for positional %s which was requested by passing the value %s",
preparedArgs[positionalName][0], this.promptPhrase);
preparedArgs[positionalName][0] =
await response.console.prompt(`"${positionalName}" Description: ` +
`${positional.description}\nPlease enter "${positionalName}":`,
{ hideText: true, secToWait: 0 });
// prompting enters as string but need to place it in array

const array = preparedArgs[positionalName][0].split(" ");
preparedArgs[positionalName] = array;
}
}
}
Expand Down Expand Up @@ -544,28 +542,26 @@ export class CommandProcessor {
}
}
// array processing
else {
if (Array.isArray(preparedArgs[option.name]) &&
preparedArgs[option.name][0] != null &&
typeof preparedArgs[option.name][0] === "string" &&
preparedArgs[option.name][0].toUpperCase() === this.promptPhrase.toUpperCase()) {
// prompt has been requested for an --option
this.log.debug("Prompting for option %s which was requested by passing the value %s",
option.name, this.promptPhrase);
preparedArgs[option.name][0] =
await response.console.prompt(`"${option.name}" Description: ` +
`${option.description}\nPlease enter "${option.name}":`,
{ hideText: true, secToWait: 0 });

const array = preparedArgs[option.name][0].split(" ");
preparedArgs[option.name] = array;
const camelCase = CliUtils.getOptionFormat(option.name).camelCase;
preparedArgs[camelCase] = preparedArgs[option.name];
if (option.aliases != null) {
for (const alias of option.aliases) {
// set each alias of the args object as well
preparedArgs[alias] = preparedArgs[option.name];
}
else if (Array.isArray(preparedArgs[option.name]) &&
preparedArgs[option.name][0] != null &&
typeof preparedArgs[option.name][0] === "string" &&
preparedArgs[option.name][0].toUpperCase() === this.promptPhrase.toUpperCase()) {
// prompt has been requested for an --option
this.log.debug("Prompting for option %s which was requested by passing the value %s",
option.name, this.promptPhrase);
preparedArgs[option.name][0] =
await response.console.prompt(`"${option.name}" Description: ` +
`${option.description}\nPlease enter "${option.name}":`,
{ hideText: true, secToWait: 0 });

const array = preparedArgs[option.name][0].split(" ");
preparedArgs[option.name] = array;
const camelCase = CliUtils.getOptionFormat(option.name).camelCase;
preparedArgs[camelCase] = preparedArgs[option.name];
if (option.aliases != null) {
for (const alias of option.aliases) {

Check warning on line 562 in packages/imperative/src/cmd/src/CommandProcessor.ts

View check run for this annotation

Codecov / codecov/patch

packages/imperative/src/cmd/src/CommandProcessor.ts#L562 

Added line #L562 was not covered by tests
// set each alias of the args object as well
preparedArgs[alias] = preparedArgs[option.name];

Check warning on line 564 in packages/imperative/src/cmd/src/CommandProcessor.ts

View check run for this annotation

Codecov / codecov/patch

packages/imperative/src/cmd/src/CommandProcessor.ts#L564 

Added line #L564 was not covered by tests
}
}
}
Expand Down
1 change: 1 addition & 0 deletions packages/imperative/src/profiles/src/validation/__tests__/ProfileValidation.unit.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
* Copyright Contributors to the Zowe Project.
*
*/

/* eslint-disable deprecation/deprecation */

import { inspect } from "util";
Expand Down
9 changes: 9 additions & 0 deletions packages/imperative/src/utilities/__tests__/CliUtils.unit.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -293,6 +293,7 @@ describe("CliUtils", () => {
it("should throw an imperative error if a required profile is not present", () => {
let error;
try {
// eslint-disable-next-line deprecation/deprecation
const args = CliUtils.getOptValueFromProfiles(
// eslint-disable-next-line deprecation/deprecation
new CommandProfiles(new Map<string, IProfile[]>()),
Expand All @@ -307,6 +308,7 @@ describe("CliUtils", () => {
});

it("should return nothing if a profile was optional and not loaded", () => {
// eslint-disable-next-line deprecation/deprecation
const args = CliUtils.getOptValueFromProfiles(
// eslint-disable-next-line deprecation/deprecation
new CommandProfiles(new Map<string, IProfile[]>()),
Expand All @@ -318,6 +320,7 @@ describe("CliUtils", () => {
it("should return args (from definitions with no hyphen in name) extracted from loaded profile", () => {
const map = new Map<string, IProfile[]>();
map.set("banana", [{ type: "banana", name: "fakebanana", nohyphen: "specified in profile" }]);
// eslint-disable-next-line deprecation/deprecation
const args = CliUtils.getOptValueFromProfiles(
// eslint-disable-next-line deprecation/deprecation
new CommandProfiles(map),
Expand All @@ -334,6 +337,7 @@ describe("CliUtils", () => {
"couldBeEither": "should be me",
"could-be-either": "should not be me"
}]);
// eslint-disable-next-line deprecation/deprecation
const args = CliUtils.getOptValueFromProfiles(
// eslint-disable-next-line deprecation/deprecation
new CommandProfiles(map),
Expand All @@ -350,6 +354,7 @@ describe("CliUtils", () => {
"fakeStringOpt": "should not be me",
"fake-string-opt": "should be me"
}]);
// eslint-disable-next-line deprecation/deprecation
const args = CliUtils.getOptValueFromProfiles(
// eslint-disable-next-line deprecation/deprecation
new CommandProfiles(map),
Expand All @@ -365,6 +370,7 @@ describe("CliUtils", () => {
"name": "fakebanana",
"could-be-either": "should be me"
}]);
// eslint-disable-next-line deprecation/deprecation
const args = CliUtils.getOptValueFromProfiles(
// eslint-disable-next-line deprecation/deprecation
new CommandProfiles(map),
Expand All @@ -380,6 +386,7 @@ describe("CliUtils", () => {
name: "fakebanana",
fakeStringOpt: "should be me"
}]);
// eslint-disable-next-line deprecation/deprecation
const args = CliUtils.getOptValueFromProfiles(
// eslint-disable-next-line deprecation/deprecation
new CommandProfiles(map),
Expand All @@ -395,6 +402,7 @@ describe("CliUtils", () => {
name: "fakebanana",
withAlias: "should have 'w' on args object too"
}]);
// eslint-disable-next-line deprecation/deprecation
const args = CliUtils.getOptValueFromProfiles(
// eslint-disable-next-line deprecation/deprecation
new CommandProfiles(map),
Expand All @@ -410,6 +418,7 @@ describe("CliUtils", () => {
name: "fakebanana",
username: "fake"
}]);
// eslint-disable-next-line deprecation/deprecation
const args = CliUtils.getOptValueFromProfiles(
// eslint-disable-next-line deprecation/deprecation
new CommandProfiles(map),
Expand Down
15 changes: 6 additions & 9 deletions packages/imperative/src/utilities/src/CliUtils.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,8 @@ export class CliUtils {
/**
* Accepts the full set of loaded profiles and attempts to match the option names supplied with profile keys.
*
* @deprecated Use `getOptValuesFromConfig` instead to load from team config
*
* @param {Map<string, IProfile[]>} profiles - the map of type to loaded profiles. The key is the profile type
* and the value is an array of profiles loaded for that type.
*
Expand Down Expand Up @@ -214,7 +216,6 @@ export class CliUtils {
}

// Build an object that contains all the options loaded from config
const fulfilled: string[] = [];
let fromCnfg: any = {};
for (const profileType of allTypes) {
const opt = ProfileUtils.getProfileOptionAndAlias(profileType)[0];
Expand All @@ -224,16 +225,13 @@ export class CliUtils {
const profileTypePrefix = profileType + "_";
let p: any = {};
if (args[opt] != null && config.api.profiles.exists(args[opt])) {
fulfilled.push(profileType);
p = config.api.profiles.get(args[opt]);
} else if (args[opt] != null && !args[opt].startsWith(profileTypePrefix) &&
config.api.profiles.exists(profileTypePrefix + args[opt])) {
fulfilled.push(profileType);
p = config.api.profiles.get(profileTypePrefix + args[opt]);
} else if (args[opt] == null &&
config.properties.defaults[profileType] != null &&
config.api.profiles.exists(config.properties.defaults[profileType])) {
fulfilled.push(profileType);
p = config.api.profiles.defaultGet(profileType);
}
fromCnfg = { ...p, ...fromCnfg };
Expand All @@ -247,14 +245,13 @@ export class CliUtils {
const profileCamel = fromCnfg[cases.camelCase];

if ((profileCamel !== undefined || profileKebab !== undefined) &&
(!Object.prototype.hasOwnProperty.call(args, cases.kebabCase) &&
!Object.prototype.hasOwnProperty.call(args, cases.camelCase))) {
(!Object.hasOwn(args, cases.kebabCase) && !Object.hasOwn(args, cases.camelCase))) {

// If both case properties are present in the profile, use the one that matches
// the option name explicitly
const value = profileKebab !== undefined && profileCamel !== undefined ?
opt.name === cases.kebabCase ? profileKebab : profileCamel :
profileKebab !== undefined ? profileKebab : profileCamel;
const shouldUseKebab = profileKebab !== undefined && profileCamel !== undefined ?
opt.name === cases.kebabCase : profileKebab !== undefined;
const value = shouldUseKebab ? profileKebab : profileCamel;
const keys = CliUtils.setOptionValue(opt.name,
"aliases" in opt ? opt.aliases : [],
value
Expand Down

0 comments on commit 48fa3da

Please sign in to comment.