From 5c19af63cb0c4fc9f37cfbe060efd17ed23bf727 Mon Sep 17 00:00:00 2001 From: "a.jashari" Date: Mon, 13 Jul 2026 18:13:29 +0200 Subject: [PATCH 1/3] SP-1383: gate early-access commands behind feature flag, replace beta Add an earlyAccess(featureKey?) marker (replacing beta): commands with a key are checked against the backend feature flag at run time and blocked with a clear message when not enabled for the team. No labels shown in help. Consolidates the old beta marker and removes the (beta) help decoration. Co-Authored-By: Claude Opus 4.8 --- .gitignore | 3 + src/commands/deployment/module.ts | 14 +-- src/content-cli.ts | 6 -- src/core/command/CustomHelp.ts | 19 ---- src/core/command/module-handler.ts | 60 +++++++------ .../feature-flag/early-access-features.ts | 6 ++ src/core/feature-flag/feature-flag.service.ts | 25 ++++++ tests/commands/action-flows/module.spec.ts | 19 ++++ tests/commands/data-pipeline/module.spec.ts | 23 +++++ tests/commands/deployment/module.spec.ts | 44 +++++++++ tests/core/command/module-handler.spec.ts | 90 +++++++++++++++++++ .../feature-flag/feature-flag.service.spec.ts | 53 +++++++++++ .../integration/commands/early-access.spec.ts | 63 +++++++++++++ tests/utls/configurator-mock.ts | 6 +- 14 files changed, 369 insertions(+), 62 deletions(-) delete mode 100644 src/core/command/CustomHelp.ts create mode 100644 src/core/feature-flag/early-access-features.ts create mode 100644 src/core/feature-flag/feature-flag.service.ts create mode 100644 tests/commands/action-flows/module.spec.ts create mode 100644 tests/commands/data-pipeline/module.spec.ts create mode 100644 tests/commands/deployment/module.spec.ts create mode 100644 tests/core/feature-flag/feature-flag.service.spec.ts create mode 100644 tests/integration/commands/early-access.spec.ts diff --git a/.gitignore b/.gitignore index 7363130f..74284100 100644 --- a/.gitignore +++ b/.gitignore @@ -15,3 +15,6 @@ npm-debug.log .idea *.iml .DS_Store + +# Test coverage output +coverage/ diff --git a/src/commands/deployment/module.ts b/src/commands/deployment/module.ts index 0cf9ead1..41fb0d29 100644 --- a/src/commands/deployment/module.ts +++ b/src/commands/deployment/module.ts @@ -6,11 +6,11 @@ import { DeploymentService } from "./deployment.service"; class Module extends IModule { public register(context: Context, configurator: Configurator): void { - const deploymentCommand = configurator.command("deployment").beta() + const deploymentCommand = configurator.command("deployment").earlyAccess() .description("Create deployments, list their history, check active deployments, and retrieve deployables and targets"); deploymentCommand.command("create") - .beta() + .earlyAccess() .description("Create a new deployment") .requiredOption("--packageKey ", "Identifier of the package to deploy") .requiredOption("--packageVersion ", "Version of the package to deploy") @@ -20,10 +20,10 @@ class Module extends IModule { .action(this.createDeployment); const listCommand = deploymentCommand.command("list") - .description("List deployment history, active deployments, deployables or targets").beta(); + .description("List deployment history, active deployments, deployables or targets").earlyAccess(); listCommand.command("history") - .beta() + .earlyAccess() .description("List deployment history") .option("--packageKey ", "Filter deployment history by package key") .option("--targetId ", "Filter deployment history by target ID") @@ -36,7 +36,7 @@ class Module extends IModule { .action(this.listDeploymentHistory); listCommand.command("active") - .beta() + .earlyAccess() .description("Get the active deployment(s) for a given target or package.\n"+ "You can use the command to list the active deployment(s) for a specific target or for a specific package.\n" + "The targetIds filter is available only for getting the active deployments for a given package. \n" + @@ -50,14 +50,14 @@ class Module extends IModule { .action(this.listActiveDeployments); listCommand.command("deployables") - .beta() + .earlyAccess() .description("List all deployables") .option("--flavor ", "Filter deployables by flavor") .option("--json", "Return the response as a JSON file") .action(this.listDeployables); listCommand.command("targets") - .beta() + .earlyAccess() .description("List all targets for a given deployable type and package key") .requiredOption("--deployableType ", "The type of the deployable") .requiredOption("--packageKey ", "Identifier of the package to list targets for") diff --git a/src/content-cli.ts b/src/content-cli.ts index c0749714..bca633e5 100644 --- a/src/content-cli.ts +++ b/src/content-cli.ts @@ -6,7 +6,6 @@ import { Configurator, IModuleConstructor, ModuleHandler } from "./core/command/ import { Context } from "./core/command/cli-context"; import { VersionUtils } from "./core/utils/version"; import { logger } from "./core/utils/logger"; -import { ContentCLIHelp } from "./core/command/CustomHelp"; /** * Celonis Content CLI. @@ -38,11 +37,6 @@ function addDefaultOptions(program: Command): void { */ export function createProgram(context: Context, opts: CreateProgramOptions = {}): Command { const program = new Command(); - program.configureHelp({ - formatHelp: (cmd, helper) => new ContentCLIHelp().formatHelp(cmd, helper), - subcommandTerm: cmd => new ContentCLIHelp().subcommandTerm(cmd), - optionTerm: opt => new ContentCLIHelp().optionTerm(opt), - }); program.version(VersionUtils.getCurrentCliVersion()); addDefaultOptions(program); diff --git a/src/core/command/CustomHelp.ts b/src/core/command/CustomHelp.ts deleted file mode 100644 index f5f38364..00000000 --- a/src/core/command/CustomHelp.ts +++ /dev/null @@ -1,19 +0,0 @@ -import { Command, Help } from "commander"; -import * as chalk from "chalk"; - -export class ContentCLIHelp extends Help { - - public subcommandTerm(cmd: Command): string { - const base = super.subcommandTerm(cmd); - return (cmd as any).isBeta === true - ? `${base} ${chalk.yellow("(beta)")}` - : base; - } - - public optionTerm(option: any): string { - const term = super.optionTerm(option); - return (option as any).isBeta === true - ? `${term} ${chalk.yellow("(beta)")}` - : term; - } -} diff --git a/src/core/command/module-handler.ts b/src/core/command/module-handler.ts index d4853679..29b005c6 100644 --- a/src/core/command/module-handler.ts +++ b/src/core/command/module-handler.ts @@ -1,9 +1,9 @@ import path = require("path"); import * as fs from "fs"; -import { Command, CommandOptions, Option, OptionValues } from "commander"; +import { Command, CommandOptions, OptionValues } from "commander"; import { Context } from "./cli-context"; import { GracefulError, logger } from "../utils/logger"; -import * as chalk from "chalk"; +import { FeatureFlagService } from "../feature-flag/feature-flag.service"; export abstract class IModule { public abstract register(context: Context, commandConfig: Configurator): void; @@ -186,13 +186,6 @@ export class CommandConfig { return this; } - public betaOption(flags: string, description?: string, defaultValue?: string | boolean | string[]): CommandConfig { - const option = new Option(flags, description).default(defaultValue); - (option as any).isBeta = true; - this.cmd.addOption(option); - return this; - } - public requiredOption(flags: string, description?: string, defaultValue?: string | boolean | string[]): CommandConfig { this.cmd.requiredOption(flags, description, defaultValue); return this; @@ -203,17 +196,24 @@ export class CommandConfig { return this; } - public beta(): CommandConfig { - (this.cmd as any).isBeta = true; + /** + * Marks a command as early access (internal, not shown in help). With a feature + * key, the command is gated at run time on that backend flag. + */ + public earlyAccess(featureKey?: string): this { + const command = this.cmd as any + command.isEarlyAccess = true; + if (featureKey) { + command.earlyAccessFeatureKey = featureKey; + } return this; } public action(handler: CommandHandler): void { this.cmd.action(async (): Promise => { try { - this.printBetaNoticeIfBetaCommand(); - this.printBetaNoticeIfBetaOptions(); this.printDeprecationNoticeIfDeprecated(); + await this.checkEarlyAccessFeatureFlag(); await handler(this.ctx, this.cmd, this.cmd.optsWithGlobals()); } catch (error) { if (error instanceof GracefulError) { @@ -232,22 +232,30 @@ export class CommandConfig { } } - private printBetaNoticeIfBetaCommand(): void { - if ((this.cmd as any).isBeta) { - logger.info(chalk.yellow("This command is in beta and may change in future releases.")); - } - } - - private printBetaNoticeIfBetaOptions(): void { - if ((this.cmd as any).isBeta) { + /** Blocks an early-access command (one with a feature key) unless its flag is enabled for the team. */ + private async checkEarlyAccessFeatureFlag(): Promise { + const featureKey = (this.cmd as any).earlyAccessFeatureKey; + if (!featureKey) { return; } - const providedOptions = this.cmd.optsWithGlobals(); - for (const option of this.cmd.options) { - const optionName = option.attributeName(); - if ((option as any).isBeta && Object.hasOwn(providedOptions, optionName)) { - logger.info(chalk.yellow(`The option '${option.long}' is in beta and may change in future releases.`)); + + let enabled: boolean; + try { + enabled = await new FeatureFlagService(this.ctx).isEnabled(featureKey); + } catch (error) { + // No profile: let the standard "no profile provided" error surface instead of masking it. + if (!this.ctx.profile) { + throw error; } + throw new GracefulError( + `Could not verify whether '${this.cmd.name()}' is enabled for your team. Please try again later or contact support.` + ); + } + + if (!enabled) { + throw new GracefulError( + `'${this.cmd.name()}' is not enabled for your team. Contact support to request access.` + ); } } } diff --git a/src/core/feature-flag/early-access-features.ts b/src/core/feature-flag/early-access-features.ts new file mode 100644 index 00000000..1cd2e39f --- /dev/null +++ b/src/core/feature-flag/early-access-features.ts @@ -0,0 +1,6 @@ +/** Backend feature-flag keys for early-access commands (passed to `earlyAccess(...)`). */ +export const EarlyAccessFeature = { + BRANCHING: "pacman.branching", +} as const; + +export type EarlyAccessFeatureKey = (typeof EarlyAccessFeature)[keyof typeof EarlyAccessFeature]; diff --git a/src/core/feature-flag/feature-flag.service.ts b/src/core/feature-flag/feature-flag.service.ts new file mode 100644 index 00000000..5ffc012e --- /dev/null +++ b/src/core/feature-flag/feature-flag.service.ts @@ -0,0 +1,25 @@ +import { Context } from "../command/cli-context"; +import { HttpClient } from "../http/http-client"; + +interface TeamFeature { + key: string; +} + +/** + * Checks whether a backend feature flag is enabled for the current team via + * `GET /api/team/features` — a flag is enabled when its key is in the returned list. + */ +export class FeatureFlagService { + private static readonly TEAM_FEATURES_URL = "/api/team/features"; + + private readonly httpClient: () => HttpClient; + + constructor(context: Context) { + this.httpClient = () => context.httpClient; + } + + public async isEnabled(featureKey: string): Promise { + const features: TeamFeature[] = await this.httpClient().get(FeatureFlagService.TEAM_FEATURES_URL); + return Array.isArray(features) && features.some(feature => feature?.key === featureKey); + } +} diff --git a/tests/commands/action-flows/module.spec.ts b/tests/commands/action-flows/module.spec.ts new file mode 100644 index 00000000..b7b3d521 --- /dev/null +++ b/tests/commands/action-flows/module.spec.ts @@ -0,0 +1,19 @@ +import Module = require("../../../src/commands/action-flows/module"); +import { testContext } from "../../utls/test-context"; +import { createMockConfigurator } from "../../utls/configurator-mock"; + +describe("Action Flows Module", () => { + describe("register", () => { + it("registers the action-flows command groups without throwing", () => { + const mockConfigurator = createMockConfigurator(); + + expect(() => new Module().register(testContext, mockConfigurator)).not.toThrow(); + + expect(mockConfigurator.command).toHaveBeenCalledWith("analyze"); + expect(mockConfigurator.command).toHaveBeenCalledWith("export"); + expect(mockConfigurator.command).toHaveBeenCalledWith("import"); + expect(mockConfigurator.command).toHaveBeenCalledWith("pull"); + expect(mockConfigurator.command).toHaveBeenCalledWith("push"); + }); + }); +}); diff --git a/tests/commands/data-pipeline/module.spec.ts b/tests/commands/data-pipeline/module.spec.ts new file mode 100644 index 00000000..5a6bce06 --- /dev/null +++ b/tests/commands/data-pipeline/module.spec.ts @@ -0,0 +1,23 @@ +import Module = require("../../../src/commands/data-pipeline/module"); +import { testContext } from "../../utls/test-context"; +import { createMockConfigurator } from "../../utls/configurator-mock"; + +describe("Data Pipeline Module", () => { + describe("register", () => { + it("registers the data-pool and connection command groups without throwing", () => { + const mockConfigurator = createMockConfigurator(); + + expect(() => new Module().register(testContext, mockConfigurator)).not.toThrow(); + + // data-pool commands + expect(mockConfigurator.command).toHaveBeenCalledWith("export"); + expect(mockConfigurator.command).toHaveBeenCalledWith("import"); + expect(mockConfigurator.command).toHaveBeenCalledWith("list"); + expect(mockConfigurator.command).toHaveBeenCalledWith("pull"); + expect(mockConfigurator.command).toHaveBeenCalledWith("push"); + // connection commands + expect(mockConfigurator.command).toHaveBeenCalledWith("get"); + expect(mockConfigurator.command).toHaveBeenCalledWith("set"); + }); + }); +}); diff --git a/tests/commands/deployment/module.spec.ts b/tests/commands/deployment/module.spec.ts new file mode 100644 index 00000000..7b370709 --- /dev/null +++ b/tests/commands/deployment/module.spec.ts @@ -0,0 +1,44 @@ +import Module = require("../../../src/commands/deployment/module"); +import { testContext } from "../../utls/test-context"; +import { createMockConfigurator } from "../../utls/configurator-mock"; + +describe("Deployment Module", () => { + describe("register", () => { + it("registers the deployment command groups without throwing", () => { + const mockConfigurator = createMockConfigurator(); + + expect(() => new Module().register(testContext, mockConfigurator)).not.toThrow(); + + expect(mockConfigurator.command).toHaveBeenCalledWith("deployment"); + expect(mockConfigurator.command).toHaveBeenCalledWith("create"); + expect(mockConfigurator.command).toHaveBeenCalledWith("list"); + expect(mockConfigurator.command).toHaveBeenCalledWith("history"); + expect(mockConfigurator.command).toHaveBeenCalledWith("active"); + expect(mockConfigurator.command).toHaveBeenCalledWith("deployables"); + expect(mockConfigurator.command).toHaveBeenCalledWith("targets"); + }); + + it("marks every deployment command as early access", () => { + const mockConfigurator = createMockConfigurator(); + + new Module().register(testContext, mockConfigurator); + + // deployment, create, list, history, active, deployables, targets + const expectedEarlyAccessCommands = 7; + expect(mockConfigurator.earlyAccess).toHaveBeenCalledTimes(expectedEarlyAccessCommands); + }); + + it("wires an action handler for every leaf subcommand", () => { + const mockConfigurator = createMockConfigurator(); + + new Module().register(testContext, mockConfigurator); + + // create, history, active, deployables, targets + const expectedLeafCommands = 5; + expect(mockConfigurator.action).toHaveBeenCalledTimes(expectedLeafCommands); + for (const call of mockConfigurator.action.mock.calls) { + expect(typeof call[0]).toBe("function"); + } + }); + }); +}); diff --git a/tests/core/command/module-handler.spec.ts b/tests/core/command/module-handler.spec.ts index 956851d9..df58d23b 100644 --- a/tests/core/command/module-handler.spec.ts +++ b/tests/core/command/module-handler.spec.ts @@ -1,8 +1,16 @@ import { Command } from "commander"; import { Configurator } from "../../../src/core/command/module-handler"; +import { Context } from "../../../src/core/command/cli-context"; import { GracefulError } from "../../../src/core/utils/logger"; import { loggingTestTransport } from "../../jest.setup"; import { testContext } from "../../utls/test-context"; +import { mockAxiosGet, mockAxiosGetError } from "../../utls/http-requests-mock"; + +const TEAM_FEATURES_URL = "https://myTeam.celonis.cloud/api/team/features"; + +function loggedMessages(): string[] { + return loggingTestTransport.logMessages.map(entry => String(entry.message)); +} describe("CommandConfig action error handling", () => { let previousExitCode: number | undefined; @@ -60,3 +68,85 @@ describe("CommandConfig action error handling", () => { ).toBe(true); }); }); + +describe("CommandConfig early access gating", () => { + let previousExitCode: number | undefined; + + beforeEach(() => { + previousExitCode = process.exitCode; + process.exitCode = 0; + loggingTestTransport.logMessages = []; + }); + + afterEach(() => { + process.exitCode = previousExitCode; + }); + + async function runEarlyAccessCommand( + featureKey: string | undefined, + context: Context, + handler: () => Promise + ): Promise { + const program = new Command(); + const configurator = new Configurator(program, context); + + configurator + .command("ea-command") + .earlyAccess(featureKey) + .action(async () => { + await handler(); + }); + + program.exitOverride(); + await program.parseAsync(["node", "content-cli", "ea-command"]); + } + + it("runs the command when the feature flag is enabled", async () => { + mockAxiosGet(TEAM_FEATURES_URL, [{ key: "pacman.branching" }]); + const handler = jest.fn(async () => undefined); + + await runEarlyAccessCommand("pacman.branching", testContext, handler); + + expect(handler).toHaveBeenCalled(); + }); + + it("blocks with a clear message when the feature flag is disabled", async () => { + mockAxiosGet(TEAM_FEATURES_URL, []); + const handler = jest.fn(async () => undefined); + + await runEarlyAccessCommand("pacman.branching", testContext, handler); + + expect(handler).not.toHaveBeenCalled(); + expect(process.exitCode ?? 0).toBe(0); + expect(loggedMessages().some(message => message.includes("is not enabled for your team"))).toBe(true); + }); + + it("shows a could-not-verify message when the feature check fails", async () => { + mockAxiosGetError(TEAM_FEATURES_URL, 500, { error: "boom" }); + const handler = jest.fn(async () => undefined); + + await runEarlyAccessCommand("pacman.branching", testContext, handler); + + expect(handler).not.toHaveBeenCalled(); + expect(loggedMessages().some(message => message.includes("Could not verify"))).toBe(true); + }); + + it("surfaces the no-profile error, not the early-access message, when no profile is set", async () => { + const noProfileContext = new Context({}); + const handler = jest.fn(async () => undefined); + + await runEarlyAccessCommand("pacman.branching", noProfileContext, handler); + + expect(handler).not.toHaveBeenCalled(); + expect(loggedMessages().some(message => message.includes("No profile provided"))).toBe(true); + expect(loggedMessages().some(message => message.includes("is not enabled for your team"))).toBe(false); + }); + + it("does not gate commands marked early access without a feature key", async () => { + const handler = jest.fn(async () => undefined); + + await runEarlyAccessCommand(undefined, testContext, handler); + + expect(handler).toHaveBeenCalled(); + }); +}); diff --git a/tests/core/feature-flag/feature-flag.service.spec.ts b/tests/core/feature-flag/feature-flag.service.spec.ts new file mode 100644 index 00000000..8ee83a06 --- /dev/null +++ b/tests/core/feature-flag/feature-flag.service.spec.ts @@ -0,0 +1,53 @@ +import { FeatureFlagService } from "../../../src/core/feature-flag/feature-flag.service"; +import { testContext } from "../../utls/test-context"; +import { mockAxiosGet, mockAxiosGetError } from "../../utls/http-requests-mock"; + +const TEAM_FEATURES_URL = "https://myTeam.celonis.cloud/api/team/features"; + +describe("FeatureFlagService", () => { + it("returns true when the feature key is present in the team features", async () => { + mockAxiosGet(TEAM_FEATURES_URL, [{ key: "pacman.branching" }, { key: "some.other.flag" }]); + + const enabled = await new FeatureFlagService(testContext).isEnabled("pacman.branching"); + + expect(enabled).toBe(true); + }); + + it("returns false when the feature key is not present", async () => { + mockAxiosGet(TEAM_FEATURES_URL, [{ key: "some.other.flag" }]); + + const enabled = await new FeatureFlagService(testContext).isEnabled("pacman.branching"); + + expect(enabled).toBe(false); + }); + + it("returns false when the team has no features enabled", async () => { + mockAxiosGet(TEAM_FEATURES_URL, []); + + const enabled = await new FeatureFlagService(testContext).isEnabled("pacman.branching"); + + expect(enabled).toBe(false); + }); + + it("returns false when the response is not an array", async () => { + mockAxiosGet(TEAM_FEATURES_URL, null); + + const enabled = await new FeatureFlagService(testContext).isEnabled("pacman.branching"); + + expect(enabled).toBe(false); + }); + + it("ignores null entries and still matches a present key", async () => { + mockAxiosGet(TEAM_FEATURES_URL, [null, { key: "pacman.branching" }]); + + const enabled = await new FeatureFlagService(testContext).isEnabled("pacman.branching"); + + expect(enabled).toBe(true); + }); + + it("rejects when the team features endpoint fails", async () => { + mockAxiosGetError(TEAM_FEATURES_URL, 500, { error: "boom" }); + + await expect(new FeatureFlagService(testContext).isEnabled("pacman.branching")).rejects.toBeDefined(); + }); +}); diff --git a/tests/integration/commands/early-access.spec.ts b/tests/integration/commands/early-access.spec.ts new file mode 100644 index 00000000..35e143a8 --- /dev/null +++ b/tests/integration/commands/early-access.spec.ts @@ -0,0 +1,63 @@ +import { IModule, Configurator } from "../../../src/core/command/module-handler"; +import { Context } from "../../../src/core/command/cli-context"; +import { EarlyAccessFeature } from "../../../src/core/feature-flag/early-access-features"; +import { CliRunResult, runCli as runCliProcess } from "../../utls/cli-runner"; +import { mockAxiosGet, mockAxiosGetError } from "../../utls/http-requests-mock"; + +const TEAM_FEATURES_URL = "https://myTeam.celonis.cloud/api/team/features"; + +const handlerSpy = jest.fn(async () => undefined); + +/** + * Throwaway module that registers a single early-access command gated on the + * branching feature flag, so we can exercise the gate end-to-end through the + * real CLI program via runCli. + */ +class EarlyAccessTestModule extends IModule { + public register(_context: Context, configurator: Configurator): void { + configurator + .command("ea-test") + .earlyAccess(EarlyAccessFeature.BRANCHING) + .action(handlerSpy); + } +} + +describe("early access command integration", () => { + beforeEach(() => { + handlerSpy.mockClear(); + }); + + async function runCli(args: string[]): Promise { + return runCliProcess(args, [EarlyAccessTestModule]); + } + + it("runs the command when the feature flag is enabled for the team", async () => { + mockAxiosGet(TEAM_FEATURES_URL, [{ key: EarlyAccessFeature.BRANCHING }]); + + const result = await runCli(["ea-test"]); + + expect(result.exitCode).toBe(0); + expect(handlerSpy).toHaveBeenCalled(); + }); + + it("blocks with a clear message when the feature flag is disabled", async () => { + mockAxiosGet(TEAM_FEATURES_URL, []); + + const result = await runCli(["ea-test"]); + + expect(result.exitCode).toBe(0); + expect(handlerSpy).not.toHaveBeenCalled(); + expect(result.output).toContain("is not enabled for your team"); + expect(result.output).toContain("Contact support to request access"); + }); + + it("shows a could-not-verify message when the feature check fails", async () => { + mockAxiosGetError(TEAM_FEATURES_URL, 500, { error: "boom" }); + + const result = await runCli(["ea-test"]); + + expect(result.exitCode).toBe(0); + expect(handlerSpy).not.toHaveBeenCalled(); + expect(result.output).toContain("Could not verify whether 'ea-test' is enabled"); + }); +}); diff --git a/tests/utls/configurator-mock.ts b/tests/utls/configurator-mock.ts index a22fe19d..cb141810 100644 --- a/tests/utls/configurator-mock.ts +++ b/tests/utls/configurator-mock.ts @@ -17,9 +17,8 @@ export interface MockConfigurator extends Configurator { requiredOption: jest.Mock; alias: jest.Mock; argument: jest.Mock; - betaOption: jest.Mock; deprecationNotice: jest.Mock; - beta: jest.Mock; + earlyAccess: jest.Mock; action: jest.Mock; } @@ -32,9 +31,8 @@ export function createMockConfigurator(): MockConfigurator { chain.requiredOption = jest.fn(() => chain); chain.alias = jest.fn(() => chain); chain.argument = jest.fn(() => chain); - chain.betaOption = jest.fn(() => chain); chain.deprecationNotice = jest.fn(() => chain); - chain.beta = jest.fn(() => chain); + chain.earlyAccess = jest.fn(() => chain); chain.action = jest.fn(); return chain as MockConfigurator; From fca9d9c753cedf74f3d01322e70322dfa78ddc16 Mon Sep 17 00:00:00 2001 From: "a.jashari" Date: Mon, 13 Jul 2026 19:32:34 +0200 Subject: [PATCH 2/3] SP-1383: add module-handler discovery + builder tests to cover new-code lines Sonar counts pre-existing module-handler code (discoverAndRegisterModules, argument) as new code for this PR due to the project's New Code Definition. Cover it with tests to clear the coverage gate. No source changes. Co-Authored-By: Claude Opus 4.8 --- tests/core/command/module-discovery.spec.ts | 155 ++++++++++++++++++++ tests/core/command/module-handler.spec.ts | 8 + 2 files changed, 163 insertions(+) create mode 100644 tests/core/command/module-discovery.spec.ts diff --git a/tests/core/command/module-discovery.spec.ts b/tests/core/command/module-discovery.spec.ts new file mode 100644 index 00000000..cffdcffc --- /dev/null +++ b/tests/core/command/module-discovery.spec.ts @@ -0,0 +1,155 @@ +import * as fs from "fs"; +import { tmpdir } from "os"; +import { join, dirname } from "path"; +import { Command } from "commander"; +import { ModuleHandler } from "../../../src/core/command/module-handler"; +import { testContext } from "../../utls/test-context"; +import { loggingTestTransport } from "../../jest.setup"; + +const createdRoots: string[] = []; + +function createCommandsTree(files: Record): string { + const root = fs.mkdtempSync(join(tmpdir(), "mh-discovery-")); + createdRoots.push(root); + fs.mkdirSync(join(root, "commands")); + for (const [relPath, content] of Object.entries(files)) { + const full = join(root, "commands", relPath); + fs.mkdirSync(dirname(full), { recursive: true }); + fs.writeFileSync(full, content); + } + return root; +} + +function newHandler(): ModuleHandler { + return new ModuleHandler(new Command(), testContext); +} + +function messages(): string[] { + return loggingTestTransport.logMessages.map(entry => String(entry.message)); +} + +describe("ModuleHandler.discoverAndRegisterModules", () => { + let exitSpy: jest.SpyInstance; + + beforeEach(() => { + loggingTestTransport.logMessages = []; + // The logger's transport calls process.exit on error-level logs; stub it so + // the error branches can be exercised without killing the test runner. + exitSpy = jest.spyOn(process, "exit").mockImplementation((() => undefined) as never); + }); + + afterEach(() => { + exitSpy.mockRestore(); + }); + + afterAll(() => { + for (const root of createdRoots) { + fs.rmSync(root, { recursive: true, force: true }); + } + }); + + it("registers a module that exports a class with a register method", () => { + const root = createCommandsTree({ "valid/module.js": "module.exports = class { register() {} };" }); + const handler = newHandler(); + + handler.discoverAndRegisterModules(root, false); + + expect(handler.registeredModules).toHaveLength(1); + }); + + it("skips a folder that has no module entry point", () => { + const root = createCommandsTree({ "empty/readme.txt": "no module here" }); + const handler = newHandler(); + + handler.discoverAndRegisterModules(root, false); + + expect(handler.registeredModules).toHaveLength(0); + }); + + it("warns when the module export is not a class", () => { + const root = createCommandsTree({ "notclass/module.js": "module.exports = { notAClass: true };" }); + const handler = newHandler(); + + handler.discoverAndRegisterModules(root, false); + + expect(handler.registeredModules).toHaveLength(0); + expect(messages().some(m => m.includes("is not a class/constructor function"))).toBe(true); + }); + + it("warns when the module class has no register method", () => { + const root = createCommandsTree({ "noregister/module.js": "module.exports = class {};" }); + const handler = newHandler(); + + handler.discoverAndRegisterModules(root, false); + + expect(handler.registeredModules).toHaveLength(0); + expect(messages().some(m => m.includes("does not have a 'register' method"))).toBe(true); + }); + + it("warns when a module cannot be required due to a missing dependency", () => { + const root = createCommandsTree({ + "badrequire/module.js": "require('a-package-that-does-not-exist-xyz');", + }); + const handler = newHandler(); + + handler.discoverAndRegisterModules(root, false); + + expect(handler.registeredModules).toHaveLength(0); + expect(messages().some(m => m.includes("Could not require module"))).toBe(true); + }); + + it("warns when requiring a module fails with an ENOENT error", () => { + const root = createCommandsTree({ + "enoent/module.js": "require('fs').readFileSync('/no/such/path/xyz-content-cli-test');", + }); + const handler = newHandler(); + + handler.discoverAndRegisterModules(root, false); + + expect(handler.registeredModules).toHaveLength(0); + expect(messages().some(m => m.includes("does not contain a compiled module.js file"))).toBe(true); + }); + + it("logs an error when the commands path is not a directory", () => { + const root = fs.mkdtempSync(join(tmpdir(), "mh-notdir-")); + createdRoots.push(root); + fs.writeFileSync(join(root, "commands"), "this is a file, not a directory"); + const handler = newHandler(); + + handler.discoverAndRegisterModules(root, false); + + expect(handler.registeredModules).toHaveLength(0); + expect(messages().some(m => m.includes("Failed to read modules directory"))).toBe(true); + }); + + it("logs an error when requiring a module throws unexpectedly", () => { + const root = createCommandsTree({ "throws/module.js": "throw new Error('boom');" }); + const handler = newHandler(); + + handler.discoverAndRegisterModules(root, false); + + expect(handler.registeredModules).toHaveLength(0); + expect(messages().some(m => m.includes("Error processing module in"))).toBe(true); + }); + + it("logs an error when the commands directory does not exist", () => { + const root = fs.mkdtempSync(join(tmpdir(), "mh-nocommands-")); + createdRoots.push(root); + const handler = newHandler(); + + handler.discoverAndRegisterModules(root, false); + + expect(handler.registeredModules).toHaveLength(0); + expect(messages().some(m => m.includes("Modules directory not found"))).toBe(true); + }); + + it("looks for module.ts (not module.js) in dev mode", () => { + const root = createCommandsTree({ "devonly/module.js": "module.exports = class { register() {} };" }); + const handler = newHandler(); + + // dev mode looks for module.ts, which is absent here, so the folder is skipped + handler.discoverAndRegisterModules(root, true); + + expect(handler.registeredModules).toHaveLength(0); + }); +}); diff --git a/tests/core/command/module-handler.spec.ts b/tests/core/command/module-handler.spec.ts index df58d23b..a5e301ac 100644 --- a/tests/core/command/module-handler.spec.ts +++ b/tests/core/command/module-handler.spec.ts @@ -150,3 +150,11 @@ describe("CommandConfig early access gating", () => { expect(handler).toHaveBeenCalled(); }); }); + +describe("CommandConfig builder methods", () => { + it("supports the argument() builder", () => { + const configurator = new Configurator(new Command(), testContext); + + expect(() => configurator.command("with-arg").argument("", "an argument")).not.toThrow(); + }); +}); From b0ddbc7f8a1f5ff284e8542fe51eb8c6cb80d46b Mon Sep 17 00:00:00 2001 From: "a.jashari" Date: Mon, 13 Jul 2026 21:06:08 +0200 Subject: [PATCH 3/3] SP-1383: parameterize module discovery tests to clear Sonar smell Co-Authored-By: Claude Opus 4.8 --- tests/core/command/module-discovery.spec.ts | 60 +++++---------------- 1 file changed, 13 insertions(+), 47 deletions(-) diff --git a/tests/core/command/module-discovery.spec.ts b/tests/core/command/module-discovery.spec.ts index cffdcffc..db07faf4 100644 --- a/tests/core/command/module-discovery.spec.ts +++ b/tests/core/command/module-discovery.spec.ts @@ -66,48 +66,24 @@ describe("ModuleHandler.discoverAndRegisterModules", () => { expect(handler.registeredModules).toHaveLength(0); }); - it("warns when the module export is not a class", () => { - const root = createCommandsTree({ "notclass/module.js": "module.exports = { notAClass: true };" }); + it.each([ + ["export is not a class", "module.exports = { notAClass: true };", "is not a class/constructor function"], + ["class has no register method", "module.exports = class {};", "does not have a 'register' method"], + ["dependency is missing", "require('a-package-that-does-not-exist-xyz');", "Could not require module"], + [ + "require fails with ENOENT", + "require('fs').readFileSync('/no/such/path/xyz-content-cli-test');", + "does not contain a compiled module.js file", + ], + ["require throws unexpectedly", "throw new Error('boom');", "Error processing module in"], + ])("logs the expected message when a module's %s", (_case, moduleContent, expectedMessage) => { + const root = createCommandsTree({ "mod/module.js": moduleContent }); const handler = newHandler(); handler.discoverAndRegisterModules(root, false); expect(handler.registeredModules).toHaveLength(0); - expect(messages().some(m => m.includes("is not a class/constructor function"))).toBe(true); - }); - - it("warns when the module class has no register method", () => { - const root = createCommandsTree({ "noregister/module.js": "module.exports = class {};" }); - const handler = newHandler(); - - handler.discoverAndRegisterModules(root, false); - - expect(handler.registeredModules).toHaveLength(0); - expect(messages().some(m => m.includes("does not have a 'register' method"))).toBe(true); - }); - - it("warns when a module cannot be required due to a missing dependency", () => { - const root = createCommandsTree({ - "badrequire/module.js": "require('a-package-that-does-not-exist-xyz');", - }); - const handler = newHandler(); - - handler.discoverAndRegisterModules(root, false); - - expect(handler.registeredModules).toHaveLength(0); - expect(messages().some(m => m.includes("Could not require module"))).toBe(true); - }); - - it("warns when requiring a module fails with an ENOENT error", () => { - const root = createCommandsTree({ - "enoent/module.js": "require('fs').readFileSync('/no/such/path/xyz-content-cli-test');", - }); - const handler = newHandler(); - - handler.discoverAndRegisterModules(root, false); - - expect(handler.registeredModules).toHaveLength(0); - expect(messages().some(m => m.includes("does not contain a compiled module.js file"))).toBe(true); + expect(messages().some(m => m.includes(expectedMessage))).toBe(true); }); it("logs an error when the commands path is not a directory", () => { @@ -122,16 +98,6 @@ describe("ModuleHandler.discoverAndRegisterModules", () => { expect(messages().some(m => m.includes("Failed to read modules directory"))).toBe(true); }); - it("logs an error when requiring a module throws unexpectedly", () => { - const root = createCommandsTree({ "throws/module.js": "throw new Error('boom');" }); - const handler = newHandler(); - - handler.discoverAndRegisterModules(root, false); - - expect(handler.registeredModules).toHaveLength(0); - expect(messages().some(m => m.includes("Error processing module in"))).toBe(true); - }); - it("logs an error when the commands directory does not exist", () => { const root = fs.mkdtempSync(join(tmpdir(), "mh-nocommands-")); createdRoots.push(root);