diff --git a/src/commands/studio/manager/package.manager.ts b/src/commands/studio/manager/package.manager.ts index 46e5f750..a5a5f512 100644 --- a/src/commands/studio/manager/package.manager.ts +++ b/src/commands/studio/manager/package.manager.ts @@ -126,7 +126,7 @@ export class PackageManager extends BaseManager { logger.error( "You cannot overwrite a package and set a new key at the same time. Please use only one of the options." ); - process.exit(); + process.exit(1); } } diff --git a/src/core/command/module-handler.ts b/src/core/command/module-handler.ts index 5c94ad11..d4853679 100644 --- a/src/core/command/module-handler.ts +++ b/src/core/command/module-handler.ts @@ -2,7 +2,7 @@ import path = require("path"); import * as fs from "fs"; import { Command, CommandOptions, Option, OptionValues } from "commander"; import { Context } from "./cli-context"; -import { logger } from "../utils/logger"; +import { GracefulError, logger } from "../utils/logger"; import * as chalk from "chalk"; export abstract class IModule { @@ -216,7 +216,12 @@ export class CommandConfig { this.printDeprecationNoticeIfDeprecated(); await handler(this.ctx, this.cmd, this.cmd.optsWithGlobals()); } catch (error) { + if (error instanceof GracefulError) { + logger.error(error.message); + return; + } logger.error(`An unexpected error occured executing a command: ${error}`); + process.exitCode = 1; } }); } diff --git a/tests/commands/studio/package.manager.spec.ts b/tests/commands/studio/package.manager.spec.ts new file mode 100644 index 00000000..2c46acdf --- /dev/null +++ b/tests/commands/studio/package.manager.spec.ts @@ -0,0 +1,34 @@ +import { PackageManager } from "../../../src/commands/studio/manager/package.manager"; +import { loggingTestTransport } from "../../jest.setup"; +import { testContext } from "../../utls/test-context"; + +describe("PackageManager", () => { + afterEach(() => { + jest.restoreAllMocks(); + }); + + it("exits with code 1 when overwrite and new key are used together", () => { + const exitSignal = new Error("process.exit(1)"); + const exitSpy = jest.spyOn(process, "exit").mockImplementation((() => { + throw exitSignal; + }) as never); + loggingTestTransport.logMessages = []; + + const manager = new PackageManager(testContext); + manager.spaceKey = "space-id"; + manager.key = "my-package"; + manager.newKey = "renamed-package"; + manager.overwrite = true; + manager.store = false; + manager.draft = false; + + expect(() => manager.getConfig()).toThrow(exitSignal); + + expect(exitSpy).toHaveBeenCalledWith(1); + expect( + loggingTestTransport.logMessages.some(entry => + String(entry.message).includes("You cannot overwrite a package and set a new key at the same time") + ) + ).toBe(true); + }); +}); diff --git a/tests/core/command/module-handler.spec.ts b/tests/core/command/module-handler.spec.ts new file mode 100644 index 00000000..956851d9 --- /dev/null +++ b/tests/core/command/module-handler.spec.ts @@ -0,0 +1,62 @@ +import { Command } from "commander"; +import { Configurator } from "../../../src/core/command/module-handler"; +import { GracefulError } from "../../../src/core/utils/logger"; +import { loggingTestTransport } from "../../jest.setup"; +import { testContext } from "../../utls/test-context"; + +describe("CommandConfig action error handling", () => { + let previousExitCode: number | undefined; + + beforeEach(() => { + previousExitCode = process.exitCode; + process.exitCode = 0; + loggingTestTransport.logMessages = []; + }); + + afterEach(() => { + process.exitCode = previousExitCode; + }); + + async function runCommand(handler: () => Promise): Promise { + const program = new Command(); + const configurator = new Configurator(program, testContext); + + configurator.command("test-command").action(async () => { + await handler(); + }); + + program.exitOverride(); + await program.parseAsync(["node", "content-cli", "test-command"]); + } + + it("logs a graceful error and keeps exitCode at zero", async () => { + await runCommand(async () => { + throw new GracefulError("graceful failure"); + }); + + expect(process.exitCode ?? 0).toBe(0); + expect( + loggingTestTransport.logMessages.some(entry => + String(entry.message).includes("graceful failure") + ) + ).toBe(true); + expect( + loggingTestTransport.logMessages.some(entry => + String(entry.message).includes("An unexpected error occured executing a command") + ) + ).toBe(false); + }); + + it("logs unexpected errors and marks the process as failed", async () => { + await runCommand(async () => { + throw new Error("boom"); + }); + + expect(process.exitCode).toBe(1); + expect( + loggingTestTransport.logMessages.some(entry => + String(entry.message).includes("An unexpected error occured executing a command") + ) + ).toBe(true); + }); +}); diff --git a/tests/integration/commands/asset-registry.spec.ts b/tests/integration/commands/asset-registry.spec.ts index 3829b358..0ab40c5e 100644 --- a/tests/integration/commands/asset-registry.spec.ts +++ b/tests/integration/commands/asset-registry.spec.ts @@ -1,12 +1,11 @@ import Module = require("../../../src/commands/asset-registry/module"); -import { Command } from "commander"; import { AssetRegistryService } from "../../../src/commands/asset-registry/asset-registry.service"; -import { buildTestProgram } from "../../utls/cli-program"; +import { CliRunResult, runCli as runCliProcess } from "../../utls/cli-runner"; +import { GracefulError } from "../../../src/core/utils/logger"; jest.mock("../../../src/commands/asset-registry/asset-registry.service"); describe("asset-registry command integration", () => { - let program: Command; let mockService: jest.Mocked; beforeEach(() => { @@ -21,12 +20,10 @@ describe("asset-registry command integration", () => { (AssetRegistryService as jest.MockedClass) .mockImplementation(() => mockService); - - program = buildTestProgram([Module]); }); - function runCli(args: string[]): Promise { - return program.parseAsync(["node", "content-cli", ...args]); + async function runCli(args: string[]): Promise { + return runCliProcess(args, [Module]); } describe("asset-registry schema", () => { @@ -141,4 +138,44 @@ describe("asset-registry command integration", () => { expect(mockService.getType).toHaveBeenCalledWith("BOARD_V2", true); }); }); + + describe("exit codes", () => { + it("exits with code 0 on a successful command", async () => { + const result = await runCli(["asset-registry", "list"]); + + expect(result.exitCode).toBe(0); + }); + + it("exits non-zero and reports the error when the service fails", async () => { + mockService.listTypes.mockRejectedValueOnce(new Error("Asset registry feature is disabled")); + + const result = await runCli(["asset-registry", "list"]); + + expect(result.exitCode).toBe(1); + expect(result.output).toContain("Asset registry feature is disabled"); + }); + + it("exits with code 0 when the service raises a GracefulError", async () => { + mockService.listTypes.mockRejectedValueOnce(new GracefulError("Nothing to list")); + + const result = await runCli(["asset-registry", "list"]); + + expect(result.exitCode).toBe(0); + expect(result.output).toContain("Nothing to list"); + }); + + it("exits non-zero for an unknown command", async () => { + const result = await runCli(["this-command-does-not-exist"]); + + expect(result.exitCode).not.toBe(0); + expect(mockService.listTypes).not.toHaveBeenCalled(); + }); + + it("exits non-zero when a required option is missing", async () => { + const result = await runCli(["asset-registry", "get"]); + + expect(result.exitCode).not.toBe(0); + expect(mockService.getType).not.toHaveBeenCalled(); + }); + }); }); diff --git a/tests/integration/commands/configuration-management.spec.ts b/tests/integration/commands/configuration-management.spec.ts index c77636bc..7864e492 100644 --- a/tests/integration/commands/configuration-management.spec.ts +++ b/tests/integration/commands/configuration-management.spec.ts @@ -1,5 +1,4 @@ import Module = require("../../../src/commands/configuration-management/module"); -import { Command } from "commander"; import { ConfigCommandService } from "../../../src/commands/configuration-management/config-command.service"; import { StagingPackageService } from "../../../src/commands/configuration-management/staging-package.service"; import { MetadataService } from "../../../src/commands/configuration-management/metadata.service"; @@ -9,8 +8,7 @@ import { PackageVersionCommandService } from "../../../src/commands/configuratio import { NodeDiffService } from "../../../src/commands/configuration-management/node-diff.service"; import { SinglePackageImportService } from "../../../src/commands/configuration-management/single-package-import.service"; import { SinglePackageExportService } from "../../../src/commands/configuration-management/single-package-export.service"; -import { buildTestProgram } from "../../utls/cli-program"; -import { loggingTestTransport } from "../../jest.setup"; +import { CliRunResult, runCli as runCliProcess } from "../../utls/cli-runner"; jest.mock("../../../src/commands/configuration-management/config-command.service"); jest.mock("../../../src/commands/configuration-management/staging-package.service"); @@ -23,7 +21,6 @@ jest.mock("../../../src/commands/configuration-management/single-package-import. jest.mock("../../../src/commands/configuration-management/single-package-export.service"); describe("configuration-management command integration", () => { - let program: Command; let mockConfigCommandService: jest.Mocked; let mockStagingPackageService: jest.Mocked; let mockMetadataService: jest.Mocked; @@ -85,28 +82,25 @@ describe("configuration-management command integration", () => { (PackageVersionCommandService as jest.MockedClass).mockImplementation(() => mockPackageVersionCommandService); (SinglePackageImportService as jest.MockedClass).mockImplementation(() => mockSinglePackageImportService); (SinglePackageExportService as jest.MockedClass).mockImplementation(() => mockSinglePackageExportService); - - program = buildTestProgram([Module]); }); - function runCli(args: string[]): Promise { - return program.parseAsync(["node", "content-cli", ...args]); + let lastResult: CliRunResult; + + async function runCli(args: string[]): Promise { + lastResult = await runCliProcess(args, [Module]); + return lastResult; } /** * Action-body validation errors (`throw new Error(...)`) are caught by - * Configurator.action and re-emitted via `logger.error(...)`, so we - * inspect the in-memory winston transport instead of asserting on - * promise rejection. The level field is colorized by `winston.format.cli()`, - * hence the substring match. + * Configurator.action, surfaced on the real process output and reflected + * in a non-zero exit code. We assert on the actual captured output and + * exit code of the last run rather than inspecting an injected logger + * transport. */ - function expectErrorLogged(message: string): void { - expect(loggingTestTransport.logMessages).toEqual(expect.arrayContaining([ - expect.objectContaining({ - level: expect.stringContaining("error"), - message: expect.stringContaining(message), - }), - ])); + function expectError(message: string): void { + expect(lastResult.exitCode).toBe(1); + expect(lastResult.output).toContain(message); } describe("config list (deprecated listPackages)", () => { @@ -117,17 +111,18 @@ describe("configuration-management command integration", () => { "--keysByVersion", "package3.1.0.0", "package4.1.0.0", ]); - expectErrorLogged("Please provide either --packageKeys or --keysByVersion, but not both."); + expectError("Please provide either --packageKeys or --keysByVersion, but not both."); expect(mockT2tcCommandService.listPackages).not.toHaveBeenCalled(); }); it("forwards only --packageKeys when provided", async () => { - await runCli([ + const result = await runCli([ "config", "list", "--packageKeys", "package1", "package2", "--json", ]); + expect(result.exitCode).toBe(0); expect(mockT2tcCommandService.listPackages).toHaveBeenCalledWith( true, undefined, @@ -201,7 +196,7 @@ describe("configuration-management command integration", () => { "--packageKeys", "package1", "package2", ]); - expectErrorLogged( + expectError( "Staging parameter is not compatible with --withDependencies, --packageKeys, --keysByVersion, --variableValue, --variableType" ); }); @@ -209,7 +204,7 @@ describe("configuration-management command integration", () => { it("rejects --staging combined with --withDependencies", async () => { await runCli(["config", "list", "--staging", "--withDependencies"]); - expectErrorLogged( + expectError( "Staging parameter is not compatible with --withDependencies, --packageKeys, --keysByVersion, --variableValue, --variableType" ); }); @@ -221,7 +216,7 @@ describe("configuration-management command integration", () => { "--keysByVersion", "package3.1.0.0", "package4.1.0.0", ]); - expectErrorLogged( + expectError( "Staging parameter is not compatible with --withDependencies, --packageKeys, --keysByVersion, --variableValue, --variableType" ); }); @@ -229,7 +224,7 @@ describe("configuration-management command integration", () => { it("rejects --staging combined with --variableValue", async () => { await runCli(["config", "list", "--staging", "--variableValue", "myValue"]); - expectErrorLogged( + expectError( "Staging parameter is not compatible with --withDependencies, --packageKeys, --keysByVersion, --variableValue, --variableType" ); }); @@ -237,7 +232,7 @@ describe("configuration-management command integration", () => { it("rejects --staging combined with --variableType", async () => { await runCli(["config", "list", "--staging", "--variableType", "myType"]); - expectErrorLogged( + expectError( "Staging parameter is not compatible with --withDependencies, --packageKeys, --keysByVersion, --variableValue, --variableType" ); }); @@ -281,14 +276,14 @@ describe("configuration-management command integration", () => { "--keysByVersion", "package3:v1", "package4:v2", ]); - expectErrorLogged("Please provide either --packageKeys or --keysByVersion, but not both."); + expectError("Please provide either --packageKeys or --keysByVersion, but not both."); expect(mockT2tcCommandService.batchExportPackages).not.toHaveBeenCalled(); }); it("rejects when neither --packageKeys nor --keysByVersion are provided", async () => { await runCli(["config", "export"]); - expectErrorLogged("Please provide either --packageKeys or --keysByVersion, but not both."); + expectError("Please provide either --packageKeys or --keysByVersion, but not both."); expect(mockT2tcCommandService.batchExportPackages).not.toHaveBeenCalled(); }); @@ -323,7 +318,7 @@ describe("configuration-management command integration", () => { "--gitProfile", "myProfile", ]); - expectErrorLogged("Please specify a branch using --gitBranch when using a Git profile."); + expectError("Please specify a branch using --gitBranch when using a Git profile."); expect(mockT2tcCommandService.batchExportPackages).not.toHaveBeenCalled(); }); @@ -384,7 +379,7 @@ describe("configuration-management command integration", () => { "--gitProfile", "myProfile", ]); - expectErrorLogged("Please provide either --packageKeys or --keysByVersion, but not both."); + expectError("Please provide either --packageKeys or --keysByVersion, but not both."); expect(mockT2tcCommandService.batchExportPackages).not.toHaveBeenCalled(); }); }); @@ -397,7 +392,7 @@ describe("configuration-management command integration", () => { "--gitProfile", "myProfile", ]); - expectErrorLogged("Please specify a branch using --gitBranch when using a Git profile."); + expectError("Please specify a branch using --gitBranch when using a Git profile."); expect(mockT2tcCommandService.batchImportPackages).not.toHaveBeenCalled(); }); @@ -553,7 +548,7 @@ describe("configuration-management command integration", () => { "--gitProfile", "myProfile", ]); - expectErrorLogged("Please specify a branch using --gitBranch when using a Git profile."); + expectError("Please specify a branch using --gitBranch when using a Git profile."); expect(mockSinglePackageImportService.importPackage).not.toHaveBeenCalled(); }); }); @@ -601,7 +596,7 @@ describe("configuration-management command integration", () => { "--gitProfile", "myProfile", ]); - expectErrorLogged("Please specify a branch using --gitBranch when using a Git profile."); + expectError("Please specify a branch using --gitBranch when using a Git profile."); expect(mockSinglePackageExportService.exportPackage).not.toHaveBeenCalled(); }); }); @@ -614,7 +609,7 @@ describe("configuration-management command integration", () => { "--keysByVersion", "key-1:1.0.0", ]); - expectErrorLogged( + expectError( "Please provide either --packageKeys or --keysByVersion/--keysByVersionFile, but not both." ); expect(mockConfigCommandService.listVariables).not.toHaveBeenCalled(); @@ -627,7 +622,7 @@ describe("configuration-management command integration", () => { "--keysByVersionFile", "mapping.json", ]); - expectErrorLogged( + expectError( "Please provide either --packageKeys or --keysByVersion/--keysByVersionFile, but not both." ); expect(mockConfigCommandService.listVariables).not.toHaveBeenCalled(); @@ -636,7 +631,7 @@ describe("configuration-management command integration", () => { it("rejects when neither staging nor versioned inputs are provided", async () => { await runCli(["config", "variables", "list"]); - expectErrorLogged( + expectError( "Please provide --packageKeys for staging, or --keysByVersion / --keysByVersionFile for versioned packages." ); expect(mockConfigCommandService.listVariables).not.toHaveBeenCalled(); @@ -681,7 +676,7 @@ describe("configuration-management command integration", () => { "--versionBumpOption", "PATCH", ]); - expectErrorLogged("Please provide either --packageVersion or --versionBumpOption, but not both."); + expectError("Please provide either --packageVersion or --versionBumpOption, but not both."); expect(mockPackageVersionCommandService.createPackageVersion).not.toHaveBeenCalled(); }); @@ -692,7 +687,7 @@ describe("configuration-management command integration", () => { "--versionBumpOption", "NONE", ]); - expectErrorLogged("Please provide either --packageVersion or --versionBumpOption PATCH."); + expectError("Please provide either --packageVersion or --versionBumpOption PATCH."); expect(mockPackageVersionCommandService.createPackageVersion).not.toHaveBeenCalled(); }); @@ -702,7 +697,7 @@ describe("configuration-management command integration", () => { "--packageKey", "my-package", ]); - expectErrorLogged("Please provide either --packageVersion or --versionBumpOption PATCH."); + expectError("Please provide either --packageVersion or --versionBumpOption PATCH."); expect(mockPackageVersionCommandService.createPackageVersion).not.toHaveBeenCalled(); }); @@ -859,7 +854,7 @@ describe("configuration-management command integration", () => { "--file", "./node.json", ]); - expectErrorLogged("Please provide either --compareVersion or --file, but not both."); + expectError("Please provide either --compareVersion or --file, but not both."); expect(mockNodeDiffService.diff).not.toHaveBeenCalled(); expect(mockNodeDiffService.diffWithFile).not.toHaveBeenCalled(); }); @@ -872,7 +867,7 @@ describe("configuration-management command integration", () => { "--baseVersion", "STAGING", ]); - expectErrorLogged("Please provide either --compareVersion or --file, but not both."); + expectError("Please provide either --compareVersion or --file, but not both."); expect(mockNodeDiffService.diff).not.toHaveBeenCalled(); expect(mockNodeDiffService.diffWithFile).not.toHaveBeenCalled(); }); diff --git a/tests/utls/cli-runner.ts b/tests/utls/cli-runner.ts new file mode 100644 index 00000000..241d00f9 --- /dev/null +++ b/tests/utls/cli-runner.ts @@ -0,0 +1,77 @@ +import { IModuleConstructor } from "../../src/core/command/module-handler"; +import { buildTestProgram } from "./cli-program"; + +export interface CliRunResult { + stdout: string; + stderr: string; + output: string; + exitCode: number; +} + +const ANSI_PATTERN = /\x1B\[[0-9;]*m/g; +const stripAnsi = (value: string): string => value.replace(ANSI_PATTERN, ""); + +class ExitSignal extends Error { + constructor(public readonly code: number) { + super(`process.exit(${code})`); + } +} + +export async function runCli(args: string[], modules: IModuleConstructor[]): Promise { + let stdout = ""; + let stderr = ""; + let exitCode = 0; + + const stdoutSpy = jest + .spyOn(process.stdout, "write") + .mockImplementation(((chunk: any): boolean => { + stdout += typeof chunk === "string" ? chunk : chunk.toString(); + return true; + }) as typeof process.stdout.write); + + const stderrSpy = jest + .spyOn(process.stderr, "write") + .mockImplementation(((chunk: any): boolean => { + stderr += typeof chunk === "string" ? chunk : chunk.toString(); + return true; + }) as typeof process.stderr.write); + + const exitSpy = jest.spyOn(process, "exit").mockImplementation(((code?: number) => { + throw new ExitSignal(code ?? 0); + }) as never); + + const previousExitCode = process.exitCode; + process.exitCode = 0; + + try { + const program = buildTestProgram(modules); + program.exitOverride(); + await program.parseAsync(["node", "content-cli", ...args]); + } catch (error) { + if (error instanceof ExitSignal) { + exitCode = error.code; + } else if (error && typeof (error as { code?: unknown }).code === "string" + && (error as { code: string }).code.startsWith("commander.")) { + exitCode = (error as { exitCode?: number }).exitCode ?? 0; + } else { + throw error; + } + } finally { + if (exitCode === 0 && process.exitCode && Number(process.exitCode) !== 0) { + exitCode = Number(process.exitCode); + } + process.exitCode = previousExitCode; + stdoutSpy.mockRestore(); + stderrSpy.mockRestore(); + exitSpy.mockRestore(); + } + + const cleanStdout = stripAnsi(stdout); + const cleanStderr = stripAnsi(stderr); + return { + stdout: cleanStdout, + stderr: cleanStderr, + output: cleanStdout + cleanStderr, + exitCode, + }; +}