Skip to content

Commit 49bd6fd

Browse files
committed
test(client): pay the four auth suites' cold start at module scope, outside every clocked window
The first case of auth-get-session-envelope, auth-rotated-session-token, organization-invitation-resend-team-placement and organization-invite-role-default paid the worker's one-time cold start (better-auth's lazy module graph, the sql.js WASM compile, first-use sync and sign-up) inside vitest's clocked test window. Each file now pays it once through its own arrangement at module scope, during collection, and pins that placement. auth-rotated-session-token's seven explicit 60_000 per-case timeouts are removed: they widened the window instead of moving the cost out of it. Claude-Session: https://claude.ai/code/session_01UYBdGBzWSrAMzpW8ah3GbP Co-authored-by: Claude <noreply@anthropic.com>
1 parent c74de10 commit 49bd6fd

4 files changed

Lines changed: 308 additions & 23 deletions

‎packages/client/src/auth-get-session-envelope.test.ts‎

Lines changed: 62 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -45,8 +45,42 @@
4545
// `data.token` is absent from the normalized body too, so a regression back
4646
// to `data.data?.token` cannot pass by accident, and the "enveloping it
4747
// would have fixed refreshToken" reading stays refuted in code.
48+
// - `⑥ the cold start stays outside every clocked window` — pins WHERE the
49+
// first scenario's one-time cost is paid (next section).
50+
//
51+
// ## [#20327] Why one throwaway scenario runs at MODULE SCOPE
52+
//
53+
// The first scenario in a worker pays a one-time cost no later one sees:
54+
// better-auth's lazily imported module graph, sql.js's WASM compile, and the
55+
// first-use costs of the sync and the sign-up. The phases are measured in
56+
// `auth-login-register-envelope.test.ts`, which carries the same arrangement
57+
// and the same fix. Here, idle on 4 vCPU at `c74de10a9`, that cost put the
58+
// first case at 1035 ms against 110-330 ms for the six after it. On a loaded
59+
// `Test Core` shard (PR #20325's run) the first case crossed vitest's 5000 ms
60+
// `testTimeout` while every other case passed. Twenty-four CPU-bound busy
61+
// loops on this box reproduce that exact signature on the first case.
62+
//
63+
// So the cost is now paid by a module-scope `await`, during COLLECTION, which
64+
// no vitest clock covers: `@vitest/runner@4.1.11` wraps hooks and test bodies
65+
// in `withTimeout(...)` and awaits the file import bare. This is the repo's
66+
// convention: "clocked windows measure behaviour, never loading" (AGENTS.md,
67+
// Build & Test; `check:test-source-alias`). The warm-up is the file's own
68+
// `signedIn()`, the arrangement six of the seven cases run, so no list of
69+
// loads can drift from what the cases really pay.
70+
//
71+
// ⛔ It shares nothing a case asserts on. Its engine and manager are its own
72+
// and are closed before any case starts. Every case still builds a fresh
73+
// engine, a fresh `AuthManager` and a fresh sign-up. What it leaves warm is
74+
// process-level: the module registry, sql.js's compiled WASM and the JIT,
75+
// which the first case used to leave to every later case.
76+
//
77+
// ⛔ Do not move it into a hook, and do not answer a recurrence by raising a
78+
// timeout: vitest clocks a hook exactly as it clocks a test body, and a wider
79+
// window only moves the cliff to a heavier shard.
4880

4981
import { describe, it, expect, afterEach } from 'vitest';
82+
import { readFileSync } from 'node:fs';
83+
import { fileURLToPath } from 'node:url';
5084
import { ObjectQL } from '@objectstack/objectql';
5185
import { SqliteWasmDriver } from '@objectstack/driver-sqlite-wasm';
5286
import { AuthManager } from '@objectstack/plugin-auth';
@@ -250,12 +284,21 @@ const principalFor = async (manager: AuthManager, token: string | undefined) =>
250284
return session?.user?.id ?? null;
251285
};
252286

253-
afterEach(async () => {
287+
/** Close every engine a scenario opened: after each case, and after the warm-up. */
288+
const closeEngines = async (): Promise<void> => {
254289
while (engines.length) {
255290
const engine = engines.pop();
256291
await (engine as unknown as { close?: () => Promise<void> })?.close?.().catch(() => {});
257292
}
258-
});
293+
};
294+
295+
// [#20327] The first scenario's one-time cost, paid during COLLECTION, which
296+
// no vitest clock covers (header, last section). ⛔ It stays at module scope,
297+
// and `⑥` below pins that.
298+
await signedIn();
299+
await closeEngines();
300+
301+
afterEach(closeEngines);
259302

260303
describe('[#16760] /get-session is lifted into the SessionResponse envelope it declares', () => {
261304
describe('① me() delivers the envelope it declares', () => {
@@ -427,4 +470,21 @@ describe('[#16760] /get-session is lifted into the SessionResponse envelope it d
427470
expect(typeof res.data.session?.token).toBe('string');
428471
});
429472
});
473+
474+
describe('⑥ the cold start stays outside every clocked window', () => {
475+
it('pays the first scenario at module scope, and no hook carries it', () => {
476+
// [#20327] Read off this file's own text, so "do not move it into a
477+
// hook" is an assertion rather than a sentence nobody reads.
478+
const code = readFileSync(fileURLToPath(import.meta.url), 'utf8');
479+
480+
// Exactly one warm-up call, and it opens its own line at column 0. So it
481+
// sits in no function body, which is what "paid during collection"
482+
// reduces to. Comment lines start with `//` and cannot match.
483+
expect(code.match(/^await signedIn\(\);$/gm) ?? []).toHaveLength(1);
484+
485+
// ⛔ No `before*` hook may come back to carry it: vitest clocks a hook
486+
// with `hookTimeout` exactly as it clocks a test body with `testTimeout`.
487+
expect(code).not.toMatch(/^\s*before(All|Each)\s*\(/m);
488+
});
489+
});
430490
});

‎packages/client/src/auth-rotated-session-token.test.ts‎

Lines changed: 88 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -41,8 +41,42 @@
4141
// that header in the shared `fetch` wrapper instead of on the rotating routes
4242
// would rewrite the stored credential on an ordinary write — and every
4343
// assertion in ① and ② would stay green.
44+
// - `④ the cold start stays outside every clocked window` — pins WHERE the
45+
// first scenario's one-time cost is paid (next section).
46+
//
47+
// ## [#20327] Why one throwaway scenario runs at MODULE SCOPE
48+
//
49+
// The first scenario in a worker pays a one-time cost no later one sees:
50+
// better-auth's lazily imported module graph, sql.js's WASM compile, and the
51+
// first-use costs of the sync and the sign-up. The phases are measured in
52+
// `auth-login-register-envelope.test.ts`, which carries the same arrangement
53+
// and the same fix. Every case here used to carry an explicit `60_000`
54+
// timeout, which WIDENED the first case's window around that cost instead of
55+
// moving the cost out of it: every budget a cost is moved into can be
56+
// exhausted by a heavier shard (`check:test-source-alias`, clocked-window
57+
// rule).
58+
//
59+
// So the cost is now paid by a module-scope `await`, during COLLECTION, which
60+
// no vitest clock covers: `@vitest/runner@4.1.11` wraps hooks and test bodies
61+
// in `withTimeout(...)` and awaits the file import bare. This is the repo's
62+
// convention: "clocked windows measure behaviour, never loading" (AGENTS.md,
63+
// Build & Test). The warm-up is the file's own `signedIn()`, the arrangement
64+
// every case runs, so no list of loads can drift from what the cases pay. With
65+
// the cost moved out, the `60_000`s are gone and every case runs on the
66+
// default `testTimeout`.
67+
//
68+
// ⛔ It shares nothing a case asserts on. Its engine and manager are its own
69+
// and are destroyed before any case starts. Every case still builds a fresh
70+
// engine, a fresh `AuthManager` and a fresh sign-up. What it leaves warm is
71+
// process-level: the module registry, sql.js's compiled WASM and the JIT,
72+
// which the first case used to leave to every later case.
73+
//
74+
// ⛔ Do not move it into a hook, and do not answer a recurrence by raising a
75+
// timeout: vitest clocks a hook exactly as it clocks a test body.
4476

4577
import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest';
78+
import { readFileSync } from 'node:fs';
79+
import { fileURLToPath } from 'node:url';
4680
import { createHmac } from 'node:crypto';
4781
import { ObjectQL } from '@objectstack/objectql';
4882
import { SqliteWasmDriver } from '@objectstack/driver-sqlite-wasm';
@@ -261,12 +295,8 @@ const signedCredentialFor = async (
261295
return String(signed);
262296
};
263297

264-
beforeEach(() => {
265-
vi.spyOn(console, 'warn').mockImplementation(() => {});
266-
vi.spyOn(console, 'error').mockImplementation(() => {});
267-
});
268-
afterEach(async () => {
269-
vi.restoreAllMocks();
298+
/** Destroy every engine a scenario opened: after each case, and after the warm-up. */
299+
const closeEngines = async (): Promise<void> => {
270300
while (engines.length) {
271301
const e = engines.pop();
272302
try {
@@ -275,6 +305,21 @@ afterEach(async () => {
275305
/* noop */
276306
}
277307
}
308+
};
309+
310+
// [#20327] The first scenario's one-time cost, paid during COLLECTION, which
311+
// no vitest clock covers (header, last section). ⛔ It stays at module scope,
312+
// and `④` below pins that.
313+
await signedIn();
314+
await closeEngines();
315+
316+
beforeEach(() => {
317+
vi.spyOn(console, 'warn').mockImplementation(() => {});
318+
vi.spyOn(console, 'error').mockImplementation(() => {});
319+
});
320+
afterEach(async () => {
321+
vi.restoreAllMocks();
322+
await closeEngines();
278323
});
279324

280325
// ───────────────────────────────────────────────────────────────────────────
@@ -335,7 +380,7 @@ describe("#16534 ① the card's own probe, with the manual re-set deleted", () =
335380
// status code: after the whole sequence the client is still holding a
336381
// credential that resolves to the same principal.
337382
expect(await principalForStoredToken(manager, client)).toBe(userId);
338-
}, 60_000);
383+
});
339384
});
340385

341386
// ───────────────────────────────────────────────────────────────────────────
@@ -358,7 +403,7 @@ describe('#16534 ② one assertion per rotating route — all three', () => {
358403
expect(await principalForStoredToken(manager, client)).toBe(userId);
359404
// …and the one it replaced is genuinely gone.
360405
expect(await principalFor(manager, before)).toBeNull();
361-
}, 60_000);
406+
});
362407

363408
it('changePassword WITHOUT revokeOtherSessions rotates nothing and stores nothing', async () => {
364409
// The other half of the same route: `token` is `null` there, and a client
@@ -374,7 +419,7 @@ describe('#16534 ② one assertion per rotating route — all three', () => {
374419
expect(result.token).toBeNull();
375420
expect(storedToken(client)).toBe(before);
376421
expect(await principalForStoredToken(manager, client)).not.toBeNull();
377-
}, 60_000);
422+
});
378423

379424
it('twoFactor.verifyTotp on the enrolment lane — the body echoes the LIVE token', async () => {
380425
const { engine, manager, client, email } = await signedIn();
@@ -390,7 +435,7 @@ describe('#16534 ② one assertion per rotating route — all three', () => {
390435
// The row behind the replaced value was deleted, so asserting only "the
391436
// stored token changed" would not have been enough.
392437
expect(await principalFor(manager, before)).toBeNull();
393-
}, 60_000);
438+
});
394439

395440
it('twoFactor.disable — the credential arrives ONLY in the `set-auth-token` header', async () => {
396441
// The route triage singled out: it answers `{ status: true }`, so an
@@ -410,7 +455,7 @@ describe('#16534 ② one assertion per rotating route — all three', () => {
410455
expect(storedToken(client)).not.toBe(before);
411456
expect(await principalForStoredToken(manager, client)).toBe(userId);
412457
expect(await principalFor(manager, before)).toBeNull();
413-
}, 60_000);
458+
});
414459
});
415460

416461
// ───────────────────────────────────────────────────────────────────────────
@@ -439,7 +484,7 @@ describe('#16534 ③ the negative control — a NON-rotating route changes nothi
439484
// Still the same live session at the end of it — the invariant is "did not
440485
// move", not "was emptied".
441486
expect(await principalForStoredToken(manager, client)).not.toBeNull();
442-
}, 60_000);
487+
});
443488

444489
it("verifyBackupCode's already-logged-in lane leaves the stored credential byte-identical", async () => {
445490
// `/two-factor/verify-backup-code` shares `AuthTwoFactorVerificationResult`
@@ -472,5 +517,35 @@ describe('#16534 ③ the negative control — a NON-rotating route changes nothi
472517
expect(result.token).not.toBe(signed);
473518
expect(storedToken(bearerClient), 'verifyBackupCode moved the stored credential').toBe(signed);
474519
expect(await principalForStoredToken(manager, bearerClient)).toBe(userId);
475-
}, 60_000);
520+
});
521+
});
522+
523+
// ───────────────────────────────────────────────────────────────────────────
524+
describe('#16534 ④ the cold start stays outside every clocked window', () => {
525+
it('pays the first scenario at module scope, no hook carries it, and no case widens its clock', () => {
526+
// [#20327] Read off this file's own text, so "do not move it into a
527+
// hook" is an assertion rather than a sentence nobody reads.
528+
const code = readFileSync(fileURLToPath(import.meta.url), 'utf8');
529+
530+
// Exactly one warm-up call, and it opens its own line at column 0. So it
531+
// sits in no function body, which is what "paid during collection"
532+
// reduces to. Comment lines start with `//` and cannot match.
533+
expect(code.match(/^await signedIn\(\);$/gm) ?? []).toHaveLength(1);
534+
535+
// ⛔ No `before*` hook may come back to carry it: vitest clocks a hook
536+
// with `hookTimeout` exactly as it clocks a test body with `testTimeout`.
537+
// This file's one hook is the console-spy `beforeEach`. Each hook is read
538+
// up to the next column-0 `});`, so a hook indented inside a `describe`
539+
// reads on to that block's close and cannot hide a scenario from this.
540+
const hooks = [...code.matchAll(/^\s*before(?:All|Each)\s*\(([\s\S]*?)^\}\);$/gm)].map(
541+
(m) => m[1],
542+
);
543+
expect(hooks).toHaveLength(1);
544+
for (const body of hooks) expect(body).not.toMatch(/\b(signedIn|arrange)\(/);
545+
546+
// ⛔ And no case widens its own clock again. Every case here used to end
547+
// `}, 60_000);`, which kept the cold start inside a wider window instead
548+
// of moving it out.
549+
expect(code).not.toMatch(/^\s*\},\s*[\d_]+\s*\);$/m);
550+
});
476551
});

‎packages/client/src/organization-invitation-resend-team-placement.test.ts‎

Lines changed: 79 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -54,9 +54,45 @@
5454
* .test.ts) plus one team, deliberately not extracted: a shared harness that
5555
* one card's edit can reshape under another card's pins is the drift both
5656
* files exist to catch.
57+
*
58+
* ⑥ pins WHERE the first scenario's one-time cost is paid (next section).
59+
*
60+
* ## [#20327] Why one throwaway scenario runs at MODULE SCOPE
61+
*
62+
* The first scenario in a worker pays a one-time cost no later one sees:
63+
* better-auth's lazily imported module graph, sql.js's WASM compile, and the
64+
* first-use costs of the sync and the sign-up. The phases are measured in
65+
* `auth-login-register-envelope.test.ts`, which carries the same arrangement
66+
* and the same fix. Here, idle on 4 vCPU at `c74de10a9`, that cost put ① at
67+
* 1245 ms against 307-411 ms for the four later cases that run `arrange()`.
68+
* Under twenty-four CPU-bound busy loops, the load that reproduces CI's
69+
* failure in the sibling suites, ① timed out at vitest's 5000 ms
70+
* `testTimeout` while every other case passed.
71+
*
72+
* So the cost is now paid by a module-scope `await`, during COLLECTION, which
73+
* no vitest clock covers: `@vitest/runner@4.1.11` wraps hooks and test bodies
74+
* in `withTimeout(...)` and awaits the file import bare. This is the repo's
75+
* convention: "clocked windows measure behaviour, never loading" (AGENTS.md,
76+
* Build & Test; `check:test-source-alias`). The warm-up is the file's own
77+
* `arrange()`, so no list of loads can drift from what the cases really pay.
78+
* It needs the posture the `beforeAll` sets for the cases, so it holds that
79+
* posture for itself and puts back what it found, even when it throws.
80+
*
81+
* ⛔ It shares nothing a case asserts on. Every case still builds a fresh
82+
* engine, a fresh `AuthManager`, a fresh owner and a fresh organization. Its
83+
* engine stays open, as every case's does (`arrange()` hands none back), so
84+
* no case runs in a state no case ran in before: every case but the first
85+
* already ran after an earlier scenario. What it leaves warm is
86+
* process-level: the module registry, sql.js's compiled WASM and the JIT.
87+
*
88+
* ⛔ Do not move it into a hook, and do not answer a recurrence by raising a
89+
* timeout: vitest clocks a hook exactly as it clocks a test body, and a wider
90+
* window only moves the cliff to a heavier shard.
5791
*/
5892

5993
import { describe, it, expect, expectTypeOf, vi, beforeAll, afterAll } from 'vitest';
94+
import { readFileSync } from 'node:fs';
95+
import { fileURLToPath } from 'node:url';
6096
import { ObjectQL } from '@objectstack/objectql';
6197
import { SqliteWasmDriver } from '@objectstack/driver-sqlite-wasm';
6298
import { AuthManager } from '@objectstack/plugin-auth';
@@ -90,14 +126,17 @@ const IDENTITY_OBJECTS = Object.values(
90126
*/
91127
const PRIOR_POSTURE = process.env.OS_TENANCY_POSTURE;
92128

129+
/** Put back the posture this file found: after the warm-up, and after the file. */
130+
const restorePosture = (): void => {
131+
if (PRIOR_POSTURE === undefined) delete process.env.OS_TENANCY_POSTURE;
132+
else process.env.OS_TENANCY_POSTURE = PRIOR_POSTURE;
133+
};
134+
93135
beforeAll(() => {
94136
process.env.OS_TENANCY_POSTURE = 'isolated';
95137
});
96138

97-
afterAll(() => {
98-
if (PRIOR_POSTURE === undefined) delete process.env.OS_TENANCY_POSTURE;
99-
else process.env.OS_TENANCY_POSTURE = PRIOR_POSTURE;
100-
});
139+
afterAll(restorePosture);
101140

102141
let seq = 0;
103142
const nextEmail = (tag: string) => `os17274-${tag}-${++seq}-${Date.now()}@example.com`;
@@ -151,6 +190,13 @@ async function arrange(): Promise<Rig> {
151190
return { client, organizationId, teamId };
152191
}
153192

193+
// [#20327] The first scenario's one-time cost, paid during COLLECTION, which
194+
// no vitest clock covers (header, last section). It holds the posture the
195+
// `beforeAll` above sets for the cases, and `.finally` puts back what it
196+
// found. ⛔ It stays at module scope, and `⑥` below pins that.
197+
process.env.OS_TENANCY_POSTURE = 'isolated';
198+
await arrange().finally(restorePosture);
199+
154200
/** What `invitations.list` answers for one invitation — the round-trip shape. */
155201
type ListedInvitation = Awaited<
156202
ReturnType<ObjectStackClient['organizations']['invitations']['list']>
@@ -454,3 +500,32 @@ export async function resendTeamIdStaysDeclared17274(): Promise<void> {
454500
// hands to `invite`: the round trip is a type-level fact, not a convention.
455501
expectTypeOf<ListedInvitation['teamId']>().toExtend<InviteRequest['teamId']>();
456502
}
503+
504+
// ─────────────────────────────────────────────────────────────────────────
505+
// ⑥ the cold start stays outside every clocked window
506+
// ─────────────────────────────────────────────────────────────────────────
507+
508+
describe('#17274 ⑥ the cold start stays outside every clocked window', () => {
509+
it('pays the first scenario at module scope, and no hook carries it', () => {
510+
// [#20327] Read off this file's own text, so "do not move it into a
511+
// hook" is an assertion rather than a sentence nobody reads.
512+
const code = readFileSync(fileURLToPath(import.meta.url), 'utf8');
513+
514+
// Exactly one warm-up call, and it opens its own line at column 0. So it
515+
// sits in no function body, which is what "paid during collection"
516+
// reduces to. Comment lines start with `//` or ` *` and cannot match.
517+
expect(code.match(/^await arrange\(\)\.finally\(restorePosture\);$/gm) ?? []).toHaveLength(1);
518+
519+
// ⛔ No `before*` hook may come back to carry it: vitest clocks a hook
520+
// with `hookTimeout` exactly as it clocks a test body with `testTimeout`.
521+
// This file's one hook is the posture `beforeAll`, which loads nothing.
522+
// Each hook is read up to the next column-0 `});`, so a hook indented
523+
// inside a `describe` reads on to that block's close and cannot hide a
524+
// scenario from this.
525+
const hooks = [...code.matchAll(/^\s*before(?:All|Each)\s*\(([\s\S]*?)^\}\);$/gm)].map(
526+
(m) => m[1],
527+
);
528+
expect(hooks).toHaveLength(1);
529+
for (const body of hooks) expect(body).not.toMatch(/\barrange\(/);
530+
});
531+
});

0 commit comments

Comments
 (0)