Skip to content

Commit 7116c84

Browse files
tianzhouclaude
andauthored
fix(test): isolate config discovery in server-spawning integration tests (#367)
* fix(test): isolate config discovery in server-spawning integration tests json-rpc-integration and http-bind-host spawn the server with DSN set, but the server discovers ./dbhub.toml relative to its own cwd and TOML config outranks the DSN env var. Running from the repo root meant DSN was silently ignored and the server tried to connect to whatever the local dbhub.toml named, died, and never bound its port — surfacing as an opaque "Server did not start within expected time". Since dbhub.toml is gitignored, this reproduced only for developers who have one, and never in CI. Both tests now spawn from an empty temp directory. json-rpc also switches from `pnpm dev` to invoking tsx directly, matching http-bind-host: `pnpm dev` starts the Vite frontend alongside the backend under concurrently, which was unnecessary and leaked a stray dev server on every run. Also captures server output into the startup failure message, which previously reported a bare timeout with no indication of the cause, and adds a guard asserting the DSN config source was used. json-rpc runtime drops from ~25s to ~1.1s. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(test): clear the SIGKILL timer on normal server exit json-rpc's afterAll left the 5s kill timer pending when the server exited cleanly. http-bind-host already clears it; this makes the two consistent and removes a latent hazard if the pool ever stops force-exiting workers. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 0769813 commit 7116c84

2 files changed

Lines changed: 66 additions & 12 deletions

File tree

src/__tests__/http-bind-host.integration.test.ts

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,18 +7,27 @@ import os from 'os';
77
describe('HTTP bind host integration', () => {
88
let serverProcess: ChildProcess | null = null;
99
let testDbPath: string;
10+
let isolatedCwd: string;
1011
const testPort = 3002;
1112
const testHost = '127.0.0.1';
1213
const startupLogs: string[] = [];
1314

1415
beforeAll(async () => {
1516
testDbPath = path.join(os.tmpdir(), `bind_host_test_${Date.now()}_${Math.random().toString(36).substr(2, 9)}.db`);
1617

18+
// The server discovers ./dbhub.toml relative to its own cwd, and TOML config
19+
// outranks the DSN env var below. Running from the repo root would therefore
20+
// silently ignore DSN and connect to whatever a developer's local (gitignored)
21+
// dbhub.toml names, so the server would fail to start on any machine that has
22+
// one. Spawn from an empty directory to isolate config discovery.
23+
isolatedCwd = fs.mkdtempSync(path.join(os.tmpdir(), 'bind_host_cwd_'));
24+
1725
// Invoke tsx directly via node to avoid pnpm.cmd resolution issues on Windows.
1826
const tsxCli = path.resolve(process.cwd(), 'node_modules', 'tsx', 'dist', 'cli.mjs');
1927
const entry = path.resolve(process.cwd(), 'src', 'index.ts');
2028

2129
serverProcess = spawn(process.execPath, [tsxCli, entry, '--transport=http'], {
30+
cwd: isolatedCwd,
2231
env: {
2332
...process.env,
2433
DSN: `sqlite://${testDbPath}`,
@@ -76,13 +85,23 @@ describe('HTTP bind host integration', () => {
7685
if (testDbPath && fs.existsSync(testDbPath)) {
7786
fs.unlinkSync(testDbPath);
7887
}
88+
if (isolatedCwd && fs.existsSync(isolatedCwd)) {
89+
fs.rmSync(isolatedCwd, { recursive: true, force: true });
90+
}
7991
});
8092

8193
it('responds on the configured host', async () => {
8294
const res = await fetch(`http://${testHost}:${testPort}/healthz`);
8395
expect(res.status).toBe(200);
8496
});
8597

98+
it('uses the DSN env var, not an ambient dbhub.toml', () => {
99+
// Guards the isolated cwd above: if the server is ever spawned from the repo
100+
// root again, a developer's local dbhub.toml wins over DSN and this reports
101+
// the cause directly instead of an opaque startup timeout.
102+
expect(startupLogs.join('')).toContain('Configuration source: environment variable');
103+
});
104+
86105
it('logs the actual bound address at startup', () => {
87106
const allLogs = startupLogs.join('');
88107
expect(allLogs).toContain(`${testHost}:${testPort}`);

src/__tests__/json-rpc-integration.test.ts

Lines changed: 47 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -8,17 +8,33 @@ describe('JSON RPC Integration Tests', () => {
88
let serverProcess: ChildProcess | null = null;
99
let testDbPath: string;
1010
let baseUrl: string;
11+
let isolatedCwd: string;
1112
const testPort = 3001;
13+
const startupLogs: string[] = [];
1214

1315
beforeAll(async () => {
1416
// Create a temporary SQLite database file
1517
const tempDir = os.tmpdir();
1618
testDbPath = path.join(tempDir, `json_rpc_test_${Date.now()}_${Math.random().toString(36).substr(2, 9)}.db`);
17-
19+
20+
// The server discovers ./dbhub.toml relative to its own cwd, and TOML config
21+
// outranks the DSN env var below. Running from the repo root would therefore
22+
// silently ignore DSN and connect to whatever a developer's local (gitignored)
23+
// dbhub.toml names, so the server would fail to start on any machine that has
24+
// one. Spawn from an empty directory to isolate config discovery.
25+
isolatedCwd = fs.mkdtempSync(path.join(tempDir, 'json_rpc_cwd_'));
26+
1827
baseUrl = `http://localhost:${testPort}`;
19-
28+
29+
// Invoke tsx directly via node rather than `pnpm dev`: `pnpm dev` runs the
30+
// backend and the Vite frontend under `concurrently`, and the frontend is both
31+
// unnecessary here and leaked a stray dev server on every run.
32+
const tsxCli = path.resolve(process.cwd(), 'node_modules', 'tsx', 'dist', 'cli.mjs');
33+
const entry = path.resolve(process.cwd(), 'src', 'index.ts');
34+
2035
// Start the server as a child process
21-
serverProcess = spawn('pnpm', ['dev'], {
36+
serverProcess = spawn(process.execPath, [tsxCli, entry, '--transport=http'], {
37+
cwd: isolatedCwd,
2238
env: {
2339
...process.env,
2440
DSN: `sqlite://${testDbPath}`,
@@ -29,13 +45,15 @@ describe('JSON RPC Integration Tests', () => {
2945
stdio: 'pipe'
3046
});
3147

32-
// Handle server output
48+
// Handle server output. Retained for the failure message below: without the
49+
// server's own logs, a startup failure surfaces only as a timeout with no
50+
// indication of the cause.
3351
serverProcess.stdout?.on('data', (data) => {
34-
console.log(`Server stdout: ${data}`);
52+
startupLogs.push(data.toString());
3553
});
3654

3755
serverProcess.stderr?.on('data', (data) => {
38-
console.error(`Server stderr: ${data}`);
56+
startupLogs.push(data.toString());
3957
});
4058

4159
// Wait for server to start up
@@ -65,7 +83,7 @@ describe('JSON RPC Integration Tests', () => {
6583
}
6684

6785
if (!serverReady) {
68-
throw new Error('Server did not start within expected time');
86+
throw new Error(`Server did not start within expected time. Logs:\n${startupLogs.join('')}`);
6987
}
7088

7189
// Create test tables and data via HTTP request
@@ -104,17 +122,22 @@ describe('JSON RPC Integration Tests', () => {
104122
serverProcess.kill('SIGTERM');
105123

106124
// Wait for process to exit
107-
await new Promise((resolve) => {
125+
await new Promise<void>((resolve) => {
108126
if (serverProcess) {
109-
serverProcess.on('exit', resolve);
110-
setTimeout(() => {
127+
// Without clearing this on normal exit, the pending timer keeps
128+
// the Vitest process alive until the 5s tail elapses.
129+
const killTimeout = setTimeout(() => {
111130
if (serverProcess && !serverProcess.killed) {
112131
serverProcess.kill('SIGKILL');
113132
}
114-
resolve(void 0);
133+
resolve();
115134
}, 5000);
135+
serverProcess.on('exit', () => {
136+
clearTimeout(killTimeout);
137+
resolve();
138+
});
116139
} else {
117-
resolve(void 0);
140+
resolve();
118141
}
119142
});
120143
}
@@ -123,6 +146,11 @@ describe('JSON RPC Integration Tests', () => {
123146
if (testDbPath && fs.existsSync(testDbPath)) {
124147
fs.unlinkSync(testDbPath);
125148
}
149+
150+
// Clean up the isolated working directory
151+
if (isolatedCwd && fs.existsSync(isolatedCwd)) {
152+
fs.rmSync(isolatedCwd, { recursive: true, force: true });
153+
}
126154
});
127155

128156
async function makeJsonRpcCall(method: string, params: any): Promise<any> {
@@ -151,6 +179,13 @@ describe('JSON RPC Integration Tests', () => {
151179
return await response.json();
152180
}
153181

182+
it('uses the DSN env var, not an ambient dbhub.toml', () => {
183+
// Guards the isolated cwd above: if the server is ever spawned from the repo
184+
// root again, a developer's local dbhub.toml wins over DSN and this reports
185+
// the cause directly instead of an opaque startup timeout.
186+
expect(startupLogs.join('')).toContain('Configuration source: environment variable');
187+
});
188+
154189
describe('execute_sql JSON RPC calls', () => {
155190
it('should execute a simple SELECT query successfully', async () => {
156191
const response = await makeJsonRpcCall('execute_sql', {

0 commit comments

Comments
 (0)