Skip to content

Commit 39740da

Browse files
authored
test(cloudflare): Use .makeRequestAndWaitForEnvelope to wait for envelopes (#20208)
There was one flaky test, which got me a little deeper into the `runner.ts` logic. This test was only passing when it was running / finishing first. With the `shuffle` flag it was consistently failing, this is why this is added in this PR as well. Furthermore a random port will be created for each runner by setting `--port 0`, this just makes sure that when running `wrangler dev` in another tab, while running the tests, the local development has the default `:8787` port. `.makeRequestAndWaitForEnvelope` has been added that is making a request and then waits for an envelope, with that we make sure that flush has been called before going to the next request. The issue is that miniflare might cancel ongoing `waitUntil` calls (where we flush)
1 parent c741030 commit 39740da

4 files changed

Lines changed: 60 additions & 19 deletions

File tree

dev-packages/cloudflare-integration-tests/runner.ts

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,12 @@ type StartResult = {
5050
path: string,
5151
options?: { headers?: Record<string, string>; data?: BodyInit; expectError?: boolean },
5252
): Promise<T | undefined>;
53+
makeRequestAndWaitForEnvelope<T>(
54+
method: 'get' | 'post',
55+
path: string,
56+
expected: Expected | Expected[],
57+
options?: { headers?: Record<string, string>; data?: BodyInit; expectError?: boolean },
58+
): Promise<T | undefined>;
5359
};
5460

5561
/** Creates a test runner */
@@ -108,6 +114,7 @@ export function createRunner(...paths: string[]) {
108114
const expectedEnvelopeCount = expectedEnvelopes.length;
109115

110116
let envelopeCount = 0;
117+
const envelopeWaiters: { expected: Expected; resolve: () => void; reject: (e: unknown) => void }[] = [];
111118
const { resolve: setWorkerPort, promise: workerPortPromise } = deferredPromise<number>();
112119
let child: ReturnType<typeof spawn> | undefined;
113120
let childSubWorker: ReturnType<typeof spawn> | undefined;
@@ -120,6 +127,12 @@ export function createRunner(...paths: string[]) {
120127
}
121128
}
122129

130+
function waitForEnvelope(expected: Expected): Promise<void> {
131+
return new Promise((resolveWaiter, rejectWaiter) => {
132+
envelopeWaiters.push({ expected, resolve: resolveWaiter, reject: rejectWaiter });
133+
});
134+
}
135+
123136
function assertEnvelopeMatches(expected: Expected, envelope: Envelope): void {
124137
if (typeof expected === 'function') {
125138
expected(envelope);
@@ -137,6 +150,18 @@ export function createRunner(...paths: string[]) {
137150
return;
138151
}
139152

153+
// Check per-request waiters first (FIFO order)
154+
if (envelopeWaiters.length > 0) {
155+
const waiter = envelopeWaiters.shift()!;
156+
try {
157+
assertEnvelopeMatches(waiter.expected, envelope);
158+
waiter.resolve();
159+
} catch (e) {
160+
waiter.reject(e);
161+
}
162+
return;
163+
}
164+
140165
try {
141166
if (unordered) {
142167
// find any matching expected envelope
@@ -242,6 +267,10 @@ export function createRunner(...paths: string[]) {
242267
`SENTRY_DSN:http://public@localhost:${mockServerPort}/1337`,
243268
'--var',
244269
`SERVER_URL:${serverUrl}`,
270+
'--port',
271+
'0',
272+
'--inspector-port',
273+
'0',
245274
...extraWranglerArgs,
246275
],
247276
{ stdio, signal },
@@ -304,6 +333,18 @@ export function createRunner(...paths: string[]) {
304333
return;
305334
}
306335
},
336+
makeRequestAndWaitForEnvelope: async function <T>(
337+
method: 'get' | 'post',
338+
path: string,
339+
expected: Expected | Expected[],
340+
options: { headers?: Record<string, string>; data?: BodyInit; expectError?: boolean } = {},
341+
): Promise<T | undefined> {
342+
const expectations = Array.isArray(expected) ? expected : [expected];
343+
const envelopePromises = expectations.map(e => waitForEnvelope(e));
344+
const result = await this.makeRequest<T>(method, path, options);
345+
await Promise.all(envelopePromises);
346+
return result;
347+
},
307348
};
308349
},
309350
};

dev-packages/cloudflare-integration-tests/suites/public-api/startSpan-streamed/test.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -197,7 +197,7 @@ it('sends a streamed span envelope with correct spans for a manually started spa
197197
},
198198
'url.port': {
199199
type: 'string',
200-
value: '8787',
200+
value: expect.stringMatching(/^\d{4,5}$/),
201201
},
202202
'url.scheme': {
203203
type: 'string',
@@ -221,7 +221,7 @@ it('sends a streamed span envelope with correct spans for a manually started spa
221221
},
222222
'http.request.header.cf_connecting_ip': {
223223
type: 'string',
224-
value: '::1',
224+
value: expect.stringMatching(/^(::1|127\.0\.0\.1)$/),
225225
},
226226
'http.request.header.host': {
227227
type: 'string',

dev-packages/cloudflare-integration-tests/suites/tracing/durableobject-spans/test.ts

Lines changed: 14 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -25,15 +25,13 @@ it.skip('sends child spans on repeated Durable Object calls', async ({ signal })
2525

2626
// All 5 child spans should be present
2727
expect(transactionEvent.spans).toHaveLength(5);
28-
expect(transactionEvent.spans).toEqual(
29-
expect.arrayContaining([
30-
expect.objectContaining({ description: 'task-1', op: 'task' }),
31-
expect.objectContaining({ description: 'task-2', op: 'task' }),
32-
expect.objectContaining({ description: 'task-3', op: 'task' }),
33-
expect.objectContaining({ description: 'task-4', op: 'task' }),
34-
expect.objectContaining({ description: 'task-5', op: 'task' }),
35-
]),
36-
);
28+
expect(transactionEvent.spans).toEqual([
29+
expect.objectContaining({ description: 'task-1', op: 'task' }),
30+
expect.objectContaining({ description: 'task-2', op: 'task' }),
31+
expect.objectContaining({ description: 'task-3', op: 'task' }),
32+
expect.objectContaining({ description: 'task-4', op: 'task' }),
33+
expect.objectContaining({ description: 'task-5', op: 'task' }),
34+
]);
3735

3836
// All child spans share the root trace_id
3937
const rootTraceId = transactionEvent.contexts?.trace?.trace_id;
@@ -43,13 +41,12 @@ it.skip('sends child spans on repeated Durable Object calls', async ({ signal })
4341
}
4442
}
4543

46-
// Expect 5 transaction envelopes — one per call.
47-
const runner = createRunner(__dirname).expectN(5, assertDoWorkEnvelope).start(signal);
44+
const runner = createRunner(__dirname).start(signal);
4845

49-
await runner.makeRequest('get', '/');
50-
await runner.makeRequest('get', '/');
51-
await runner.makeRequest('get', '/');
52-
await runner.makeRequest('get', '/');
53-
await runner.makeRequest('get', '/');
54-
await runner.completed();
46+
// Each request waits for its envelope to be received and validated before proceeding.
47+
await runner.makeRequestAndWaitForEnvelope('get', '/', assertDoWorkEnvelope);
48+
await runner.makeRequestAndWaitForEnvelope('get', '/', assertDoWorkEnvelope);
49+
await runner.makeRequestAndWaitForEnvelope('get', '/', assertDoWorkEnvelope);
50+
await runner.makeRequestAndWaitForEnvelope('get', '/', assertDoWorkEnvelope);
51+
await runner.makeRequestAndWaitForEnvelope('get', '/', assertDoWorkEnvelope);
5552
});

dev-packages/cloudflare-integration-tests/vite.config.mts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,9 @@ export default defineConfig({
2828
singleThread: true,
2929
},
3030
},
31+
sequence: {
32+
shuffle: true,
33+
},
3134
reporters: process.env.DEBUG
3235
? ['default', { summary: false }]
3336
: process.env.GITHUB_ACTIONS

0 commit comments

Comments
 (0)