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

feat(cli)!: In case of errors, exit with code 1 #41

Merged
merged 1 commit into from
Mar 22, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
7 changes: 6 additions & 1 deletion src/cli/base.ts
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
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
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