TNC-2432 / v2.2 / feat(testing): publish the testing entry, with withSpies, UnmockedCallError and the fixture builders

[agent-heimdall]

Third slice, on top of #56, and the one that makes the entry real: @truenas/api-client/testing is a published subpath now, so it moves under the same semver rules as the main entry.

import { createFakeClient, fakeApiError, withSpies } from '@truenas/api-client/testing';

What it adds

./testing subpath export	built by tsup alongside the main entry, covered by check:dist, publint and attw
UnmockedCallError	createFakeClient({ strict: true }) fails an unscripted call instead of hanging
withSpies(client, vi.fn)	wraps the client's verbs in the caller's spies, in place
fakeApiError, fakeApiVersion, fakeAuthResponse	the remaining fixture builders from the proposal, beside fakeJob
AuthResponseType	now a value export from both entries — a string enum rejects its own literals, so without it only the default arm of fakeAuthResponse / failNextLogin was reachable
TypeDoc	documents and link-checks the testing entry; both modules are named after their import paths

Two packaging findings worth a reviewer's attention

splitting: true is load-bearing, not tidy. Two entries built from one source each bundle their own copy of every class they use — tsup splits for ESM by default, for CJS it does not. The values still compare equal, because the enums are strings, so nothing looks wrong until a consumer writes expect(client).toBeInstanceOf(TrueNasApiClient) against a fake from the other entry under Jest and is told it is not one. check-dist.mjs now builds a fake client and asserts it is an instance of the main entry's class, in both formats; removing the flag fails that check while every other gate stays green.

The alias guard had stopped covering the output. It scanned a hardcoded list of eight files, which was the whole output until splitting arrived — after it, dist/index.js is an 858-byte re-export stub and the code and shared types live in content-hashed chunks no list can name. An import("@/…") in the emitted .d.ts passed in silence. It walks dist now and fails if the walk finds fewer files than the two entries in two formats plus their types.

Docs URLs move on the next release. With a second entry point TypeDoc switches to module mode, so every page path gains a module prefix — classes/TrueNasApiClient.html becomes classes/_truenas_api-client.TrueNasApiClient.html. Existing deep links to the reference will 404; worth a line in the release notes.

typesVersions maps the subpath for node10, which attw fails without: modern Node honours exports at runtime, but ts-jest's legacy resolution finds the subpath's types only through that mapping. All four resolution modes are 🟢 for both entries.

UnmockedCallError is opt-in, against the proposal's line

The proposal said send throws for an unregistered method, full stop. It cannot: at send time a frame that mock will answer and a frame that connection.reply will answer two lines later are indistinguishable. Refusing unconditionally would break every hand-driven spec — 24 in this repo's own suite. So it is createFakeClient({ strict: true }), which is the spec's claim that it scripted everything. The client's own core.subscribe frames are exempt.

withSpies installs unbound, deliberately

Both runners invoke the implementation with the this of the call, so a verb reached as client.api.call(…) gets its own object either way. Binding would make a detached const { call } = client.api work under spies where it throws without them — a spied client more permissive than an unspied one is the divergence this package exists to prevent, in the helper meant to observe it.

Two things follow that took a while to get right, and both have tests:

• SpyFactory is exported so a consumer can pass something other than vi.fn, and the contract it cannot express in the type is that the returned function must forward this. An arrow does not, nor does (impl) => vi.fn((...args) => impl(...args)) — the shape you reach for wanting a spy and something of your own. Both type-check. The wrapper names the problem instead of letting the real method fail on its own first line with Cannot read properties of undefined.
• The wrapper calls the method on every path rather than refusing on a missing this. callAndGetJobId's body is inside a defer, so it tolerates a detached call and returns an observable — refusing made the spied client stricter than the real one, which is the same divergence pointing the other way.

The fixtures are checked against middleware, not against each other

fakeApiError produces the frame /api/<version> actually sends — a JSON-RPC error with the TrueNAS payload under data, code: -32001. Its trace is an object rather than null, because both arms that send -32001 pass sys.exc_info(); and class and repr are derived so the pair is one an appliance could send, with formatted explicitly outside that guarantee.

That derivation needs a Python repr(), and pythonRepr is checked against python3 rather than against expectations written here — a differential over every code point plus random strings. The one limit it cannot escape is documented on the class it uses: \p{Cn} is a property of a code point in a Unicode version, and the engine's table and the appliance's Python's are not the same, so the two disagree on code points one has assigned and the other has not.

fakeAuthResponse builds per arm, because auth.login_ex returns a discriminated union: SUCCESS carries user_info/authenticator/reconnect_token, OTP_REQUIRED carries username, REDIRECT carries urls, and AUTH_ERR and EXPIRED carry nothing. The arms are a Record keyed by the enum, so a member added to AuthResponseType fails the build rather than silently taking an empty arm.

Two type gaps this surfaced, both outside the diff

• AuthResponse declares max_session_age and max_inactivity. Neither is on any arm of middleware's union at any version — max_session_age exists only as an internal AAL attribute. The builder never invents them.
• AuthResponseType is two arms short: AuthLoginExResult.result has seven, and DENIED and SCRAM_RESPONSE are missing, so this package cannot name two of the responses a v26+ appliance can send. AuthRespScram requires scram_type and rfc_str, so adding it is not just an enum edit — which is why ARMS fails the build when it arrives.

Happy to take either as a follow-up.

Verification

Seven local review rounds before the CI review, one after. 642 tests (43 files), and all eleven gates exit 0: tsc base/spec/scripts, eslint, vitest, tsup, check-dist, typedoc, publint, pack, attw.

Round 7 is the first clean one, and the honest summary of the other six is that the top finding in each was inside the previous round's repair — a shared array introduced by an exhaustiveness fix, a repr quoter that escaped three of thirty-three characters, a this guard that renamed every spied verb. Every one is now pinned by a test that fails when the mechanism is broken.

The CI review then found two HIGHs, both fixed in 4ba575f: AuthResponseType was on neither entry, and fakeApiVersion was the one builder not going through present(), so an explicit undefined landed on a required field. The testing entry's runtime exports are now pinned by src/testing/index.spec.ts, as the main barrel's are — the enum went missing because every spec imported it from @/types. The two cheap LOWs were taken too (JobUpdate aliases FakeJobOverrides; TypeDoc covers the entry). A local review against the CI prompt and rubric came back with no findings at MEDIUM or above.

Six LOWs are open and not addressed, per direction on the previous PR:

1. callAndGetJobId is the one verb a non-forwarding spy factory still fails opaquely on, and two docblocks read as though it does not.
2. A comment says both runners copy the implementation's name and length; jest copies length, not name.
3. Wrapping a thrown error changes its class, so "the outcome is always the real method's" is true of a return and not of a throw.
4. this === null still reaches the opaque failure rather than the named one.
5. FakeAuthenticator's own answers (success(), succeedNextLogin, failNextLogin) still build bare responses rather than going through fakeAuthResponse, so an unscripted login has no user_info.
6. The last line of the synthetic trace.formatted is the repr; a real traceback ends with Class: reason, or the bare class.

🤖 Generated with Claude Code

[bugclerk]

Jira URL: https://ixsystemsinc.atlassian.net/browse/TNC-2432

[github-actions]

LOW — This entry becomes public surface here, but typedoc.config.mjs still has entryPoints: ['src/index.ts'], and src/index.ts exports nothing from src/testing. TypeDoc only walks what is reachable from an entry point, so none of the fifteen symbols below appear in the published docs, and docs:check (which runs with validation.invalidLink: true and treatWarningsAsErrors: true) never looks at these files.

That second half is load-bearing rather than cosmetic: src/testing/create-fake-client.ts now writes {@link UnmockedCallError} and {@link FakeConnectionOptions.strict} in the strict docblock, and that file imports neither name. Both would be reported as unresolved links the moment the entry point is added — which is the check the rest of the package already gets.

Adding 'src/testing/index.ts' to entryPoints covers both, at the cost of fixing those two links first.

[github-actions]

LOW — The diagnosis only reaches verbs that read this synchronously. callAndGetJobId's body is entirely inside a defer, so with a non-forwarding factory it neither throws here nor returns anything usable — the failure surfaces on subscribe, as Cannot read properties of undefined (reading 'dispatch') out of a bundled chunk, naming neither spies nor this.

Concretely, with the shape the SpyFactory docblock calls out as the one a consumer reaches for:

const c = withSpies(createFakeClient({ version: 'v27.0.0' }), impl => vi.fn((...a) => impl(...a)));
c.api.callAndGetJobId('app.delete', ['plex']).subscribe();  // TypeError, unexplained

with-spies.spec.ts exercises the guard through c.api.call only, so the gap is not visible from the suite either. Worth either narrowing the SpyFactory docblock's "withSpies detects that and says so" to the synchronous case, or wrapping the returned value when it is an Observable so the deferred verbs get the same sentence.

[github-actions]

Three findings: three LOW. No human review required.

The packaging work is the strongest part of this change. check-dist.mjs going from a hardcoded list of eight files to readdirSync(recursive) is a real fix — with splitting: true the alias guard was scanning stubs and skipping the chunks that hold the shared code — and the instanceof assertion across both formats turns "splitting is tidy" into a gate that fails if the flag is removed. Exporting AuthResponseType as a value from both entries closes a genuine hole: a string enum rejects its own literals, so without it only the default arm of fakeAuthResponse and failNextLogin was reachable from outside the package.

The three findings, all inline:

• LOW — fake-auth-response.ts:61: FakeAuthenticator.success() / failNextLogin still hand-build { response_type }, so every unscripted login answers with the bare shape fakeAuthResponse was added to retire — and sessionLifetime falls through to the client default rather than the user_info.attributes.preferences.lifetime a real appliance sends. fake-authenticator.ts is outside this diff, so it is a reuse gap, not a regression.
• LOW — with-spies.ts:60: CONNECTION_METHODS is the one of the three lists with no pin against its class. API_VERBS has a runtime walk and an Exclude, AUTHENTICATOR_METHODS has an Exclude; this has neither, so a new outbound method on TrueNasConnection would be classified by fake-connection.spec.ts and still go unspied.
• LOW — fake-auth-response.ts:39: reconnect_token: null is unconditional and present() drops an undefined override, so no success response can be built without the field — which v25.10 never sends. The docblock note saying so was dropped in 3739898.

[github-actions]

Needs a human review. 2 finding(s): 2 LOW.

Nothing blocks, but this change is one a person should decide on:

• package.json:20 adds ./testing to exports, placing fifteen new symbols permanently under the package's semver rules
• tsup.config.ts:9 enables splitting, reshaping the already-published main entry into stubs over chunks via tsup's experimental CJS splitting

[github-actions]

HIGH — AuthResponseType is not exported from either entry, so no consumer can name any arm but the default.

FakeAuthResponseOverrides['response_type'] is the string enum AuthResponseType, declared in src/types/auth.type.ts and re-exported by neither src/index.ts (which exports only type { AuthResponse }) nor this file. A string enum does not accept its own literal, so from outside the package:

import { fakeAuthResponse } from '@truenas/api-client/testing';

fakeAuthResponse({ response_type: 'AUTH_ERR' });
//                               ^ Type '"AUTH_ERR"' is not assignable to type 'AuthResponseType'

and there is no import that fixes it — the value simply is not on the surface. Four of the five arms the function's own JSDoc table documents (OTP_REQUIRED, REDIRECT, AUTH_ERR, EXPIRED) are unreachable without a cast through NonNullable<FakeAuthResponseOverrides['response_type']>.

FakeAuthenticator (line 16) has the same gap: failNextLogin(responseType: AuthResponseType) compiles only at its default, and succeedNextLogin(Partial<AuthResponse>) cannot set a response_type either.

This is exactly the argument src/index.ts already makes for AppState: "Value export, not type-only: … without the enum itself a consumer has nothing to compare it against." The fix is one line — export { AuthResponseType } from '@/types/auth.type'; in src/index.ts, and/or here alongside the other re-export at line 29.

[github-actions]

HIGH — the one builder in this PR that does not go through present(), and every field it can drop is required.

FakeApiVersionOverrides = Partial<ApiVersion> and exactOptionalPropertyTypes is off, so an explicit undefined type-checks and this spread applies it:

fakeApiVersion('v27.0.0', { websocketPath: legacy ? '/websocket' : undefined })
// → { version: 'v27.0.0', year: 27, minor: 0, patch: 0, websocketPath: undefined }

ApiVersion.websocketPath is string, so that object is a lie its consumers read — createFakeClient would hand it to new FakeConnection({ websocketPath: undefined }), which silently falls back to '/api/current', and a spec asserting on the path it asked for fails somewhere other than here. All five fields of ApiVersion are required, so all five are exposed.

This is the failure present.ts was extracted in this PR to prevent, and its docblock claims the guard is universal — "Every builder here takes a Partial<…> … Dropping them makes an absent field mean 'unchanged', which is what a partial means everywhere else here." fakeJob, fakeApiError and fakeAuthResponse all apply it; this one was missed.

Suggested change

	return { ...parsed, ...overrides };
	return { ...parsed, ...present(overrides) };

(plus import { present } from './present';)

[github-actions]

LOW — FakeJobOverrides<R> and MockAnswers' JobUpdate<R> (mock-answers.ts:89) are now the same type written twice, and both are exported from the entry:

export type JobUpdate<R> = Partial<Omit<Job<R>, 'progress'>> & {
  progress?: Partial<JobProgress>;
};

Same fields, same one-level-down progress, same stated reason in both docblocks. Since mock.job completes each update through fakeJob, JobUpdate<R> = FakeJobOverrides<R> is not just a coincidence of shape — it is the same contract. Aliasing it (export type JobUpdate<R> = FakeJobOverrides<R>;) keeps the name a consumer already reads while leaving one statement of the shape; as it stands a change to one silently leaves the other behind, and consumers see two public names for one thing.

[github-actions]

Changes requested. 5 finding(s): 2 HIGH, 3 LOW.

Findings at MEDIUM and above are on the lines they are about. Fix them, or say on the PR why one was mis-rated.

[github-actions]

LOW — the builder lands, but the one place in this package that actually produces an AuthResponse still hand-builds a bare one.

FakeAuthenticator.success() returns { response_type: AuthResponseType.Success }, and failNextLogin returns { response_type } — the same three-field-and-cast shape this docblock exists to retire. It is the default answer for every unscripted login, so createFakeClient() plus client.authenticator.loginWithUserPass('root', 'pw') hands the real TrueNasAuthenticator a SUCCESS with no user_info, and truenas-authenticator.ts:280-282 falls through to DefaultSessionLifetime instead of reading user_info.attributes.preferences.lifetime — which fakeAuthResponse() supplies (300) and a real appliance always sends on success.

fake-authenticator.ts is not in this diff, so this is a reuse gap rather than a regression: success() → fakeAuthResponse(), failNextLogin → fakeAuthResponse({ response_type }), and succeedNextLogin(response) → fakeAuthResponse(response) would all be drop-ins, and would make the fake authenticator's answers the same shape as the ones a spec scripts by hand.

[github-actions]

LOW — this is the one of the three lists with nothing pinning it against the class it describes.

API_VERBS gets the runtime walk in with-spies.spec.ts:112 and a type-level Exclude over keyof TrueNasApi; AUTHENTICATOR_METHODS gets the Exclude over keyof TrueNasAuthenticator. CONNECTION_METHODS gets neither, so the claim above — that send is the whole of the connection's outbound surface — is unenforced.

fake-connection.spec.ts's DRIVEN/INERT inventory does fail when a member is added to TrueNasConnection, but it only asks is it driven or inert; nothing routes a new outbound method here, so it would go unspied and a spec reaching for it would fail with "not a spy" rather than with anything about the call. A keyof TrueNasConnection-based Exclude in the spec, matching the two it already has, would close it.

[github-actions]

LOW — reconnect_token is unconditional on the SUCCESS arm and, because present() drops an override of literally undefined, there is no way to build a success response without the field: fakeAuthResponse({ reconnect_token: undefined }) still returns null. v25.10 does not declare it at all (auth.type.ts:24 — "Absent below v26"), so a spec pinned to that version gets a shape the appliance never sends, and 'reconnect_token' in response is true on every response this builder makes.

Nothing in the client branches on its presence, so the blast radius is a consumer's own assertion. The note that used to say this — "this builder has no way to say that … do not read 'reconnect_token' in response as a claim about the version" — went out with the docblock condensing in 3739898, so the limitation is now undocumented on a helper that is published public surface as of this PR. Worth a line in the table above rather than a code change.

[github-actions]

Approved. 3 finding(s): 3 LOW.

Nothing blocks and no human review is needed.

[github-actions]

🎉 This PR is included in version 6.0.2 🎉

The release is available on:

• npm package (@latest dist-tag)
• GitHub release

Your semantic-release bot 📦🚀