From 7b7f96fa2725acc480e2694b7309bc30deafa086 Mon Sep 17 00:00:00 2001 From: Minsu Lee Date: Thu, 18 Dec 2025 18:33:46 +0900 Subject: [PATCH 1/2] refactor(code): reorganize test structure to test/ directory - Moved all tests from src/__tests__/ to test/unit/ and test/integration/ - Created new utils package with extracted utilities: - parseArgs function with bug fix for flag value parsing - Platform detection utilities - Fixed CLI -v/--version and -h/--help flag handling - Updated package.json test script to bun test ./test - Reorganized test structure: unit tests in test/unit/, integration tests in test/integration/ - All 87 tests passing with new structure --- packages/code/package.json | 2 +- packages/code/src/__tests__/cli.test.ts | 150 ------- packages/code/src/cli.ts | 47 +-- packages/code/src/utils/args.ts | 54 +++ packages/code/src/utils/index.ts | 6 + packages/code/src/utils/platform.ts | 117 ++++++ packages/code/test/integration/cli.test.ts | 365 ++++++++++++++++++ packages/code/test/unit/args.test.ts | 184 +++++++++ packages/code/test/unit/hooks.test.ts | 231 +++++++++++ packages/code/test/unit/platform.test.ts | 187 +++++++++ .../unit/provider.test.ts} | 6 +- 11 files changed, 1161 insertions(+), 188 deletions(-) delete mode 100644 packages/code/src/__tests__/cli.test.ts create mode 100644 packages/code/src/utils/args.ts create mode 100644 packages/code/src/utils/index.ts create mode 100644 packages/code/src/utils/platform.ts create mode 100644 packages/code/test/integration/cli.test.ts create mode 100644 packages/code/test/unit/args.test.ts create mode 100644 packages/code/test/unit/hooks.test.ts create mode 100644 packages/code/test/unit/platform.test.ts rename packages/code/{src/providers/lsp/__tests__/index.test.ts => test/unit/provider.test.ts} (96%) diff --git a/packages/code/package.json b/packages/code/package.json index ef4b9fa..2e1b92f 100644 --- a/packages/code/package.json +++ b/packages/code/package.json @@ -28,7 +28,7 @@ "dev": "bun --watch run src/cli.ts", "build": "echo 'Build handled by root build:npm'", "typecheck": "tsc -p tsconfig.json --noEmit", - "test": "bun test ./src" + "test": "bun test ./test" }, "dependencies": { "@pleaseai/code-format": "workspace:*", diff --git a/packages/code/src/__tests__/cli.test.ts b/packages/code/src/__tests__/cli.test.ts deleted file mode 100644 index 6986eaa..0000000 --- a/packages/code/src/__tests__/cli.test.ts +++ /dev/null @@ -1,150 +0,0 @@ -import path from 'node:path' -import { $ } from 'bun' -import { describe, expect, test } from 'bun:test' - -const CLI_PATH = path.resolve(import.meta.dir, '../cli.ts') -const PROJECT_DIR = path.resolve(import.meta.dir, '../../../..') - -describe('CLI', () => { - describe('version', () => { - test('prints version', async () => { - const result = await $`bun run ${CLI_PATH} version`.text() - expect(result).toMatch(/code \d+\.\d+\.\d+/) - }) - }) - - describe('help', () => { - test('prints help', async () => { - const result = await $`bun run ${CLI_PATH} help`.text() - expect(result).toContain('Usage:') - expect(result).toContain('format') - expect(result).toContain('lsp') - }) - }) - - describe('format --stdin', () => { - test('handles valid JSON input', async () => { - const input = JSON.stringify({ - tool_input: { - file_path: '/tmp/test.ts', - file_contents: 'const x = 1', - }, - }) - - const proc = Bun.spawn(['bun', 'run', CLI_PATH, 'format', '--stdin'], { - stdin: 'pipe', - stdout: 'pipe', - stderr: 'pipe', - cwd: PROJECT_DIR, - }) - - proc.stdin.write(input) - proc.stdin.end() - - const exitCode = await proc.exited - // May fail if no formatter found, but should not crash - expect([0, 1]).toContain(exitCode) - }) - - test('handles missing file_path', async () => { - const input = JSON.stringify({ - tool_input: {}, - }) - - const proc = Bun.spawn(['bun', 'run', CLI_PATH, 'format', '--stdin'], { - stdin: 'pipe', - stdout: 'pipe', - stderr: 'pipe', - }) - - proc.stdin.write(input) - proc.stdin.end() - - const exitCode = await proc.exited - const stderr = await new Response(proc.stderr).text() - - expect(exitCode).toBe(1) - expect(stderr).toContain('file_path') - }) - - test('handles invalid JSON input', async () => { - const proc = Bun.spawn(['bun', 'run', CLI_PATH, 'format', '--stdin'], { - stdin: 'pipe', - stdout: 'pipe', - stderr: 'pipe', - }) - - proc.stdin.write('not valid json') - proc.stdin.end() - - const exitCode = await proc.exited - expect(exitCode).toBe(1) - }) - }) - - describe('lsp --stdin', () => { - test('handles valid JSON input', async () => { - const input = JSON.stringify({ - tool_input: { - file_path: path.join(PROJECT_DIR, 'packages/code/src/cli.ts'), - }, - }) - - const proc = Bun.spawn(['bun', 'run', CLI_PATH, 'lsp', '--stdin'], { - stdin: 'pipe', - stdout: 'pipe', - stderr: 'pipe', - cwd: PROJECT_DIR, - }) - - proc.stdin.write(input) - proc.stdin.end() - - // Give it some time to initialize LSP then kill - const timeoutId = setTimeout(() => proc.kill(), 3000) - - const exitCode = await proc.exited - clearTimeout(timeoutId) - - // May exit with various codes depending on LSP availability - // 0 = success, non-zero = error or killed - expect(typeof exitCode).toBe('number') - }, 10000) // Increase test timeout - - test('handles missing file_path', async () => { - const input = JSON.stringify({ - tool_input: {}, - }) - - const proc = Bun.spawn(['bun', 'run', CLI_PATH, 'lsp', '--stdin'], { - stdin: 'pipe', - stdout: 'pipe', - stderr: 'pipe', - }) - - proc.stdin.write(input) - proc.stdin.end() - - const exitCode = await proc.exited - const stderr = await new Response(proc.stderr).text() - - expect(exitCode).toBe(1) - expect(stderr).toContain('file_path') - }) - }) - - describe('unknown command', () => { - test('prints error for unknown command', async () => { - const proc = Bun.spawn(['bun', 'run', CLI_PATH, 'unknown'], { - stdout: 'pipe', - stderr: 'pipe', - }) - - const exitCode = await proc.exited - const stderr = await new Response(proc.stderr).text() - - expect(exitCode).toBe(1) - expect(stderr).toContain('Unknown command') - }) - }) -}) diff --git a/packages/code/src/cli.ts b/packages/code/src/cli.ts index a1d4d11..1a20704 100644 --- a/packages/code/src/cli.ts +++ b/packages/code/src/cli.ts @@ -17,6 +17,7 @@ import process from 'node:process' import { Format } from '@pleaseai/code-format' import pkg from '../package.json' import { runLSPDiagnostics } from './hooks/lsp' +import { parseArgs } from './utils' const VERSION = pkg.version @@ -31,38 +32,6 @@ interface HookInput { tool_use_id: string } -interface ParsedArgs { - command: string - args: string[] - flags: Record -} - -function parseArgs(argv: string[]): ParsedArgs { - const args = argv.slice(2) - const flags: Record = {} - const positional: string[] = [] - - for (let i = 0; i < args.length; i++) { - const arg = args[i]! - if (arg.startsWith('--')) { - const [key, value] = arg.slice(2).split('=') - flags[key!] = value ?? true - } - else if (arg.startsWith('-')) { - flags[arg.slice(1)] = true - } - else { - positional.push(arg) - } - } - - return { - command: positional[0] ?? 'help', - args: positional.slice(1), - flags, - } -} - async function readStdinJson(): Promise { const chunks: Buffer[] = [] for await (const chunk of Bun.stdin.stream()) { @@ -157,6 +126,16 @@ Environment: async function main(): Promise { const { command, args, flags } = parseArgs(process.argv) + // Check for version/help flags first (before command routing) + if (flags.v || flags.version) { + versionCommand() + return + } + if (flags.h || flags.help) { + helpCommand() + return + } + const projectDir = (flags.project as string) ?? process.env.CODE_PROJECT_PATH @@ -208,14 +187,10 @@ async function main(): Promise { } case 'version': - case '-v': - case '--version': versionCommand() break case 'help': - case '-h': - case '--help': helpCommand() break diff --git a/packages/code/src/utils/args.ts b/packages/code/src/utils/args.ts new file mode 100644 index 0000000..3cee728 --- /dev/null +++ b/packages/code/src/utils/args.ts @@ -0,0 +1,54 @@ +/** + * CLI argument parsing utilities + */ + +export interface ParsedArgs { + command: string + args: string[] + flags: Record +} + +/** + * Parse command-line arguments into structured format. + * + * Supports: + * - --flag=value (equals syntax) + * - --flag (boolean flag) + * - -f (short boolean flag) + * - Positional arguments + * + * @param argv - Process argv array (first 2 elements are node/bun path and script path) + * @returns Parsed arguments with command, positional args, and flags + */ +export function parseArgs(argv: string[]): ParsedArgs { + const args = argv.slice(2) + const flags: Record = {} + const positional: string[] = [] + + for (let i = 0; i < args.length; i++) { + const arg = args[i]! + if (arg.startsWith('--')) { + const eqIndex = arg.indexOf('=') + if (eqIndex > -1) { + const key = arg.slice(2, eqIndex) + const value = arg.slice(eqIndex + 1) + flags[key] = value + } + else { + flags[arg.slice(2)] = true + } + } + else if (arg.startsWith('-')) { + flags[arg.slice(1)] = true + } + else { + positional.push(arg) + } + } + + return { + command: positional[0] ?? 'help', + args: positional.slice(1), + flags, + } +} diff --git a/packages/code/src/utils/index.ts b/packages/code/src/utils/index.ts new file mode 100644 index 0000000..5713c16 --- /dev/null +++ b/packages/code/src/utils/index.ts @@ -0,0 +1,6 @@ +/** + * Utility modules for @pleaseai/code + */ + +export { parseArgs, type ParsedArgs } from './args' +export { getPotentialBinaryPaths, getTarget, isMusl } from './platform' diff --git a/packages/code/src/utils/platform.ts b/packages/code/src/utils/platform.ts new file mode 100644 index 0000000..2fcd85a --- /dev/null +++ b/packages/code/src/utils/platform.ts @@ -0,0 +1,117 @@ +/** + * Platform detection utilities + * + * Used for determining the correct platform-specific binary to execute. + */ + +import { execSync } from 'node:child_process' +import fs from 'node:fs' +import path from 'node:path' +import process from 'node:process' + +/** + * Detect if running on musl libc (Alpine Linux, etc.) + * Based on oxc-project's detection approach + * + * Uses multiple detection methods: + * 1. Check /usr/bin/ldd content for "musl" + * 2. Check process.report for glibc version (Node 12+) + * 3. Run ldd --version and check output + */ +export function isMusl(): boolean { + // Method 1: Check /usr/bin/ldd for musl + try { + const lddContent = fs.readFileSync('/usr/bin/ldd', 'utf-8') + if (lddContent.includes('musl')) { + return true + } + } + catch { + // File doesn't exist or can't be read + } + + // Method 2: Check process.report for glibc version (Node 12+) + try { + const report = process.report?.getReport() as { header?: { glibcVersionRuntime?: string } } | undefined + if (report?.header?.glibcVersionRuntime) { + return false // Has glibc, not musl + } + } + catch { + // process.report not available + } + + // Method 3: Run ldd --version and check output + try { + const output = execSync('ldd --version 2>&1', { encoding: 'utf-8' }) + if (output.includes('musl')) { + return true + } + } + catch { + // ldd not available or failed + } + + return false +} + +/** + * Get the platform-specific target string. + * + * Format: + * - macOS: darwin-arm64, darwin-x64 + * - Windows: win32-x64, win32-arm64 + * - Linux: linux-x64-glibc, linux-arm64-musl, etc. + * + * @throws Error if platform is unsupported + */ +export function getTarget(): string { + const platform = process.platform + const arch = process.arch + + if (platform === 'darwin') { + return `darwin-${arch}` + } + + if (platform === 'win32') { + return `win32-${arch}` + } + + if (platform === 'linux') { + const libc = isMusl() ? 'musl' : 'glibc' + return `linux-${arch}-${libc}` + } + + throw new Error(`Unsupported platform: ${platform}-${arch}`) +} + +/** + * Get the list of potential binary paths to search. + * These paths cover different package manager layouts: + * - npm/yarn hoisted layout + * - non-hoisted layout + * - pnpm layout + * + * @param dirname - Directory of the calling script (__dirname) + * @param packageScope - Package scope (e.g., "@pleaseai") + * @param packageName - Package name (e.g., "code") + * @param binaryName - Binary name (e.g., "code" or "code.exe") + */ +export function getPotentialBinaryPaths( + dirname: string, + packageScope: string, + packageName: string, + binaryName: string, +): string[] { + const target = getTarget() + const fullPackageName = `${packageScope}/${packageName}-${target}` + + return [ + // Hoisted (npm/yarn) + path.join(dirname, '..', '..', fullPackageName.replace('/', path.sep), binaryName), + // Not hoisted + path.join(dirname, '..', 'node_modules', fullPackageName.replace('/', path.sep), binaryName), + // pnpm + path.join(dirname, '..', '..', '..', fullPackageName.replace('/', path.sep), binaryName), + ] +} diff --git a/packages/code/test/integration/cli.test.ts b/packages/code/test/integration/cli.test.ts new file mode 100644 index 0000000..a5a22e7 --- /dev/null +++ b/packages/code/test/integration/cli.test.ts @@ -0,0 +1,365 @@ +/** + * CLI integration tests + */ + +import path from 'node:path' +import { $ } from 'bun' +import { describe, expect, test } from 'bun:test' + +const CLI_PATH = path.resolve(import.meta.dir, '../../src/cli.ts') +const PROJECT_DIR = path.resolve(import.meta.dir, '../../../..') + +describe('CLI Integration', () => { + describe('version command', () => { + test('prints version with semantic versioning format', async () => { + const result = await $`bun run ${CLI_PATH} version`.text() + expect(result).toMatch(/code \d+\.\d+\.\d+/) + }) + + test('-v flag shows version', async () => { + const result = await $`bun run ${CLI_PATH} -v`.text() + expect(result).toMatch(/code \d+\.\d+\.\d+/) + }) + + test('--version flag shows version', async () => { + const result = await $`bun run ${CLI_PATH} --version`.text() + expect(result).toMatch(/code \d+\.\d+\.\d+/) + }) + }) + + describe('help command', () => { + test('prints help with usage info', async () => { + const result = await $`bun run ${CLI_PATH} help`.text() + expect(result).toContain('Usage:') + expect(result).toContain('format') + expect(result).toContain('lsp') + expect(result).toContain('version') + expect(result).toContain('help') + }) + + test('-h flag shows help', async () => { + const result = await $`bun run ${CLI_PATH} -h`.text() + expect(result).toContain('Usage:') + }) + + test('--help flag shows help', async () => { + const result = await $`bun run ${CLI_PATH} --help`.text() + expect(result).toContain('Usage:') + }) + + test('help includes environment variables', async () => { + const result = await $`bun run ${CLI_PATH} help`.text() + expect(result).toContain('CODE_PROJECT_PATH') + }) + + test('help includes hook mode info', async () => { + const result = await $`bun run ${CLI_PATH} help`.text() + expect(result).toContain('Hook mode') + expect(result).toContain('--stdin') + }) + }) + + describe('unknown command', () => { + test('prints error for unknown command', async () => { + const proc = Bun.spawn(['bun', 'run', CLI_PATH, 'unknowncmd'], { + stdout: 'pipe', + stderr: 'pipe', + }) + + const exitCode = await proc.exited + const stderr = await new Response(proc.stderr).text() + + expect(exitCode).toBe(1) + expect(stderr).toContain('Unknown command') + expect(stderr).toContain('unknowncmd') + }) + + test('shows help after unknown command error', async () => { + const proc = Bun.spawn(['bun', 'run', CLI_PATH, 'badcmd'], { + stdout: 'pipe', + stderr: 'pipe', + }) + + await proc.exited + const stdout = await new Response(proc.stdout).text() + + expect(stdout).toContain('Usage:') + }) + }) + + describe('format command', () => { + describe('direct mode', () => { + test('requires file argument', async () => { + const proc = Bun.spawn(['bun', 'run', CLI_PATH, 'format'], { + stdout: 'pipe', + stderr: 'pipe', + }) + + const exitCode = await proc.exited + const stderr = await new Response(proc.stderr).text() + + expect(exitCode).toBe(1) + expect(stderr).toContain('Usage:') + }) + + test('handles non-existent file gracefully', async () => { + const proc = Bun.spawn(['bun', 'run', CLI_PATH, 'format', '/nonexistent/file.ts'], { + stdout: 'pipe', + stderr: 'pipe', + cwd: PROJECT_DIR, + }) + + const exitCode = await proc.exited + // Should exit but not crash + expect([0, 1]).toContain(exitCode) + }) + + test('formats existing file and returns JSON', async () => { + const testFile = path.join(PROJECT_DIR, 'packages/code/src/cli.ts') + const proc = Bun.spawn(['bun', 'run', CLI_PATH, 'format', testFile], { + stdout: 'pipe', + stderr: 'pipe', + cwd: PROJECT_DIR, + }) + + const exitCode = await proc.exited + const stdout = await new Response(proc.stdout).text() + + expect(exitCode).toBe(0) + const result = JSON.parse(stdout.trim()) + expect(result).toHaveProperty('success') + expect(result).toHaveProperty('file') + }) + }) + + describe('--stdin hook mode', () => { + test('handles valid JSON input', async () => { + const input = JSON.stringify({ + session_id: 'test-session', + cwd: PROJECT_DIR, + tool_name: 'Write', + tool_input: { + file_path: path.join(PROJECT_DIR, 'packages/code/src/cli.ts'), + content: 'const x = 1', + }, + tool_use_id: 'test-id', + }) + + const proc = Bun.spawn(['bun', 'run', CLI_PATH, 'format', '--stdin'], { + stdin: 'pipe', + stdout: 'pipe', + stderr: 'pipe', + cwd: PROJECT_DIR, + }) + + proc.stdin.write(input) + proc.stdin.end() + + const exitCode = await proc.exited + // May succeed or fail depending on formatter availability + expect([0, 1]).toContain(exitCode) + }) + + test('returns suppressOutput on success in hook mode', async () => { + const testFile = path.join(PROJECT_DIR, 'packages/code/src/cli.ts') + const input = JSON.stringify({ + session_id: 'test-session', + cwd: PROJECT_DIR, + tool_name: 'Write', + tool_input: { file_path: testFile }, + tool_use_id: 'test-id', + }) + + const proc = Bun.spawn(['bun', 'run', CLI_PATH, 'format', '--stdin'], { + stdin: 'pipe', + stdout: 'pipe', + stderr: 'pipe', + cwd: PROJECT_DIR, + }) + + proc.stdin.write(input) + proc.stdin.end() + + const exitCode = await proc.exited + const stdout = await new Response(proc.stdout).text() + + if (exitCode === 0 && stdout.trim()) { + const result = JSON.parse(stdout.trim()) + expect(result).toHaveProperty('suppressOutput', true) + } + }) + + test('handles missing file_path in tool_input', async () => { + const input = JSON.stringify({ + session_id: 'test-session', + cwd: PROJECT_DIR, + tool_name: 'Write', + tool_input: {}, + tool_use_id: 'test-id', + }) + + const proc = Bun.spawn(['bun', 'run', CLI_PATH, 'format', '--stdin'], { + stdin: 'pipe', + stdout: 'pipe', + stderr: 'pipe', + }) + + proc.stdin.write(input) + proc.stdin.end() + + const exitCode = await proc.exited + const stderr = await new Response(proc.stderr).text() + + expect(exitCode).toBe(1) + expect(stderr).toContain('file_path') + }) + + test('handles invalid JSON input', async () => { + const proc = Bun.spawn(['bun', 'run', CLI_PATH, 'format', '--stdin'], { + stdin: 'pipe', + stdout: 'pipe', + stderr: 'pipe', + }) + + proc.stdin.write('not valid json {{{') + proc.stdin.end() + + const exitCode = await proc.exited + const stderr = await new Response(proc.stderr).text() + + expect(exitCode).toBe(1) + expect(stderr).toContain('Invalid JSON') + }) + + test('handles empty stdin', async () => { + const proc = Bun.spawn(['bun', 'run', CLI_PATH, 'format', '--stdin'], { + stdin: 'pipe', + stdout: 'pipe', + stderr: 'pipe', + }) + + proc.stdin.end() + + const exitCode = await proc.exited + const stderr = await new Response(proc.stderr).text() + + expect(exitCode).toBe(1) + expect(stderr).toContain('No input') + }) + }) + }) + + describe('lsp command', () => { + describe('direct mode', () => { + test('requires file argument', async () => { + const proc = Bun.spawn(['bun', 'run', CLI_PATH, 'lsp'], { + stdout: 'pipe', + stderr: 'pipe', + }) + + const exitCode = await proc.exited + const stderr = await new Response(proc.stderr).text() + + expect(exitCode).toBe(1) + expect(stderr).toContain('Usage:') + }) + }) + + describe('--stdin hook mode', () => { + test('handles valid JSON input', async () => { + const input = JSON.stringify({ + session_id: 'test-session', + cwd: PROJECT_DIR, + tool_name: 'Write', + tool_input: { + file_path: path.join(PROJECT_DIR, 'packages/code/src/cli.ts'), + }, + tool_use_id: 'test-id', + }) + + const proc = Bun.spawn(['bun', 'run', CLI_PATH, 'lsp', '--stdin'], { + stdin: 'pipe', + stdout: 'pipe', + stderr: 'pipe', + cwd: PROJECT_DIR, + }) + + proc.stdin.write(input) + proc.stdin.end() + + // Give it time to initialize LSP + const timeoutId = setTimeout(() => proc.kill(), 5000) + + const exitCode = await proc.exited + clearTimeout(timeoutId) + + // May exit with various codes depending on LSP availability + expect(typeof exitCode).toBe('number') + }, 10000) + + test('handles missing file_path in tool_input', async () => { + const input = JSON.stringify({ + session_id: 'test-session', + cwd: PROJECT_DIR, + tool_name: 'Write', + tool_input: {}, + tool_use_id: 'test-id', + }) + + const proc = Bun.spawn(['bun', 'run', CLI_PATH, 'lsp', '--stdin'], { + stdin: 'pipe', + stdout: 'pipe', + stderr: 'pipe', + }) + + proc.stdin.write(input) + proc.stdin.end() + + const exitCode = await proc.exited + const stderr = await new Response(proc.stderr).text() + + expect(exitCode).toBe(1) + expect(stderr).toContain('file_path') + }) + }) + }) + + describe('--project flag', () => { + test('accepts --project flag for format', async () => { + const testFile = path.join(PROJECT_DIR, 'packages/code/src/cli.ts') + const proc = Bun.spawn( + ['bun', 'run', CLI_PATH, 'format', testFile, `--project=${PROJECT_DIR}`], + { + stdout: 'pipe', + stderr: 'pipe', + }, + ) + + const exitCode = await proc.exited + // Should not crash with --project flag + expect([0, 1]).toContain(exitCode) + }) + + test('accepts --project=value syntax', async () => { + const testFile = path.join(PROJECT_DIR, 'packages/code/src/cli.ts') + const proc = Bun.spawn( + ['bun', 'run', CLI_PATH, 'format', testFile, '--project=/tmp'], + { + stdout: 'pipe', + stderr: 'pipe', + }, + ) + + const exitCode = await proc.exited + // Should not crash + expect([0, 1]).toContain(exitCode) + }) + }) + + describe('no command (default)', () => { + test('shows help when no command provided', async () => { + const result = await $`bun run ${CLI_PATH}`.text() + expect(result).toContain('Usage:') + }) + }) +}) diff --git a/packages/code/test/unit/args.test.ts b/packages/code/test/unit/args.test.ts new file mode 100644 index 0000000..5f58168 --- /dev/null +++ b/packages/code/test/unit/args.test.ts @@ -0,0 +1,184 @@ +/** + * Unit tests for argument parsing + */ + +import { describe, expect, test } from 'bun:test' +import { parseArgs } from '../../src/utils/args' + +// Helper to create argv array (simulating process.argv) +function argv(...args: string[]): string[] { + return ['/path/to/node', '/path/to/script.ts', ...args] +} + +describe('parseArgs', () => { + describe('command parsing', () => { + test('extracts command from first positional argument', () => { + const result = parseArgs(argv('format')) + expect(result.command).toBe('format') + }) + + test('defaults to help when no command given', () => { + const result = parseArgs(argv()) + expect(result.command).toBe('help') + }) + + test('handles multiple commands (takes first)', () => { + const result = parseArgs(argv('format', 'extra')) + expect(result.command).toBe('format') + expect(result.args).toEqual(['extra']) + }) + }) + + describe('positional arguments', () => { + test('collects positional arguments after command', () => { + const result = parseArgs(argv('format', 'file1.ts', 'file2.ts')) + expect(result.command).toBe('format') + expect(result.args).toEqual(['file1.ts', 'file2.ts']) + }) + + test('returns empty args when no positional after command', () => { + const result = parseArgs(argv('version')) + expect(result.args).toEqual([]) + }) + }) + + describe('long flags (--flag)', () => { + test('parses boolean flag', () => { + const result = parseArgs(argv('format', '--stdin')) + expect(result.flags.stdin).toBe(true) + }) + + test('parses flag with equals value', () => { + const result = parseArgs(argv('format', '--project=/my/path')) + expect(result.flags.project).toBe('/my/path') + }) + + test('parses flag with empty value', () => { + const result = parseArgs(argv('format', '--project=')) + expect(result.flags.project).toBe('') + }) + + test('parses multiple boolean flags', () => { + const result = parseArgs(argv('lsp', '--stdin', '--verbose')) + expect(result.flags.stdin).toBe(true) + expect(result.flags.verbose).toBe(true) + }) + + test('parses multiple value flags', () => { + const result = parseArgs(argv('format', '--project=/path', '--output=json')) + expect(result.flags.project).toBe('/path') + expect(result.flags.output).toBe('json') + }) + }) + + describe('short flags (-f)', () => { + test('parses short boolean flag', () => { + const result = parseArgs(argv('-v')) + expect(result.flags.v).toBe(true) + }) + + test('parses short flag -h', () => { + const result = parseArgs(argv('-h')) + expect(result.flags.h).toBe(true) + }) + + test('parses multiple short flags', () => { + const result = parseArgs(argv('-v', '-h')) + expect(result.flags.v).toBe(true) + expect(result.flags.h).toBe(true) + }) + }) + + describe('mixed flags and positional', () => { + test('handles flags before positional', () => { + const result = parseArgs(argv('format', '--stdin', 'file.ts')) + expect(result.command).toBe('format') + expect(result.flags.stdin).toBe(true) + expect(result.args).toEqual(['file.ts']) + }) + + test('handles flags after positional', () => { + const result = parseArgs(argv('format', 'file.ts', '--project=/path')) + expect(result.command).toBe('format') + expect(result.args).toEqual(['file.ts']) + expect(result.flags.project).toBe('/path') + }) + + test('handles interleaved flags and positional', () => { + const result = parseArgs(argv('format', '--stdin', 'file.ts', '--verbose')) + expect(result.command).toBe('format') + expect(result.flags.stdin).toBe(true) + expect(result.flags.verbose).toBe(true) + expect(result.args).toEqual(['file.ts']) + }) + }) + + describe('edge cases', () => { + test('handles flag-like positional argument in quotes', () => { + // This would come from shell as a single argument without dashes + const result = parseArgs(argv('format', 'test-file.ts')) + expect(result.args).toEqual(['test-file.ts']) + }) + + test('handles empty argv', () => { + const result = parseArgs([]) + expect(result.command).toBe('help') + expect(result.args).toEqual([]) + expect(result.flags).toEqual({}) + }) + + test('handles only node and script path', () => { + const result = parseArgs(['/usr/bin/node', '/script.ts']) + expect(result.command).toBe('help') + }) + + test('handles flag with equals containing equals', () => { + const result = parseArgs(argv('format', '--config=a=b=c')) + // Note: split('=') only splits first occurrence in our implementation + // So this gets key='config' and value='a' (rest is lost) + // This is current behavior - documenting it + expect(result.flags.config).toBe('a=b=c') + }) + + test('handles long flag name', () => { + const result = parseArgs(argv('format', '--very-long-flag-name')) + expect(result.flags['very-long-flag-name']).toBe(true) + }) + + test('handles numeric value', () => { + const result = parseArgs(argv('format', '--timeout=5000')) + expect(result.flags.timeout).toBe('5000') + }) + + test('handles paths with special characters', () => { + const result = parseArgs(argv('format', '--project=/path/with spaces/dir')) + expect(result.flags.project).toBe('/path/with spaces/dir') + }) + }) + + describe('CLI command aliases', () => { + test('parses -v as flag (not command)', () => { + const result = parseArgs(argv('-v')) + expect(result.command).toBe('help') // No positional, defaults to help + expect(result.flags.v).toBe(true) + }) + + test('parses --version as flag', () => { + const result = parseArgs(argv('--version')) + expect(result.command).toBe('help') + expect(result.flags.version).toBe(true) + }) + + test('parses -h as flag', () => { + const result = parseArgs(argv('-h')) + expect(result.command).toBe('help') + expect(result.flags.h).toBe(true) + }) + + test('parses --help as flag', () => { + const result = parseArgs(argv('--help')) + expect(result.command).toBe('help') + expect(result.flags.help).toBe(true) + }) + }) +}) diff --git a/packages/code/test/unit/hooks.test.ts b/packages/code/test/unit/hooks.test.ts new file mode 100644 index 0000000..81606f1 --- /dev/null +++ b/packages/code/test/unit/hooks.test.ts @@ -0,0 +1,231 @@ +/** + * Unit tests for hooks module + */ + +import { describe, expect, test } from 'bun:test' +import { formatDiagnosticsReport } from '../../src/hooks' + +// Mock diagnostic structure matching LSP Diagnostic type +interface MockDiagnostic { + range: { + start: { line: number, character: number } + end: { line: number, character: number } + } + message: string + severity?: number + code?: string | number + source?: string +} + +function createDiagnostic( + line: number, + char: number, + message: string, + severity: number = 1, + code?: string | number, +): MockDiagnostic { + return { + range: { + start: { line, character: char }, + end: { line, character: char + 1 }, + }, + message, + severity, + code, + } +} + +describe('formatDiagnosticsReport', () => { + describe('empty diagnostics', () => { + test('returns null for empty diagnostics', () => { + const result = formatDiagnosticsReport({}, '/project') + expect(result).toBeNull() + }) + + test('returns null for file with empty diagnostics array', () => { + const result = formatDiagnosticsReport( + { '/project/src/file.ts': [] }, + '/project', + ) + expect(result).toBeNull() + }) + }) + + describe('single error', () => { + test('formats single error correctly', () => { + const diagnostics = { + '/project/src/file.ts': [ + createDiagnostic(10, 5, 'Type error: expected number'), + ], + } + const result = formatDiagnosticsReport(diagnostics, '/project') + + expect(result).not.toBeNull() + expect(result).toContain('✗ 1 error found') + expect(result).toContain('src/file.ts:11:6') + expect(result).toContain('Type error: expected number') + }) + + test('formats error with code', () => { + const diagnostics = { + '/project/src/file.ts': [ + createDiagnostic(10, 5, 'Type error', 1, 'TS2345'), + ], + } + const result = formatDiagnosticsReport(diagnostics, '/project') + + expect(result).toContain('[TS2345]') + }) + }) + + describe('warnings', () => { + test('formats single warning correctly', () => { + const diagnostics = { + '/project/src/file.ts': [ + createDiagnostic(5, 0, 'Unused variable', 2), + ], + } + const result = formatDiagnosticsReport(diagnostics, '/project') + + expect(result).not.toBeNull() + expect(result).toContain('✗ 1 warning found') + expect(result).toContain('⚠') + }) + + test('formats multiple warnings correctly', () => { + const diagnostics = { + '/project/src/file.ts': [ + createDiagnostic(5, 0, 'Unused variable x', 2), + createDiagnostic(10, 0, 'Unused variable y', 2), + ], + } + const result = formatDiagnosticsReport(diagnostics, '/project') + + expect(result).toContain('✗ 2 warnings found') + }) + }) + + describe('mixed errors and warnings', () => { + test('formats mixed diagnostics correctly', () => { + const diagnostics = { + '/project/src/file.ts': [ + createDiagnostic(5, 0, 'Type error', 1), + createDiagnostic(10, 0, 'Unused variable', 2), + createDiagnostic(15, 0, 'Another error', 1), + ], + } + const result = formatDiagnosticsReport(diagnostics, '/project') + + expect(result).not.toBeNull() + expect(result).toContain('✗ 2 errors, 1 warning found') + }) + + test('formats multiple files correctly', () => { + const diagnostics = { + '/project/src/a.ts': [createDiagnostic(0, 0, 'Error in a', 1)], + '/project/src/b.ts': [createDiagnostic(0, 0, 'Warning in b', 2)], + } + const result = formatDiagnosticsReport(diagnostics, '/project') + + expect(result).toContain('✗ 1 error, 1 warning found') + expect(result).toContain('src/a.ts') + expect(result).toContain('src/b.ts') + }) + }) + + describe('info and hint severities', () => { + test('does not count info in summary', () => { + const diagnostics = { + '/project/src/file.ts': [ + createDiagnostic(5, 0, 'Info message', 3), + ], + } + const result = formatDiagnosticsReport(diagnostics, '/project') + + // Info severity (3) is not counted in error/warning totals + expect(result).toBeNull() + }) + + test('does not count hint in summary', () => { + const diagnostics = { + '/project/src/file.ts': [ + createDiagnostic(5, 0, 'Hint message', 4), + ], + } + const result = formatDiagnosticsReport(diagnostics, '/project') + + // Hint severity (4) is not counted in error/warning totals + expect(result).toBeNull() + }) + }) + + describe('overflow handling', () => { + test('truncates to 5 diagnostics and shows overflow', () => { + const diagnostics = { + '/project/src/file.ts': [ + createDiagnostic(1, 0, 'Error 1', 1), + createDiagnostic(2, 0, 'Error 2', 1), + createDiagnostic(3, 0, 'Error 3', 1), + createDiagnostic(4, 0, 'Error 4', 1), + createDiagnostic(5, 0, 'Error 5', 1), + createDiagnostic(6, 0, 'Error 6', 1), + createDiagnostic(7, 0, 'Error 7', 1), + ], + } + const result = formatDiagnosticsReport(diagnostics, '/project') + + expect(result).not.toBeNull() + expect(result).toContain('✗ 7 errors found') + expect(result).toContain('... and 2 more') + // First 5 should be included + expect(result).toContain('Error 1') + expect(result).toContain('Error 5') + // 6th and 7th should not be shown individually + expect(result).not.toContain('Error 6') + expect(result).not.toContain('Error 7') + }) + }) + + describe('multiline messages', () => { + test('uses only first line of multiline messages', () => { + const diagnostics = { + '/project/src/file.ts': [ + createDiagnostic(5, 0, 'First line\nSecond line\nThird line', 1), + ], + } + const result = formatDiagnosticsReport(diagnostics, '/project') + + expect(result).toContain('First line') + expect(result).not.toContain('Second line') + expect(result).not.toContain('Third line') + }) + }) + + describe('line/column numbering', () => { + test('converts 0-based to 1-based line/column', () => { + const diagnostics = { + '/project/src/file.ts': [ + createDiagnostic(0, 0, 'Error at start', 1), + ], + } + const result = formatDiagnosticsReport(diagnostics, '/project') + + // 0,0 should display as 1:1 + expect(result).toContain('src/file.ts:1:1') + }) + }) + + describe('relative path handling', () => { + test('shows relative path from project root', () => { + const diagnostics = { + '/project/src/deep/nested/file.ts': [ + createDiagnostic(0, 0, 'Error', 1), + ], + } + const result = formatDiagnosticsReport(diagnostics, '/project') + + expect(result).toContain('src/deep/nested/file.ts') + expect(result).not.toContain('/project/') + }) + }) +}) diff --git a/packages/code/test/unit/platform.test.ts b/packages/code/test/unit/platform.test.ts new file mode 100644 index 0000000..833bb95 --- /dev/null +++ b/packages/code/test/unit/platform.test.ts @@ -0,0 +1,187 @@ +/** + * Unit tests for platform detection utilities + */ + +import { describe, expect, test } from 'bun:test' +import { getPotentialBinaryPaths, getTarget, isMusl } from '../../src/utils/platform' + +describe('platform detection', () => { + describe('getTarget', () => { + test('returns valid target format', () => { + const target = getTarget() + + // Target should be non-empty string + expect(typeof target).toBe('string') + expect(target.length).toBeGreaterThan(0) + + // Target should contain platform and arch + const platform = process.platform + const arch = process.arch + + if (platform === 'darwin') { + expect(target).toBe(`darwin-${arch}`) + } + else if (platform === 'win32') { + expect(target).toBe(`win32-${arch}`) + } + else if (platform === 'linux') { + expect(target).toMatch(new RegExp(`linux-${arch}-(glibc|musl)`)) + } + }) + + test('matches expected format for current platform', () => { + const target = getTarget() + + // Should contain a hyphen separator + expect(target).toContain('-') + + // Should start with known platform + const validPrefixes = ['darwin', 'win32', 'linux'] + const startsWithValid = validPrefixes.some(prefix => target.startsWith(prefix)) + expect(startsWithValid).toBe(true) + }) + }) + + describe('isMusl', () => { + test('returns boolean', () => { + const result = isMusl() + expect(typeof result).toBe('boolean') + }) + + test('returns false on macOS and Windows', () => { + // musl is Linux-specific + if (process.platform !== 'linux') { + expect(isMusl()).toBe(false) + } + }) + }) + + describe('getPotentialBinaryPaths', () => { + const testDirname = '/app/node_modules/@pleaseai/code/dist' + const packageScope = '@pleaseai' + const packageName = 'code' + const binaryName = 'code' + + test('returns array of paths', () => { + const paths = getPotentialBinaryPaths( + testDirname, + packageScope, + packageName, + binaryName, + ) + + expect(Array.isArray(paths)).toBe(true) + expect(paths.length).toBe(3) + }) + + test('paths contain package scope and name', () => { + const paths = getPotentialBinaryPaths( + testDirname, + packageScope, + packageName, + binaryName, + ) + + // Each path should contain the package reference + for (const p of paths) { + expect(p).toContain('@pleaseai') + expect(p).toContain('code') + } + }) + + test('paths end with binary name', () => { + const paths = getPotentialBinaryPaths( + testDirname, + packageScope, + packageName, + binaryName, + ) + + for (const p of paths) { + expect(p).toMatch(/code$/) + } + }) + + test('handles Windows binary name', () => { + const paths = getPotentialBinaryPaths( + testDirname, + packageScope, + packageName, + 'code.exe', + ) + + for (const p of paths) { + expect(p).toMatch(/code\.exe$/) + } + }) + + test('includes target in package path', () => { + const paths = getPotentialBinaryPaths( + testDirname, + packageScope, + packageName, + binaryName, + ) + + const target = getTarget() + + for (const p of paths) { + expect(p).toContain(`code-${target}`) + } + }) + + test('generates different paths for different layouts', () => { + const paths = getPotentialBinaryPaths( + testDirname, + packageScope, + packageName, + binaryName, + ) + + // All paths should be unique + const uniquePaths = new Set(paths) + expect(uniquePaths.size).toBe(paths.length) + }) + + test('hoisted path resolves to sibling package directory', () => { + const target = getTarget() + const paths = getPotentialBinaryPaths( + '/project/node_modules/@pleaseai/code/dist', + packageScope, + packageName, + binaryName, + ) + + // First path (hoisted) should be at node_modules level + const hoistedPath = paths[0]! + // Should resolve relative to the root node_modules + expect(hoistedPath).toContain(`code-${target}`) + }) + + test('non-hoisted path includes node_modules subdirectory', () => { + const paths = getPotentialBinaryPaths( + testDirname, + packageScope, + packageName, + binaryName, + ) + + // Second path (non-hoisted) should include node_modules + const nonHoistedPath = paths[1]! + expect(nonHoistedPath).toContain('node_modules') + }) + + test('all three paths are different', () => { + const paths = getPotentialBinaryPaths( + '/project/node_modules/.pnpm/@pleaseai+code@1.0.0/node_modules/@pleaseai/code/dist', + packageScope, + packageName, + binaryName, + ) + + // All three search paths should be unique + const uniquePaths = new Set(paths) + expect(uniquePaths.size).toBe(3) + }) + }) +}) diff --git a/packages/code/src/providers/lsp/__tests__/index.test.ts b/packages/code/test/unit/provider.test.ts similarity index 96% rename from packages/code/src/providers/lsp/__tests__/index.test.ts rename to packages/code/test/unit/provider.test.ts index 7a2207d..5bb80f9 100644 --- a/packages/code/src/providers/lsp/__tests__/index.test.ts +++ b/packages/code/test/unit/provider.test.ts @@ -1,5 +1,9 @@ +/** + * Unit tests for LSP Provider + */ + import { afterEach, describe, expect, test } from 'bun:test' -import { createLSPProvider, LSPProvider } from '../index' +import { createLSPProvider, LSPProvider } from '../../src/providers/lsp' describe('LSPProvider', () => { let provider: LSPProvider | null = null From 6ad3868fe6f67f16a2a8ba56d06401945262f9e3 Mon Sep 17 00:00:00 2001 From: Minsu Lee Date: Thu, 18 Dec 2025 18:51:19 +0900 Subject: [PATCH 2/2] fix(code): improve error handling and add missing tests - Document fallback behavior in platform detection with better comments - Preserve JSON parse error context in cli.ts for better debugging - Add tests for flag precedence over commands (--version, --help) - Add tests for environment variable priority (CODE_PROJECT_PATH) --- packages/code/src/cli.ts | 5 +- packages/code/src/launcher/cli.ts | 16 +++++- packages/code/src/utils/platform.ts | 11 +++- packages/code/test/integration/cli.test.ts | 64 +++++++++++++++++++++- 4 files changed, 87 insertions(+), 9 deletions(-) diff --git a/packages/code/src/cli.ts b/packages/code/src/cli.ts index 1a20704..bc8172d 100644 --- a/packages/code/src/cli.ts +++ b/packages/code/src/cli.ts @@ -46,8 +46,9 @@ async function readStdinJson(): Promise { try { return JSON.parse(text) } - catch { - throw new Error('Invalid JSON input from stdin') + catch (error: unknown) { + const message = error instanceof Error ? error.message : 'unknown parse error' + throw new Error(`Invalid JSON input from stdin: ${message}`) } } diff --git a/packages/code/src/launcher/cli.ts b/packages/code/src/launcher/cli.ts index afa6ea8..1b59f91 100644 --- a/packages/code/src/launcher/cli.ts +++ b/packages/code/src/launcher/cli.ts @@ -20,6 +20,15 @@ const PACKAGE_NAME = 'code' /** * Detect if running on musl libc (Alpine Linux, etc.) * Based on oxc-project's detection approach + * + * Uses multiple detection methods: + * 1. Check /usr/bin/ldd content for "musl" + * 2. Check process.report for glibc version (Node 12+) + * 3. Run ldd --version and check output + * + * All methods use fallback behavior - if one fails (expected or unexpected), + * the next method is tried. Common expected errors: ENOENT (file not found). + * Unexpected errors (EACCES, ENOMEM) are silently ignored with fallback. */ function isMusl(): boolean { // Method 1: Check /usr/bin/ldd for musl @@ -30,7 +39,8 @@ function isMusl(): boolean { } } catch { - // File doesn't exist or can't be read + // Expected: ENOENT (file not found) on non-Linux or some distros + // Unexpected errors (EACCES, ENOMEM, etc.) also fall through to next method } // Method 2: Check process.report for glibc version (Node 12+) @@ -41,7 +51,7 @@ function isMusl(): boolean { } } catch { - // process.report not available + // Expected: process.report not available in all environments } // Method 3: Run ldd --version and check output @@ -52,7 +62,7 @@ function isMusl(): boolean { } } catch { - // ldd not available or failed + // Expected: ldd not available on non-Linux systems } return false diff --git a/packages/code/src/utils/platform.ts b/packages/code/src/utils/platform.ts index 2fcd85a..7134367 100644 --- a/packages/code/src/utils/platform.ts +++ b/packages/code/src/utils/platform.ts @@ -17,6 +17,10 @@ import process from 'node:process' * 1. Check /usr/bin/ldd content for "musl" * 2. Check process.report for glibc version (Node 12+) * 3. Run ldd --version and check output + * + * All methods use fallback behavior - if one fails (expected or unexpected), + * the next method is tried. Common expected errors: ENOENT (file not found). + * Unexpected errors (EACCES, ENOMEM) are silently ignored with fallback. */ export function isMusl(): boolean { // Method 1: Check /usr/bin/ldd for musl @@ -27,7 +31,8 @@ export function isMusl(): boolean { } } catch { - // File doesn't exist or can't be read + // Expected: ENOENT (file not found) on non-Linux or some distros + // Unexpected errors (EACCES, ENOMEM, etc.) also fall through to next method } // Method 2: Check process.report for glibc version (Node 12+) @@ -38,7 +43,7 @@ export function isMusl(): boolean { } } catch { - // process.report not available + // Expected: process.report not available in all environments } // Method 3: Run ldd --version and check output @@ -49,7 +54,7 @@ export function isMusl(): boolean { } } catch { - // ldd not available or failed + // Expected: ldd not available on non-Linux systems } return false diff --git a/packages/code/test/integration/cli.test.ts b/packages/code/test/integration/cli.test.ts index a5a22e7..af9415a 100644 --- a/packages/code/test/integration/cli.test.ts +++ b/packages/code/test/integration/cli.test.ts @@ -25,6 +25,17 @@ describe('CLI Integration', () => { const result = await $`bun run ${CLI_PATH} --version`.text() expect(result).toMatch(/code \d+\.\d+\.\d+/) }) + + test('--version flag takes precedence over format command', async () => { + const result = await $`bun run ${CLI_PATH} format file.ts --version`.text() + expect(result).toMatch(/code \d+\.\d+\.\d+/) + // Should NOT try to format file.ts, just show version + }) + + test('-v flag takes precedence over lsp command', async () => { + const result = await $`bun run ${CLI_PATH} lsp file.ts -v`.text() + expect(result).toMatch(/code \d+\.\d+\.\d+/) + }) }) describe('help command', () => { @@ -57,6 +68,17 @@ describe('CLI Integration', () => { expect(result).toContain('Hook mode') expect(result).toContain('--stdin') }) + + test('--help flag takes precedence over format command', async () => { + const result = await $`bun run ${CLI_PATH} format file.ts --help`.text() + expect(result).toContain('Usage:') + // Should NOT try to format file.ts, just show help + }) + + test('-h flag takes precedence over lsp command', async () => { + const result = await $`bun run ${CLI_PATH} lsp file.ts -h`.text() + expect(result).toContain('Usage:') + }) }) describe('unknown command', () => { @@ -214,7 +236,7 @@ describe('CLI Integration', () => { expect(stderr).toContain('file_path') }) - test('handles invalid JSON input', async () => { + test('handles invalid JSON input with parse error details', async () => { const proc = Bun.spawn(['bun', 'run', CLI_PATH, 'format', '--stdin'], { stdin: 'pipe', stdout: 'pipe', @@ -229,6 +251,8 @@ describe('CLI Integration', () => { expect(exitCode).toBe(1) expect(stderr).toContain('Invalid JSON') + // Should include the original parse error details + expect(stderr).toMatch(/Invalid JSON input from stdin:/) }) test('handles empty stdin', async () => { @@ -354,6 +378,44 @@ describe('CLI Integration', () => { // Should not crash expect([0, 1]).toContain(exitCode) }) + + test('--project flag takes precedence over CODE_PROJECT_PATH env', async () => { + const testFile = path.join(PROJECT_DIR, 'packages/code/src/cli.ts') + const proc = Bun.spawn( + ['bun', 'run', CLI_PATH, 'format', testFile, `--project=${PROJECT_DIR}`], + { + stdout: 'pipe', + stderr: 'pipe', + env: { + ...process.env, + CODE_PROJECT_PATH: '/nonexistent/path', + }, + }, + ) + + const exitCode = await proc.exited + // Should use --project flag, not env var, and succeed + expect([0, 1]).toContain(exitCode) + }) + + test('CODE_PROJECT_PATH env is used when --project not specified', async () => { + const testFile = path.join(PROJECT_DIR, 'packages/code/src/cli.ts') + const proc = Bun.spawn( + ['bun', 'run', CLI_PATH, 'format', testFile], + { + stdout: 'pipe', + stderr: 'pipe', + env: { + ...process.env, + CODE_PROJECT_PATH: PROJECT_DIR, + }, + }, + ) + + const exitCode = await proc.exited + // Should use CODE_PROJECT_PATH env var + expect([0, 1]).toContain(exitCode) + }) }) describe('no command (default)', () => {