Skip to content

Commit 85c04e4

Browse files
committed
SP-1090: assert real CLI output and exit codes; fail commands with non-zero exit
1 parent 1c15350 commit 85c04e4

4 files changed

Lines changed: 231 additions & 2 deletions

File tree

src/commands/studio/manager/package.manager.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -126,7 +126,7 @@ export class PackageManager extends BaseManager {
126126
logger.error(
127127
"You cannot overwrite a package and set a new key at the same time. Please use only one of the options."
128128
);
129-
process.exit();
129+
process.exit(1);
130130
}
131131
}
132132

src/core/command/module-handler.ts

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ import path = require("path");
22
import * as fs from "fs";
33
import { Command, CommandOptions, Option, OptionValues } from "commander";
44
import { Context } from "./cli-context";
5-
import { logger } from "../utils/logger";
5+
import { GracefulError, logger } from "../utils/logger";
66
import * as chalk from "chalk";
77

88
export abstract class IModule {
@@ -216,7 +216,12 @@ export class CommandConfig {
216216
this.printDeprecationNoticeIfDeprecated();
217217
await handler(this.ctx, this.cmd, this.cmd.optsWithGlobals());
218218
} catch (error) {
219+
if (error instanceof GracefulError) {
220+
logger.error(error.message);
221+
return;
222+
}
219223
logger.error(`An unexpected error occured executing a command: ${error}`);
224+
process.exitCode = 1;
220225
}
221226
});
222227
}
Lines changed: 112 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,112 @@
1+
import { Command, OptionValues } from "commander";
2+
import { runCli } from "../utls/cli-runner";
3+
import { mockAxiosGet, mockAxiosGetError } from "../utls/http-requests-mock";
4+
import {
5+
ASSET_REGISTRY_DISABLED_ERROR,
6+
ASSET_REGISTRY_DISABLED_USER_MESSAGE,
7+
} from "../../src/commands/asset-registry/asset-registry-error";
8+
import { AssetRegistryMetadata } from "../../src/commands/asset-registry/asset-registry.interfaces";
9+
import { Configurator, IModule } from "../../src/core/command/module-handler";
10+
import { Context } from "../../src/core/command/cli-context";
11+
import { FatalError, GracefulError } from "../../src/core/utils/logger";
12+
13+
import AssetRegistryModule = require("../../src/commands/asset-registry/module");
14+
15+
const GRACEFUL_MESSAGE = "graceful failure - should not fail the process";
16+
17+
class DiagnosticsModule extends IModule {
18+
public register(context: Context, configurator: Configurator): void {
19+
const diag = configurator.command("diag").description("Diagnostics test commands");
20+
21+
diag.command("graceful")
22+
.description("Throws a GracefulError")
23+
.action(async (_ctx: Context, _cmd: Command, _opts: OptionValues): Promise<void> => {
24+
throw new GracefulError(GRACEFUL_MESSAGE);
25+
});
26+
27+
diag.command("fatal")
28+
.description("Throws a FatalError")
29+
.action(async (_ctx: Context, _cmd: Command, _opts: OptionValues): Promise<void> => {
30+
throw new FatalError("boom");
31+
});
32+
}
33+
}
34+
35+
describe("CLI process output and exit codes", () => {
36+
const TYPES_URL = "https://myTeam.celonis.cloud/pacman/api/core/asset-registry/types";
37+
38+
const metadata: AssetRegistryMetadata = {
39+
types: {
40+
BOARD_V2: {
41+
assetType: "BOARD_V2",
42+
displayName: "View",
43+
description: null,
44+
group: "DASHBOARDS",
45+
assetSchema: { version: "2.1.0" },
46+
service: { basePath: "/blueprint/api" },
47+
endpoints: {
48+
schema: "/schema/board_v2",
49+
validate: "/validate/board_v2",
50+
examples: "/examples/board_v2",
51+
},
52+
contributions: { pigEntityTypes: [], dataPipelineEntityTypes: [], actionTypes: [] },
53+
},
54+
},
55+
};
56+
57+
describe("successful commands", () => {
58+
it("Should print the version and exit with code 0", async () => {
59+
const result = await runCli(["-V"], [AssetRegistryModule]);
60+
61+
expect(result.exitCode).toBe(0);
62+
expect(result.stdout).toContain("test-version");
63+
});
64+
65+
it("Should print asset types to stdout and exit with code 0", async () => {
66+
mockAxiosGet(TYPES_URL, metadata);
67+
68+
const result = await runCli(["asset-registry", "list"], [AssetRegistryModule]);
69+
70+
expect(result.exitCode).toBe(0);
71+
expect(result.output).toContain("BOARD_V2 - View [DASHBOARDS]");
72+
});
73+
});
74+
75+
describe("failing commands produce a non-zero exit code", () => {
76+
it("Should exit non-zero and report the friendly message when the feature flag is disabled", async () => {
77+
mockAxiosGetError(TYPES_URL, 403, { error: ASSET_REGISTRY_DISABLED_ERROR });
78+
79+
const result = await runCli(["asset-registry", "list"], [AssetRegistryModule]);
80+
81+
expect(result.exitCode).toBe(1);
82+
expect(result.output).toContain(ASSET_REGISTRY_DISABLED_USER_MESSAGE);
83+
});
84+
85+
it("Should exit non-zero for an unknown command", async () => {
86+
const result = await runCli(["this-command-does-not-exist"], [AssetRegistryModule]);
87+
88+
expect(result.exitCode).toBe(1);
89+
});
90+
91+
it("Should exit non-zero when a required option is missing", async () => {
92+
const result = await runCli(["asset-registry", "get"], [AssetRegistryModule]);
93+
94+
expect(result.exitCode).toBe(1);
95+
});
96+
});
97+
98+
describe("action-wrapper error semantics", () => {
99+
it("Should report a GracefulError without forcing a non-zero exit code", async () => {
100+
const result = await runCli(["diag", "graceful"], [DiagnosticsModule]);
101+
102+
expect(result.exitCode).toBe(0);
103+
expect(result.output).toContain(GRACEFUL_MESSAGE);
104+
});
105+
106+
it("Should exit non-zero for a regular error thrown by a command", async () => {
107+
const result = await runCli(["diag", "fatal"], [DiagnosticsModule]);
108+
109+
expect(result.exitCode).toBe(1);
110+
});
111+
});
112+
});

tests/utls/cli-runner.ts

Lines changed: 112 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,112 @@
1+
import { Command } from "commander";
2+
import { IModuleConstructor, ModuleHandler } from "../../src/core/command/module-handler";
3+
import { Context } from "../../src/core/command/cli-context";
4+
import { HttpClient } from "../../src/core/http/http-client";
5+
6+
export interface CliRunResult {
7+
stdout: string;
8+
stderr: string;
9+
output: string;
10+
exitCode: number;
11+
}
12+
13+
const ANSI_PATTERN = /\x1B\[[0-9;]*m/g;
14+
const stripAnsi = (value: string): string => value.replace(ANSI_PATTERN, "");
15+
16+
class ExitSignal extends Error {
17+
constructor(public readonly code: number) {
18+
super(`process.exit(${code})`);
19+
}
20+
}
21+
22+
function buildTestContext(): Context {
23+
const context = new Context({});
24+
context.profile = {
25+
name: "test",
26+
type: "Key",
27+
team: "https://myTeam.celonis.cloud/",
28+
apiToken: "test-token",
29+
authenticationType: "Bearer",
30+
};
31+
context._httpClient = new HttpClient(context);
32+
return context;
33+
}
34+
35+
function buildProgram(context: Context, modules: IModuleConstructor[]): Command {
36+
const program = new Command();
37+
program.exitOverride();
38+
program.version("test-version");
39+
program.option("-q, --quietmode", "Reduce output to a minimum", false);
40+
program.option("-p, --profile [profile]");
41+
program.option("--gitProfile [gitProfile]", "Git profile to use");
42+
program.option("--debug", "Print debug messages", false);
43+
program.option("--dev", "Development Mode", false);
44+
45+
const moduleHandler = new ModuleHandler(program, context);
46+
moduleHandler.configurator.command("list").description("Commands to list content.").alias("ls");
47+
48+
for (const ModuleClass of modules) {
49+
new ModuleClass().register(context, moduleHandler.configurator);
50+
}
51+
52+
return program;
53+
}
54+
55+
export async function runCli(args: string[], modules: IModuleConstructor[]): Promise<CliRunResult> {
56+
let stdout = "";
57+
let stderr = "";
58+
let exitCode = 0;
59+
60+
const stdoutSpy = jest
61+
.spyOn(process.stdout, "write")
62+
.mockImplementation(((chunk: any): boolean => {
63+
stdout += typeof chunk === "string" ? chunk : chunk.toString();
64+
return true;
65+
}) as typeof process.stdout.write);
66+
67+
const stderrSpy = jest
68+
.spyOn(process.stderr, "write")
69+
.mockImplementation(((chunk: any): boolean => {
70+
stderr += typeof chunk === "string" ? chunk : chunk.toString();
71+
return true;
72+
}) as typeof process.stderr.write);
73+
74+
const exitSpy = jest.spyOn(process, "exit").mockImplementation(((code?: number) => {
75+
throw new ExitSignal(code ?? 0);
76+
}) as never);
77+
78+
const previousExitCode = process.exitCode;
79+
process.exitCode = 0;
80+
81+
try {
82+
const context = buildTestContext();
83+
const program = buildProgram(context, modules);
84+
await program.parseAsync(["node", "content-cli", ...args]);
85+
} catch (error) {
86+
if (error instanceof ExitSignal) {
87+
exitCode = error.code;
88+
} else if (error && typeof (error as { code?: unknown }).code === "string"
89+
&& (error as { code: string }).code.startsWith("commander.")) {
90+
exitCode = (error as { exitCode?: number }).exitCode ?? 0;
91+
} else {
92+
throw error;
93+
}
94+
} finally {
95+
if (exitCode === 0 && process.exitCode && Number(process.exitCode) !== 0) {
96+
exitCode = Number(process.exitCode);
97+
}
98+
process.exitCode = previousExitCode;
99+
stdoutSpy.mockRestore();
100+
stderrSpy.mockRestore();
101+
exitSpy.mockRestore();
102+
}
103+
104+
const cleanStdout = stripAnsi(stdout);
105+
const cleanStderr = stripAnsi(stderr);
106+
return {
107+
stdout: cleanStdout,
108+
stderr: cleanStderr,
109+
output: cleanStdout + cleanStderr,
110+
exitCode,
111+
};
112+
}

0 commit comments

Comments
 (0)