ReviewOS
Reload

also looking at this

stacks/buddy

test: import the CLI without parsing, and pin its wiring in-process

#1448
Merged glennmichael123 wants to merge test/cli-wiring into main
2 files +145 -2

Review threads live on the whole diff, not on one commit, so none are shown here - a thread's line means something in the branch's final form, and painting it into an intermediate step would put it on code it is not about.

bin/cli.tsmodified+8-2
@@ -204,7 +204,7 @@ async function collectPackageContext(
204204 }
205205}
206206
207const cli = new CLI('buddy')
207const cli: CLI = new CLI('buddy')
208208
209209cli.usage(`[command] [options]
210210
@@ -3517,4 +3517,10 @@ cli.command('help', 'Show this help text').action(() => {
35173517cli.option('--config <path>', 'Path to a buddy config file')
35183518cli.version(version)
35193519cli.help()
3520cli.parse()
3520
3521// Exported for the in-process wiring tests: importing this module must not
3522// parse argv or run a command — only direct execution does.
3523export { cli }
3524
3525if (import.meta.main)
3526 cli.parse()
test/cli-wiring.test.tsadded+137-0
@@ -0,0 +1,137 @@
1import { describe, expect, it } from 'bun:test'
2import { cli } from '../bin/cli'
3
4/**
5 * In-process wiring tests for the CLI surface. `bin/cli.ts` guards its
6 * `cli.parse()` behind `import.meta.main`, so importing it registers the 32
7 * commands without parsing argv or running an action — and `parse(argv,
8 * { run: false })` exercises the real flag plumbing without side effects.
9 * The subprocess smoke suite (`cli-surface.test.ts`) stays authoritative for
10 * process-level behavior; this file pins the wiring drift it cannot see.
11 */
12
13const argv = (...parts: string[]): string[] => ['bun', 'buddy', ...parts]
14
15function command(name: string): any {
16 const found = (cli as any).commands.find((c: any) => c.name === name)
17 if (!found)
18 throw new Error(`command '${name}' is not registered`)
19 return found
20}
21
22describe('cli wiring', () => {
23 describe('command surface', () => {
24 it('success case - registers exactly the documented commands', () => {
25 const names = (cli as any).commands.map((c: any) => c.name).sort()
26 expect(names).toEqual([
27 'check',
28 'cleanup',
29 'compare',
30 'dashboard',
31 'deps',
32 'doctor',
33 'exists',
34 'fix-ci',
35 'gate',
36 'generate-workflows',
37 'handle-comment',
38 'handle-issue',
39 'help',
40 'info',
41 'latest',
42 'list-branches',
43 'open-settings',
44 'post-merge',
45 'rebase',
46 'report',
47 'review',
48 'run',
49 'scan',
50 'schedule',
51 'search',
52 'security',
53 'setup',
54 'touch',
55 'update',
56 'update-check',
57 'version',
58 'versions',
59 ])
60 })
61
62 it('success case - every command and option carries a description', () => {
63 for (const c of (cli as any).commands) {
64 expect(c.description, `command '${c.name}'`).toBeTruthy()
65 for (const o of c.options ?? [])
66 expect(o.description, `${c.name} ${o.rawName}`).toBeTruthy()
67 }
68 })
69
70 it('success case - init aliases setup', () => {
71 expect(command('setup').aliasNames).toContain('init')
72 })
73
74 it('success case - the global --config option and version are wired', () => {
75 const globalOptions = ((cli as any).globalCommand?.options ?? []).map((o: any) => o.name)
76 expect(globalOptions).toContain('config')
77 expect((cli as any).globalCommand?.versionNumber ?? (cli as any).commands[0]?.versionNumber).toBeTruthy()
78 })
79 })
80
81 describe('strategy flags', () => {
82 it('failure case - no strategy flag carries a CAC default that would shadow the config', () => {
83 // The defect #1441 fixed: `{ default: 'all' }` on --strategy made
84 // `options.strategy ?? config.packages.strategy` unreachable.
85 for (const c of (cli as any).commands) {
86 const strategy = (c.options ?? []).find((o: any) => o.name === 'strategy')
87 if (strategy)
88 expect(strategy.config?.default, `${c.name} --strategy`).toBeUndefined()
89 }
90 })
91
92 it('success case - exactly the four update-driving commands take --strategy', () => {
93 const withStrategy = (cli as any).commands
94 .filter((c: any) => (c.options ?? []).some((o: any) => o.name === 'strategy'))
95 .map((c: any) => c.name)
96 .sort()
97 expect(withStrategy).toEqual(['check', 'scan', 'schedule', 'update'])
98 })
99 })
100
101 describe('parse without running', () => {
102 it('success case - scan flags land as parsed options', () => {
103 cli.parse(argv('scan', '--strategy', 'minor', '--verbose'), { run: false })
104 expect((cli as any).matchedCommandName).toBe('scan')
105 expect((cli as any).options.strategy).toBe('minor')
106 expect((cli as any).options.verbose).toBe(true)
107 })
108
109 it('success case - negation flips respect-latest off', () => {
110 cli.parse(argv('update', '--no-respect-latest'), { run: false })
111 expect((cli as any).matchedCommandName).toBe('update')
112 expect((cli as any).options.respectLatest).toBe(false)
113 })
114
115 it('success case - review flags parse with their defaults intact', () => {
116 cli.parse(argv('review', '--fail-on', 'high', '--light'), { run: false })
117 expect((cli as any).options.failOn).toBe('high')
118 expect((cli as any).options.light).toBe(true)
119 // --format was not given: the declared default applies
120 expect((cli as any).options.format).toBe('pretty')
121 })
122
123 it('success case - declared defaults surface when the flag is omitted', () => {
124 cli.parse(argv('cleanup', '--dry-run'), { run: false })
125 // The default is declared as the string '7' — actions parse it. If this
126 // ever becomes a number at the declaration site, update the pin.
127 expect((cli as any).options.days).toBe('7')
128 expect((cli as any).options.dryRun).toBe(true)
129 })
130
131 it('success case - positional arguments are captured', () => {
132 cli.parse(argv('exists', 'lodash'), { run: false })
133 expect((cli as any).matchedCommandName).toBe('exists')
134 expect((cli as any).args).toContain('lodash')
135 })
136 })
137})