Skip to content

Commit fbfcc3b

Browse files
author
cod1k
committed
Refactor tests to simplify waitUntil handling
Replaced custom delayed `waitUntil` implementations with `Promise.withResolvers` for improved readability and consistency. Removed unnecessary timer usage and adjusted test logic for better maintainability.
1 parent f57c6eb commit fbfcc3b

File tree

5 files changed

+84
-97
lines changed

5 files changed

+84
-97
lines changed

‎packages/cloudflare/test/durableobject.test.ts‎

Lines changed: 5 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import type { ExecutionContext } from '@cloudflare/workers-types';
22
import * as SentryCore from '@sentry/core';
3-
import { afterEach, describe, expect, it, onTestFinished, vi } from 'vitest';
3+
import { afterEach, describe, expect, it, vi } from 'vitest';
44
import { instrumentDurableObjectWithSentry } from '../src';
55
import { isInstrumented } from '../src/instrument';
66

@@ -122,15 +122,13 @@ describe('instrumentDurableObjectWithSentry', () => {
122122
});
123123

124124
it('flush performs after all waitUntil promises are finished', async () => {
125-
vi.useFakeTimers();
126-
onTestFinished(() => {
127-
vi.useRealTimers();
128-
});
129125
const flush = vi.spyOn(SentryCore.Client.prototype, 'flush');
130126
const waitUntil = vi.fn();
127+
const { promise, resolve } = Promise.withResolvers();
128+
process.nextTick(resolve);
131129
const testClass = vi.fn(context => ({
132130
fetch: () => {
133-
context.waitUntil(new Promise(res => setTimeout(res)));
131+
context.waitUntil(promise);
134132
return new Response('test');
135133
},
136134
}));
@@ -142,8 +140,7 @@ describe('instrumentDurableObjectWithSentry', () => {
142140
expect(() => dObject.fetch(new Request('https://example.com'))).not.toThrow();
143141
expect(flush).not.toBeCalled();
144142
expect(waitUntil).toHaveBeenCalledOnce();
145-
vi.advanceTimersToNextTimer();
146-
await Promise.all(waitUntil.mock.calls.map(([p]) => p));
143+
await Promise.all(waitUntil.mock.calls.map(call => call[0]));
147144
expect(flush).toBeCalled();
148145
});
149146

‎packages/cloudflare/test/flush.test.ts‎

Lines changed: 6 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,10 @@
11
import { type ExecutionContext } from '@cloudflare/workers-types';
2-
import { describe, expect, it, onTestFinished, vi } from 'vitest';
2+
import { describe, expect, it, vi } from 'vitest';
33
import { makeFlushLock } from '../src/flush';
44

55
describe('Flush buffer test', () => {
6-
const waitUntilPromises: Promise<void>[] = [];
76
const mockExecutionContext: ExecutionContext = {
8-
waitUntil: vi.fn(promise => {
9-
waitUntilPromises.push(promise);
10-
}),
7+
waitUntil: vi.fn(),
118
passThroughOnException: vi.fn(),
129
props: null,
1310
};
@@ -16,16 +13,10 @@ describe('Flush buffer test', () => {
1613
await expect(finalize()).resolves.toBeUndefined();
1714
});
1815
it('should flush buffer only after all waitUntil were finished', async () => {
19-
vi.useFakeTimers();
20-
onTestFinished(() => {
21-
vi.useRealTimers();
22-
});
23-
const task = new Promise(resolve => setTimeout(resolve, 100));
16+
const { promise, resolve } = Promise.withResolvers();
2417
const lock = makeFlushLock(mockExecutionContext);
25-
mockExecutionContext.waitUntil(task);
26-
void lock.finalize();
27-
vi.advanceTimersToNextTimer();
28-
await Promise.all(waitUntilPromises);
29-
await expect(lock.ready).resolves.toBeUndefined();
18+
mockExecutionContext.waitUntil(promise);
19+
process.nextTick(resolve);
20+
await expect(lock.finalize()).resolves.toBeUndefined();
3021
});
3122
});

‎packages/cloudflare/test/handler.test.ts‎

Lines changed: 55 additions & 59 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ import type {
1010
} from '@cloudflare/workers-types';
1111
import type { Event } from '@sentry/core';
1212
import * as SentryCore from '@sentry/core';
13-
import { beforeEach, describe, expect, onTestFinished, test, vi } from 'vitest';
13+
import { beforeEach, describe, expect, test, vi } from 'vitest';
1414
import { CloudflareClient } from '../src/client';
1515
import { withSentry } from '../src/handler';
1616
import { markAsInstrumented } from '../src/instrument';
@@ -30,8 +30,29 @@ const MOCK_ENV = {
3030
SENTRY_RELEASE: '1.1.1',
3131
};
3232

33-
function addDelayedWaitUntil(context: ExecutionContext) {
34-
context.waitUntil(new Promise<void>(resolve => setTimeout(() => resolve())));
33+
function makeWaitUntilAndTask<
34+
M extends ReturnType<typeof vi.fn>,
35+
T extends M & {
36+
readonly ready: Promise<M>;
37+
readonly task: { readonly promise: Promise<void>; readonly resolve: M };
38+
},
39+
>(): T {
40+
const waitUntil = vi.fn();
41+
const { promise, resolve } = Promise.withResolvers();
42+
const resolver = vi.fn().mockImplementation(resolve).mockName('waitUntil.ready');
43+
Object.defineProperties(waitUntil, {
44+
ready: {
45+
get: () => Promise.all(waitUntil.mock.calls.map(call => call[0])).then(() => resolver),
46+
enumerable: true,
47+
configurable: true,
48+
},
49+
task: {
50+
value: { promise, resolve: resolver },
51+
enumerable: true,
52+
configurable: true,
53+
},
54+
});
55+
return waitUntil as T;
3556
}
3657

3758
describe('withSentry', () => {
@@ -135,27 +156,22 @@ describe('withSentry', () => {
135156

136157
test('flush must be called when all waitUntil are done', async () => {
137158
const flush = vi.spyOn(SentryCore.Client.prototype, 'flush');
138-
vi.useFakeTimers();
139-
onTestFinished(() => {
140-
vi.useRealTimers();
141-
});
159+
const waitUntil = makeWaitUntilAndTask();
160+
process.nextTick(waitUntil.task.resolve);
142161
const handler = {
143-
fetch(_request, _env, _context) {
144-
addDelayedWaitUntil(_context);
162+
fetch(...args) {
163+
args[2].waitUntil(waitUntil.task.promise);
145164
return new Response('test');
146165
},
147166
} satisfies ExportedHandler<typeof MOCK_ENV>;
148167

149168
const wrappedHandler = withSentry(vi.fn(), handler);
150-
const waits: Promise<unknown>[] = [];
151-
const waitUntil = vi.fn(promise => waits.push(promise));
152169
await wrappedHandler.fetch?.(new Request('https://example.com'), MOCK_ENV, {
153170
waitUntil,
154171
} as unknown as ExecutionContext);
155172
expect(flush).not.toBeCalled();
156-
expect(waitUntil).toBeCalled();
157-
vi.advanceTimersToNextTimer().runAllTimers();
158-
await Promise.all(waits);
173+
const ready = await waitUntil.ready;
174+
expect(flush).toHaveBeenCalledAfter(ready);
159175
expect(flush).toHaveBeenCalledOnce();
160176
});
161177
});
@@ -375,28 +391,23 @@ describe('withSentry', () => {
375391

376392
test('flush must be called when all waitUntil are done', async () => {
377393
const flush = vi.spyOn(SentryCore.Client.prototype, 'flush');
378-
vi.useFakeTimers();
379-
onTestFinished(() => {
380-
vi.useRealTimers();
381-
});
394+
const waitUntil = makeWaitUntilAndTask();
395+
process.nextTick(waitUntil.task.resolve);
382396
const handler = {
383-
scheduled(_controller, _env, _context) {
384-
addDelayedWaitUntil(_context);
397+
scheduled(...args) {
398+
args[2].waitUntil(waitUntil.task.promise);
385399
return;
386400
},
387401
} satisfies ExportedHandler<typeof MOCK_ENV>;
388402

389403
const wrappedHandler = withSentry(vi.fn(), handler);
390-
const waits: Promise<unknown>[] = [];
391-
const waitUntil = vi.fn(promise => waits.push(promise));
392404
await wrappedHandler.scheduled?.(createMockScheduledController(), MOCK_ENV, {
393405
waitUntil,
394406
} as unknown as ExecutionContext);
395407
expect(flush).not.toBeCalled();
396-
expect(waitUntil).toBeCalled();
397-
vi.advanceTimersToNextTimer().runAllTimers();
398-
await Promise.all(waits);
408+
await waitUntil.ready;
399409
expect(flush).toHaveBeenCalledOnce();
410+
expect(flush).toHaveBeenCalledAfter(waitUntil);
400411
});
401412
});
402413

@@ -614,28 +625,24 @@ describe('withSentry', () => {
614625

615626
test('flush must be called when all waitUntil are done', async () => {
616627
const flush = vi.spyOn(SentryCore.Client.prototype, 'flush');
617-
vi.useFakeTimers();
618-
onTestFinished(() => {
619-
vi.useRealTimers();
620-
});
628+
const waitUntil = makeWaitUntilAndTask();
629+
630+
process.nextTick(waitUntil.task.resolve);
621631
const handler = {
622-
email(_controller, _env, _context) {
623-
addDelayedWaitUntil(_context);
632+
email(...args) {
633+
args[2].waitUntil(waitUntil.task.promise);
624634
return;
625635
},
626636
} satisfies ExportedHandler<typeof MOCK_ENV>;
627637

628638
const wrappedHandler = withSentry(vi.fn(), handler);
629-
const waits: Promise<unknown>[] = [];
630-
const waitUntil = vi.fn(promise => waits.push(promise));
631639
await wrappedHandler.email?.(createMockEmailMessage(), MOCK_ENV, {
632640
waitUntil,
633641
} as unknown as ExecutionContext);
634642
expect(flush).not.toBeCalled();
635-
expect(waitUntil).toBeCalled();
636-
vi.advanceTimersToNextTimer().runAllTimers();
637-
await Promise.all(waits);
643+
const ready = await waitUntil.ready;
638644
expect(flush).toHaveBeenCalledOnce();
645+
expect(flush).toHaveBeenCalledAfter(ready);
639646
});
640647
});
641648

@@ -857,27 +864,22 @@ describe('withSentry', () => {
857864

858865
test('flush must be called when all waitUntil are done', async () => {
859866
const flush = vi.spyOn(SentryCore.Client.prototype, 'flush');
860-
vi.useFakeTimers();
861-
onTestFinished(() => {
862-
vi.useRealTimers();
863-
});
867+
const waitUntil = makeWaitUntilAndTask();
868+
process.nextTick(waitUntil.task.resolve);
864869
const handler = {
865-
queue(_controller, _env, _context) {
866-
addDelayedWaitUntil(_context);
870+
queue(...args) {
871+
args[2].waitUntil(waitUntil.task.promise);
867872
return;
868873
},
869874
} satisfies ExportedHandler<typeof MOCK_ENV>;
870875

871876
const wrappedHandler = withSentry(vi.fn(), handler);
872-
const waits: Promise<unknown>[] = [];
873-
const waitUntil = vi.fn(promise => waits.push(promise));
874877
await wrappedHandler.queue?.(createMockQueueBatch(), MOCK_ENV, {
875878
waitUntil,
876879
} as unknown as ExecutionContext);
877880
expect(flush).not.toBeCalled();
878-
expect(waitUntil).toBeCalled();
879-
vi.advanceTimersToNextTimer().runAllTimers();
880-
await Promise.all(waits);
881+
882+
expect(flush).toHaveBeenCalledAfter(await waitUntil.ready);
881883
expect(flush).toHaveBeenCalledOnce();
882884
});
883885
});
@@ -1054,29 +1056,23 @@ describe('withSentry', () => {
10541056

10551057
test('flush must be called when all waitUntil are done', async () => {
10561058
const flush = vi.spyOn(SentryCore.Client.prototype, 'flush');
1057-
vi.useFakeTimers();
1058-
onTestFinished(() => {
1059-
vi.useRealTimers();
1060-
flush.mockRestore();
1061-
});
1059+
const waitUntil = makeWaitUntilAndTask();
1060+
process.nextTick(waitUntil.task.resolve);
10621061
const handler = {
1063-
tail(_controller, _env, _context) {
1064-
addDelayedWaitUntil(_context);
1062+
tail(...args) {
1063+
args[2].waitUntil(waitUntil.task.promise);
10651064
return;
10661065
},
10671066
} satisfies ExportedHandler<typeof MOCK_ENV>;
10681067

10691068
const wrappedHandler = withSentry(vi.fn(), handler);
1070-
const waits: Promise<unknown>[] = [];
1071-
const waitUntil = vi.fn(promise => waits.push(promise));
10721069
await wrappedHandler.tail?.(createMockTailEvent(), MOCK_ENV, {
10731070
waitUntil,
10741071
} as unknown as ExecutionContext);
10751072
expect(flush).not.toBeCalled();
1076-
expect(waitUntil).toBeCalled();
1077-
vi.advanceTimersToNextTimer().runAllTimers();
1078-
await Promise.all(waits);
1073+
const ready = await waitUntil.ready;
10791074
expect(flush).toHaveBeenCalledOnce();
1075+
expect(flush).toHaveBeenCalledAfter(ready);
10801076
});
10811077
});
10821078

‎packages/cloudflare/test/request.test.ts‎

Lines changed: 10 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44
import type { ExecutionContext } from '@cloudflare/workers-types';
55
import type { Event } from '@sentry/core';
66
import * as SentryCore from '@sentry/core';
7-
import { beforeAll, beforeEach, describe, expect, onTestFinished, test, vi } from 'vitest';
7+
import { beforeAll, beforeEach, describe, expect, test, vi } from 'vitest';
88
import { setAsyncLocalStorageAsyncContextStrategy } from '../src/async';
99
import type { CloudflareOptions } from '../src/client';
1010
import { CloudflareClient } from '../src/client';
@@ -14,10 +14,6 @@ const MOCK_OPTIONS: CloudflareOptions = {
1414
dsn: 'https://public@dsn.ingest.sentry.io/1337',
1515
};
1616

17-
function addDelayedWaitUntil(context: ExecutionContext) {
18-
context.waitUntil(new Promise<void>(resolve => setTimeout(() => resolve())));
19-
}
20-
2117
describe('withSentry', () => {
2218
beforeAll(() => {
2319
setAsyncLocalStorageAsyncContextStrategy();
@@ -70,25 +66,24 @@ describe('withSentry', () => {
7066

7167
test('flush must be called when all waitUntil are done', async () => {
7268
const flush = vi.spyOn(SentryCore.Client.prototype, 'flush');
73-
vi.useFakeTimers();
74-
onTestFinished(() => {
75-
vi.useRealTimers();
76-
});
77-
const waits: Promise<unknown>[] = [];
78-
const waitUntil = vi.fn(promise => waits.push(promise));
69+
const waitUntil = vi.fn();
7970

8071
const context = {
8172
waitUntil,
8273
} as unknown as ExecutionContext;
8374

75+
const { promise, resolve } = Promise.withResolvers();
76+
const resolved = vi.fn(() => resolve());
77+
process.nextTick(resolved);
78+
8479
await wrapRequestHandler({ options: MOCK_OPTIONS, request: new Request('https://example.com'), context }, () => {
85-
addDelayedWaitUntil(context);
80+
context.waitUntil(promise);
8681
return new Response('test');
8782
});
8883
expect(flush).not.toBeCalled();
8984
expect(waitUntil).toBeCalled();
90-
vi.advanceTimersToNextTimerAsync().then(() => vi.runAllTimers());
91-
await Promise.all(waits);
85+
await Promise.all(waitUntil.mock.calls.map(call => call[0]));
86+
expect(flush).toHaveBeenCalledAfter(resolved);
9287
expect(flush).toHaveBeenCalledOnce();
9388
});
9489

@@ -336,5 +331,5 @@ function createMockExecutionContext(): ExecutionContext {
336331
return {
337332
waitUntil: vi.fn(),
338333
passThroughOnException: vi.fn(),
339-
};
334+
} as unknown as ExecutionContext;
340335
}

‎packages/cloudflare/test/types.d.ts‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
// Not required in TypeScript 5.7+
2+
interface PromiseConstructor {
3+
withResolvers<T = unknown, E = unknown>(): {
4+
promise: PromiseConstructor<T>;
5+
resolve: (value?: T) => void;
6+
reject: (err: E) => void;
7+
};
8+
}

0 commit comments

Comments
 (0)