diff --git a/__tests__/cli-ui-command.test.ts b/__tests__/cli-ui-command.test.ts index d2bd848982..82c287d9ec 100644 --- a/__tests__/cli-ui-command.test.ts +++ b/__tests__/cli-ui-command.test.ts @@ -12,7 +12,7 @@ * observable fact rather than a promise. */ -import { describe, it, expect, beforeAll, afterAll } from 'vitest'; +import { describe, it, expect, beforeAll, afterAll, vi } from 'vitest'; import { execFileSync, spawn, type ChildProcess } from 'child_process'; import * as fs from 'fs'; import * as http from 'http'; @@ -223,11 +223,23 @@ describe('codegraph ui — serving', () => { const markerFile = (): string => path.join(markerDir, 'opened.txt'); - /** The opener is async (detached); give it a moment before concluding. */ + /** + * The opener is async (detached); give it a moment before concluding. + * + * Waits for CONTENT, not merely for the file to appear. Shell redirection + * creates the target before the command writes into it — `cmd.exe` opens the + * `>` target as it parses the line, ahead of `echo` — so an existence check + * can return a file that is real but still empty. Callers that expect no + * launch still get null: a file that never gains content times out the same + * as one that never appears. + */ async function waitForMarker(timeoutMs: number): Promise { const deadline = Date.now() + timeoutMs; for (;;) { - if (fs.existsSync(markerFile())) return fs.readFileSync(markerFile(), 'utf-8'); + if (fs.existsSync(markerFile())) { + const body = fs.readFileSync(markerFile(), 'utf-8'); + if (body.trim() !== '') return body; + } if (Date.now() > deadline) return null; await new Promise((r) => setTimeout(r, 50)); } @@ -280,11 +292,11 @@ describe('codegraph ui — serving', () => { }, 60_000); it('CODEGRAPH_BROWSER=none suppresses the launch like --no-open', async () => { - fs.rmSync(markerFile(), { force: true }); const viewer = await startViewer(['--port', '0', projectDir], { CODEGRAPH_BROWSER: 'none' }); try { expect((await get(viewer.port, '/')).status).toBe(200); - expect(await waitForMarker(1_000)).toBeNull(); + await vi.waitFor(() => expect(viewer.output()).toContain('Open that URL in a browser')); + expect(viewer.output()).not.toContain('Opening your browser'); } finally { await stopViewer(viewer.child); } diff --git a/__tests__/rm-temp.ts b/__tests__/rm-temp.ts new file mode 100644 index 0000000000..eb2b7f25a5 --- /dev/null +++ b/__tests__/rm-temp.ts @@ -0,0 +1,24 @@ +/** + * Removing a temp tree whose last handle the OS releases late. + * + * The watcher suite's end-to-end test drives a real fs.watch. unwatch() + * returns before Windows releases the directory handle, and a removal in that + * window fails with EPERM, while POSIX unlinks regardless. There is no event to + * wait on, so this retries the removal for a short while. + * + * Only use it for that kind of residue. A holder the test owns (a database + * connection, a tracked child) should be closed or awaited where it is opened. + */ +import * as fs from 'node:fs'; + +export async function rmTempDir(dir: string, attempts = 40, delayMs = 50): Promise { + for (let i = 0; ; i++) { + try { + fs.rmSync(dir, { recursive: true, force: true }); + return; + } catch (err) { + if (i >= attempts) throw err; + await new Promise((resolve) => setTimeout(resolve, delayMs)); + } + } +} diff --git a/__tests__/ui-server.test.ts b/__tests__/ui-server.test.ts index 4b4e63fc8c..361d374fbd 100644 --- a/__tests__/ui-server.test.ts +++ b/__tests__/ui-server.test.ts @@ -13,12 +13,14 @@ */ import { describe, it, expect, beforeAll, afterAll, vi } from 'vitest'; +import * as childProcess from 'child_process'; import * as http from 'http'; import * as fs from 'fs'; import * as os from 'os'; import * as path from 'path'; import { browserOpenCommand, + openBrowser, cacheControlFor, contentTypeFor, describeBindFailure, @@ -33,6 +35,11 @@ import { type UiServerHandle, } from '../src/ui-server'; +vi.mock('child_process', async (importOriginal) => ({ + ...await importOriginal(), + spawn: vi.fn(), +})); + interface Response { status: number; headers: http.IncomingHttpHeaders; @@ -697,6 +704,22 @@ describe('port fallback on each platform', () => { }); describe('browserOpenCommand', () => { + it('does not spawn a browser when CODEGRAPH_BROWSER=none', () => { + const spawn = vi.spyOn(childProcess, 'spawn').mockImplementation(() => { + throw new Error('Unexpected browser launch'); + }); + vi.stubEnv('CODEGRAPH_BROWSER', 'none'); + try { + for (const platform of ['win32', 'darwin', 'linux'] as const) { + expect(openBrowser('http://127.0.0.1:4747', platform)).toBe(false); + } + expect(spawn).not.toHaveBeenCalled(); + } finally { + spawn.mockRestore(); + vi.unstubAllEnvs(); + } + }); + it('uses the platform opener', () => { expect(browserOpenCommand('http://x', 'darwin')).toEqual({ command: 'open', args: ['http://x'] }); expect(browserOpenCommand('http://x', 'linux')).toEqual({ command: 'xdg-open', args: ['http://x'] }); diff --git a/__tests__/watcher.test.ts b/__tests__/watcher.test.ts index e0bb14723c..cdbddf2e45 100644 --- a/__tests__/watcher.test.ts +++ b/__tests__/watcher.test.ts @@ -31,6 +31,7 @@ import { type WatchOptions, } from '../src/sync/watcher'; import CodeGraph from '../src/index'; +import { rmTempDir } from './rm-temp'; // Keep real filesystem operations, with writable exports for failure injection. vi.mock('fs', async (importOriginal) => ({ ...await importOriginal() })); @@ -104,7 +105,12 @@ describe('FileWatcher', () => { __setFsWatchForTests(null); // reset the injected fs.watch seam vi.restoreAllMocks(); if (fs.existsSync(testDir)) { - fs.rmSync(testDir, { recursive: true, force: true }); + // The end-to-end test drives a real fs.watch, whose directory handle the + // OS releases a beat after unwatch() returns. Windows fails the removal + // with EPERM in that window; POSIX unlinks regardless, which is why only + // Windows sees it. Every holder this suite owns is already closed at its + // own site — this waits out the release, it does not stand in for it. + await rmTempDir(testDir); } });