Skip to content

Commit

Permalink
feat(cli): In case of errors, exit with code 1
Browse files Browse the repository at this point in the history 
This change alignes UI5 linter with ESLint (see [1]):
* No errors, maybe warnings: Use exit code 0
* At least one error: Use exit code 1
* Unexpected error (typically caused by an exception): Use exit code 2

[1]: https://eslint.org/docs/latest/use/command-line-interface#exit-codes
  • Loading branch information
@RandomByte
RandomByte committed Mar 20, 2024
1 parent d313912 commit f395f78
Show file tree
Hide file tree
Showing 3 changed files with 35 additions and 76 deletions.
7 changes: 6 additions & 1 deletion src/cli/base.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -141,6 +141,11 @@ async function handleLint(argv: ArgumentsCamelCase<LinterArg>) {
if (profile) {
await profile.stop();
}

if (res.some((file) => !!file.errorCount)) {
// At least one error is reported. Exit with non-zero exit code.
process.exit(1);
}
}

export default function base(cli: Argv) {
Expand Down Expand Up @@ -191,7 +196,7 @@ export default function base(cli: Argv) {
process.stderr.write("\n");
process.stderr.write(chalk.dim(`See 'ui5lint --help'`) + "\n");
}
process.exit(1);
process.exit(2);
});

cli.command(lintCommand);
Expand Down
14 changes: 9 additions & 5 deletions test/lib/cli/base.integration.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,13 +13,15 @@ const test = anyTest as TestFn<{
consoleLogStub: SinonStub;
processCwdStub: SinonStub;
processStdoutWriteStub: SinonStub;
processExitStub: SinonStub;
cli: Argv;
}>;

test.beforeEach((t) => {
t.context.consoleLogStub = sinon.stub(console, "log");
t.context.processCwdStub = sinon.stub(process, "cwd").returns(sampleProjectPath);
t.context.processStdoutWriteStub = sinon.stub(process.stdout, "write").returns(true);
t.context.processExitStub = sinon.stub(process, "exit");
t.context.cli = yargs();
cliBase(t.context.cli);
});
Expand All @@ -30,15 +32,17 @@ test.afterEach.always(() => {

// Test if standard output is parsable JSON
test.serial("ui5lint --format json", async (t) => {
const {cli} = t.context;
const {cli, consoleLogStub, processCwdStub, processStdoutWriteStub, processExitStub} = t.context;

await cli.parseAsync(["--format", "json"]);

t.is(t.context.consoleLogStub.callCount, 0, "console.log should not be used");
t.true(t.context.processCwdStub.callCount > 0, "process.cwd was called");
t.is(t.context.processStdoutWriteStub.callCount, 1, "process.stdout.write is only used once");
t.is(consoleLogStub.callCount, 0, "console.log should not be used");
t.true(processCwdStub.callCount > 0, "process.cwd was called");
t.is(processStdoutWriteStub.callCount, 1, "process.stdout.write is only used once");
t.is(processExitStub.callCount, 1, "process.exit got called once");
t.is(processExitStub.firstCall.firstArg, 1, "process.exit got called with exit code 1");

const resultProcessStdout = t.context.processStdoutWriteStub.firstCall.firstArg;
const resultProcessStdout = processStdoutWriteStub.firstCall.firstArg;
let parsedJson: LintResult[];

t.notThrows(() => parsedJson = JSON.parse(resultProcessStdout), "Output of process.stdout.write is JSON-formatted");
Expand Down
90 changes: 20 additions & 70 deletions test/lib/cli/base.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ const test = anyTest as TestFn<{
processErrWrite: SinonStub;
formatText: SinonStub;
formatJson: SinonStub;
createExitStub: () => SinonStub;
cli: Argv;
base: typeof Base;
}>;
Expand Down Expand Up @@ -78,6 +79,10 @@ test.beforeEach(async (t) => {
});

t.context.base(t.context.cli);

t.context.createExitStub = () => {
return sinon.stub(process, "exit");
};
});

test.afterEach.always((t) => {
Expand Down Expand Up @@ -150,17 +155,8 @@ test.serial("ui5lint --format json ", async (t) => {
});

test.serial("Yargs error handling", async (t) => {
const {processStdErrWriteStub, consoleWriterStopStub, cli} = t.context;

const processExit = new Promise((resolve) => {
const processExitStub = sinon.stub(process, "exit");
// @ts-expect-error callsFake definition returns never, instead of void which is not that correct.
processExitStub.callsFake((errorCode) => {
processExitStub.restore();
resolve(errorCode);
});
});

const {processStdErrWriteStub, consoleWriterStopStub, cli, createExitStub} = t.context;
const processExitStub = createExitStub();
cli.command({
command: "foo",
describe: "This is a task",
Expand All @@ -170,9 +166,7 @@ test.serial("Yargs error handling", async (t) => {

await cli.parseAsync(["invalid"]);

const errorCode = await processExit;

t.is(errorCode, 1, "Should exit with error code 1");
t.is(processExitStub.firstCall.firstArg, 2, "Should exit with error code 2");
t.is(consoleWriterStopStub.callCount, 0, "ConsoleWriter.stop did not get called");
t.is(processStdErrWriteStub.callCount, 5);
t.deepEqual(processStdErrWriteStub.getCall(0).args, [
Expand All @@ -190,17 +184,8 @@ test.serial("Yargs error handling", async (t) => {
});

test.serial("Exception error handling", async (t) => {
const {cli, processStdErrWriteStub, consoleWriterStopStub} = t.context;

const processExit = new Promise((resolve) => {
const processExitStub = sinon.stub(process, "exit");
// @ts-expect-error callsFake definition returns never, instead of void which is not that correct.
processExitStub.callsFake((errorCode) => {
processExitStub.restore();
resolve(errorCode);
});
});

const {cli, processStdErrWriteStub, consoleWriterStopStub, createExitStub} = t.context;
const processExitStub = createExitStub();
const error = new Error("Some error from foo command");

cli.command({
Expand All @@ -215,9 +200,7 @@ test.serial("Exception error handling", async (t) => {
is: error,
});

const errorCode = await processExit;

t.is(errorCode, 1, "Should exit with error code 1");
t.is(processExitStub.firstCall.firstArg, 2, "Should exit with error code 2");
t.is(consoleWriterStopStub.callCount, 1, "ConsoleWriter.stop got called once");
t.is(processStdErrWriteStub.callCount, 7);
t.deepEqual(processStdErrWriteStub.getCall(1).args, [
Expand All @@ -236,19 +219,10 @@ test.serial("Exception error handling", async (t) => {
});

test.serial("Exception error handling without logging (silent)", async (t) => {
const {cli, processStdErrWriteStub, isLogLevelEnabledStub, consoleWriterStopStub} = t.context;

const {cli, processStdErrWriteStub, isLogLevelEnabledStub, consoleWriterStopStub, createExitStub} = t.context;
const processExitStub = createExitStub();
isLogLevelEnabledStub.withArgs("error").returns(false);

const processExit = new Promise((resolve) => {
const processExitStub = sinon.stub(process, "exit");
// @ts-expect-error callsFake definition returns never, instead of void which is not that correct.
processExitStub.callsFake((errorCode) => {
processExitStub.restore();
resolve(errorCode);
});
});

const error = new Error("Some error from foo command");

cli.command({
Expand All @@ -263,28 +237,17 @@ test.serial("Exception error handling without logging (silent)", async (t) => {
is: error,
});

const errorCode = await processExit;

t.is(errorCode, 1, "Should exit with error code 1");
t.is(processExitStub.firstCall.firstArg, 2, "Should exit with error code 2");
t.is(consoleWriterStopStub.callCount, 1, "ConsoleWriter.stop got called once");
t.is(processStdErrWriteStub.callCount, 0);
t.is(t.context.consoleLogStub.callCount, 0, "console.log should not be used");
});

test.serial("Exception error handling with verbose logging", async (t) => {
const {cli, processStdErrWriteStub, isLogLevelEnabledStub} = t.context;

const {cli, processStdErrWriteStub, isLogLevelEnabledStub, createExitStub} = t.context;
const processExitStub = createExitStub();
isLogLevelEnabledStub.withArgs("verbose").returns(true);

const processExit = new Promise((resolve) => {
const processExitStub = sinon.stub(process, "exit");
// @ts-expect-error callsFake definition returns never, instead of void which is not that correct.
processExitStub.callsFake((errorCode) => {
processExitStub.restore();
resolve(errorCode);
});
});

const error = new Error("Some error from foo command");

cli.command({
Expand All @@ -299,9 +262,7 @@ test.serial("Exception error handling with verbose logging", async (t) => {
is: error,
});

const errorCode = await processExit;

t.is(errorCode, 1, "Should exit with error code 1");
t.is(processExitStub.firstCall.firstArg, 2, "Should exit with error code 2");
t.is(processStdErrWriteStub.callCount, 10);
t.deepEqual(processStdErrWriteStub.getCall(1).args, [
chalk.bold.red("⚠️ Process Failed With Error") + "\n",
Expand All @@ -326,17 +287,8 @@ test.serial("Exception error handling with verbose logging", async (t) => {
});

test.serial("Unexpected error handling", async (t) => {
const {processStdErrWriteStub, consoleWriterStopStub, cli} = t.context;

const processExit = new Promise((resolve) => {
const processExitStub = sinon.stub(process, "exit");
// @ts-expect-error callsFake definition returns never, instead of void which is not that correct.
processExitStub.callsFake((errorCode) => {
processExitStub.restore();
resolve(errorCode);
});
});

const {processStdErrWriteStub, consoleWriterStopStub, cli, createExitStub} = t.context;
const processExitStub = createExitStub();
const typeError = new TypeError("Cannot do this");

cli.command({
Expand All @@ -351,9 +303,7 @@ test.serial("Unexpected error handling", async (t) => {
is: typeError,
});

const errorCode = await processExit;

t.is(errorCode, 1, "Should exit with error code 1");
t.is(processExitStub.firstCall.firstArg, 2, "Should exit with error code 2");
t.is(consoleWriterStopStub.callCount, 1, "ConsoleWriter.stop got called once");
t.is(processStdErrWriteStub.callCount, 10);
t.deepEqual(processStdErrWriteStub.getCall(1).args, [
Expand Down

0 comments on commit f395f78

Please sign in to comment.