From e8173b3012297a239087e92090e47db02da65b3c Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Wed, 4 Mar 2026 10:10:31 +0100 Subject: [PATCH] Address review feedback: role.ts, repo start params, test coverage - Remove unnecessary optional chaining on getRoleInfo().role since it throws for unknown roles. - Add parameters declaration to repo start so [packages...] shows in help output. - Add tests verifying cleye strips Backstage flags from args before forwarding to Jest, including legacy camelCase flag support. Signed-off-by: Patrik Oldsberg Made-with: Cursor --- packages/cli/cli-report.md | 4 +- .../src/modules/build/commands/repo/start.ts | 3 +- packages/cli/src/modules/build/lib/role.ts | 2 +- .../modules/test/commands/repo/test.test.ts | 56 +++++++++++++++++++ 4 files changed, 61 insertions(+), 4 deletions(-) diff --git a/packages/cli/cli-report.md b/packages/cli/cli-report.md index f8d1db7019..88b82dda21 100644 --- a/packages/cli/cli-report.md +++ b/packages/cli/cli-report.md @@ -33,7 +33,7 @@ Commands: ### `backstage-cli build-workspace` ``` -Usage: backstage-cli build-workspace +Usage: backstage-cli build-workspace [packages...] Options: --always-pack @@ -511,7 +511,7 @@ Options: ### `backstage-cli repo start` ``` -Usage: backstage-cli repo start +Usage: backstage-cli repo start [packages...] Options: --config diff --git a/packages/cli/src/modules/build/commands/repo/start.ts b/packages/cli/src/modules/build/commands/repo/start.ts index 8d36d0732b..6acd4f1463 100644 --- a/packages/cli/src/modules/build/commands/repo/start.ts +++ b/packages/cli/src/modules/build/commands/repo/start.ts @@ -41,7 +41,8 @@ export default async ({ args, info }: CommandContext) => { _: namesOrPaths, } = cli( { - help: info, + help: { ...info, usage: `${info.usage} [packages...]` }, + parameters: ['[packages...]'], flags: { plugin: { type: [String], diff --git a/packages/cli/src/modules/build/lib/role.ts b/packages/cli/src/modules/build/lib/role.ts index bfcd1ecfb2..26c9da7cd0 100644 --- a/packages/cli/src/modules/build/lib/role.ts +++ b/packages/cli/src/modules/build/lib/role.ts @@ -23,7 +23,7 @@ export async function findRoleFromCommand(opts: { role?: string; }): Promise { if (opts.role) { - return PackageRoles.getRoleInfo(opts.role)?.role; + return PackageRoles.getRoleInfo(opts.role).role; } const pkg = await fs.readJson(targetPaths.resolve('package.json')); diff --git a/packages/cli/src/modules/test/commands/repo/test.test.ts b/packages/cli/src/modules/test/commands/repo/test.test.ts index d2f2485717..fe0e733f8c 100644 --- a/packages/cli/src/modules/test/commands/repo/test.test.ts +++ b/packages/cli/src/modules/test/commands/repo/test.test.ts @@ -14,6 +14,7 @@ * limitations under the License. */ +import { cli } from 'cleye'; import { createFlagFinder } from './test'; describe('createFlagFinder', () => { @@ -45,3 +46,58 @@ describe('createFlagFinder', () => { expect(find('--qux')).toBe(true); }); }); + +describe('repo test arg forwarding', () => { + // Mirrors the cleye configuration used in the repo test command handler + function parseRepoTestArgs(args: string[]) { + return cli( + { + help: false, + flags: { + since: { type: String }, + successCache: { type: Boolean }, + successCacheDir: { type: String }, + jestHelp: { type: Boolean }, + }, + ignoreArgv: type => type === 'unknown-flag' || type === 'argument', + }, + undefined, + args, + ); + } + + it('strips Backstage flags from args while preserving Jest flags and arguments', () => { + const args = [ + '--since', + 'main', + '--success-cache', + '--coverage', + '--watch', + 'path/to/test', + ]; + + const { flags } = parseRepoTestArgs(args); + + expect(flags.since).toBe('main'); + expect(flags.successCache).toBe(true); + expect(args).toEqual(['--coverage', '--watch', 'path/to/test']); + }); + + it('supports legacy camelCase flag names', () => { + const args = ['--successCache', '--successCacheDir', '/tmp/cache']; + + const { flags } = parseRepoTestArgs(args); + + expect(flags.successCache).toBe(true); + expect(flags.successCacheDir).toBe('/tmp/cache'); + expect(args).toEqual([]); + }); + + it('leaves args untouched when no Backstage flags are present', () => { + const args = ['--coverage', '--verbose', '--bail']; + + parseRepoTestArgs(args); + + expect(args).toEqual(['--coverage', '--verbose', '--bail']); + }); +});