Skip to content

Commit 2752b13

Browse files
amikofalvyclaude
amikofalvy
and
claude
authored
Refactor CLI tests: subprocess → integration, fast unit tests (#2944)
* Refactor CLI tests: move subprocess tests to integration, add fast unit tests Extract `createProgram()` from index.ts into program.ts so CLI command registration can be imported without side effects (env loading, instrumentation). This enables fast unit testing via Commander's programmatic API. Move the subprocess-based cli.test.ts to integration/cli-subprocess.test.ts (real `execSync` spawning of the compiled binary). Replace with new unit tests that use Commander's `exitOverride()` — runs in 14ms vs 33s. Integration tests are chained in the `test` script so they run in CI after unit tests pass. Also available standalone via `pnpm test:integration`. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Address PR review feedback - Add retry: 2 to integration config for flaky subprocess tests - Chain integration tests in test:ci script - Remove no-op global.gc() from subprocess test cleanup - Remove unnecessary comment in CI config Skipping push --help unit test: Commander subcommand help doesn't propagate through parent's configureOutput(), so the programmatic option inspection is more reliable for unit tests. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Fix lint: replace non-null assertions with optional chaining Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent a929847 commit 2752b13

File tree

8 files changed

+560
-364
lines changed

8 files changed

+560
-364
lines changed

‎agents-cli/package.json‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,11 +20,12 @@
2020
"cli": "node ./dist/index.js",
2121
"prepare": "pnpm build --no-dts",
2222
"dev": "pnpm build --watch",
23-
"test": "node -e \"process.exit(process.env.CI ? 0 : 1)\" && vitest --run --config vitest.config.ci.ts || vitest --run",
23+
"test": "(node -e \"process.exit(process.env.CI ? 0 : 1)\" && vitest --run --config vitest.config.ci.ts || vitest --run) && vitest --run --config vitest.integration.config.ts",
2424
"test:debug": "vitest --run --reporter=verbose --no-coverage",
2525
"test:watch": "vitest",
2626
"test:coverage": "node -e \"process.exit(process.env.CI ? 0 : 1)\" && vitest --run --coverage --config vitest.config.ci.ts || vitest --run --coverage",
27-
"test:ci": "vitest --run --config vitest.config.ci.ts",
27+
"test:ci": "vitest --run --config vitest.config.ci.ts && vitest --run --config vitest.integration.config.ts",
28+
"test:integration": "vitest --run --config vitest.integration.config.ts",
2829
"typecheck": "tsc --noEmit --project tsconfig.typecheck.json",
2930
"typecheck:watch": "tsc --noEmit --watch --project tsconfig.typecheck.json"
3031
},

‎agents-cli/src/__tests__/cli.test.ts‎

Lines changed: 109 additions & 131 deletions
Original file line numberDiff line numberDiff line change
@@ -1,186 +1,164 @@
1-
import { execSync } from 'node:child_process';
2-
import { existsSync, readFileSync } from 'node:fs';
1+
import { readFileSync } from 'node:fs';
32
import { dirname, join } from 'node:path';
43
import { fileURLToPath } from 'node:url';
4+
import type { Command } from 'commander';
55
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';
66

77
const __filename = fileURLToPath(import.meta.url);
88
const __dirname = dirname(__filename);
99

10-
// Path to the compiled CLI
11-
const cliPath = join(__dirname, '..', '..', 'dist', 'index.js');
12-
13-
// Helper function to execute CLI commands
14-
function runCli(args: string[]): { stdout: string; stderr: string; exitCode: number } {
15-
// Check if CLI binary exists before attempting to run
16-
if (!existsSync(cliPath)) {
17-
return {
18-
stdout: '',
19-
stderr: `CLI binary not found at ${cliPath}`,
20-
exitCode: 1,
21-
};
22-
}
10+
vi.mock('../commands/push', () => ({ pushCommand: vi.fn() }));
11+
vi.mock('../commands/add', () => ({ addCommand: vi.fn() }));
12+
vi.mock('../commands/config', () => ({
13+
configGetCommand: vi.fn(),
14+
configSetCommand: vi.fn(),
15+
configListCommand: vi.fn(),
16+
}));
17+
vi.mock('../commands/dev', () => ({ devCommand: vi.fn() }));
18+
vi.mock('../commands/init', () => ({ initCommand: vi.fn() }));
19+
vi.mock('../commands/list-agents', () => ({ listAgentsCommand: vi.fn() }));
20+
vi.mock('../commands/login', () => ({ loginCommand: vi.fn() }));
21+
vi.mock('../commands/logout', () => ({ logoutCommand: vi.fn() }));
22+
vi.mock('../commands/profile', () => ({
23+
profileAddCommand: vi.fn(),
24+
profileCurrentCommand: vi.fn(),
25+
profileListCommand: vi.fn(),
26+
profileRemoveCommand: vi.fn(),
27+
profileUseCommand: vi.fn(),
28+
}));
29+
vi.mock('../commands/pull-v4/introspect', () => ({ pullV4Command: vi.fn() }));
30+
vi.mock('../commands/status', () => ({ statusCommand: vi.fn() }));
31+
vi.mock('../commands/update', () => ({ updateCommand: vi.fn() }));
32+
vi.mock('../commands/whoami', () => ({ whoamiCommand: vi.fn() }));
33+
34+
function parseArgs(program: Command, args: string[]): { stdout: string; stderr: string } {
35+
let stdout = '';
36+
let stderr = '';
37+
38+
program.exitOverride();
39+
program.configureOutput({
40+
writeOut: (str) => {
41+
stdout += str;
42+
},
43+
writeErr: (str) => {
44+
stderr += str;
45+
},
46+
});
2347

2448
try {
25-
const stdout = execSync(`node ${cliPath} ${args.join(' ')}`, {
26-
encoding: 'utf8',
27-
stdio: ['pipe', 'pipe', 'pipe'],
28-
timeout: 30000, // 30 second timeout for CI
29-
killSignal: 'SIGTERM', // Use SIGTERM first for cleaner shutdown
30-
windowsHide: true, // Hide windows on Windows
31-
env: {
32-
...process.env,
33-
// Test environment
34-
CI: 'true', // Signal to CLI that it's running in CI
35-
NODE_OPTIONS: '--max-old-space-size=256', // Limit memory usage
36-
},
37-
});
38-
return { stdout, stderr: '', exitCode: 0 };
39-
} catch (error: any) {
40-
// Handle timeout specifically - check for various timeout indicators
41-
const isTimeout =
42-
error.code === 'ETIMEDOUT' ||
43-
error.signal === 'SIGTERM' ||
44-
error.killed ||
45-
error.message?.toLowerCase().includes('timedout');
46-
47-
if (isTimeout) {
48-
return {
49-
stdout: error.stdout || '',
50-
stderr: 'Command timed out',
51-
exitCode: 124, // Standard timeout exit code
52-
};
53-
}
54-
55-
return {
56-
stdout: error.stdout || '',
57-
stderr: error.stderr || error.message || 'Unknown error',
58-
exitCode: error.status || 1,
59-
};
49+
program.parse(['node', 'inkeep', ...args]);
50+
} catch {
51+
// Commander throws on --version, --help, and errors when exitOverride is set
6052
}
53+
54+
return { stdout, stderr };
6155
}
6256

6357
describe('Inkeep CLI', () => {
64-
beforeEach(() => {
65-
// Mock console methods to capture output during tests
58+
let createProgram: () => Command;
59+
60+
beforeEach(async () => {
6661
vi.spyOn(console, 'log').mockImplementation(() => {});
6762
vi.spyOn(console, 'error').mockImplementation(() => {});
63+
({ createProgram } = await import('../program'));
6864
});
6965

70-
afterEach(async () => {
66+
afterEach(() => {
7167
vi.restoreAllMocks();
72-
// Small delay to allow processes to fully terminate
73-
await new Promise((resolve) => setTimeout(resolve, 100));
74-
// Force garbage collection to clean up any hanging references
75-
if (global.gc) {
76-
global.gc();
77-
}
7868
});
7969

8070
describe('--version command', () => {
8171
it('should display the version number', () => {
82-
const result = runCli(['--version']);
83-
84-
expect(result.exitCode).toBe(0);
85-
expect(result.stdout.trim()).toMatch(/^\d+\.\d+\.\d+$/);
72+
const { stdout } = parseArgs(createProgram(), ['--version']);
73+
expect(stdout.trim()).toMatch(/^\d+\.\d+\.\d+$/);
8674
});
8775

8876
it('should match the version in package.json', () => {
8977
const packageJsonPath = join(__dirname, '..', '..', 'package.json');
9078
const packageJson = JSON.parse(readFileSync(packageJsonPath, 'utf-8'));
91-
const expectedVersion = packageJson.version;
9279

93-
const result = runCli(['--version']);
94-
95-
expect(result.stdout.trim()).toBe(expectedVersion);
80+
const { stdout } = parseArgs(createProgram(), ['--version']);
81+
expect(stdout.trim()).toBe(packageJson.version);
9682
});
9783
});
9884

99-
describe('push command', () => {
100-
it('should work without required arguments', () => {
101-
const result = runCli(['push']);
102-
103-
// Skip test if command timed out (CI performance issue, not CLI bug)
104-
if (result.exitCode === 124) {
105-
console.log('Skipping assertion: push command timed out in CI');
106-
return;
107-
}
108-
109-
// The push command now tries to detect project automatically
110-
expect(result.exitCode).toBe(1);
111-
// It should fail because configuration or project is missing in test environment
112-
expect(result.stderr.toLowerCase()).toMatch(/tenant id|index\.ts|config/);
113-
});
85+
describe('--help command', () => {
86+
it('should display help information', () => {
87+
const { stdout } = parseArgs(createProgram(), ['--help']);
11488

115-
it('should accept --agents-api-url option', () => {
116-
const result = runCli([
117-
'push',
118-
'--project',
119-
'non-existent',
120-
'--agents-api-url',
121-
'http://example.com',
122-
]);
123-
124-
// Will fail because project doesn't exist, but should accept the option
125-
expect(result.exitCode).toBe(1);
126-
// Should fail for project not found, not for invalid option
127-
expect(result.stderr).not.toContain('unknown option');
89+
expect(stdout).toContain('CLI tool for Inkeep Agent Framework');
90+
expect(stdout).toContain('Commands:');
91+
expect(stdout).toContain('push');
92+
expect(stdout).toContain('config');
93+
expect(stdout).toContain('list-agent');
12894
});
129-
});
13095

131-
describe('list-agent command', () => {
132-
it('should require --project option and accept --url option', () => {
133-
const result = runCli(['list-agent', '--help']);
96+
it('should have push command with correct description', () => {
97+
const program = createProgram();
98+
const pushCmd = program.commands.find((c) => c.name() === 'push');
13499

135-
expect(result.exitCode).toBe(0);
136-
expect(result.stdout).toContain('List all available agents for a specific project');
137-
expect(result.stdout).toContain('--project <project-id>');
138-
expect(result.stdout).toContain('--agents-api-url');
100+
expect(pushCmd).toBeDefined();
101+
expect(pushCmd?.description()).toContain('Push a project configuration');
139102
});
140103
});
141104

142-
describe('--help command', () => {
143-
it('should display help information', () => {
144-
const result = runCli(['--help']);
145-
146-
expect(result.exitCode).toBe(0);
147-
expect(result.stdout).toContain('CLI tool for Inkeep Agent Framework');
148-
expect(result.stdout).toContain('Commands:');
149-
expect(result.stdout).toContain('push');
150-
expect(result.stdout).toContain('config');
151-
expect(result.stdout).toContain('list-agent');
105+
describe('push command', () => {
106+
it('should accept --agents-api-url option', () => {
107+
const program = createProgram();
108+
const pushCmd = program.commands.find((c) => c.name() === 'push');
109+
110+
expect(pushCmd).toBeDefined();
111+
const optionNames = pushCmd?.options.map((o) => o.long);
112+
expect(optionNames).toContain('--agents-api-url');
113+
expect(optionNames).toContain('--project');
114+
expect(optionNames).toContain('--config');
115+
expect(optionNames).toContain('--force');
152116
});
117+
});
153118

154-
it('should display help for push command', () => {
155-
const result = runCli(['push', '--help']);
156-
157-
expect(result.exitCode).toBe(0);
158-
expect(result.stdout).toContain('Push a project configuration');
159-
expect(result.stdout).toContain('--agents-api-url');
119+
describe('list-agent command', () => {
120+
it('should have expected description and options', () => {
121+
const program = createProgram();
122+
const listAgentCmd = program.commands.find((c) => c.name() === 'list-agent');
123+
124+
expect(listAgentCmd).toBeDefined();
125+
expect(listAgentCmd?.description()).toBe('List all available agents for a specific project');
126+
const optionNames = listAgentCmd?.options.map((o) => o.long);
127+
expect(optionNames).toContain('--project');
128+
expect(optionNames).toContain('--agents-api-url');
160129
});
161130
});
162131

163132
describe('invalid commands', () => {
164133
it('should show error for unknown command', () => {
165-
const result = runCli(['unknown-command']);
166-
167-
expect(result.exitCode).toBe(1);
168-
expect(result.stderr).toContain('error: unknown command');
134+
const { stderr } = parseArgs(createProgram(), ['unknown-command']);
135+
expect(stderr).toContain('unknown command');
169136
});
170137
});
171138

172139
describe('CLI structure', () => {
173140
it('should have correct CLI name', () => {
174-
const result = runCli(['--help']);
175-
176-
expect(result.stdout).toContain('inkeep');
141+
const { stdout } = parseArgs(createProgram(), ['--help']);
142+
expect(stdout).toContain('inkeep');
177143
});
178144

179-
it('should be executable', () => {
180-
// This test ensures the CLI can be executed without throwing
181-
const result = runCli(['--version']);
182-
183-
expect(result.exitCode).toBe(0);
145+
it('should register all expected commands', () => {
146+
const program = createProgram();
147+
const commandNames = program.commands.map((c) => c.name());
148+
149+
expect(commandNames).toContain('push');
150+
expect(commandNames).toContain('pull');
151+
expect(commandNames).toContain('list-agent');
152+
expect(commandNames).toContain('config');
153+
expect(commandNames).toContain('dev');
154+
expect(commandNames).toContain('login');
155+
expect(commandNames).toContain('logout');
156+
expect(commandNames).toContain('status');
157+
expect(commandNames).toContain('profile');
158+
expect(commandNames).toContain('add');
159+
expect(commandNames).toContain('init');
160+
expect(commandNames).toContain('update');
161+
expect(commandNames).toContain('whoami');
184162
});
185163
});
186164
});

0 commit comments

Comments
 (0)