test(auth): stub the clock in both floor regression tests
Copilot round 3 on #2256: both detectors were one-sided. Each measured the sub-floor remainder against the real clock, so a preempted worker overshoots the deadline, the remainder goes negative, and the unfixed `remainingMs <= 0` bound takes the same branch and prints the same message — the test passes on an implementation with no floor. It could never fail wrongly, but it could silently stop testing anything, which on a flake fix is the wrong half of that guarantee to keep. Both now stub `performance.now()`: the web case advances it inside the fetch so the second grant's remainder is exactly MIN_REVOCATION_REQUEST_BUDGET_MS - 1, and the CLI case freezes it so the remainder at the check is the budget itself. Positive and under the floor on every machine, which is the only state that separates the two bounds. Verified as detectors with the clock stubbed: reverting the core floor gives 1 failed / 57 passed, reverting the CLI floor 1 failed / 14 passed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017wEXtbs8UEHUMxDEAxbs99 Signed-off-by: cliffhall <cliff@futurescale.com>
This commit is contained in:
parent
c736f1d185
commit
927a70872d
2 changed files with 40 additions and 23 deletions
|
|
@ -390,6 +390,12 @@ describe("clearStoredAuthForRelogin", () => {
|
|||
const fetchSpy = vi
|
||||
.spyOn(globalThis, "fetch")
|
||||
.mockResolvedValue(new Response(null, { status: 200 }));
|
||||
// Frozen, so the remainder at the check is exactly the budget. Left to
|
||||
// the real clock this is a one-sided detector: a worker preempted for
|
||||
// more than the budget reaches the check with a NEGATIVE remainder, where
|
||||
// the unfixed `remainingMs <= 0` bound takes the same branch and prints
|
||||
// the same message — passing without the floor (Copilot).
|
||||
const nowSpy = vi.spyOn(performance, "now").mockReturnValue(1_000);
|
||||
try {
|
||||
const outcome = await clearStoredAuthForRelogin("https://example.com", {
|
||||
budgetMs: MIN_REVOCATION_REQUEST_BUDGET_MS - 1,
|
||||
|
|
@ -405,6 +411,7 @@ describe("clearStoredAuthForRelogin", () => {
|
|||
'budget was exhausted before "',
|
||||
);
|
||||
} finally {
|
||||
nowSpy.mockRestore();
|
||||
fetchSpy.mockRestore();
|
||||
}
|
||||
});
|
||||
|
|
|
|||
|
|
@ -872,34 +872,44 @@ describe("revokeStoredOAuthTokens (plan + execute)", () => {
|
|||
tokenTypeHint: "refresh_token" as const,
|
||||
});
|
||||
const timeoutMs = 40;
|
||||
// Lands the second grant inside the floor but *above* zero — the sliver the
|
||||
// old bound would have spent. A slow machine only pushes the remainder
|
||||
// further below the floor, so the assertion cannot flip the other way.
|
||||
const firstRequestMs = timeoutMs - MIN_REVOCATION_REQUEST_BUDGET_MS + 1;
|
||||
// The clock is stubbed rather than slept against. A real sleep makes this a
|
||||
// ONE-SIDED detector: under contention the first request overruns, the
|
||||
// remainder goes negative, and the unfixed `remainingMs <= 0` bound skips
|
||||
// the second grant for the wrong reason — so the test passes on an
|
||||
// implementation that has no floor at all. Advancing a stub inside the
|
||||
// fetch puts the second grant's remainder at exactly
|
||||
// `MIN_REVOCATION_REQUEST_BUDGET_MS - 1` on every machine: positive, and
|
||||
// under the floor, which is the only state that tells the two apart.
|
||||
let now = 1_000;
|
||||
const nowSpy = vi.spyOn(performance, "now").mockImplementation(() => now);
|
||||
const fetchFn = vi.fn<typeof fetch>(async () => {
|
||||
await new Promise((resolve) => setTimeout(resolve, firstRequestMs));
|
||||
now += timeoutMs - MIN_REVOCATION_REQUEST_BUDGET_MS + 1;
|
||||
return new Response(null, { status: 200 });
|
||||
});
|
||||
|
||||
const outcome = await executeOAuthRevocation(
|
||||
{
|
||||
serverUrl: SERVER_URL,
|
||||
grants: [grant("a"), grant("b")],
|
||||
failures: [],
|
||||
endpoint: REVOKE_URL,
|
||||
supportedAuthMethods: [],
|
||||
metadataIssuer: "https://as.example.com",
|
||||
},
|
||||
{ fetchFn, timeoutMs },
|
||||
);
|
||||
try {
|
||||
const outcome = await executeOAuthRevocation(
|
||||
{
|
||||
serverUrl: SERVER_URL,
|
||||
grants: [grant("a"), grant("b")],
|
||||
failures: [],
|
||||
endpoint: REVOKE_URL,
|
||||
supportedAuthMethods: [],
|
||||
metadataIssuer: "https://as.example.com",
|
||||
},
|
||||
{ fetchFn, timeoutMs },
|
||||
);
|
||||
|
||||
expect(fetchFn).toHaveBeenCalledTimes(1);
|
||||
// The unattempted grant outranks the first one's success, so the caller
|
||||
// hears that a grant may still be live rather than that all was well.
|
||||
expect(outcome).toMatchObject({ status: "failed" });
|
||||
expect(outcome.status === "failed" ? outcome.detail : "").toContain(
|
||||
"budget was exhausted",
|
||||
);
|
||||
expect(fetchFn).toHaveBeenCalledTimes(1);
|
||||
// The unattempted grant outranks the first one's success, so the caller
|
||||
// hears that a grant may still be live rather than that all was well.
|
||||
expect(outcome).toMatchObject({ status: "failed" });
|
||||
expect(outcome.status === "failed" ? outcome.detail : "").toContain(
|
||||
"budget was exhausted",
|
||||
);
|
||||
} finally {
|
||||
nowSpy.mockRestore();
|
||||
}
|
||||
});
|
||||
|
||||
// A grant bound to an issuer the cached metadata does not describe cannot be
|
||||
|
|
|
|||
Loading…
Reference in a new issue