From 21949aac0a8407b76389ebf5a73b48f185e003be Mon Sep 17 00:00:00 2001 From: Merlin Beutlberger Date: Fri, 21 Apr 2023 17:13:01 +0200 Subject: [PATCH] cli/config: Update texts, fix edge cases, fix tests * Fix 'ui5 set ' 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 ' * 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 --- lib/cli/commands/config.js | 82 ++++++++------ test/lib/cli/commands/config.js | 187 ++++++++++++++++++++++++++++---- 2 files changed, 211 insertions(+), 58 deletions(-) diff --git a/lib/cli/commands/config.js b/lib/cli/commands/config.js index 34f81d1a..ef9035b3 100644 --- a/lib/cli/commands/config.js +++ b/lib/cli/commands/config.js @@ -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 ", "Sets a property in `.ui5rc`", { + .demandCommand(1, "Command required. Available commands are 'set', 'get' and 'list'") + .command("set [value]", + "Set the value for a given configuration key. Provide no unset the configuration", { + handler: handleConfig, + builder: noop, + middlewares: [baseMiddleware], + }) + .command("get ", "Get the value for a given configuration key", { handler: handleConfig, builder: noop, middlewares: [baseMiddleware], }) - .command("get ", "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; diff --git a/test/lib/cli/commands/config.js b/test/lib/cli/commands/config.js index 2bd7c87a..eae2e3f1 100644 --- a/test/lib/cli/commands/config.js +++ b/test/lib/cli/commands/config.js @@ -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 @@ -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) => { @@ -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) => { @@ -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'", + }); +});