Skip to content

Commit 8657802

Browse files
authored
Fix flaky hook test by replacing setTimeout with deterministic awaits (vercel#1347)
1 parent 5811beb commit 8657802

1 file changed

Lines changed: 36 additions & 45 deletions

File tree

packages/core/src/workflow/hook.test.ts

Lines changed: 36 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import { WorkflowRuntimeError } from '@workflow/errors';
2+
import { withResolvers } from '@workflow/utils';
23
import type { Event } from '@workflow/world';
34
import * as nanoid from 'nanoid';
45
import { monotonicFactory } from 'ulid';
@@ -8,8 +9,8 @@ import { WorkflowSuspension } from '../global.js';
89
import type { WorkflowOrchestratorContext } from '../private.js';
910
import { dehydrateStepReturnValue } from '../serialization.js';
1011
import { createContext } from '../vm/index.js';
11-
import { createCreateHook } from './hook.js';
1212
import { createWebhook } from './create-hook.js';
13+
import { createCreateHook } from './hook.js';
1314

1415
// Helper to setup context to simulate a workflow run
1516
function setupWorkflowContext(events: Event[]): WorkflowOrchestratorContext {
@@ -68,20 +69,16 @@ describe('createCreateHook', () => {
6869
it('should throw WorkflowSuspension when no events are available', async () => {
6970
const ctx = setupWorkflowContext([]);
7071

71-
let workflowError: Error | undefined;
72-
ctx.onWorkflowError = (err) => {
73-
workflowError = err;
74-
};
72+
const errorReceived = withResolvers<Error>();
73+
ctx.onWorkflowError = errorReceived.resolve;
7574

7675
const createHook = createCreateHook(ctx);
7776
const hook = createHook();
7877

7978
// Start awaiting the hook - it will process events asynchronously
8079
const hookPromise = hook.then((v) => v);
8180

82-
// Wait for the error handler to be called
83-
await new Promise((resolve) => setTimeout(resolve, 10));
84-
81+
const workflowError = await errorReceived.promise;
8582
expect(workflowError).toBeInstanceOf(WorkflowSuspension);
8683
});
8784

@@ -101,20 +98,16 @@ describe('createCreateHook', () => {
10198
},
10299
]);
103100

104-
let workflowError: Error | undefined;
105-
ctx.onWorkflowError = (err) => {
106-
workflowError = err;
107-
};
101+
const errorReceived = withResolvers<Error>();
102+
ctx.onWorkflowError = errorReceived.resolve;
108103

109104
const createHook = createCreateHook(ctx);
110105
const hook = createHook();
111106

112107
// Start awaiting the hook - it will process events asynchronously
113108
const hookPromise = hook.then((v) => v);
114109

115-
// Wait for the error handler to be called
116-
await new Promise((resolve) => setTimeout(resolve, 10));
117-
110+
const workflowError = await errorReceived.promise;
118111
expect(workflowError).toBeInstanceOf(WorkflowRuntimeError);
119112
expect(workflowError?.message).toContain('Unexpected event type for hook');
120113
expect(workflowError?.message).toContain('hook_01K11TFZ62YS0YYFDQ3E8B9YCV');
@@ -181,8 +174,10 @@ describe('createCreateHook', () => {
181174
const createHook = createCreateHook(ctx);
182175
const hook = createHook();
183176

184-
// Wait for event processing
185-
await new Promise((resolve) => setTimeout(resolve, 10));
177+
// Wait for event processing (hook_disposed removes from invocationsQueue)
178+
await vi.waitFor(() => {
179+
expect(ctx.invocationsQueue.size).toBe(0);
180+
});
186181

187182
// The hook consumer should have finished (returned EventConsumerResult.Finished)
188183
// and should not have called onWorkflowError with a RuntimeError
@@ -273,10 +268,8 @@ describe('createCreateHook', () => {
273268
},
274269
]);
275270

276-
let workflowError: Error | undefined;
277-
ctx.onWorkflowError = (err) => {
278-
workflowError = err;
279-
};
271+
const errorReceived = withResolvers<Error>();
272+
ctx.onWorkflowError = errorReceived.resolve;
280273

281274
const createHook = createCreateHook(ctx);
282275
// Create hook with a specific token
@@ -285,9 +278,7 @@ describe('createCreateHook', () => {
285278
// Start awaiting the hook
286279
const hookPromise = hook.then((v) => v);
287280

288-
// Wait for the error handler to be called
289-
await new Promise((resolve) => setTimeout(resolve, 10));
290-
281+
const workflowError = await errorReceived.promise;
291282
expect(workflowError).toBeInstanceOf(WorkflowRuntimeError);
292283
expect(workflowError?.message).toContain('my-custom-token');
293284
});
@@ -448,8 +439,11 @@ describe('createCreateHook', () => {
448439
const createHook = createCreateHook(ctx);
449440
const hook = createHook();
450441

451-
// Wait for events to process
452-
await new Promise((resolve) => setTimeout(resolve, 10));
442+
// Wait for events to process (hook_created sets hasCreatedEvent on queue item)
443+
await vi.waitFor(() => {
444+
const item = ctx.invocationsQueue.get('hook_01K11TFZ62YS0YYFDQ3E8B9YCV');
445+
expect(item?.type === 'hook' && item.hasCreatedEvent).toBe(true);
446+
});
453447

454448
// Dispose — hook_created was replayed but no hook_disposed in log
455449
hook.dispose();
@@ -588,8 +582,11 @@ describe('createCreateHook', () => {
588582
const createHook = createCreateHook(ctx);
589583
const hook = createHook();
590584

591-
// Wait for events to process
592-
await new Promise((resolve) => setTimeout(resolve, 10));
585+
// Wait for events to process (hook_created sets hasCreatedEvent on queue item)
586+
await vi.waitFor(() => {
587+
const item = ctx.invocationsQueue.values().next().value;
588+
expect(item?.type === 'hook' && item.hasCreatedEvent).toBe(true);
589+
});
593590

594591
// Dispose after hook_created was replayed
595592
hook.dispose();
@@ -776,7 +773,6 @@ describe('createCreateHook', () => {
776773
});
777774

778775
it('should drain pending promises and trigger suspension when dispose is called while awaiting', async () => {
779-
const ops: Promise<any>[] = [];
780776
const ctx = setupWorkflowContext([
781777
{
782778
eventId: 'evnt_0',
@@ -788,16 +784,17 @@ describe('createCreateHook', () => {
788784
},
789785
]);
790786

791-
let workflowError: Error | undefined;
792-
ctx.onWorkflowError = (err) => {
793-
workflowError = err;
794-
};
787+
const errorReceived = withResolvers<Error>();
788+
ctx.onWorkflowError = errorReceived.resolve;
795789

796790
const createHook = createCreateHook(ctx);
797791
const hook = createHook();
798792

799-
// Wait for events to process (hook_created consumed, then null → eventLogEmpty)
800-
await new Promise((resolve) => setTimeout(resolve, 10));
793+
// Wait for events to process (hook_created sets hasCreatedEvent on queue item)
794+
await vi.waitFor(() => {
795+
const item = ctx.invocationsQueue.get('hook_01K11TFZ62YS0YYFDQ3E8B9YCV');
796+
expect(item?.type === 'hook' && item.hasCreatedEvent).toBe(true);
797+
});
801798

802799
// Start awaiting — this pushes a resolver to promises[] since payloadsQueue is empty
803800
const hookPromise = hook.then((v) => v);
@@ -806,9 +803,7 @@ describe('createCreateHook', () => {
806803
// and trigger suspension (not leave an orphaned promise)
807804
hook.dispose();
808805

809-
// Wait for the async suspension handler
810-
await new Promise((resolve) => setTimeout(resolve, 10));
811-
806+
const workflowError = await errorReceived.promise;
812807
expect(workflowError).toBeInstanceOf(WorkflowSuspension);
813808

814809
// The suspension should include the disposed hook
@@ -820,10 +815,8 @@ describe('createCreateHook', () => {
820815
it('should suspend when awaiting a disposed hook on first invocation', async () => {
821816
const ctx = setupWorkflowContext([]);
822817

823-
let workflowError: Error | undefined;
824-
ctx.onWorkflowError = (err) => {
825-
workflowError = err;
826-
};
818+
const errorReceived = withResolvers<Error>();
819+
ctx.onWorkflowError = errorReceived.resolve;
827820

828821
const createHook = createCreateHook(ctx);
829822
const hook = createHook();
@@ -834,9 +827,7 @@ describe('createCreateHook', () => {
834827
// Then await — the event log is empty, so this should trigger suspension
835828
const hookPromise = hook.then((v) => v);
836829

837-
// Wait for the async error handler
838-
await new Promise((resolve) => setTimeout(resolve, 10));
839-
830+
const workflowError = await errorReceived.promise;
840831
expect(workflowError).toBeInstanceOf(WorkflowSuspension);
841832

842833
// The suspension should include the disposed hook item

0 commit comments

Comments
 (0)