Skip to content

Commit 866e9ec

Browse files
committed
test: CI coverage gate, harden command assertions, injectable health timeout
- Enforce coverage thresholds in CI (npm run test:coverage) - Replace 12 bare .toHaveBeenCalled() in commands.test.ts with specific checks - Add healthCheckTimeout to CustomConfig (default 5s, injectable for tests) - Custom backend test suite drops from ~5.2s to ~0.4s
1 parent 7c36253 commit 866e9ec

6 files changed

Lines changed: 156 additions & 19 deletions

File tree

.github/workflows/ci.yml

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24,8 +24,8 @@ jobs:
2424
- name: Install dependencies
2525
run: npm ci
2626

27-
- name: Run tests
28-
run: npm test
27+
- name: Run tests with coverage
28+
run: npm run test:coverage
2929

3030
- name: Build
3131
run: npm run build

src/backends/custom.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ export class CustomBackend implements TtsBackend {
2323
await new Promise<void>((resolve, reject) => {
2424
const req = client.get(
2525
`${url.origin}/health`,
26-
{ timeout: 5000 },
26+
{ timeout: this.config.healthCheckTimeout ?? 5000 },
2727
(res) => {
2828
res.resume();
2929
if (res.statusCode === 200) resolve();

src/types.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,8 @@ export interface F5Config {
5757
/** Custom HTTP endpoint backend configuration. */
5858
export interface CustomConfig {
5959
endpoint: string;
60+
/** Health-check timeout in ms. Defaults to 5000 (5 s). */
61+
healthCheckTimeout?: number;
6062
/** Synthesis HTTP request timeout in ms. Defaults to 120000 (2 min). */
6163
synthesisTimeout?: number;
6264
}
Lines changed: 128 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,128 @@
1+
# Test Suite Review V2 — Combined Findings (2026-02-20)
2+
3+
## Documents Merged
4+
5+
- `a1b2c3-test-suite-review.md`
6+
- `test-suite-review-2026-02-20-a7k3p9.md`
7+
8+
## Executive Summary
9+
10+
The test suite is already strong and well-structured, with high coverage and stable green runs. The highest-value improvements are not broad rewrites, but targeted hardening in four areas: CI quality gates, assertion precision, slow timeout-path tests, and selected uncovered orchestration/platform branches.
11+
12+
## Current State Snapshot
13+
14+
- Test files: 16
15+
- Tests: 245
16+
- Status: 245/245 passing
17+
- Runtime: ~5.5s
18+
- Coverage:
19+
- Statements: 95.64%
20+
- Branches: 87.12%
21+
- Functions: 93.79%
22+
- Lines: 97.05%
23+
24+
## Consolidated Strengths
25+
26+
- Clear module-based test organization (commands, setup, provider, chunker, backends).
27+
- Good mix of unit and integration-style tests.
28+
- Existing coverage of cancellation/error behavior in critical paths.
29+
- Extension workflow behaviors are exercised via VS Code mocks.
30+
31+
## Consolidated Gaps
32+
33+
### 1) CI Gate Incomplete (Process Risk)
34+
35+
- CI currently runs tests/build/typecheck but does not enforce coverage thresholds via coverage run.
36+
- Impact: coverage regressions can merge undetected.
37+
38+
### 2) Assertion Strictness (Quality Risk)
39+
40+
- Some tests rely on permissive assertions (for example: broad “called” checks, `>= 1` chunk expectations).
41+
- Impact: behavior regressions may pass despite changing semantics.
42+
43+
### 3) Timeout Test Cost (Speed/Flake Risk)
44+
45+
- `customBackend` timeout-path tests consume several seconds due to real timeout waits.
46+
- Impact: slower feedback loops and greater CI variability.
47+
48+
### 4) Targeted Branch Gaps (Coverage Quality Risk)
49+
50+
- Remaining uncovered lines are concentrated in orchestration/platform and edge handling branches.
51+
- High-value targets:
52+
- `src/commands.ts` wrapper/error branches
53+
- `src/setup.ts` URL validation / custom endpoint path
54+
- `src/extension.ts` runtime reconfiguration branches
55+
- `src/player.ts` and `src/installer.ts` platform-specific branches
56+
- `src/chunker.ts` buffer-full/backpressure wait branch
57+
- `src/backends/kokoro.ts` abort-after-generation branch
58+
59+
### 5) Mock Fidelity and Real Host Validation (Confidence Risk)
60+
61+
- Mock-driven integration tests are good but cannot fully replicate Extension Host behavior.
62+
- Child process and filesystem behavior can be tested with higher-fidelity simulation.
63+
64+
## Final Prioritized Plan
65+
66+
## P0 — Immediate
67+
68+
1. Enforce coverage in CI
69+
- Add `npm run test:coverage` to CI workflow for PR gating.
70+
71+
2. Harden permissive assertions in high-risk tests
72+
- Focus first on:
73+
- `test/chunker.test.ts`
74+
- `test/extensionIntegration.test.ts`
75+
- `test/commands.test.ts`
76+
77+
## P1 — Next
78+
79+
3. Optimize timeout-path tests
80+
- Prefer injectable short timeout values in tests for determinism.
81+
- Optional: fake timers only where request/response behavior remains deterministic.
82+
- Target: no single timeout-path unit test > ~250ms.
83+
84+
4. Close highest-value branch gaps
85+
- Add focused tests for specific uncovered orchestration/platform branches listed above.
86+
87+
5. Improve mock fidelity where behavior matters
88+
- Expand process lifecycle simulation (`SIGSTOP`/`SIGCONT`) and filesystem interaction checks in backend flow tests.
89+
90+
## P2 — Maturity
91+
92+
6. Add true VS Code E2E smoke tests
93+
- Introduce `@vscode/test-electron` (Insiders-targeted) for minimal smoke coverage:
94+
- activation
95+
- command registration
96+
- status bar visibility
97+
- walkthrough open behavior
98+
99+
7. Pilot mutation testing on critical modules
100+
- Scope first pass to small set (`chunker`, `commands`, `speechProvider`) and track mutation score trend.
101+
102+
## Recommended Implementation Sequence (Low-Risk, High-ROI)
103+
104+
1. CI coverage gate
105+
2. Assertion hardening pass
106+
3. Timeout-path speedup
107+
4. Branch-gap closure pass
108+
5. E2E smoke introduction
109+
6. Mutation pilot
110+
111+
## Success Criteria
112+
113+
- Coverage thresholds enforced in CI for pull requests.
114+
- Reduced suite runtime and no multi-second single-test bottlenecks in routine local runs.
115+
- Measurable branch coverage increase in orchestration/platform modules.
116+
- Fewer permissive assertions in critical-path tests.
117+
- Basic real Extension Host E2E smoke tests passing.
118+
119+
## Sources
120+
121+
1. Vitest Coverage Guide (official): https://vitest.dev/guide/coverage.html
122+
Credibility: 10/10 (official, current, directly applicable)
123+
124+
2. VS Code Extension Testing Guide (official): https://code.visualstudio.com/api/working-with-extensions/testing-extension
125+
Credibility: 10/10 (official Microsoft guidance, current)
126+
127+
3. Stryker Mutation Testing Docs (official): https://stryker-mutator.io/docs/
128+
Credibility: 8/10 (official tool docs, valuable for mutation strategy)

test/commands.test.ts

Lines changed: 18 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -136,7 +136,7 @@ describe("commands", () => {
136136
disableTts(services);
137137

138138
expect(services.speechRegistration).toBeUndefined();
139-
expect(backend.dispose).toHaveBeenCalled();
139+
expect(backend.dispose).toHaveBeenCalledTimes(1);
140140
});
141141

142142
it("is safe when no backend or registration", () => {
@@ -163,7 +163,7 @@ describe("commands", () => {
163163

164164
togglePause(services);
165165

166-
expect(spy).toHaveBeenCalled();
166+
expect(spy).toHaveBeenCalledTimes(1);
167167
});
168168
});
169169

@@ -216,7 +216,9 @@ describe("commands", () => {
216216

217217
await readSelectionAloud(services);
218218

219-
expect(mockPlayerPlay).toHaveBeenCalled();
219+
expect(mockPlayerPlay).toHaveBeenCalledWith(
220+
expect.objectContaining({ sampleRate: 24000 })
221+
);
220222
});
221223
});
222224

@@ -230,7 +232,7 @@ describe("commands", () => {
230232

231233
await initializeAndRegister(context, services, backend);
232234

233-
expect(backend.initialize).toHaveBeenCalled();
235+
expect(backend.initialize).toHaveBeenCalledTimes(1);
234236
expect(vscode.speech.registerSpeechProvider).toHaveBeenCalledWith(
235237
"eloquent",
236238
services.provider
@@ -261,7 +263,7 @@ describe("commands", () => {
261263

262264
await initializeAndRegister(context, services, backend);
263265

264-
expect(oldDispose).toHaveBeenCalled();
266+
expect(oldDispose).toHaveBeenCalledTimes(1);
265267
});
266268

267269
it("sets status bar to loading then active", async () => {
@@ -285,7 +287,9 @@ describe("commands", () => {
285287
// testVoice plays audio
286288
await initializeAndRegister(context, services, fakeBackend());
287289

288-
expect(mockPlayerPlay).toHaveBeenCalled();
290+
expect(mockPlayerPlay).toHaveBeenCalledWith(
291+
expect.objectContaining({ sampleRate: 24000 })
292+
);
289293
});
290294
});
291295

@@ -298,7 +302,9 @@ describe("commands", () => {
298302

299303
await testVoice(services, fakeBackend());
300304

301-
expect(mockPlayerPlay).toHaveBeenCalled();
305+
expect(mockPlayerPlay).toHaveBeenCalledWith(
306+
expect.objectContaining({ sampleRate: 24000 })
307+
);
302308
expect(spy).toHaveBeenCalledWith(true); // finally block
303309
});
304310

@@ -340,7 +346,7 @@ describe("commands", () => {
340346
const services = makeServices();
341347
await setupBackend(makeContext(), services);
342348

343-
expect(backend.initialize).toHaveBeenCalled();
349+
expect(backend.initialize).toHaveBeenCalledTimes(1);
344350
expect(services.speechRegistration).toBeDefined();
345351
});
346352
});
@@ -353,7 +359,7 @@ describe("commands", () => {
353359

354360
await enableTts(makeContext(), makeServices());
355361

356-
expect(mockRunSetupWizard).toHaveBeenCalled();
362+
expect(mockRunSetupWizard).toHaveBeenCalledTimes(1);
357363
});
358364

359365
it("creates and initializes configured backend", async () => {
@@ -365,7 +371,7 @@ describe("commands", () => {
365371
await enableTts(makeContext(), services);
366372

367373
expect(mockCreateBackend).toHaveBeenCalledWith("kokoro", expect.anything());
368-
expect(backend.initialize).toHaveBeenCalled();
374+
expect(backend.initialize).toHaveBeenCalledTimes(1);
369375
});
370376
});
371377

@@ -391,7 +397,7 @@ describe("commands", () => {
391397
await toggleTts(makeContext(), makeServices());
392398

393399
// Falls through to enableTts → setupBackend because no backend configured
394-
expect(mockRunSetupWizard).toHaveBeenCalled();
400+
expect(mockRunSetupWizard).toHaveBeenCalledTimes(1);
395401
});
396402
});
397403

@@ -428,7 +434,7 @@ describe("commands", () => {
428434
await changeVoice(makeContext(), services);
429435

430436
expect(mockCreateBackend).toHaveBeenCalledWith("kokoro", expect.anything());
431-
expect(backend.initialize).toHaveBeenCalled();
437+
expect(backend.initialize).toHaveBeenCalledTimes(1);
432438
});
433439
});
434440
});

test/customBackend.test.ts

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -169,16 +169,17 @@ describe("CustomBackend", () => {
169169
});
170170

171171
try {
172-
const backend = new CustomBackend({ endpoint: hangUrl });
173-
// The health check has a 5s timeout — but we can't wait that long.
174-
// Verify the timeout path by checking the error message.
172+
const backend = new CustomBackend({
173+
endpoint: hangUrl,
174+
healthCheckTimeout: 200,
175+
});
175176
await expect(backend.initialize()).rejects.toThrow(
176177
"Custom TTS endpoint timed out"
177178
);
178179
} finally {
179180
hangServer.close();
180181
}
181-
}, 10_000);
182+
});
182183

183184
it("selects https client for https:// endpoints", async () => {
184185
// Use an HTTPS URL pointing at a port that will refuse the connection.

0 commit comments

Comments
 (0)