From 16664b1b65566046f253c7ff6cf271cb1a2c1288 Mon Sep 17 00:00:00 2001 From: Sean McGuire Date: Tue, 22 Sep 2026 16:23:30 -0700 Subject: [PATCH 1/4] add locatoroperation class --- .../understudy/locatorOperation.test.ts | 229 ++++++++++++++++++ .../extension/understudy/locatorOperation.ts | 100 ++++++++ 2 files changed, 329 insertions(+) create mode 100644 packages/extension/understudy/locatorOperation.test.ts create mode 100644 packages/extension/understudy/locatorOperation.ts diff --git a/packages/extension/understudy/locatorOperation.test.ts b/packages/extension/understudy/locatorOperation.test.ts new file mode 100644 index 000000000..96e01d0f2 --- /dev/null +++ b/packages/extension/understudy/locatorOperation.test.ts @@ -0,0 +1,229 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import { TimeoutError } from "../errors.js"; +import { LocatorOperation, runLocatorOperation } from "./locatorOperation.js"; + +function deferred() { + let resolve!: (value: T) => void; + const promise = new Promise((resolvePromise) => { + resolve = resolvePromise; + }); + return { promise, resolve }; +} + +describe("locator operation deadlines", () => { + beforeEach(() => { + vi.useFakeTimers({ toFake: ["setTimeout", "clearTimeout", "performance", "Date"] }); + }); + + afterEach(() => { + vi.useRealTimers(); + vi.restoreAllMocks(); + }); + + it("uses elapsed monotonic time, independent of changes to the wall clock", async () => { + await runLocatorOperation({ name: "locator.click", timeout: 100 }, async (operation) => { + expect(operation.remainingMs()).toBe(100); + vi.setSystemTime(Date.now() + 60_000); + expect(operation.remainingMs()).toBe(100); + await vi.advanceTimersByTimeAsync(40); + expect(operation.remainingMs()).toBe(60); + expect(operation.signal.aborted).toBe(false); + }); + expect(vi.getTimerCount()).toBe(0); + }); + + it("rejects at the deadline and keeps the same timeout reason", async () => { + const gate = deferred(); + let operation!: LocatorOperation; + const result = runLocatorOperation({ name: "locator.click", timeout: 100 }, async (context) => { + operation = context; + await gate.promise; + }); + const rejected = expect(result).rejects.toThrow("locator.click timed out after 100ms"); + + await vi.advanceTimersByTimeAsync(100); + await rejected; + expect(operation.remainingMs()).toBe(0); + expect(operation.signal.reason).toBeInstanceOf(TimeoutError); + expect(() => operation.throwIfStopped()).toThrow(operation.signal.reason); + expect(vi.getTimerCount()).toBe(0); + gate.resolve(); + }); + + it("checks expiry even before the deadline timer gets a turn", async () => { + await expect( + runLocatorOperation({ name: "locator.click", timeout: 100 }, async (operation) => { + vi.spyOn(performance, "now").mockReturnValue(101); + expect(() => operation.throwIfStopped()).toThrow(TimeoutError); + expect(operation.signal.aborted).toBe(true); + }), + ).rejects.toThrow(TimeoutError); + expect(vi.getTimerCount()).toBe(0); + }); + + it("does not return success when work finishes after its deadline", async () => { + await expect( + runLocatorOperation({ name: "locator.click", timeout: 100 }, async () => { + vi.spyOn(performance, "now").mockReturnValue(101); + return "too late"; + }), + ).rejects.toThrow(TimeoutError); + }); + + it("runs without a deadline or timer when timeout is zero", async () => { + const gate = deferred(); + let operation!: LocatorOperation; + const result = runLocatorOperation({ name: "locator.type", timeout: 0 }, async (context) => { + operation = context; + return gate.promise; + }); + await Promise.resolve(); + expect(vi.getTimerCount()).toBe(0); + await vi.advanceTimersByTimeAsync(1_000_000); + expect(operation.remainingMs()).toBe(Infinity); + expect(operation.signal.aborted).toBe(false); + expect(() => operation.throwIfStopped()).not.toThrow(); + gate.resolve("done"); + await expect(result).resolves.toBe("done"); + }); + + it("reuses the parent context and leaves its timer running after nested work", async () => { + const gate = deferred(); + const result = runLocatorOperation({ name: "locator.fill", timeout: 100 }, async (parent) => { + await vi.advanceTimersByTimeAsync(60); + await runLocatorOperation(parent, async (child) => { + expect(child).toBe(parent); + expect(child.remainingMs()).toBe(40); + expect(vi.getTimerCount()).toBe(1); + }); + expect(vi.getTimerCount()).toBe(1); + await gate.promise; + }); + const rejected = expect(result).rejects.toThrow("locator.fill timed out after 100ms"); + + // Let the nested call complete before advancing the parent's remaining budget. + await vi.advanceTimersByTimeAsync(0); + await vi.advanceTimersByTimeAsync(40); + await rejected; + expect(vi.getTimerCount()).toBe(0); + gate.resolve(); + }); + + it("does not let a failed nested call dispose the parent's deadline", async () => { + const gate = deferred(); + const error = new Error("nested lookup failed"); + const result = runLocatorOperation({ name: "locator.fill", timeout: 100 }, async (parent) => { + await expect( + runLocatorOperation(parent, async () => { + throw error; + }), + ).rejects.toBe(error); + expect(vi.getTimerCount()).toBe(1); + await gate.promise; + }); + const rejected = expect(result).rejects.toThrow(TimeoutError); + + await vi.advanceTimersByTimeAsync(100); + await rejected; + gate.resolve(); + }); + + it("does not invoke nested work after the parent expires", async () => { + const gate = deferred(); + let operation!: LocatorOperation; + const result = runLocatorOperation({ name: "locator.fill", timeout: 100 }, async (context) => { + operation = context; + return gate.promise; + }); + const rejected = expect(result).rejects.toThrow(TimeoutError); + await vi.advanceTimersByTimeAsync(100); + await rejected; + + const work = vi.fn(async () => {}); + await expect(runLocatorOperation(operation, work)).rejects.toBe(operation.signal.reason); + expect(work).not.toHaveBeenCalled(); + gate.resolve(); + }); + + it("keeps concurrent operations independent", async () => { + const firstGate = deferred(); + const secondGate = deferred(); + let second!: LocatorOperation; + const first = runLocatorOperation( + { name: "locator.click", timeout: 10 }, + () => firstGate.promise, + ); + const other = runLocatorOperation({ name: "locator.click", timeout: 100 }, async (context) => { + second = context; + return secondGate.promise; + }); + const rejected = expect(first).rejects.toThrow(TimeoutError); + + await vi.advanceTimersByTimeAsync(10); + await rejected; + expect(second.signal.aborted).toBe(false); + expect(second.remainingMs()).toBe(90); + secondGate.resolve("done"); + await expect(other).resolves.toBe("done"); + expect(vi.getTimerCount()).toBe(0); + firstGate.resolve(); + }); + + it.each(["success", "failure", "synchronous failure"] as const)( + "disposes the timer and abort listener on %s", + async (outcome) => { + const error = new Error("action failed"); + let signal!: AbortSignal; + let removeListener!: ReturnType; + const result = runLocatorOperation({ name: "locator.click", timeout: 100 }, (operation) => { + signal = operation.signal; + removeListener = vi.spyOn(signal, "removeEventListener"); + if (outcome === "synchronous failure") throw error; + return outcome === "success" ? Promise.resolve("done") : Promise.reject(error); + }); + + if (outcome === "success") await expect(result).resolves.toBe("done"); + else await expect(result).rejects.toBe(error); + + expect(removeListener).toHaveBeenCalledWith("abort", expect.any(Function)); + expect(vi.getTimerCount()).toBe(0); + await vi.advanceTimersByTimeAsync(100); + expect(signal.aborted).toBe(false); + }, + ); + + it("does not truncate timeouts larger than one timer interval", async () => { + const maxTimerMs = 2_147_483_647; + const gate = deferred(); + let operation!: LocatorOperation; + const result = runLocatorOperation( + { name: "locator.type", timeout: maxTimerMs + 100 }, + async (context) => { + operation = context; + return gate.promise; + }, + ); + const rejected = expect(result).rejects.toThrow(TimeoutError); + + await vi.advanceTimersByTimeAsync(maxTimerMs); + expect(operation.signal.aborted).toBe(false); + expect(operation.remainingMs()).toBe(100); + expect(vi.getTimerCount()).toBe(1); + await vi.advanceTimersByTimeAsync(100); + await rejected; + expect(vi.getTimerCount()).toBe(0); + gate.resolve(); + }); + + it.each([-1, NaN, Infinity, -Infinity])( + "rejects invalid timeout %s before starting work", + async (timeout) => { + const work = vi.fn(async () => {}); + await expect(runLocatorOperation({ name: "locator.click", timeout }, work)).rejects.toThrow( + RangeError, + ); + expect(work).not.toHaveBeenCalled(); + expect(vi.getTimerCount()).toBe(0); + }, + ); +}); diff --git a/packages/extension/understudy/locatorOperation.ts b/packages/extension/understudy/locatorOperation.ts new file mode 100644 index 000000000..7d2f59f0f --- /dev/null +++ b/packages/extension/understudy/locatorOperation.ts @@ -0,0 +1,100 @@ +import { TimeoutError } from "../errors.js"; + +type LocatorOperationOptions = { + name: string; + /** Milliseconds for the whole operation. Zero disables the deadline. */ + timeout: number; +}; + +const MAX_TIMER_MS = 2_147_483_647; + +/** One invocation's lifetime, shared by its frame resolution and action steps. */ +export class LocatorOperation { + private readonly controller = new AbortController(); + private readonly deadline: number | null; + private timer: ReturnType | undefined; + + constructor( + readonly name: string, + private readonly timeout: number, + ) { + if (!Number.isFinite(timeout) || timeout < 0) { + throw new RangeError("Locator timeout must be a finite, non-negative number"); + } + this.deadline = timeout === 0 ? null : performance.now() + timeout; + if (this.deadline !== null) this.scheduleDeadline(); + } + + get signal(): AbortSignal { + return this.controller.signal; + } + + remainingMs(): number { + return this.deadline === null ? Infinity : Math.max(0, this.deadline - performance.now()); + } + + throwIfStopped(): void { + // Check the clock too: the deadline timer may not have run yet. + this.expireIfNeeded(); + this.signal.throwIfAborted(); + } + + /** Only the runner that created this context owns its timer. */ + dispose(): void { + clearTimeout(this.timer); + this.timer = undefined; + } + + private expireIfNeeded(): void { + if (!this.signal.aborted && this.remainingMs() === 0) { + this.dispose(); + this.controller.abort(new TimeoutError(this.name, this.timeout)); + } + } + + private scheduleDeadline(): void { + // Long timeouts need several timer intervals; never truncate the deadline. + this.timer = setTimeout( + () => { + this.expireIfNeeded(); + if (!this.signal.aborted) this.scheduleDeadline(); + }, + Math.min(MAX_TIMER_MS, Math.ceil(this.remainingMs())), + ); + } +} + +/** + * Own a new operation, or reuse a parent's context without resetting its clock. + * Expiry stops waiting; work must check the context before continuing its steps. + */ +export async function runLocatorOperation( + options: LocatorOperationOptions | LocatorOperation, + work: (operation: LocatorOperation) => Promise, +): Promise { + const ownsOperation = !(options instanceof LocatorOperation); + const operation = + options instanceof LocatorOperation + ? options + : new LocatorOperation(options.name, options.timeout); + + let onAbort: (() => void) | undefined; + try { + operation.throwIfStopped(); + const stopped = new Promise((_, reject) => { + onAbort = () => reject(operation.signal.reason); + operation.signal.addEventListener("abort", onAbort, { once: true }); + }); + // Attach both handlers before invoking work, including work that throws synchronously. + const pending = Promise.resolve().then(() => { + operation.throwIfStopped(); + return work(operation); + }); + const result = await Promise.race([pending, stopped]); + operation.throwIfStopped(); + return result; + } finally { + if (onAbort) operation.signal.removeEventListener("abort", onAbort); + if (ownsOperation) operation.dispose(); + } +} From de616c0f14bc552fd19d13f77aba1cfd2a1adc74 Mon Sep 17 00:00:00 2001 From: Sean McGuire Date: Tue, 22 Sep 2026 16:33:28 -0700 Subject: [PATCH 2/4] add run, delay, cleanup to locatorOperation class --- .../extension/understudy/locatorOperation.ts | 116 ++++++- .../understudy/locatorOperationSteps.test.ts | 304 ++++++++++++++++++ 2 files changed, 404 insertions(+), 16 deletions(-) create mode 100644 packages/extension/understudy/locatorOperationSteps.test.ts diff --git a/packages/extension/understudy/locatorOperation.ts b/packages/extension/understudy/locatorOperation.ts index 7d2f59f0f..4986ae140 100644 --- a/packages/extension/understudy/locatorOperation.ts +++ b/packages/extension/understudy/locatorOperation.ts @@ -7,12 +7,14 @@ type LocatorOperationOptions = { }; const MAX_TIMER_MS = 2_147_483_647; +const CLEANUP_TIMEOUT_MS = 1_000; /** One invocation's lifetime, shared by its frame resolution and action steps. */ export class LocatorOperation { private readonly controller = new AbortController(); private readonly deadline: number | null; private timer: ReturnType | undefined; + private readonly phases = new Map(); constructor( readonly name: string, @@ -39,6 +41,99 @@ export class LocatorOperation { this.signal.throwIfAborted(); } + /** + * Start work only while active, and stop waiting at the shared deadline. + * onLateResult releases a resource that could not be delivered to the caller. + * Resources delivered successfully remain the caller's responsibility. + */ + async run( + phase: string, + work: () => Promise, + onLateResult?: (value: T) => void | Promise, + ): Promise { + this.throwIfStopped(); + const step = Symbol(); + this.phases.set(step, phase); + let onAbort: (() => void) | undefined; + let abandoned = false; + let result: { value: T } | undefined; + const discard = (value: T) => { + if (onLateResult) void this.cleanup(() => onLateResult(value)); + }; + + try { + const stopped = new Promise((_, reject) => { + onAbort = () => reject(this.signal.reason); + this.signal.addEventListener("abort", onAbort, { once: true }); + }); + // Observe rejection before invoking work, including synchronous throws. + const pending = Promise.resolve() + .then(() => { + this.throwIfStopped(); + return work(); + }) + .then((value) => { + if (abandoned) discard(value); + else result = { value }; + return value; + }); + const value = await Promise.race([pending, stopped]); + this.throwIfStopped(); + return value; + } catch (error) { + abandoned = true; + // The result may have arrived just before expiry, but not been delivered. + if (result) discard(result.value); + throw error; + } finally { + if (onAbort) this.signal.removeEventListener("abort", onAbort); + this.phases.delete(step); + } + } + + /** Sleep within the operation's budget and remove the sleep timer on expiry. */ + async delay(ms: number): Promise { + if (!Number.isFinite(ms) || ms < 0) { + throw new RangeError("Locator delay must be a finite, non-negative number"); + } + let timer: ReturnType | undefined; + try { + await this.run("waiting between steps", () => { + const deadline = performance.now() + ms; + return new Promise((resolve) => { + const tick = () => { + const remaining = deadline - performance.now(); + if (remaining <= 0) resolve(); + else timer = setTimeout(tick, Math.min(MAX_TIMER_MS, Math.ceil(remaining))); + }; + tick(); + }); + }); + } finally { + clearTimeout(timer); + } + } + + /** + * Attempt cleanup even after expiry, waiting at most one second. + * Failures are best-effort; issued cleanup commands may still finish later. + */ + async cleanup(work: () => void | Promise): Promise { + let timer: ReturnType | undefined; + try { + await Promise.race([ + Promise.resolve().then(work), + new Promise((resolve) => { + timer = setTimeout(resolve, CLEANUP_TIMEOUT_MS); + }), + ]); + } catch { + // Cleanup must not replace the action's result or timeout error. + } finally { + clearTimeout(timer); + } + } + /** Only the runner that created this context owns its timer. */ dispose(): void { clearTimeout(this.timer); @@ -48,7 +143,10 @@ export class LocatorOperation { private expireIfNeeded(): void { if (!this.signal.aborted && this.remainingMs() === 0) { this.dispose(); - this.controller.abort(new TimeoutError(this.name, this.timeout)); + const error = new TimeoutError(this.name, this.timeout); + const phase = [...this.phases.values()].at(-1); + if (phase && phase !== this.name) error.message += ` while ${phase}`; + this.controller.abort(error); } } @@ -78,23 +176,9 @@ export async function runLocatorOperation( ? options : new LocatorOperation(options.name, options.timeout); - let onAbort: (() => void) | undefined; try { - operation.throwIfStopped(); - const stopped = new Promise((_, reject) => { - onAbort = () => reject(operation.signal.reason); - operation.signal.addEventListener("abort", onAbort, { once: true }); - }); - // Attach both handlers before invoking work, including work that throws synchronously. - const pending = Promise.resolve().then(() => { - operation.throwIfStopped(); - return work(operation); - }); - const result = await Promise.race([pending, stopped]); - operation.throwIfStopped(); - return result; + return await operation.run(operation.name, () => work(operation)); } finally { - if (onAbort) operation.signal.removeEventListener("abort", onAbort); if (ownsOperation) operation.dispose(); } } diff --git a/packages/extension/understudy/locatorOperationSteps.test.ts b/packages/extension/understudy/locatorOperationSteps.test.ts new file mode 100644 index 000000000..68859031b --- /dev/null +++ b/packages/extension/understudy/locatorOperationSteps.test.ts @@ -0,0 +1,304 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import { TimeoutError } from "../errors.js"; +import { LocatorOperation, runLocatorOperation } from "./locatorOperation.js"; + +function deferred() { + let resolve!: (value: T) => void; + let reject!: (error: Error) => void; + const promise = new Promise((resolvePromise, rejectPromise) => { + resolve = resolvePromise; + reject = rejectPromise; + }); + return { promise, resolve, reject }; +} + +describe("locator operation steps", () => { + let operation: LocatorOperation; + + beforeEach(() => { + vi.useFakeTimers({ toFake: ["setTimeout", "clearTimeout", "performance"] }); + operation = new LocatorOperation("locator.click", 100); + }); + + afterEach(() => { + operation.dispose(); + vi.useRealTimers(); + vi.restoreAllMocks(); + }); + + it("does not dispatch work after expiry, even before the timer runs", async () => { + const command = vi.fn(async () => {}); + const pending = operation.run("clicking", command); + const rejected = expect(pending).rejects.toThrow(TimeoutError); + + // Expire between accepting the callback and actually invoking it. + vi.spyOn(performance, "now").mockReturnValue(101); + await rejected; + expect(command).not.toHaveBeenCalled(); + }); + + it("uses the remaining budget and does not resume after a late response", async () => { + const command = deferred(); + const nextStep = vi.fn(async () => {}); + await vi.advanceTimersByTimeAsync(60); + const pending = runLocatorOperation(operation, async (context) => { + await context.run("reading geometry", () => command.promise); + await context.run("clicking", nextStep); + }); + const rejected = expect(pending).rejects.toThrow( + "locator.click timed out after 100ms while reading geometry", + ); + + await vi.advanceTimersByTimeAsync(40); + await rejected; + command.resolve(); + await vi.advanceTimersByTimeAsync(0); + expect(nextStep).not.toHaveBeenCalled(); + expect(vi.getTimerCount()).toBe(0); + }); + + it("delivers successful results without releasing the caller's resource", async () => { + const handle = { objectId: "node" }; + const release = vi.fn(async () => {}); + await expect(operation.run("finding node", async () => handle, release)).resolves.toBe(handle); + expect(release).not.toHaveBeenCalled(); + }); + + it("releases a late resource exactly once after timeout", async () => { + const command = deferred<{ objectId: string }>(); + const handle = { objectId: "late-node" }; + const release = vi.fn(async () => {}); + const pending = operation.run("finding node", () => command.promise, release); + const rejected = expect(pending).rejects.toThrow(TimeoutError); + + await vi.advanceTimersByTimeAsync(100); + await rejected; + expect(release).not.toHaveBeenCalled(); + command.resolve(handle); + await vi.advanceTimersByTimeAsync(0); + expect(release).toHaveBeenCalledExactlyOnceWith(handle); + expect(vi.getTimerCount()).toBe(0); + }); + + it("releases an undelivered result when work crosses the deadline before the timer runs", async () => { + const handle = { objectId: "node" }; + const release = vi.fn(async () => {}); + const pending = operation.run( + "finding node", + async () => { + vi.spyOn(performance, "now").mockReturnValue(101); + return handle; + }, + release, + ); + + await expect(pending).rejects.toThrow(TimeoutError); + expect(release).toHaveBeenCalledExactlyOnceWith(handle); + expect(vi.getTimerCount()).toBe(0); + }); + + it("handles a command rejection arriving after timeout", async () => { + const command = deferred(); + const release = vi.fn(async () => {}); + const pending = operation.run("finding node", () => command.promise, release); + const rejected = expect(pending).rejects.toThrow(TimeoutError); + + await vi.advanceTimersByTimeAsync(100); + await rejected; + command.reject(new Error("late browser error")); + await vi.advanceTimersByTimeAsync(0); + expect(release).not.toHaveBeenCalled(); + expect(operation.signal.reason).toBeInstanceOf(TimeoutError); + }); + + it.each(["success", "failure", "synchronous failure", "timeout"] as const)( + "removes the step's abort listener on %s", + async (outcome) => { + const add = vi.spyOn(operation.signal, "addEventListener"); + const remove = vi.spyOn(operation.signal, "removeEventListener"); + const command = deferred(); + const error = new Error("browser error"); + const pending = operation.run("reading geometry", () => { + if (outcome === "synchronous failure") throw error; + return command.promise; + }); + + if (outcome === "success") { + command.resolve("done"); + await expect(pending).resolves.toBe("done"); + } else if (outcome === "timeout") { + const rejected = expect(pending).rejects.toThrow(TimeoutError); + await vi.advanceTimersByTimeAsync(100); + await rejected; + command.resolve("too late"); + } else { + const rejected = expect(pending).rejects.toBe(error); + if (outcome === "failure") { + await Promise.resolve(); + command.reject(error); + } + await rejected; + } + + expect(add).toHaveBeenCalledOnce(); + expect(remove).toHaveBeenCalledExactlyOnceWith("abort", add.mock.calls[0]![1]); + }, + ); + + it("reports a pending phase rather than a concurrent phase that already finished", async () => { + const command = deferred(); + const pending = operation.run("waiting for frame", () => command.promise); + const rejected = expect(pending).rejects.toThrow( + "locator.click timed out after 100ms while waiting for frame", + ); + await operation.run("inspecting another frame", async () => {}); + await vi.advanceTimersByTimeAsync(100); + await rejected; + command.resolve(); + }); + + it("interrupts a delay and clears its timer", async () => { + const pending = operation.delay(1_000); + const rejected = expect(pending).rejects.toThrow(TimeoutError); + await vi.advanceTimersByTimeAsync(0); + expect(vi.getTimerCount()).toBe(2); + + await vi.advanceTimersByTimeAsync(100); + await rejected; + expect(vi.getTimerCount()).toBe(0); + }); + + it("clears a completed delay's timer without stopping the operation", async () => { + const pending = operation.delay(25); + await vi.advanceTimersByTimeAsync(25); + await pending; + expect(operation.remainingMs()).toBe(75); + expect(operation.signal.aborted).toBe(false); + expect(vi.getTimerCount()).toBe(1); + }); + + it("supports zero delay without allocating a sleep timer", async () => { + await operation.delay(0); + expect(operation.remainingMs()).toBe(100); + expect(vi.getTimerCount()).toBe(1); + }); + + it("supports long delays when the operation has no deadline", async () => { + operation.dispose(); + operation = new LocatorOperation("locator.type", 0); + const maxTimerMs = 2_147_483_647; + let finished = false; + const pending = operation.delay(maxTimerMs + 100).then(() => { + finished = true; + }); + + await vi.advanceTimersByTimeAsync(maxTimerMs); + expect(finished).toBe(false); + await vi.advanceTimersByTimeAsync(100); + await pending; + expect(finished).toBe(true); + expect(operation.signal.aborted).toBe(false); + expect(vi.getTimerCount()).toBe(0); + }); + + it.each([-1, NaN, Infinity])("rejects invalid delay %s", async (ms) => { + await expect(operation.delay(ms)).rejects.toThrow(RangeError); + expect(vi.getTimerCount()).toBe(1); + }); + + it("attempts cleanup after expiry and bounds its wait even if the command stalls", async () => { + await vi.advanceTimersByTimeAsync(100); + const command = deferred(); + const cleanup = vi.fn(() => command.promise); + let finished = false; + const pending = operation.cleanup(cleanup).then(() => { + finished = true; + }); + + await vi.advanceTimersByTimeAsync(999); + expect(cleanup).toHaveBeenCalledOnce(); + expect(finished).toBe(false); + await vi.advanceTimersByTimeAsync(1); + await pending; + expect(finished).toBe(true); + expect(operation.signal.reason).toBeInstanceOf(TimeoutError); + expect(vi.getTimerCount()).toBe(0); + + // A rejection after the cleanup wait ends must still be observed. + command.reject(new Error("late cleanup failure")); + await vi.advanceTimersByTimeAsync(0); + }); + + it.each(["success", "failure", "synchronous failure"] as const)( + "clears the cleanup timer on %s", + async (outcome) => { + const error = new Error("cleanup failure"); + await operation.cleanup(() => { + if (outcome === "synchronous failure") throw error; + return outcome === "success" ? Promise.resolve() : Promise.reject(error); + }); + expect(vi.getTimerCount()).toBe(1); + }, + ); + + it("preserves the primary error when cleanup fails", async () => { + const error = new Error("element is detached"); + await expect( + runLocatorOperation(operation, async (context) => { + try { + throw error; + } finally { + await context.cleanup(async () => { + throw new Error("release failed"); + }); + } + }), + ).rejects.toBe(error); + }); + + it("does not hold the caller past timeout while finally cleanup is stalled", async () => { + const command = deferred(); + const release = deferred(); + const cleanup = vi.fn(() => release.promise); + const pending = runLocatorOperation(operation, async (context) => { + try { + await context.run("finding node", () => command.promise); + } finally { + await context.cleanup(cleanup); + } + }); + const rejected = expect(pending).rejects.toThrow(TimeoutError); + + await vi.advanceTimersByTimeAsync(100); + await rejected; + expect(cleanup).toHaveBeenCalledOnce(); + await vi.advanceTimersByTimeAsync(1_000); + expect(vi.getTimerCount()).toBe(0); + command.resolve(); + release.resolve(); + await vi.advanceTimersByTimeAsync(0); + }); + + it.each(["synchronous failure", "failure", "stall"] as const)( + "handles late resource cleanup ending in %s", + async (outcome) => { + const command = deferred(); + const release = deferred(); + const cleanup = vi.fn(() => { + if (outcome === "synchronous failure") throw new Error("release failed"); + if (outcome === "failure") return Promise.reject(new Error("release failed")); + return release.promise; + }); + const pending = operation.run("finding node", () => command.promise, cleanup); + const rejected = expect(pending).rejects.toThrow(TimeoutError); + + await vi.advanceTimersByTimeAsync(100); + await rejected; + command.resolve("late-node"); + await vi.advanceTimersByTimeAsync(1_000); + expect(cleanup).toHaveBeenCalledExactlyOnceWith("late-node"); + expect(vi.getTimerCount()).toBe(0); + release.resolve(); + }, + ); +}); From 828b75bbbeeeb8e9c2107282d7725ef26712bc0c Mon Sep 17 00:00:00 2001 From: Sean McGuire Date: Tue, 22 Sep 2026 16:39:31 -0700 Subject: [PATCH 3/4] consolidate tests --- .../understudy/locatorOperation.test.ts | 402 ++++++++++-------- .../understudy/locatorOperationSteps.test.ts | 304 ------------- 2 files changed, 227 insertions(+), 479 deletions(-) delete mode 100644 packages/extension/understudy/locatorOperationSteps.test.ts diff --git a/packages/extension/understudy/locatorOperation.test.ts b/packages/extension/understudy/locatorOperation.test.ts index 96e01d0f2..e850512f1 100644 --- a/packages/extension/understudy/locatorOperation.test.ts +++ b/packages/extension/understudy/locatorOperation.test.ts @@ -4,226 +4,278 @@ import { LocatorOperation, runLocatorOperation } from "./locatorOperation.js"; function deferred() { let resolve!: (value: T) => void; - const promise = new Promise((resolvePromise) => { + let reject!: (error: Error) => void; + const promise = new Promise((resolvePromise, rejectPromise) => { resolve = resolvePromise; + reject = rejectPromise; }); - return { promise, resolve }; + return { promise, resolve, reject }; } -describe("locator operation deadlines", () => { +describe("locator operations", () => { + const operations: LocatorOperation[] = []; + function createOperation(timeout = 100) { + const operation = new LocatorOperation("locator.click", timeout); + operations.push(operation); + return operation; + } + beforeEach(() => { vi.useFakeTimers({ toFake: ["setTimeout", "clearTimeout", "performance", "Date"] }); }); afterEach(() => { + operations.splice(0).forEach((operation) => operation.dispose()); + expect(vi.getTimerCount()).toBe(0); vi.useRealTimers(); vi.restoreAllMocks(); }); - it("uses elapsed monotonic time, independent of changes to the wall clock", async () => { - await runLocatorOperation({ name: "locator.click", timeout: 100 }, async (operation) => { - expect(operation.remainingMs()).toBe(100); + it.each([0, 100])( + "tracks a %s ms budget independently of wall-clock changes", + async (timeout) => { + const operation = createOperation(timeout); vi.setSystemTime(Date.now() + 60_000); - expect(operation.remainingMs()).toBe(100); + expect(operation.remainingMs()).toBe(timeout || Infinity); await vi.advanceTimersByTimeAsync(40); - expect(operation.remainingMs()).toBe(60); + expect(operation.remainingMs()).toBe(timeout ? 60 : Infinity); expect(operation.signal.aborted).toBe(false); - }); - expect(vi.getTimerCount()).toBe(0); - }); - - it("rejects at the deadline and keeps the same timeout reason", async () => { - const gate = deferred(); - let operation!: LocatorOperation; - const result = runLocatorOperation({ name: "locator.click", timeout: 100 }, async (context) => { - operation = context; - await gate.promise; - }); - const rejected = expect(result).rejects.toThrow("locator.click timed out after 100ms"); - - await vi.advanceTimersByTimeAsync(100); - await rejected; - expect(operation.remainingMs()).toBe(0); - expect(operation.signal.reason).toBeInstanceOf(TimeoutError); - expect(() => operation.throwIfStopped()).toThrow(operation.signal.reason); - expect(vi.getTimerCount()).toBe(0); - gate.resolve(); - }); - - it("checks expiry even before the deadline timer gets a turn", async () => { - await expect( - runLocatorOperation({ name: "locator.click", timeout: 100 }, async (operation) => { - vi.spyOn(performance, "now").mockReturnValue(101); - expect(() => operation.throwIfStopped()).toThrow(TimeoutError); - expect(operation.signal.aborted).toBe(true); - }), - ).rejects.toThrow(TimeoutError); - expect(vi.getTimerCount()).toBe(0); - }); - - it("does not return success when work finishes after its deadline", async () => { - await expect( - runLocatorOperation({ name: "locator.click", timeout: 100 }, async () => { - vi.spyOn(performance, "now").mockReturnValue(101); - return "too late"; - }), - ).rejects.toThrow(TimeoutError); - }); - - it("runs without a deadline or timer when timeout is zero", async () => { - const gate = deferred(); - let operation!: LocatorOperation; - const result = runLocatorOperation({ name: "locator.type", timeout: 0 }, async (context) => { - operation = context; - return gate.promise; - }); - await Promise.resolve(); - expect(vi.getTimerCount()).toBe(0); - await vi.advanceTimersByTimeAsync(1_000_000); - expect(operation.remainingMs()).toBe(Infinity); - expect(operation.signal.aborted).toBe(false); - expect(() => operation.throwIfStopped()).not.toThrow(); - gate.resolve("done"); - await expect(result).resolves.toBe("done"); - }); + expect(vi.getTimerCount()).toBe(timeout ? 1 : 0); + }, + ); - it("reuses the parent context and leaves its timer running after nested work", async () => { - const gate = deferred(); - const result = runLocatorOperation({ name: "locator.fill", timeout: 100 }, async (parent) => { + it.each(["success", "failure"] as const)( + "reuses the parent's remaining budget after nested %s", + async (outcome) => { + const operation = createOperation(); + const error = new Error("nested failure"); await vi.advanceTimersByTimeAsync(60); - await runLocatorOperation(parent, async (child) => { - expect(child).toBe(parent); + const nested = runLocatorOperation(operation, async (child) => { + expect(child).toBe(operation); expect(child.remainingMs()).toBe(40); - expect(vi.getTimerCount()).toBe(1); + if (outcome === "failure") throw error; }); - expect(vi.getTimerCount()).toBe(1); - await gate.promise; - }); - const rejected = expect(result).rejects.toThrow("locator.fill timed out after 100ms"); - - // Let the nested call complete before advancing the parent's remaining budget. - await vi.advanceTimersByTimeAsync(0); - await vi.advanceTimersByTimeAsync(40); - await rejected; - expect(vi.getTimerCount()).toBe(0); - gate.resolve(); - }); + if (outcome === "failure") await expect(nested).rejects.toBe(error); + else await nested; - it("does not let a failed nested call dispose the parent's deadline", async () => { - const gate = deferred(); - const error = new Error("nested lookup failed"); - const result = runLocatorOperation({ name: "locator.fill", timeout: 100 }, async (parent) => { - await expect( - runLocatorOperation(parent, async () => { - throw error; - }), - ).rejects.toBe(error); expect(vi.getTimerCount()).toBe(1); - await gate.promise; - }); - const rejected = expect(result).rejects.toThrow(TimeoutError); - - await vi.advanceTimersByTimeAsync(100); - await rejected; - gate.resolve(); - }); - - it("does not invoke nested work after the parent expires", async () => { - const gate = deferred(); - let operation!: LocatorOperation; - const result = runLocatorOperation({ name: "locator.fill", timeout: 100 }, async (context) => { - operation = context; - return gate.promise; - }); - const rejected = expect(result).rejects.toThrow(TimeoutError); - await vi.advanceTimersByTimeAsync(100); - await rejected; - - const work = vi.fn(async () => {}); - await expect(runLocatorOperation(operation, work)).rejects.toBe(operation.signal.reason); - expect(work).not.toHaveBeenCalled(); - gate.resolve(); - }); + await vi.advanceTimersByTimeAsync(40); + expect(operation.remainingMs()).toBe(0); + expect(operation.signal.reason).toBeInstanceOf(TimeoutError); + expect(() => operation.throwIfStopped()).toThrow(operation.signal.reason); + }, + ); it("keeps concurrent operations independent", async () => { - const firstGate = deferred(); - const secondGate = deferred(); - let second!: LocatorOperation; - const first = runLocatorOperation( - { name: "locator.click", timeout: 10 }, - () => firstGate.promise, - ); - const other = runLocatorOperation({ name: "locator.click", timeout: 100 }, async (context) => { - second = context; - return secondGate.promise; - }); - const rejected = expect(first).rejects.toThrow(TimeoutError); - + const short = createOperation(10); + const long = createOperation(100); + const gate = deferred(); + const first = expect(short.run("reading", () => gate.promise)).rejects.toThrow(TimeoutError); + const second = long.run("reading", () => gate.promise); await vi.advanceTimersByTimeAsync(10); - await rejected; - expect(second.signal.aborted).toBe(false); - expect(second.remainingMs()).toBe(90); - secondGate.resolve("done"); - await expect(other).resolves.toBe("done"); - expect(vi.getTimerCount()).toBe(0); - firstGate.resolve(); + await first; + expect(long.signal.aborted).toBe(false); + expect(long.remainingMs()).toBe(90); + gate.resolve("done"); + await expect(second).resolves.toBe("done"); }); - it.each(["success", "failure", "synchronous failure"] as const)( - "disposes the timer and abort listener on %s", + it.each(["success", "failure", "synchronous failure", "timeout"] as const)( + "disposes the owning runner's timer and listener on %s", async (outcome) => { + const gate = deferred(); const error = new Error("action failed"); let signal!: AbortSignal; - let removeListener!: ReturnType; - const result = runLocatorOperation({ name: "locator.click", timeout: 100 }, (operation) => { + let remove!: ReturnType; + const pending = runLocatorOperation({ name: "locator.click", timeout: 100 }, (operation) => { signal = operation.signal; - removeListener = vi.spyOn(signal, "removeEventListener"); + remove = vi.spyOn(signal, "removeEventListener"); if (outcome === "synchronous failure") throw error; - return outcome === "success" ? Promise.resolve("done") : Promise.reject(error); + return gate.promise; }); + await Promise.resolve(); + if (outcome === "success") { + gate.resolve("done"); + await expect(pending).resolves.toBe("done"); + } else if (outcome === "timeout") { + const rejected = expect(pending).rejects.toThrow(TimeoutError); + await vi.advanceTimersByTimeAsync(100); + await rejected; + gate.resolve("late"); + } else { + const rejected = expect(pending).rejects.toBe(error); + if (outcome === "failure") gate.reject(error); + await rejected; + } + expect(remove).toHaveBeenCalledExactlyOnceWith("abort", expect.any(Function)); + expect(vi.getTimerCount()).toBe(0); + await vi.advanceTimersByTimeAsync(100); + expect(signal.aborted).toBe(outcome === "timeout"); + }, + ); - if (outcome === "success") await expect(result).resolves.toBe("done"); - else await expect(result).rejects.toBe(error); + it("checks expiry before dispatch, even before the deadline timer runs", async () => { + const operation = createOperation(); + const command = vi.fn(async () => {}); + const pending = operation.run("clicking", command); + vi.spyOn(performance, "now").mockReturnValue(101); + await expect(pending).rejects.toThrow(TimeoutError); + await expect(runLocatorOperation(operation, command)).rejects.toBe(operation.signal.reason); + expect(command).not.toHaveBeenCalled(); + }); + + it("delivers a successful resource without invoking late cleanup", async () => { + const operation = createOperation(); + const release = vi.fn(async () => {}); + await expect(operation.run("finding node", async () => "node", release)).resolves.toBe("node"); + expect(release).not.toHaveBeenCalled(); + }); - expect(removeListener).toHaveBeenCalledWith("abort", expect.any(Function)); + it.each(["resolve", "reject"] as const)( + "does not resume timed-out work when a late command %ss", + async (outcome) => { + const operation = createOperation(); + const command = deferred(); + const nextStep = vi.fn(async () => {}); + const release = vi.fn(async () => {}); + const remove = vi.spyOn(operation.signal, "removeEventListener"); + await vi.advanceTimersByTimeAsync(60); + const pending = runLocatorOperation(operation, async (context) => { + await context.run("finding node", () => command.promise, release); + await context.run("clicking", nextStep); + }); + const rejected = expect(pending).rejects.toThrow( + "locator.click timed out after 100ms while finding node", + ); + // A finished concurrent phase must not replace the phase that is still waiting. + await operation.run("inspecting another frame", async () => {}); + await vi.advanceTimersByTimeAsync(40); + await rejected; + expect(remove).toHaveBeenCalledTimes(3); + if (outcome === "resolve") command.resolve("late-node"); + else command.reject(new Error("late browser error")); + await vi.advanceTimersByTimeAsync(0); + expect(nextStep).not.toHaveBeenCalled(); + if (outcome === "resolve") expect(release).toHaveBeenCalledExactlyOnceWith("late-node"); + else expect(release).not.toHaveBeenCalled(); expect(vi.getTimerCount()).toBe(0); - await vi.advanceTimersByTimeAsync(100); - expect(signal.aborted).toBe(false); }, ); - it("does not truncate timeouts larger than one timer interval", async () => { - const maxTimerMs = 2_147_483_647; - const gate = deferred(); - let operation!: LocatorOperation; - const result = runLocatorOperation( - { name: "locator.type", timeout: maxTimerMs + 100 }, - async (context) => { - operation = context; - return gate.promise; + it("releases a result that crosses the deadline before delivery", async () => { + const operation = createOperation(); + const release = vi.fn(async () => {}); + const pending = operation.run( + "finding node", + async () => { + vi.spyOn(performance, "now").mockReturnValue(101); + return "node"; }, + release, ); - const rejected = expect(result).rejects.toThrow(TimeoutError); + await expect(pending).rejects.toThrow(TimeoutError); + expect(release).toHaveBeenCalledExactlyOnceWith("node"); + }); - await vi.advanceTimersByTimeAsync(maxTimerMs); - expect(operation.signal.aborted).toBe(false); - expect(operation.remainingMs()).toBe(100); - expect(vi.getTimerCount()).toBe(1); - await vi.advanceTimersByTimeAsync(100); - await rejected; - expect(vi.getTimerCount()).toBe(0); - gate.resolve(); + it.each([0, 25, 1_000])("bounds a %s ms delay and removes its sleep timer", async (ms) => { + const operation = createOperation(); + const pending = operation.delay(ms); + if (ms > 100) { + const rejected = expect(pending).rejects.toThrow(TimeoutError); + await vi.advanceTimersByTimeAsync(100); + await rejected; + expect(vi.getTimerCount()).toBe(0); + } else { + await vi.advanceTimersByTimeAsync(ms); + await pending; + expect(operation.remainingMs()).toBe(100 - ms); + expect(operation.signal.aborted).toBe(false); + expect(vi.getTimerCount()).toBe(1); + } }); - it.each([-1, NaN, Infinity, -Infinity])( - "rejects invalid timeout %s before starting work", - async (timeout) => { - const work = vi.fn(async () => {}); - await expect(runLocatorOperation({ name: "locator.click", timeout }, work)).rejects.toThrow( - RangeError, - ); - expect(work).not.toHaveBeenCalled(); + it.each(["deadline", "unlimited delay"] as const)( + "does not truncate a long %s to one timer interval", + async (kind) => { + const maxTimerMs = 2_147_483_647; + const gate = deferred(); + const operation = createOperation(kind === "deadline" ? maxTimerMs + 100 : 0); + const pending = + kind === "deadline" + ? operation.run("waiting", () => gate.promise) + : operation.delay(maxTimerMs + 100); + const result = + kind === "deadline" + ? expect(pending).rejects.toThrow(TimeoutError) + : expect(pending).resolves.toBeUndefined(); + await vi.advanceTimersByTimeAsync(maxTimerMs); + expect(operation.signal.aborted).toBe(false); + expect(vi.getTimerCount()).toBe(1); + expect(operation.remainingMs()).toBe(kind === "deadline" ? 100 : Infinity); + await vi.advanceTimersByTimeAsync(100); + await result; expect(vi.getTimerCount()).toBe(0); + gate.resolve(); }, ); + + it.each([-1, NaN, Infinity, -Infinity])("rejects invalid time values: %s", async (timeout) => { + const command = vi.fn(async () => {}); + await expect(runLocatorOperation({ name: "locator.click", timeout }, command)).rejects.toThrow( + RangeError, + ); + await expect(createOperation().delay(timeout)).rejects.toThrow(RangeError); + expect(command).not.toHaveBeenCalled(); + }); + + it.each(["success", "failure", "synchronous failure"] as const)( + "preserves the primary error and clears the cleanup timer after cleanup %s", + async (outcome) => { + const operation = createOperation(); + const error = new Error("element detached"); + await expect( + runLocatorOperation(operation, async (context) => { + try { + throw error; + } finally { + await context.cleanup(() => { + if (outcome === "synchronous failure") throw new Error("release failed"); + return outcome === "success" + ? Promise.resolve() + : Promise.reject(new Error("release failed")); + }); + } + }), + ).rejects.toBe(error); + expect(vi.getTimerCount()).toBe(1); + }, + ); + + it("returns on timeout while bounding cleanup and observing its late rejection", async () => { + const operation = createOperation(); + const command = deferred(); + const release = deferred(); + const cleanup = vi.fn(() => release.promise); + let cleanedUp = false; + const pending = runLocatorOperation(operation, async (context) => { + try { + await context.run("finding node", () => command.promise); + } finally { + await context.cleanup(cleanup); + cleanedUp = true; + } + }); + const rejected = expect(pending).rejects.toThrow(TimeoutError); + await vi.advanceTimersByTimeAsync(100); + await rejected; + expect(cleanup).toHaveBeenCalledOnce(); + await vi.advanceTimersByTimeAsync(999); + expect(cleanedUp).toBe(false); + await vi.advanceTimersByTimeAsync(1); + expect(cleanedUp).toBe(true); + expect(vi.getTimerCount()).toBe(0); + release.reject(new Error("late cleanup failure")); + command.resolve(); + await vi.advanceTimersByTimeAsync(0); + }); }); diff --git a/packages/extension/understudy/locatorOperationSteps.test.ts b/packages/extension/understudy/locatorOperationSteps.test.ts deleted file mode 100644 index 68859031b..000000000 --- a/packages/extension/understudy/locatorOperationSteps.test.ts +++ /dev/null @@ -1,304 +0,0 @@ -import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; -import { TimeoutError } from "../errors.js"; -import { LocatorOperation, runLocatorOperation } from "./locatorOperation.js"; - -function deferred() { - let resolve!: (value: T) => void; - let reject!: (error: Error) => void; - const promise = new Promise((resolvePromise, rejectPromise) => { - resolve = resolvePromise; - reject = rejectPromise; - }); - return { promise, resolve, reject }; -} - -describe("locator operation steps", () => { - let operation: LocatorOperation; - - beforeEach(() => { - vi.useFakeTimers({ toFake: ["setTimeout", "clearTimeout", "performance"] }); - operation = new LocatorOperation("locator.click", 100); - }); - - afterEach(() => { - operation.dispose(); - vi.useRealTimers(); - vi.restoreAllMocks(); - }); - - it("does not dispatch work after expiry, even before the timer runs", async () => { - const command = vi.fn(async () => {}); - const pending = operation.run("clicking", command); - const rejected = expect(pending).rejects.toThrow(TimeoutError); - - // Expire between accepting the callback and actually invoking it. - vi.spyOn(performance, "now").mockReturnValue(101); - await rejected; - expect(command).not.toHaveBeenCalled(); - }); - - it("uses the remaining budget and does not resume after a late response", async () => { - const command = deferred(); - const nextStep = vi.fn(async () => {}); - await vi.advanceTimersByTimeAsync(60); - const pending = runLocatorOperation(operation, async (context) => { - await context.run("reading geometry", () => command.promise); - await context.run("clicking", nextStep); - }); - const rejected = expect(pending).rejects.toThrow( - "locator.click timed out after 100ms while reading geometry", - ); - - await vi.advanceTimersByTimeAsync(40); - await rejected; - command.resolve(); - await vi.advanceTimersByTimeAsync(0); - expect(nextStep).not.toHaveBeenCalled(); - expect(vi.getTimerCount()).toBe(0); - }); - - it("delivers successful results without releasing the caller's resource", async () => { - const handle = { objectId: "node" }; - const release = vi.fn(async () => {}); - await expect(operation.run("finding node", async () => handle, release)).resolves.toBe(handle); - expect(release).not.toHaveBeenCalled(); - }); - - it("releases a late resource exactly once after timeout", async () => { - const command = deferred<{ objectId: string }>(); - const handle = { objectId: "late-node" }; - const release = vi.fn(async () => {}); - const pending = operation.run("finding node", () => command.promise, release); - const rejected = expect(pending).rejects.toThrow(TimeoutError); - - await vi.advanceTimersByTimeAsync(100); - await rejected; - expect(release).not.toHaveBeenCalled(); - command.resolve(handle); - await vi.advanceTimersByTimeAsync(0); - expect(release).toHaveBeenCalledExactlyOnceWith(handle); - expect(vi.getTimerCount()).toBe(0); - }); - - it("releases an undelivered result when work crosses the deadline before the timer runs", async () => { - const handle = { objectId: "node" }; - const release = vi.fn(async () => {}); - const pending = operation.run( - "finding node", - async () => { - vi.spyOn(performance, "now").mockReturnValue(101); - return handle; - }, - release, - ); - - await expect(pending).rejects.toThrow(TimeoutError); - expect(release).toHaveBeenCalledExactlyOnceWith(handle); - expect(vi.getTimerCount()).toBe(0); - }); - - it("handles a command rejection arriving after timeout", async () => { - const command = deferred(); - const release = vi.fn(async () => {}); - const pending = operation.run("finding node", () => command.promise, release); - const rejected = expect(pending).rejects.toThrow(TimeoutError); - - await vi.advanceTimersByTimeAsync(100); - await rejected; - command.reject(new Error("late browser error")); - await vi.advanceTimersByTimeAsync(0); - expect(release).not.toHaveBeenCalled(); - expect(operation.signal.reason).toBeInstanceOf(TimeoutError); - }); - - it.each(["success", "failure", "synchronous failure", "timeout"] as const)( - "removes the step's abort listener on %s", - async (outcome) => { - const add = vi.spyOn(operation.signal, "addEventListener"); - const remove = vi.spyOn(operation.signal, "removeEventListener"); - const command = deferred(); - const error = new Error("browser error"); - const pending = operation.run("reading geometry", () => { - if (outcome === "synchronous failure") throw error; - return command.promise; - }); - - if (outcome === "success") { - command.resolve("done"); - await expect(pending).resolves.toBe("done"); - } else if (outcome === "timeout") { - const rejected = expect(pending).rejects.toThrow(TimeoutError); - await vi.advanceTimersByTimeAsync(100); - await rejected; - command.resolve("too late"); - } else { - const rejected = expect(pending).rejects.toBe(error); - if (outcome === "failure") { - await Promise.resolve(); - command.reject(error); - } - await rejected; - } - - expect(add).toHaveBeenCalledOnce(); - expect(remove).toHaveBeenCalledExactlyOnceWith("abort", add.mock.calls[0]![1]); - }, - ); - - it("reports a pending phase rather than a concurrent phase that already finished", async () => { - const command = deferred(); - const pending = operation.run("waiting for frame", () => command.promise); - const rejected = expect(pending).rejects.toThrow( - "locator.click timed out after 100ms while waiting for frame", - ); - await operation.run("inspecting another frame", async () => {}); - await vi.advanceTimersByTimeAsync(100); - await rejected; - command.resolve(); - }); - - it("interrupts a delay and clears its timer", async () => { - const pending = operation.delay(1_000); - const rejected = expect(pending).rejects.toThrow(TimeoutError); - await vi.advanceTimersByTimeAsync(0); - expect(vi.getTimerCount()).toBe(2); - - await vi.advanceTimersByTimeAsync(100); - await rejected; - expect(vi.getTimerCount()).toBe(0); - }); - - it("clears a completed delay's timer without stopping the operation", async () => { - const pending = operation.delay(25); - await vi.advanceTimersByTimeAsync(25); - await pending; - expect(operation.remainingMs()).toBe(75); - expect(operation.signal.aborted).toBe(false); - expect(vi.getTimerCount()).toBe(1); - }); - - it("supports zero delay without allocating a sleep timer", async () => { - await operation.delay(0); - expect(operation.remainingMs()).toBe(100); - expect(vi.getTimerCount()).toBe(1); - }); - - it("supports long delays when the operation has no deadline", async () => { - operation.dispose(); - operation = new LocatorOperation("locator.type", 0); - const maxTimerMs = 2_147_483_647; - let finished = false; - const pending = operation.delay(maxTimerMs + 100).then(() => { - finished = true; - }); - - await vi.advanceTimersByTimeAsync(maxTimerMs); - expect(finished).toBe(false); - await vi.advanceTimersByTimeAsync(100); - await pending; - expect(finished).toBe(true); - expect(operation.signal.aborted).toBe(false); - expect(vi.getTimerCount()).toBe(0); - }); - - it.each([-1, NaN, Infinity])("rejects invalid delay %s", async (ms) => { - await expect(operation.delay(ms)).rejects.toThrow(RangeError); - expect(vi.getTimerCount()).toBe(1); - }); - - it("attempts cleanup after expiry and bounds its wait even if the command stalls", async () => { - await vi.advanceTimersByTimeAsync(100); - const command = deferred(); - const cleanup = vi.fn(() => command.promise); - let finished = false; - const pending = operation.cleanup(cleanup).then(() => { - finished = true; - }); - - await vi.advanceTimersByTimeAsync(999); - expect(cleanup).toHaveBeenCalledOnce(); - expect(finished).toBe(false); - await vi.advanceTimersByTimeAsync(1); - await pending; - expect(finished).toBe(true); - expect(operation.signal.reason).toBeInstanceOf(TimeoutError); - expect(vi.getTimerCount()).toBe(0); - - // A rejection after the cleanup wait ends must still be observed. - command.reject(new Error("late cleanup failure")); - await vi.advanceTimersByTimeAsync(0); - }); - - it.each(["success", "failure", "synchronous failure"] as const)( - "clears the cleanup timer on %s", - async (outcome) => { - const error = new Error("cleanup failure"); - await operation.cleanup(() => { - if (outcome === "synchronous failure") throw error; - return outcome === "success" ? Promise.resolve() : Promise.reject(error); - }); - expect(vi.getTimerCount()).toBe(1); - }, - ); - - it("preserves the primary error when cleanup fails", async () => { - const error = new Error("element is detached"); - await expect( - runLocatorOperation(operation, async (context) => { - try { - throw error; - } finally { - await context.cleanup(async () => { - throw new Error("release failed"); - }); - } - }), - ).rejects.toBe(error); - }); - - it("does not hold the caller past timeout while finally cleanup is stalled", async () => { - const command = deferred(); - const release = deferred(); - const cleanup = vi.fn(() => release.promise); - const pending = runLocatorOperation(operation, async (context) => { - try { - await context.run("finding node", () => command.promise); - } finally { - await context.cleanup(cleanup); - } - }); - const rejected = expect(pending).rejects.toThrow(TimeoutError); - - await vi.advanceTimersByTimeAsync(100); - await rejected; - expect(cleanup).toHaveBeenCalledOnce(); - await vi.advanceTimersByTimeAsync(1_000); - expect(vi.getTimerCount()).toBe(0); - command.resolve(); - release.resolve(); - await vi.advanceTimersByTimeAsync(0); - }); - - it.each(["synchronous failure", "failure", "stall"] as const)( - "handles late resource cleanup ending in %s", - async (outcome) => { - const command = deferred(); - const release = deferred(); - const cleanup = vi.fn(() => { - if (outcome === "synchronous failure") throw new Error("release failed"); - if (outcome === "failure") return Promise.reject(new Error("release failed")); - return release.promise; - }); - const pending = operation.run("finding node", () => command.promise, cleanup); - const rejected = expect(pending).rejects.toThrow(TimeoutError); - - await vi.advanceTimersByTimeAsync(100); - await rejected; - command.resolve("late-node"); - await vi.advanceTimersByTimeAsync(1_000); - expect(cleanup).toHaveBeenCalledExactlyOnceWith("late-node"); - expect(vi.getTimerCount()).toBe(0); - release.resolve(); - }, - ); -}); From 1d16972905ba78098b6f305d5b74112572b473dd Mon Sep 17 00:00:00 2001 From: Sean McGuire Date: Tue, 22 Sep 2026 17:07:51 -0700 Subject: [PATCH 4/4] throw timeout err even if late commands throw --- .../understudy/locatorOperation.test.ts | 26 ++++++++++++++++--- .../extension/understudy/locatorOperation.ts | 1 + 2 files changed, 23 insertions(+), 4 deletions(-) diff --git a/packages/extension/understudy/locatorOperation.test.ts b/packages/extension/understudy/locatorOperation.test.ts index e850512f1..2ba2422fb 100644 --- a/packages/extension/understudy/locatorOperation.test.ts +++ b/packages/extension/understudy/locatorOperation.test.ts @@ -124,6 +124,24 @@ describe("locator operations", () => { expect(command).not.toHaveBeenCalled(); }); + it.each(["throws", "rejects"] as const)( + "reports timeout when work %s after the deadline before the timer runs", + async (failure) => { + const operation = createOperation(); + const pending = operation.run("reading geometry", () => { + vi.spyOn(performance, "now").mockReturnValue(101); + expect(operation.signal.aborted).toBe(false); + const error = new Error("browser error"); + if (failure === "throws") throw error; + return Promise.reject(error); + }); + await expect(pending).rejects.toThrow( + "locator.click timed out after 100ms while reading geometry", + ); + expect(operation.signal.reason).toBeInstanceOf(TimeoutError); + }, + ); + it("delivers a successful resource without invoking late cleanup", async () => { const operation = createOperation(); const release = vi.fn(async () => {}); @@ -131,8 +149,8 @@ describe("locator operations", () => { expect(release).not.toHaveBeenCalled(); }); - it.each(["resolve", "reject"] as const)( - "does not resume timed-out work when a late command %ss", + it.each(["resolves", "rejects"] as const)( + "does not resume timed-out work when a late command %s", async (outcome) => { const operation = createOperation(); const command = deferred(); @@ -152,11 +170,11 @@ describe("locator operations", () => { await vi.advanceTimersByTimeAsync(40); await rejected; expect(remove).toHaveBeenCalledTimes(3); - if (outcome === "resolve") command.resolve("late-node"); + if (outcome === "resolves") command.resolve("late-node"); else command.reject(new Error("late browser error")); await vi.advanceTimersByTimeAsync(0); expect(nextStep).not.toHaveBeenCalled(); - if (outcome === "resolve") expect(release).toHaveBeenCalledExactlyOnceWith("late-node"); + if (outcome === "resolves") expect(release).toHaveBeenCalledExactlyOnceWith("late-node"); else expect(release).not.toHaveBeenCalled(); expect(vi.getTimerCount()).toBe(0); }, diff --git a/packages/extension/understudy/locatorOperation.ts b/packages/extension/understudy/locatorOperation.ts index 4986ae140..3ba335a5a 100644 --- a/packages/extension/understudy/locatorOperation.ts +++ b/packages/extension/understudy/locatorOperation.ts @@ -84,6 +84,7 @@ export class LocatorOperation { abandoned = true; // The result may have arrived just before expiry, but not been delivered. if (result) discard(result.value); + this.throwIfStopped(); throw error; } finally { if (onAbort) this.signal.removeEventListener("abort", onAbort);