also looking at this
fix(test): defuse the suite's landmines, and cover the dossier's untested modules
#1436
11 files
+617
-12
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.
| @@ -1034,9 +1034,15 @@ bun test | ||
| 1034 | 1034 | ## Build From Source |
| 1035 | 1035 | |
| 1036 | 1036 | ```bash |
| 1037 | bun install # not optional — see the note below | |
| 1037 | 1038 | bun run build |
| 1038 | 1039 | ``` |
| 1039 | 1040 | |
| 1041 | > [!NOTE] | |
| 1042 | > `bun install` is a hard prerequisite for every dev command. On a fresh clone without `node_modules`, `bun test` fails with over a hundred phantom errors (`Cannot find package 'ts-pantry'` and friends) that look like real bugs and are not. | |
| 1043 | > | |
| 1044 | > Git hooks (staged lint on `pre-commit`, gitlint on `commit-msg`) are declared in `package.json` but are not installed automatically — run `bunx bun-git-hooks` once after cloning to activate them. | |
| 1045 | ||
| 1040 | 1046 | ## Changelog |
| 1041 | 1047 | |
| 1042 | 1048 | Please see our [releases](https://github.com/stacksjs/buddy/releases) page for more information on what has changed recently. |
| @@ -0,0 +1,191 @@ | ||
| 1 | import type { AiCompletionRequest } from '../src/ai/types' | |
| 2 | import { beforeEach, describe, expect, it, mock } from 'bun:test' | |
| 3 | import { createAnthropicProvider } from '../src/ai/providers/anthropic' | |
| 4 | import { AiProviderError } from '../src/ai/types' | |
| 5 | ||
| 6 | /** | |
| 7 | * The google and openai providers were tested directly; this one — the | |
| 8 | * default when several keys are present — was not. The SDK is faked at the | |
| 9 | * module seam so `complete` runs its real mapping code: block collection, | |
| 10 | * stop-reason normalisation, usage mapping, and the lazy client. | |
| 11 | * | |
| 12 | * `mock.module` is process-global; that is safe here because nothing else in | |
| 13 | * the suite imports `@anthropic-ai/sdk` (it is an optional peer the provider | |
| 14 | * itself loads lazily). The fake delegates nowhere — there is no real network | |
| 15 | * client any test should ever construct. | |
| 16 | */ | |
| 17 | interface FakeResponse { | |
| 18 | content: Array<{ type: 'text', text: string } | { type: 'tool_use', id: string, name: string, input: unknown }> | |
| 19 | stop_reason: string | null | |
| 20 | usage: { input_tokens: number, output_tokens: number, cache_read_input_tokens?: number } | |
| 21 | model: string | |
| 22 | } | |
| 23 | ||
| 24 | const OK_RESPONSE: FakeResponse = { | |
| 25 | content: [{ type: 'text', text: 'hello' }], | |
| 26 | stop_reason: 'end_turn', | |
| 27 | usage: { input_tokens: 10, output_tokens: 5 }, | |
| 28 | model: 'claude-test-1', | |
| 29 | } | |
| 30 | ||
| 31 | let respondWith: () => FakeResponse = () => OK_RESPONSE | |
| 32 | let createCalls: any[] = [] | |
| 33 | let constructorCalls: any[] = [] | |
| 34 | ||
| 35 | class FakeAnthropic { | |
| 36 | messages = { | |
| 37 | create: async (params: unknown): Promise<FakeResponse> => { | |
| 38 | createCalls.push(params) | |
| 39 | return respondWith() | |
| 40 | }, | |
| 41 | } | |
| 42 | ||
| 43 | constructor(options: unknown) { | |
| 44 | constructorCalls.push(options) | |
| 45 | } | |
| 46 | } | |
| 47 | ||
| 48 | mock.module('@anthropic-ai/sdk', () => ({ default: FakeAnthropic })) | |
| 49 | ||
| 50 | /** A minimal request; tests spread overrides onto it. */ | |
| 51 | function request(overrides: Partial<AiCompletionRequest> = {}): AiCompletionRequest { | |
| 52 | return { messages: [{ role: 'user', content: 'hi' }], ...overrides } as AiCompletionRequest | |
| 53 | } | |
| 54 | ||
| 55 | beforeEach(() => { | |
| 56 | respondWith = () => OK_RESPONSE | |
| 57 | createCalls = [] | |
| 58 | constructorCalls = [] | |
| 59 | }) | |
| 60 | ||
| 61 | describe('anthropic provider', () => { | |
| 62 | describe('response mapping', () => { | |
| 63 | it('success case - concatenates text blocks and collects tool calls', async () => { | |
| 64 | respondWith = () => ({ | |
| 65 | ...OK_RESPONSE, | |
| 66 | content: [ | |
| 67 | { type: 'text', text: 'part one ' }, | |
| 68 | { type: 'tool_use', id: 't1', name: 'read_file', input: { path: 'a.ts' } }, | |
| 69 | { type: 'text', text: 'part two' }, | |
| 70 | ], | |
| 71 | stop_reason: 'tool_use', | |
| 72 | }) | |
| 73 | ||
| 74 | const provider = createAnthropicProvider({ apiKey: 'sk-test' }) | |
| 75 | const response = await provider.complete(request(), 'claude-test-1') | |
| 76 | ||
| 77 | expect(response.text).toBe('part one part two') | |
| 78 | expect(response.toolCalls).toEqual([{ id: 't1', name: 'read_file', input: { path: 'a.ts' } }]) | |
| 79 | expect(response.stopReason).toBe('tool_use') | |
| 80 | }) | |
| 81 | ||
| 82 | it('success case - maps usage, including the cache read count only when present', async () => { | |
| 83 | const provider = createAnthropicProvider({ apiKey: 'sk-test' }) | |
| 84 | ||
| 85 | const plain = await provider.complete(request(), 'claude-test-1') | |
| 86 | expect(plain.usage).toEqual({ inputTokens: 10, outputTokens: 5 }) | |
| 87 | ||
| 88 | respondWith = () => ({ ...OK_RESPONSE, usage: { input_tokens: 10, output_tokens: 5, cache_read_input_tokens: 7 } }) | |
| 89 | const cached = await provider.complete(request(), 'claude-test-1') | |
| 90 | expect(cached.usage.cachedInputTokens).toBe(7) | |
| 91 | }) | |
| 92 | ||
| 93 | it('edge case - every stop reason lands in the normalized set', async () => { | |
| 94 | // `refusal` in particular must survive: it arrives as a successful HTTP | |
| 95 | // response with empty content, and a caller that cannot tell would read | |
| 96 | // the empty text as a valid answer. | |
| 97 | const cases: Array<[string | null, string]> = [ | |
| 98 | ['end_turn', 'end'], | |
| 99 | ['stop_sequence', 'end'], | |
| 100 | ['tool_use', 'tool_use'], | |
| 101 | ['max_tokens', 'max_tokens'], | |
| 102 | ['refusal', 'refusal'], | |
| 103 | ['something_new', 'other'], | |
| 104 | [null, 'other'], | |
| 105 | ] | |
| 106 | ||
| 107 | const provider = createAnthropicProvider({ apiKey: 'sk-test' }) | |
| 108 | for (const [wire, normalized] of cases) { | |
| 109 | respondWith = () => ({ ...OK_RESPONSE, stop_reason: wire }) | |
| 110 | const response = await provider.complete(request(), 'claude-test-1') | |
| 111 | expect(response.stopReason).toBe(normalized as typeof response.stopReason) | |
| 112 | } | |
| 113 | }) | |
| 114 | ||
| 115 | it('success case - parses json only for schema-constrained requests', async () => { | |
| 116 | respondWith = () => ({ ...OK_RESPONSE, content: [{ type: 'text', text: '{"answer":42}' }] }) | |
| 117 | const provider = createAnthropicProvider({ apiKey: 'sk-test' }) | |
| 118 | ||
| 119 | const unconstrained = await provider.complete(request(), 'claude-test-1') | |
| 120 | expect(unconstrained.json).toBeUndefined() | |
| 121 | ||
| 122 | const constrained = await provider.complete(request({ jsonSchema: { type: 'object' } }), 'claude-test-1') | |
| 123 | expect(constrained.json).toEqual({ answer: 42 }) | |
| 124 | }) | |
| 125 | ||
| 126 | it('failure case - malformed json for a schema-constrained request throws', async () => { | |
| 127 | respondWith = () => ({ ...OK_RESPONSE, content: [{ type: 'text', text: 'not json' }] }) | |
| 128 | const provider = createAnthropicProvider({ apiKey: 'sk-test' }) | |
| 129 | ||
| 130 | await expect(provider.complete(request({ jsonSchema: { type: 'object' } }), 'claude-test-1')) | |
| 131 | .rejects.toBeInstanceOf(AiProviderError) | |
| 132 | }) | |
| 133 | }) | |
| 134 | ||
| 135 | describe('request mapping', () => { | |
| 136 | it('success case - passes model, system, tools and output config through', async () => { | |
| 137 | // The schema-constrained request parses the reply as JSON on the way out. | |
| 138 | respondWith = () => ({ ...OK_RESPONSE, content: [{ type: 'text', text: '{}' }] }) | |
| 139 | const provider = createAnthropicProvider({ apiKey: 'sk-test' }) | |
| 140 | await provider.complete(request({ | |
| 141 | system: 'be brief', | |
| 142 | effort: 'low', | |
| 143 | jsonSchema: { type: 'object' }, | |
| 144 | tools: [{ name: 'grep', description: 'search', parameters: { type: 'object' } }], | |
| 145 | }), 'claude-test-1') | |
| 146 | ||
| 147 | const params = createCalls[0] | |
| 148 | expect(params.model).toBe('claude-test-1') | |
| 149 | expect(params.system).toBe('be brief') | |
| 150 | expect(params.tools).toEqual([{ name: 'grep', description: 'search', input_schema: { type: 'object' } }]) | |
| 151 | expect(params.output_config).toEqual({ effort: 'low', format: { type: 'json_schema', schema: { type: 'object' } } }) | |
| 152 | }) | |
| 153 | ||
| 154 | it('edge case - omits the optional keys rather than sending undefined', async () => { | |
| 155 | const provider = createAnthropicProvider({ apiKey: 'sk-test' }) | |
| 156 | await provider.complete(request(), 'claude-test-1') | |
| 157 | ||
| 158 | const params = createCalls[0] | |
| 159 | expect('system' in params).toBe(false) | |
| 160 | expect('tools' in params).toBe(false) | |
| 161 | expect('output_config' in params).toBe(false) | |
| 162 | // The default ceiling is sized to stay under the SDK's HTTP timeout. | |
| 163 | expect(params.max_tokens).toBe(16000) | |
| 164 | }) | |
| 165 | ||
| 166 | it('success case - an explicit maxTokens overrides the default', async () => { | |
| 167 | const provider = createAnthropicProvider({ apiKey: 'sk-test' }) | |
| 168 | await provider.complete(request({ maxTokens: 512 }), 'claude-test-1') | |
| 169 | ||
| 170 | expect(createCalls[0].max_tokens).toBe(512) | |
| 171 | }) | |
| 172 | }) | |
| 173 | ||
| 174 | describe('client construction', () => { | |
| 175 | it('success case - the client is built once and reused', async () => { | |
| 176 | const provider = createAnthropicProvider({ apiKey: 'sk-test' }) | |
| 177 | await provider.complete(request(), 'claude-test-1') | |
| 178 | await provider.complete(request(), 'claude-test-1') | |
| 179 | ||
| 180 | expect(constructorCalls).toHaveLength(1) | |
| 181 | expect(constructorCalls[0]).toEqual({ apiKey: 'sk-test' }) | |
| 182 | }) | |
| 183 | ||
| 184 | it('success case - a gateway base URL reaches the SDK', async () => { | |
| 185 | const provider = createAnthropicProvider({ apiKey: 'sk-test', baseUrl: 'https://gateway.example' }) | |
| 186 | await provider.complete(request(), 'claude-test-1') | |
| 187 | ||
| 188 | expect(constructorCalls[0]).toEqual({ apiKey: 'sk-test', baseURL: 'https://gateway.example' }) | |
| 189 | }) | |
| 190 | }) | |
| 191 | }) | |
| @@ -0,0 +1,118 @@ | ||
| 1 | import { afterEach, describe, expect, it } from 'bun:test' | |
| 2 | import { mkdtempSync, rmSync, writeFileSync } from 'node:fs' | |
| 3 | import { tmpdir } from 'node:os' | |
| 4 | import { join } from 'node:path' | |
| 5 | import process from 'node:process' | |
| 6 | import { | |
| 7 | detectRequiredPackageManagers, | |
| 8 | getAllLockFilePaths, | |
| 9 | hasLockFile, | |
| 10 | regenerateLockFile, | |
| 11 | } from '../src/utils/lock-file' | |
| 12 | ||
| 13 | /** | |
| 14 | * 188 lines, publicly re-exported, and no direct test — the dossier's | |
| 15 | * shortlist. The spawn-heavy path is exercised through its non-fatal error | |
| 16 | * contract; the pure parts are asserted directly. | |
| 17 | */ | |
| 18 | describe('lock-file', () => { | |
| 19 | const cleanups: Array<() => void> = [] | |
| 20 | ||
| 21 | afterEach(() => { | |
| 22 | while (cleanups.length > 0) | |
| 23 | cleanups.pop()?.() | |
| 24 | }) | |
| 25 | ||
| 26 | /** A temp directory removed after the test. */ | |
| 27 | function tempDir(): string { | |
| 28 | const dir = mkdtempSync(join(tmpdir(), 'buddy-lock-')) | |
| 29 | cleanups.push(() => rmSync(dir, { recursive: true, force: true })) | |
| 30 | return dir | |
| 31 | } | |
| 32 | ||
| 33 | describe('getAllLockFilePaths', () => { | |
| 34 | it('success case - names every lock file a regeneration may touch', () => { | |
| 35 | const paths = getAllLockFilePaths() | |
| 36 | ||
| 37 | // The staging step after regeneration adds exactly these; a manager | |
| 38 | // whose lock file is missing here gets regenerated and then not | |
| 39 | // committed, which looks like the update never ran. | |
| 40 | for (const expected of ['bun.lock', 'bun.lockb', 'package-lock.json', 'yarn.lock', 'pnpm-lock.yaml', 'composer.lock']) | |
| 41 | expect(paths).toContain(expected) | |
| 42 | }) | |
| 43 | }) | |
| 44 | ||
| 45 | describe('hasLockFile', () => { | |
| 46 | it('success case - finds each manager by its lock file', () => { | |
| 47 | const dir = tempDir() | |
| 48 | writeFileSync(join(dir, 'yarn.lock'), '') | |
| 49 | writeFileSync(join(dir, 'composer.lock'), '{}') | |
| 50 | ||
| 51 | expect(hasLockFile('yarn', dir)).toBe(true) | |
| 52 | expect(hasLockFile('composer', dir)).toBe(true) | |
| 53 | expect(hasLockFile('npm', dir)).toBe(false) | |
| 54 | expect(hasLockFile('pnpm', dir)).toBe(false) | |
| 55 | }) | |
| 56 | ||
| 57 | it('edge case - either bun lock format counts', () => { | |
| 58 | const textual = tempDir() | |
| 59 | writeFileSync(join(textual, 'bun.lock'), '') | |
| 60 | expect(hasLockFile('bun', textual)).toBe(true) | |
| 61 | ||
| 62 | const binary = tempDir() | |
| 63 | writeFileSync(join(binary, 'bun.lockb'), '') | |
| 64 | expect(hasLockFile('bun', binary)).toBe(true) | |
| 65 | ||
| 66 | expect(hasLockFile('bun', tempDir())).toBe(false) | |
| 67 | }) | |
| 68 | }) | |
| 69 | ||
| 70 | describe('detectRequiredPackageManagers', () => { | |
| 71 | // These run from the repository root, whose bun.lock makes the JS | |
| 72 | // manager detection deterministic. | |
| 73 | it('success case - a package.json update needs the JS manager', () => { | |
| 74 | expect(detectRequiredPackageManagers(['package.json'])).toEqual(['bun']) | |
| 75 | expect(detectRequiredPackageManagers(['packages/app/package.json'])).toEqual(['bun']) | |
| 76 | }) | |
| 77 | ||
| 78 | it('success case - a composer.json update needs composer', () => { | |
| 79 | expect(detectRequiredPackageManagers(['composer.json'])).toEqual(['composer']) | |
| 80 | }) | |
| 81 | ||
| 82 | it('success case - mixed updates need both, once each', () => { | |
| 83 | const managers = detectRequiredPackageManagers([ | |
| 84 | 'package.json', | |
| 85 | 'composer.json', | |
| 86 | 'sub/package.json', | |
| 87 | ]) | |
| 88 | ||
| 89 | expect(managers.sort()).toEqual(['bun', 'composer']) | |
| 90 | }) | |
| 91 | ||
| 92 | it('edge case - unrelated files need nothing', () => { | |
| 93 | expect(detectRequiredPackageManagers(['deps.yaml', 'README.md', 'Dockerfile'])).toEqual([]) | |
| 94 | }) | |
| 95 | }) | |
| 96 | ||
| 97 | describe('regenerateLockFile', () => { | |
| 98 | it('failure case - a missing binary resolves rather than throwing', async () => { | |
| 99 | // The contract is non-fatal: a machine without the manager installed | |
| 100 | // gets a result object naming the failure, not an unhandled rejection | |
| 101 | // that sinks the whole update run. | |
| 102 | const originalPath = process.env.PATH | |
| 103 | process.env.PATH = tempDir() | |
| 104 | cleanups.push(() => { | |
| 105 | process.env.PATH = originalPath | |
| 106 | }) | |
| 107 | ||
| 108 | const result = await regenerateLockFile('yarn', tempDir()) | |
| 109 | ||
| 110 | expect(result.success).toBe(false) | |
| 111 | expect(result.packageManager).toBe('yarn') | |
| 112 | // Which channel reports the missing binary is platform-dependent — | |
| 113 | // node fires the `error` event, Bun closes with a negative code. The | |
| 114 | // contract is only that the failure is named, not how it was caught. | |
| 115 | expect(result.message).toMatch(/Failed to run yarn|exited with code/) | |
| 116 | }) | |
| 117 | }) | |
| 118 | }) | |