Skip to content

Commit 2aa73e8

Browse files
committed
fix(test): type the driver double and narrow the dispatch result so the hidden test layer type-checks (#13408)
1 parent 0e2fdb8 commit 2aa73e8

3 files changed

Lines changed: 91 additions & 45 deletions

File tree

packages/objectql/src/engine-primary-datasource.test.ts

Lines changed: 17 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -16,9 +16,20 @@
1616
// the refusal happens and is not silently a name.
1717

1818
import { describe, it, expect } from 'vitest';
19+
import type { IDataDriver } from '@objectstack/spec/contracts';
1920
import { ObjectQL } from './engine.js';
2021

21-
const driver = (name: string) => ({
22+
/**
23+
* A registrable driver double.
24+
*
25+
* Annotated `IDataDriver` deliberately, rather than left as an inferred object
26+
* literal: `registerDriver` takes the real contract, so an un-annotated double
27+
* is checked only at the call site and silently drifts as the interface grows.
28+
* The annotation makes THIS declaration the thing that fails when a member is
29+
* added — which is how the first version of this fixture was found short of
30+
* `upsert` and `dropTable`.
31+
*/
32+
const driver = (name: string): IDataDriver => ({
2233
name,
2334
version: '1.0.0',
2435
supports: {},
@@ -27,18 +38,20 @@ const driver = (name: string) => ({
2738
checkHealth: async () => true,
2839
find: async () => [],
2940
findOne: async () => null,
30-
create: async (_o: string, data: any) => ({ id: '1', ...data }),
31-
update: async (_o: string, id: string, data: any) => ({ id, ...data }),
41+
create: async (_o, data) => ({ id: '1', ...data }),
42+
update: async (_o, id, data) => ({ id, ...data }),
43+
upsert: async (_o, data) => ({ id: '1', ...data }),
3244
delete: async () => true,
3345
count: async () => 0,
3446
bulkCreate: async () => [],
3547
bulkUpdate: async () => [],
3648
bulkDelete: async () => {},
37-
execute: async () => ({}),
49+
execute: async () => null,
3850
beginTransaction: async () => ({}),
3951
commit: async () => {},
4052
rollback: async () => {},
4153
syncSchema: async () => {},
54+
dropTable: async () => {},
4255
});
4356

4457
function newEngine(): ObjectQL {

packages/runtime/src/http-dispatcher.ready.test.ts

Lines changed: 59 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import { describe, it, expect, vi } from 'vitest';
2-
import { HttpDispatcher } from './http-dispatcher.js';
2+
import { HttpDispatcher, type HttpDispatcherResult } from './http-dispatcher.js';
33

44
function kernel(state: string, dataService?: unknown): any {
55
return {
@@ -10,6 +10,34 @@ function kernel(state: string, dataService?: unknown): any {
1010
}
1111
const ctx: any = {};
1212

13+
/**
14+
* `HttpDispatcherResult.response` is OPTIONAL, so every read of it is a
15+
* `possibly undefined` in a type-checked program — and this package's test
16+
* layer IS type-checked, by `check:type-check-debt --re-measure` against a
17+
* shrink-only ledger, even though `pnpm test` never sees it. That asymmetry is
18+
* exactly how 19 fresh errors got in here behind a green `pnpm --filter
19+
* @objectstack/runtime typecheck`: the package's own tsconfig excludes every
20+
* `.test.ts` file, so the program that reported zero had never read this one.
21+
*
22+
* Narrow once, and narrow LOUDLY — the shape is lifted from the #8287 suite in
23+
* `http-dispatcher.keys.test.ts`, deliberately rather than invented again.
24+
* `expect(res.response).toBeDefined()` would satisfy a reader and narrow
25+
* nothing (vitest's matchers are not assertion signatures), and a `!` would
26+
* silence the compiler while leaving the failure to surface as `undefined is
27+
* not an object` three lines later. A probe that answered no response at all is
28+
* a different defect from one that answered the wrong status; this keeps them
29+
* distinguishable.
30+
*
31+
* Applied to the WHOLE file, not just the #13408 suite: the reads are identical
32+
* in kind, the repair is one call each, and leaving the older ones would bank a
33+
* green while knowingly holding fixable errors in a file already open.
34+
*/
35+
function responseOf(res: HttpDispatcherResult): NonNullable<HttpDispatcherResult['response']> {
36+
const { response } = res;
37+
if (!response) throw new Error('GET /ready answered no response at all');
38+
return response;
39+
}
40+
1341
/** An engine whose `checkDriversHealth` reports the given verdicts. */
1442
function engine(results: Array<{ driverName: string; healthy: boolean }>) {
1543
return { checkDriversHealth: vi.fn(async () => results) };
@@ -38,14 +66,14 @@ describe('HttpDispatcher — GET /ready readiness probe', () => {
3866
it('returns 200 when the kernel is running', async () => {
3967
const res = await new HttpDispatcher(kernel('running')).dispatch('GET', '/ready', undefined, undefined, ctx);
4068
expect(res.handled).toBe(true);
41-
expect(res.response.status).toBe(200);
42-
expect(res.response.body.data.state).toBe('running');
69+
expect(responseOf(res).status).toBe(200);
70+
expect(responseOf(res).body.data.state).toBe('running');
4371
});
4472

4573
it('returns 503 while booting or shutting down', async () => {
4674
for (const state of ['idle', 'initializing', 'stopping', 'stopped']) {
4775
const res = await new HttpDispatcher(kernel(state)).dispatch('GET', '/ready', undefined, undefined, ctx);
48-
expect(res.response.status).toBe(503);
76+
expect(responseOf(res).status).toBe(503);
4977
}
5078
});
5179

@@ -57,7 +85,7 @@ describe('HttpDispatcher — GET /ready readiness probe', () => {
5785
kernel('running', engine([{ driverName: 'sql', healthy: true }])),
5886
).dispatch('GET', '/ready', undefined, undefined, ctx);
5987

60-
expect(res.response.status).toBe(200);
88+
expect(responseOf(res).status).toBe(200);
6189
});
6290

6391
it('returns 503 naming the driver when one is down, even though the kernel runs', async () => {
@@ -68,9 +96,9 @@ describe('HttpDispatcher — GET /ready readiness probe', () => {
6896
])),
6997
).dispatch('GET', '/ready', undefined, undefined, ctx);
7098

71-
expect(res.response.status).toBe(503);
72-
expect(res.response.body.error.message).toBe('Data driver unavailable');
73-
expect(res.response.body.error.details).toEqual({ state: 'running', drivers: ['sql'] });
99+
expect(responseOf(res).status).toBe(503);
100+
expect(responseOf(res).body.error.message).toBe('Data driver unavailable');
101+
expect(responseOf(res).body.error.details).toEqual({ state: 'running', drivers: ['sql'] });
74102
});
75103

76104
it('does not re-probe within the memo TTL — k8s polls every few seconds', async () => {
@@ -91,14 +119,14 @@ describe('HttpDispatcher — GET /ready readiness probe', () => {
91119
}),
92120
).dispatch('GET', '/ready', undefined, undefined, ctx);
93121

94-
expect(res.response.status).toBe(200);
122+
expect(responseOf(res).status).toBe(200);
95123
});
96124

97125
it('stays ready on an engine predating checkDriversHealth', async () => {
98126
const res = await new HttpDispatcher(kernel('running', { find: async () => [] }))
99127
.dispatch('GET', '/ready', undefined, undefined, ctx);
100128

101-
expect(res.response.status).toBe(200);
129+
expect(responseOf(res).status).toBe(200);
102130
});
103131
});
104132
});
@@ -122,21 +150,21 @@ describe('HttpDispatcher — GET /ready primary-vs-secondary drain (#13408)', ()
122150
kernel('running', engineWithPrimary([{ driverName: 'pg', healthy: false }], primary('pg'))),
123151
).dispatch('GET', '/ready', undefined, undefined, ctx);
124152

125-
expect(res.response.status).toBe(503);
126-
expect(res.response.body.error.message).toBe('Data driver unavailable');
153+
expect(responseOf(res).status).toBe(503);
154+
expect(responseOf(res).body.error.message).toBe('Data driver unavailable');
127155
// Byte-identical to the shape #3756 shipped — an operator's alerting on
128156
// this body must not be able to tell that the handler changed.
129-
expect(res.response.body.error.details).toEqual({ state: 'running', drivers: ['pg'] });
157+
expect(responseOf(res).body.error.details).toEqual({ state: 'running', drivers: ['pg'] });
130158
});
131159

132160
it('the all-healthy 200 body carries NO degraded key', async () => {
133161
const res = await new HttpDispatcher(
134162
kernel('running', engineWithPrimary([{ driverName: 'pg', healthy: true }], primary('pg'))),
135163
).dispatch('GET', '/ready', undefined, undefined, ctx);
136164

137-
expect(res.response.status).toBe(200);
138-
expect(res.response.body.data).toEqual({ status: 'ready', state: 'running' });
139-
expect(res.response.body.data).not.toHaveProperty('degraded');
165+
expect(responseOf(res).status).toBe(200);
166+
expect(responseOf(res).body.data).toEqual({ status: 'ready', state: 'running' });
167+
expect(responseOf(res).body.data).not.toHaveProperty('degraded');
140168
});
141169

142170
it('does not even ASK which datasource is primary while everything is healthy', () => {
@@ -164,11 +192,11 @@ describe('HttpDispatcher — GET /ready primary-vs-secondary drain (#13408)', ()
164192
)),
165193
).dispatch('GET', '/ready', undefined, undefined, ctx);
166194

167-
expect(res.response.status).toBe(200);
168-
expect(res.response.body.data.status).toBe('ready');
195+
expect(responseOf(res).status).toBe(200);
196+
expect(responseOf(res).body.data.status).toBe('ready');
169197
// ⛔ The rejected fourth option — filtering the bad driver out so it
170198
// becomes invisible — would show an EMPTY degraded list here.
171-
expect(res.response.body.data.degraded).toEqual({
199+
expect(responseOf(res).body.data.degraded).toEqual({
172200
drivers: ['tenant_mongo'],
173201
primaryDatasource: 'pg',
174202
});
@@ -182,8 +210,8 @@ describe('HttpDispatcher — GET /ready primary-vs-secondary drain (#13408)', ()
182210
)),
183211
).dispatch('GET', '/ready', undefined, undefined, ctx);
184212

185-
expect(res.response.status).toBe(503);
186-
expect(res.response.body.error.details.drivers).toEqual(['pg']);
213+
expect(responseOf(res).status).toBe(503);
214+
expect(responseOf(res).body.error.details.drivers).toEqual(['pg']);
187215
});
188216

189217
it('drains when BOTH are down — the primary is in the unhealthy set', async () => {
@@ -194,8 +222,8 @@ describe('HttpDispatcher — GET /ready primary-vs-secondary drain (#13408)', ()
194222
)),
195223
).dispatch('GET', '/ready', undefined, undefined, ctx);
196224

197-
expect(res.response.status).toBe(503);
198-
expect(res.response.body.error.details.drivers).toEqual(['pg', 'tenant_mongo']);
225+
expect(responseOf(res).status).toBe(503);
226+
expect(responseOf(res).body.error.details.drivers).toEqual(['pg', 'tenant_mongo']);
199227
});
200228
});
201229

@@ -215,8 +243,8 @@ describe('HttpDispatcher — GET /ready primary-vs-secondary drain (#13408)', ()
215243
const res = await new HttpDispatcher(kernel('running', engine(secondaryDown)))
216244
.dispatch('GET', '/ready', undefined, undefined, ctx);
217245

218-
expect(res.response.status).toBe(503);
219-
expect(res.response.body.error.details).toEqual({
246+
expect(responseOf(res).status).toBe(503);
247+
expect(responseOf(res).body.error.details).toEqual({
220248
state: 'running',
221249
drivers: ['tenant_mongo'],
222250
});
@@ -228,7 +256,7 @@ describe('HttpDispatcher — GET /ready primary-vs-secondary drain (#13408)', ()
228256
resolvePrimaryDatasource: () => { throw new Error('registry exploded'); },
229257
})).dispatch('GET', '/ready', undefined, undefined, ctx);
230258

231-
expect(res.response.status).toBe(503);
259+
expect(responseOf(res).status).toBe(503);
232260
});
233261

234262
it.each([
@@ -241,7 +269,7 @@ describe('HttpDispatcher — GET /ready primary-vs-secondary drain (#13408)', ()
241269
kernel('running', engineWithPrimary(secondaryDown, verdict)),
242270
).dispatch('GET', '/ready', undefined, undefined, ctx);
243271

244-
expect(res.response.status).toBe(503);
272+
expect(responseOf(res).status).toBe(503);
245273
});
246274

247275
it.each([
@@ -255,7 +283,7 @@ describe('HttpDispatcher — GET /ready primary-vs-secondary drain (#13408)', ()
255283
kernel('running', engineWithPrimary(secondaryDown, verdict)),
256284
).dispatch('GET', '/ready', undefined, undefined, ctx);
257285

258-
expect(res.response.status).toBe(503);
286+
expect(responseOf(res).status).toBe(503);
259287
});
260288

261289
it('NON-VACUITY: the same fixture returns 200 the moment the criterion resolves', async () => {
@@ -265,7 +293,7 @@ describe('HttpDispatcher — GET /ready primary-vs-secondary drain (#13408)', ()
265293
kernel('running', engineWithPrimary(secondaryDown, primary('pg'))),
266294
).dispatch('GET', '/ready', undefined, undefined, ctx);
267295

268-
expect(res.response.status).toBe(200);
296+
expect(responseOf(res).status).toBe(200);
269297
});
270298
});
271299

@@ -294,8 +322,8 @@ describe('HttpDispatcher — GET /health liveness probe', () => {
294322
const res = await new HttpDispatcher(kernel('running', e))
295323
.dispatch('GET', '/health', undefined, undefined, ctx);
296324

297-
expect(res.response.status).toBe(200);
298-
expect(res.response.body.data.status).toBe('ok');
325+
expect(responseOf(res).status).toBe(200);
326+
expect(responseOf(res).body.data.status).toBe('ok');
299327
expect(e.checkDriversHealth).not.toHaveBeenCalled();
300328
});
301329
});

scripts/check-type-check-coverage.mjs

Lines changed: 15 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -901,18 +901,23 @@ const TEST_DEBT = {
901901
+ 'ead731756, which is what the lowering at the top of this note closed.',
902902
},
903903
'@objectstack/runtime': {
904-
errors: 217,
905-
note: 'TS18048 x98 (possibly-undefined), TS2345 x26, TS18046 x20, TS2339 x16, TS2493 x15, TS2835 x11, '
906-
+ 'TS2554 x11. LOWERED 227 -> 217 at ead731756 (#12723) -- the entry that card was filed on, where '
907-
+ 'the slack turned out to be 10 rather than the 1 it reported: the 226 it quoted was itself a '
908-
+ 'reading taken days earlier, which is the card\'s own point about a number that drifts. The tally '
909-
+ 'above was taken at 227 and is NOT re-tallied here; that sweep measured per-entry TOTALS only. Src '
910-
+ 'graduated in #4311 (declares `typecheck`); this is purely the hidden test layer. Measured 220 -> '
904+
errors: 206,
905+
note: 'TS18048 x91 (possibly-undefined), TS18046 x27, TS2339 x17, TS2493 x15, TS2835 x13, TS2345 x10, '
906+
+ 'TS7006 x8, TS6133 x6, TS2554 x4, TS2353 x4, TS2571 x3, TS2550 x2 -- RE-TALLIED at 206 (#13408). '
907+
+ 'The previous note carried its composition from a 227-era sweep that measured per-entry TOTALS '
908+
+ 'only and said so; this one is a fresh per-code count of the same program the ratchet measures. '
909+
+ 'LOWERED 217 -> 206 (#13408), and the -11 is fully attributed to ONE file: '
910+
+ 'src/http-dispatcher.ready.test.ts held 30 TS18048 reads of the optional '
911+
+ '`HttpDispatcherResult.response` -- 19 added by that card\'s own new /ready suite and 11 that '
912+
+ 'pre-dated it -- and all 30 were replaced by a `responseOf()` narrowing helper, the shape already '
913+
+ 'used by the #8287 suite in src/http-dispatcher.keys.test.ts. That card found them the hard way: '
914+
+ 'the package `typecheck` excludes test files, so its green said nothing about the 19 it had just '
915+
+ 'added, and only this ratchet saw them. Nothing else in the package moved. Earlier lineage: 220 -> '
911916
+ '218 (5ab08428, one of only two entries that ever shrank; TS6133 x25 collapsed to x7 while '
912917
+ 'possibly-undefined grew, so that net -2 hid a much larger churn in both directions) -> 227 '
913-
+ '(e8db1a230). The latest +9 is fully attributed and is ONE file: every one of the nine is a '
914-
+ 'TS18048 in src/domains/meta-item-envelope.test.ts, added by #5563 / PR #5895 (the '
915-
+ '`GET /meta/:type/:name` envelope convergence). Nothing else in the package moved.',
918+
+ '(e8db1a230, +9 all TS18048 in src/domains/meta-item-envelope.test.ts from #5563 / PR #5895) -> '
919+
+ '217 (ead731756, #12723). Src graduated in #4311 (declares `typecheck`); this is purely the '
920+
+ 'hidden test layer.',
916921
},
917922
'@objectstack/cli': {
918923
errors: 144,

0 commit comments

Comments
 (0)