Skip to content

Commit 45a2e16

Browse files
baozhoutaoclaude
andcommitted
fix(client): compile tests/integration/ and point its discovery assertions at the surface that exists
`packages/client/tests/integration/01-discovery.test.ts` was read by neither gate: `tsconfig.test.json`'s `include` stopped at `src/**/*` and no `exclude` anywhere named the file (the #5476 shape — outside every program, not inside an excluded region), while the regular `vitest.config.ts` excludes `tests/integration/**` because the suite needs a live server. Two channels, both blind, and the suite had drifted onto a `client.discovery` property `ObjectStackClient` does not have. - `tsconfig.test.json`: `include` gains `tests/**/*`. The vitest split is untouched — compiling the file does not run it, and the suite still needs the external server its README describes. tsc is the gate that works without one. - `01-discovery.test.ts`, all three errors the inclusion surfaces: - TC-DISC-004 read `client.discovery`, which never existed on the class (TS2339 x2). The payload lives on the private `discoveryInfo` field, read here through the bracket-notation escape hatch exactly as this package's `src/client.hono.test.ts` already reads it. No public API was invented for the suite. The case also now asserts `routes` is populated — the thing "Route Resolution" is named for, and what `getRoute()` steers every later call with — so assertion strength goes up, not down. - TC-DISC-002's `discovery.apiName.length` (TS18048) is reached optionally and asserted, per #5449's convention in this package: a missing value fails `toBeGreaterThan` rather than being waved through by a `!` or a `?? ''`. Which KEY that assertion should name is a producer-side contract question, not a test's to settle — metadata-protocol emits the deprecated `apiName` and no `name`, the runtime dispatcher emits `name` and no `apiName` — so the spelling is left alone and the evidence is recorded on #4828. Reverse verification, both directions predicted before running: restoring the old assertions under the new `include` reports exactly the 3 original errors; restoring the old `include` as well makes them vanish — rolling this change back does not go red, it goes BLIND, which is the defect being fixed. Fixes #5544 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016FNvXhtSdnEGEfLEsMmvxh
1 parent 229d29e commit 45a2e16

2 files changed

Lines changed: 35 additions & 19 deletions

File tree

packages/client/tests/integration/01-discovery.test.ts

Lines changed: 24 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -36,9 +36,13 @@ describe('Discovery & Connection', () => {
3636

3737
// Version should be a semantic version or API version string
3838
expect(discovery.version).toMatch(/^v?\d+/);
39-
40-
// API name should be non-empty
41-
expect(discovery.apiName.length).toBeGreaterThan(0);
39+
40+
// API name should be non-empty. `apiName` is optional on the discovery
41+
// payload (spec `protocol.zod.ts` keeps it as the deprecated alias for
42+
// `name`), so it is reached optionally and asserted — a missing value
43+
// fails `toBeGreaterThan` rather than being waved through by a `!` or a
44+
// `?? ''` default (#5449).
45+
expect(discovery.apiName?.length).toBeGreaterThan(0);
4246
});
4347
});
4448

@@ -56,13 +60,23 @@ describe('Discovery & Connection', () => {
5660
test('should resolve API routes from discovery info', async () => {
5761
const client = new ObjectStackClient({ baseUrl: TEST_SERVER_URL });
5862
await client.connect();
59-
60-
// After connection, client should have discovery info
61-
expect(client.discovery).toBeDefined();
62-
expect(client.discovery?.version).toBeDefined();
63-
64-
// Verify that subsequent API calls can be made (routes are resolved)
65-
// This implicitly tests route resolution
63+
64+
// After connection, the client should have retained the discovery info.
65+
// `ObjectStackClient` has no public `discovery` property — the one this
66+
// case asserted until #5544 never existed on the class; the payload is
67+
// held on the private `discoveryInfo` field (`src/index.ts`), which is
68+
// what `getRoute()` steers every subsequent call with. It is read here
69+
// through the bracket-notation escape hatch, exactly as this package's
70+
// `src/client.hono.test.ts` already reads the same field — no public API
71+
// is invented on behalf of a suite no type checker had ever compiled.
72+
const discoveryInfo = client['discoveryInfo'];
73+
expect(discoveryInfo).toBeDefined();
74+
expect(discoveryInfo?.version).toBeDefined();
75+
76+
// Route resolution is what this case is named for: the routes map
77+
// `getRoute()` reads has to be populated for subsequent API calls to be
78+
// steered at all.
79+
expect(discoveryInfo?.routes).toBeDefined();
6680
});
6781
});
6882
});

packages/client/tsconfig.test.json

Lines changed: 11 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -24,14 +24,16 @@
2424
// Nothing here may loosen a type rule; if a test does not compile, that is
2525
// the finding.
2626
//
27-
// `include` deliberately stops at `src`, matching the build config's root, and
28-
// none of the files it leaves out carries a `@ts-expect-error`, so no pin is
29-
// hiding there. `tests/integration/` — the suite `vitest.integration.config.ts`
30-
// runs against a live server — is in no tsconfig at all: a second,
31-
// differently-shaped hole (1 file / 3 errors, one of them a real API drift, the
32-
// suite reading a `client.discovery` property `ObjectStackClient` does not
33-
// have) that wants its own change rather than a rider on this one. Filed as
34-
// #5544.
27+
// `include` covers BOTH test roots this package has. `src/**/*` is the layer
28+
// the build config excludes; `tests/**/*` is `tests/integration/`, the suite
29+
// `vitest.integration.config.ts` runs against a live server and the regular
30+
// `vitest.config.ts` excludes. Until #5544 it was named by no `include` and no
31+
// `exclude` anywhere — the #5476 shape, outside every program rather than
32+
// inside an excluded region — so neither vitest's regular run nor tsc ever read
33+
// it, and it had drifted onto a `client.discovery` property `ObjectStackClient`
34+
// does not have. Compiling it does NOT run it: the vitest split is unchanged,
35+
// the suite still needs a server. tsc reading a file is the cheaper of the two
36+
// gates and the only one that works without one.
3537
//
3638
// The per-file ledger beside this config (`test-typecheck-debt.json`) is small
3739
// on purpose. Under the repaired config the whole test layer came to 13 errors;
@@ -57,6 +59,6 @@
5759
"lib": ["ES2022", "DOM", "DOM.Iterable"],
5860
"types": ["node"]
5961
},
60-
"include": ["src/**/*"],
62+
"include": ["src/**/*", "tests/**/*"],
6163
"exclude": ["node_modules", "dist"]
6264
}

0 commit comments

Comments
 (0)