From 45a2e1658c06878fc817d33ce7931e7a30fb5cfb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=8C=85=E5=91=A8=E6=B6=9B?= Date: Wed, 5 Aug 2026 18:32:59 +0000 Subject: [PATCH] fix(client): compile `tests/integration/` and point its discovery assertions at the surface that exists MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `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 Claude-Session: https://claude.ai/code/session_016FNvXhtSdnEGEfLEsMmvxh --- .../tests/integration/01-discovery.test.ts | 34 +++++++++++++------ packages/client/tsconfig.test.json | 20 ++++++----- 2 files changed, 35 insertions(+), 19 deletions(-) diff --git a/packages/client/tests/integration/01-discovery.test.ts b/packages/client/tests/integration/01-discovery.test.ts index 699784287a..3a9e171ca2 100644 --- a/packages/client/tests/integration/01-discovery.test.ts +++ b/packages/client/tests/integration/01-discovery.test.ts @@ -36,9 +36,13 @@ describe('Discovery & Connection', () => { // Version should be a semantic version or API version string expect(discovery.version).toMatch(/^v?\d+/); - - // API name should be non-empty - expect(discovery.apiName.length).toBeGreaterThan(0); + + // API name should be non-empty. `apiName` is optional on the discovery + // payload (spec `protocol.zod.ts` keeps it as the deprecated alias for + // `name`), so it is reached optionally and asserted — a missing value + // fails `toBeGreaterThan` rather than being waved through by a `!` or a + // `?? ''` default (#5449). + expect(discovery.apiName?.length).toBeGreaterThan(0); }); }); @@ -56,13 +60,23 @@ describe('Discovery & Connection', () => { test('should resolve API routes from discovery info', async () => { const client = new ObjectStackClient({ baseUrl: TEST_SERVER_URL }); await client.connect(); - - // After connection, client should have discovery info - expect(client.discovery).toBeDefined(); - expect(client.discovery?.version).toBeDefined(); - - // Verify that subsequent API calls can be made (routes are resolved) - // This implicitly tests route resolution + + // After connection, the client should have retained the discovery info. + // `ObjectStackClient` has no public `discovery` property — the one this + // case asserted until #5544 never existed on the class; the payload is + // held on the private `discoveryInfo` field (`src/index.ts`), which is + // what `getRoute()` steers every subsequent call with. It is read here + // through the bracket-notation escape hatch, exactly as this package's + // `src/client.hono.test.ts` already reads the same field — no public API + // is invented on behalf of a suite no type checker had ever compiled. + const discoveryInfo = client['discoveryInfo']; + expect(discoveryInfo).toBeDefined(); + expect(discoveryInfo?.version).toBeDefined(); + + // Route resolution is what this case is named for: the routes map + // `getRoute()` reads has to be populated for subsequent API calls to be + // steered at all. + expect(discoveryInfo?.routes).toBeDefined(); }); }); }); diff --git a/packages/client/tsconfig.test.json b/packages/client/tsconfig.test.json index 2ec76f91f8..f9df0f8d54 100644 --- a/packages/client/tsconfig.test.json +++ b/packages/client/tsconfig.test.json @@ -24,14 +24,16 @@ // Nothing here may loosen a type rule; if a test does not compile, that is // the finding. // -// `include` deliberately stops at `src`, matching the build config's root, and -// none of the files it leaves out carries a `@ts-expect-error`, so no pin is -// hiding there. `tests/integration/` — the suite `vitest.integration.config.ts` -// runs against a live server — is in no tsconfig at all: a second, -// differently-shaped hole (1 file / 3 errors, one of them a real API drift, the -// suite reading a `client.discovery` property `ObjectStackClient` does not -// have) that wants its own change rather than a rider on this one. Filed as -// #5544. +// `include` covers BOTH test roots this package has. `src/**/*` is the layer +// the build config excludes; `tests/**/*` is `tests/integration/`, the suite +// `vitest.integration.config.ts` runs against a live server and the regular +// `vitest.config.ts` excludes. Until #5544 it was named by no `include` and no +// `exclude` anywhere — the #5476 shape, outside every program rather than +// inside an excluded region — so neither vitest's regular run nor tsc ever read +// it, and it had drifted onto a `client.discovery` property `ObjectStackClient` +// does not have. Compiling it does NOT run it: the vitest split is unchanged, +// the suite still needs a server. tsc reading a file is the cheaper of the two +// gates and the only one that works without one. // // The per-file ledger beside this config (`test-typecheck-debt.json`) is small // on purpose. Under the repaired config the whole test layer came to 13 errors; @@ -57,6 +59,6 @@ "lib": ["ES2022", "DOM", "DOM.Iterable"], "types": ["node"] }, - "include": ["src/**/*"], + "include": ["src/**/*", "tests/**/*"], "exclude": ["node_modules", "dist"] }