Skip to content

Commit

Permalink
cli/config: Update texts, fix edge cases, fix tests
Browse files Browse the repository at this point in the history 
* Fix 'ui5 set <key>' without value or empty string to unset
* Log everything to stderr, except machine parsable output like
  configuration values returned by 'ui5 list' and 'ui5 get <key>'
* Do not mention location of ui5rc in config commands. In the future we
  might have multiple locations and @ui5/project/config/Configuration
  would take care of deciding where to read what from. The CLI should
  not have that knowledge
* Move imports and variable definitions to handler in order to only
  execute them if the command is actually 'ui5 config'
* Tests now mock Configuration in order to not modify the actual
  configuration of the user running the tests
  • Loading branch information
@RandomByte
RandomByte committed Apr 21, 2023
1 parent 8c6ccb8 commit 21949aa
Show file tree
Hide file tree
Showing 2 changed files with 211 additions and 58 deletions.
82 changes: 48 additions & 34 deletions lib/cli/commands/config.js
View file
Original file line number Diff line number Diff line change
@@ -1,76 +1,90 @@
import path from "node:path";
import os from "node:os";
import chalk from "chalk";
import process from "node:process";
import baseMiddleware from "../middlewares/base.js";
import Configuration from "@ui5/project/config/Configuration";

const ALLOWED_KEYS = ["mavenSnapshotEndpointUrl"];
const ui5RcFilePath = path.resolve(path.join(os.homedir(), ".ui5rc"));

const configCommand = {
command: "config",
describe: "Configures the `.ui5rc` file.",
describe: "Get and set UI5 Tooling configuration options",
middlewares: [baseMiddleware],
handler: handleConfig
};

configCommand.builder = function(cli) {
return cli
.command("set <key> <value>", "Sets a property in `.ui5rc`", {
.demandCommand(1, "Command required. Available commands are 'set', 'get' and 'list'")
.command("set <key> [value]",
"Set the value for a given configuration key. Provide no unset the configuration", {
handler: handleConfig,
builder: noop,
middlewares: [baseMiddleware],
})
.command("get <key>", "Get the value for a given configuration key", {
handler: handleConfig,
builder: noop,
middlewares: [baseMiddleware],
})
.command("get <key>", "Gets a property from `.ui5rc`", {
.command("list", "Display the current configuration", {
handler: handleConfig,
builder: noop,
middlewares: [baseMiddleware],
})
.command("list", "List settings stored in `.ui5rc`", {
handler: handleConfig,
builder: noop,
middlewares: [baseMiddleware],
});
.example("$0 set mavenSnapshotEndpointUrl http://example.com/snapshots/",
"Set a value for the mavenSnapshotEndpointUrl configuration")
.example("$0 set mavenSnapshotEndpointUrl",
"Unset the current value of the mavenSnapshotEndpointUrl configuration");
};

function noop() {}

async function handleConfig(argv) {
const {_: commandArgs, key, value} = argv;
const command = commandArgs[commandArgs.length - 1];

if (key && !ALLOWED_KEYS.includes(key)) {
throw new Error(`The provided key is not part of the .ui5rc allowed options: ${ALLOWED_KEYS.join(", ")}`);
const {default: Configuration} = await import( "@ui5/project/config/Configuration");
const allowedKeys = ["mavenSnapshotEndpointUrl"];

if (key && !allowedKeys.includes(key)) {
throw new Error(
`The provided key is not a valid configuration option. Valid options are: ${allowedKeys.join(", ")}`);
}

const config = await Configuration.fromFile();

if (commandArgs.includes("list")) {
console.log(`Listing properties from ${chalk.dim(ui5RcFilePath)}:
${formatJsonForOutput(config.toJSON())}`);
} else if (commandArgs.includes("get")) {
console.log(`Getting property ${chalk.bold(key)} from ${chalk.dim(ui5RcFilePath)}:
${formatJsonForOutput(config.toJSON(), key)}`);
} else if (commandArgs.includes("set")) {
const jsonConfig = config.toJSON();
if (value) {
jsonConfig[key] = value;
} else {
delete jsonConfig[key];
if (command === "list") {
process.stdout.write(formatJsonForOutput(config.toJson()));
} else if (command === "get") {
let configValue = config.toJson()[key];
if (configValue === undefined) {
configValue = "";
}

console.log(`Set property ${chalk.bold(key)} into ${chalk.dim(ui5RcFilePath)}:
process.stdout.write(`${configValue}\n`);
} else if (command === "set") {
const jsonConfig = config.toJson();
if (value === undefined || value === "") {
delete jsonConfig[key];
process.stderr.write(`Configuration option ${chalk.bold(key)} has been unset\n`);
} else {
jsonConfig[key] = value;
process.stderr.write(`Configuration option ${chalk.bold(key)} has been updated:
${formatJsonForOutput(jsonConfig, key)}`);
}

await Configuration.toFile(new Configuration(jsonConfig));
} else {
throw new Error(`Unknown 'ui5 config' command '${command}'`);
}
}

function formatJsonForOutput(config, filterKey) {
return Object.keys(config)
.filter((key) => !filterKey || filterKey === key)
.map(
(key) => ` ${key} = ${config[key]}\n`
).join("");
.filter((key) => config[key] !== undefined) // Don't print undefined config
.map((key) => {
let configValue = config[key];
if (configValue === undefined) {
configValue = "";
}
return ` ${key} = ${configValue}\n`;
}).join("");
}

export default configCommand;
187 changes: 163 additions & 24 deletions test/lib/cli/commands/config.js
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import test from "ava";
import sinon from "sinon";
import esmock from "esmock";
import chalk from "chalk";

function getDefaultArgv() {
// This has been taken from the actual argv object yargs provides
Expand Down Expand Up @@ -30,9 +31,18 @@ test.beforeEach(async (t) => {
t.context.argv = getDefaultArgv();

t.context.builder = sinon.stub().resolves();
t.context.consoleLog = sinon.stub(console, "log");
t.context.stdoutWriteStub = sinon.stub(process.stdout, "write");
t.context.stderrWriteStub = sinon.stub(process.stderr, "write");
t.context.configToJson = sinon.stub().returns({});

t.context.config = await esmock.p("../../../../lib/cli/commands/config.js");
const {default: Configuration} = await import( "@ui5/project/config/Configuration");
t.context.Configuration = Configuration;
sinon.stub(Configuration, "fromFile").resolves(new Configuration({}));
sinon.stub(Configuration, "toFile").resolves();

t.context.config = await esmock.p("../../../../lib/cli/commands/config.js", {
"@ui5/project/config/Configuration": t.context.Configuration
});
});

test.afterEach.always((t) => {
Expand All @@ -41,56 +51,175 @@ test.afterEach.always((t) => {
});

test.serial("ui5 config list", async (t) => {
const {config, argv, consoleLog} = t.context;
const {config, argv, stdoutWriteStub, stderrWriteStub, Configuration} = t.context;

const configurationStub = new Configuration({});
sinon.stub(configurationStub, "toJson").returns({
mavenSnapshotEndpointUrl: "my/url",
otherConfig: false,
pony: 130,
horses: null,
cows: undefined // Won't be rendered
});
Configuration.fromFile.resolves(configurationStub);

argv["_"] = ["list"];
await config.handler(argv);

t.is(stdoutWriteStub.firstCall.firstArg, ` mavenSnapshotEndpointUrl = my/url
otherConfig = false
pony = 130
horses = null
`, "Logged expected text to stdout");
t.is(stderrWriteStub.callCount, 0, "Nothing written to stderr");
});

test.serial("ui5 config list: No config", async (t) => {
const {config, argv, stdoutWriteStub, stderrWriteStub} = t.context;

argv["_"] = ["list"];
await config.handler(argv);

t.true(consoleLog.getCall(0).args[0].startsWith("Listing properties from "), "Lists properties via console.log");
t.is(stdoutWriteStub.firstCall.firstArg, "",
"Logged no text to stdout");
t.is(stderrWriteStub.callCount, 0, "Nothing written to stderr");
});

test.serial("ui5 config get", async (t) => {
const {config, argv, consoleLog} = t.context;
const {config, argv, stdoutWriteStub, stderrWriteStub, Configuration} = t.context;

Configuration.fromFile.resolves(new Configuration({
mavenSnapshotEndpointUrl: "my/url"
}));

argv["_"] = ["get"];
argv["key"] = "mavenSnapshotEndpointUrl";
await config.handler(argv);

t.is(stdoutWriteStub.firstCall.firstArg, "my/url\n",
"Logged configuration value to stdout");
t.is(stderrWriteStub.callCount, 0, "Nothing written to stderr");
});

test.serial("ui5 config get: Empty value", async (t) => {
const {config, argv, stdoutWriteStub} = t.context;

argv["_"] = ["get"];
argv["key"] = "mavenSnapshotEndpointUrl";
await config.handler(argv);

t.true(
consoleLog.getCall(0)
.args[0].startsWith("Getting property "),
"Getting key from config file"
);
t.is(stdoutWriteStub.firstCall.firstArg, "\n", "Logged no value to console");
});

test.serial("ui5 config set", async (t) => {
const {config, argv, consoleLog} = t.context;
const {config, argv, stderrWriteStub, stdoutWriteStub, Configuration} = t.context;

const configurationStub = new Configuration({});
sinon.stub(configurationStub, "toJson").returns({
mavenSnapshotEndpointUrl: "my/url"
});

Configuration.fromFile.resolves(configurationStub);

argv["_"] = ["set"];
argv["key"] = "mavenSnapshotEndpointUrl";
argv["value"] = "https://_snapshot_endpoint_";
await config.handler(argv);

t.true(
consoleLog.getCall(0)
.args[0].startsWith("Set property "),
"Setting value into file"
);
t.is(stderrWriteStub.firstCall.firstArg,
`Configuration option ${chalk.bold("mavenSnapshotEndpointUrl")} has been updated:
mavenSnapshotEndpointUrl = https://_snapshot_endpoint_\n`,
"Logged expected message to stderr");
t.is(stdoutWriteStub.callCount, 0, "Nothing written to stdout");

t.is(Configuration.toFile.callCount, 1, "Configuration#toFile got called once");
t.deepEqual(Configuration.toFile.firstCall.firstArg.toJson(), {
mavenSnapshotEndpointUrl: "https://_snapshot_endpoint_"
}, "Configuration#toFile got called with expected argument");
});

test.serial("ui5 config set without value should delete", async (t) => {
const {config, argv, stderrWriteStub, stdoutWriteStub, Configuration} = t.context;

argv["_"] = ["set"];
argv["key"] = "mavenSnapshotEndpointUrl";
argv["value"] = undefined; // Simulating no value parameter provided to Yargs
await config.handler(argv);

t.is(stderrWriteStub.firstCall.firstArg,
`Configuration option ${chalk.bold("mavenSnapshotEndpointUrl")} has been unset\n`,
"Logged expected message to stderr");
t.is(stdoutWriteStub.callCount, 0, "Nothing written to stdout");

t.is(Configuration.toFile.callCount, 1, "Configuration#toFile got called once");
t.deepEqual(Configuration.toFile.firstCall.firstArg.toJson(), {
mavenSnapshotEndpointUrl: undefined
}, "Configuration#toFile got called with expected argument");
});

test.serial("ui5 config set empty value should delete", async (t) => {
const {config, argv, consoleLog} = t.context;
const {config, argv, stderrWriteStub, stdoutWriteStub, Configuration} = t.context;

argv["_"] = ["set"];
argv["key"] = "mavenSnapshotEndpointUrl";
argv["value"] = ""; // Simulating empty value provided to Yargs using quotes ""
await config.handler(argv);

t.is(stderrWriteStub.firstCall.firstArg,
`Configuration option ${chalk.bold("mavenSnapshotEndpointUrl")} has been unset\n`,
"Logged expected message to stderr");
t.is(stdoutWriteStub.callCount, 0, "Nothing written to stdout");

t.is(Configuration.toFile.callCount, 1, "Configuration#toFile got called once");
t.deepEqual(Configuration.toFile.firstCall.firstArg.toJson(), {
mavenSnapshotEndpointUrl: undefined
}, "Configuration#toFile got called with expected argument");
});

test.serial("ui5 config set null should update", async (t) => {
const {config, argv, stderrWriteStub, stdoutWriteStub, Configuration} = t.context;

argv["_"] = ["set"];
argv["key"] = "mavenSnapshotEndpointUrl";
argv["value"] = "";

// Yargs would never provide us with other types than string. Still, our code should
// check for empty strings and nothing else (like falsy)
argv["value"] = 0;
await config.handler(argv);

t.true(
consoleLog.getCall(0)
.args[0].startsWith("Set property "),
"Setting value into file"
);
t.is(stderrWriteStub.firstCall.firstArg,
`Configuration option ${chalk.bold("mavenSnapshotEndpointUrl")} has been updated:
mavenSnapshotEndpointUrl = 0\n`,
"Logged expected message to stderr");
t.is(stdoutWriteStub.callCount, 0, "Nothing written to stdout");

t.is(Configuration.toFile.callCount, 1, "Configuration#toFile got called once");
t.deepEqual(Configuration.toFile.firstCall.firstArg.toJson(), {
mavenSnapshotEndpointUrl: 0
}, "Configuration#toFile got called with expected argument");
});

test.serial("ui5 config set false should update", async (t) => {
const {config, argv, stderrWriteStub, stdoutWriteStub, Configuration} = t.context;

argv["_"] = ["set"];
argv["key"] = "mavenSnapshotEndpointUrl";

// Yargs would never provide us with other types than string. Still, our code should
// check for empty strings and nothing else (like falsyness)
argv["value"] = false;
await config.handler(argv);

t.is(stderrWriteStub.firstCall.firstArg,
`Configuration option ${chalk.bold("mavenSnapshotEndpointUrl")} has been updated:
mavenSnapshotEndpointUrl = false\n`,
"Logged expected message to stderr");
t.is(stdoutWriteStub.callCount, 0, "Nothing written to stdout");

t.is(Configuration.toFile.callCount, 1, "Configuration#toFile got called once");
t.deepEqual(Configuration.toFile.firstCall.firstArg.toJson(), {
mavenSnapshotEndpointUrl: false
}, "Configuration#toFile got called with expected argument");
});

test.serial("ui5 config invalid key", async (t) => {
Expand All @@ -102,7 +231,17 @@ test.serial("ui5 config invalid key", async (t) => {

await t.throwsAsync(config.handler(argv), {
message:
"The provided key is not part of the .ui5rc allowed options: mavenSnapshotEndpointUrl",
"The provided key is not a valid configuration option. Valid options are: mavenSnapshotEndpointUrl",
});
});

test.serial("ui5 config unknown command", async (t) => {
const {config, argv} = t.context;

argv["_"] = ["foo"];

await t.throwsAsync(config.handler(argv), {
message:
"Unknown 'ui5 config' command 'foo'",
});
});

0 comments on commit 21949aa

Please sign in to comment.