Skip to content

Commit 2dac8d6

Browse files
Refactor incremental render tests to use custom waitFor function
- Replaced inline wait functions with a new `waitFor` utility to improve test reliability and readability. - Updated tests to utilize `waitFor` for asynchronous expectations, ensuring proper handling of processing times. - Simplified the test structure by removing redundant wait logic, enhancing maintainability.
1 parent 10e3865 commit 2dac8d6

File tree

3 files changed

+90
-29
lines changed

3 files changed

+90
-29
lines changed

react_on_rails_pro/packages/node-renderer/src/worker/handleIncrementalRenderStream.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -76,7 +76,7 @@ export async function handleIncrementalRenderStream(
7676
const { response, shouldContinue: continueFlag } = result;
7777

7878
// eslint-disable-next-line no-await-in-loop
79-
await onResponseStart(response);
79+
void onResponseStart(response);
8080

8181
if (!continueFlag) {
8282
return;

react_on_rails_pro/packages/node-renderer/tests/helper.ts

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -144,4 +144,48 @@ export function readRenderingRequest(projectName: string, commit: string, reques
144144
return fs.readFileSync(path.resolve(__dirname, renderingRequestRelativePath), 'utf8');
145145
}
146146

147+
/**
148+
* Custom waitFor function that retries an expect statement until it passes or timeout is reached
149+
* @param expectFn - Function containing Jest expect statements
150+
* @param options - Configuration options
151+
* @param options.timeout - Maximum time to wait in milliseconds (default: 1000)
152+
* @param options.interval - Time between retries in milliseconds (default: 10)
153+
* @param options.message - Custom error message when timeout is reached
154+
*/
155+
export const waitFor = async (
156+
expectFn: () => void,
157+
options: {
158+
timeout?: number;
159+
interval?: number;
160+
message?: string;
161+
} = {},
162+
): Promise<void> => {
163+
const { timeout = 1000, interval = 10, message } = options;
164+
const startTime = Date.now();
165+
166+
while (Date.now() - startTime < timeout) {
167+
try {
168+
expectFn();
169+
// If we get here, the expect passed, so we can return
170+
return;
171+
} catch (error) {
172+
// Expect failed, continue retrying
173+
if (Date.now() - startTime >= timeout) {
174+
// Timeout reached, re-throw the last error
175+
throw error;
176+
}
177+
}
178+
179+
// Wait before next retry
180+
// eslint-disable-next-line no-await-in-loop
181+
await new Promise<void>((resolve) => {
182+
setTimeout(resolve, interval);
183+
});
184+
}
185+
186+
// Timeout reached, throw error with descriptive message
187+
const defaultMessage = `Expect condition not met within ${timeout}ms`;
188+
throw new Error(message || defaultMessage);
189+
};
190+
147191
setConfig('helper');

react_on_rails_pro/packages/node-renderer/tests/incrementalRender.test.ts

Lines changed: 45 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ import path from 'path';
44
import worker, { disableHttp2 } from '../src/worker';
55
import packageJson from '../src/shared/packageJson';
66
import * as incremental from '../src/worker/handleIncrementalRenderRequest';
7-
import { createVmBundle, BUNDLE_TIMESTAMP } from './helper';
7+
import { createVmBundle, BUNDLE_TIMESTAMP, waitFor } from './helper';
88
import type { ResponseResult } from '../src/shared/utils';
99

1010
// Disable HTTP/2 for testing like other tests do
@@ -114,16 +114,6 @@ describe('incremental render NDJSON endpoint', () => {
114114
});
115115
};
116116

117-
const waitForProcessing = (ms = 50) =>
118-
new Promise<void>((resolve) => {
119-
setTimeout(resolve, ms);
120-
});
121-
122-
const waitForSinkEnd = (ms = 10) =>
123-
new Promise<void>((resolve) => {
124-
setTimeout(resolve, ms);
125-
});
126-
127117
beforeAll(async () => {
128118
await app.ready();
129119
await app.listen({ port: 0 });
@@ -162,8 +152,10 @@ describe('incremental render NDJSON endpoint', () => {
162152
const initialObj = createInitialObject(SERVER_BUNDLE_TIMESTAMP);
163153
req.write(`${JSON.stringify(initialObj)}\n`);
164154

165-
// Wait a brief moment for the server to process the first object
166-
await waitForProcessing();
155+
// Wait for the server to process the first object
156+
await waitFor(() => {
157+
expect(handleSpy).toHaveBeenCalledTimes(1);
158+
});
167159

168160
// Verify handleIncrementalRenderRequest was called immediately after first chunk
169161
expect(handleSpy).toHaveBeenCalledTimes(1);
@@ -182,9 +174,11 @@ describe('incremental render NDJSON endpoint', () => {
182174
// Write the chunk
183175
req.write(`${JSON.stringify(chunk)}\n`);
184176

185-
// Wait a brief moment for processing
177+
// Wait for the chunk to be processed
186178
// eslint-disable-next-line no-await-in-loop
187-
await waitForProcessing();
179+
await waitFor(() => {
180+
expect(sinkAddCalls).toHaveLength(expectedCallsBeforeWrite + 1);
181+
});
188182

189183
// Verify the chunk was processed immediately
190184
expect(sinkAddCalls).toHaveLength(expectedCallsBeforeWrite + 1);
@@ -197,7 +191,9 @@ describe('incremental render NDJSON endpoint', () => {
197191
await responsePromise;
198192

199193
// Wait for the sink.end to be called
200-
await waitForSinkEnd();
194+
await waitFor(() => {
195+
expect(sinkEnd).toHaveBeenCalledTimes(1);
196+
});
201197

202198
// Final verification: all chunks were processed in the correct order
203199
expect(handleSpy).toHaveBeenCalledTimes(1);
@@ -282,8 +278,10 @@ describe('incremental render NDJSON endpoint', () => {
282278
const initialObj = createInitialObject(SERVER_BUNDLE_TIMESTAMP);
283279
req.write(`${JSON.stringify(initialObj)}\n`);
284280

285-
// Wait a brief moment for the server to process the first object
286-
await waitForProcessing();
281+
// Wait for the server to process the first object and set up the response
282+
await waitFor(() => {
283+
expect(handleSpy).toHaveBeenCalledTimes(1);
284+
});
287285

288286
// Verify handleIncrementalRenderRequest was called
289287
expect(handleSpy).toHaveBeenCalledTimes(1);
@@ -292,7 +290,9 @@ describe('incremental render NDJSON endpoint', () => {
292290
req.write(`${JSON.stringify({ a: 1 })}\n`);
293291

294292
// Wait for processing
295-
await waitForProcessing();
293+
await waitFor(() => {
294+
expect(sinkAddCalls).toHaveLength(1);
295+
});
296296

297297
// Verify the valid chunk was processed
298298
expect(sinkAddCalls).toHaveLength(1);
@@ -309,15 +309,20 @@ describe('incremental render NDJSON endpoint', () => {
309309
await responsePromise;
310310

311311
// Wait for the sink.end to be called
312-
await waitForSinkEnd();
312+
await waitFor(() => {
313+
expect(sinkEnd).toHaveBeenCalledTimes(1);
314+
});
313315

314316
// Verify that processing continued after the malformed chunk
315317
// The malformed chunk should be skipped, but valid chunks should be processed
316-
expect(sinkAddCalls).toEqual([{ a: 1 }, { d: 4 }]);
317318

318319
// Verify that the stream completed successfully
319-
expect(sinkEnd).toHaveBeenCalledTimes(1);
320-
expect(sinkAbort).not.toHaveBeenCalled();
320+
await waitFor(() => {
321+
expect(sinkAddCalls).toEqual([{ a: 1 }, { d: 4 }]);
322+
expect(sinkEnd).toHaveBeenCalledTimes(1);
323+
expect(sinkAbort).not.toHaveBeenCalled();
324+
});
325+
console.log('sinkAddCalls');
321326
});
322327

323328
test('handles empty lines gracefully in the stream', async () => {
@@ -348,7 +353,9 @@ describe('incremental render NDJSON endpoint', () => {
348353
req.write(`${JSON.stringify(initialObj)}\n`);
349354

350355
// Wait for processing
351-
await waitForProcessing();
356+
await waitFor(() => {
357+
expect(handleSpy).toHaveBeenCalledTimes(1);
358+
});
352359

353360
// Send chunks with empty lines mixed in
354361
req.write('\n'); // Empty line
@@ -363,7 +370,9 @@ describe('incremental render NDJSON endpoint', () => {
363370
await responsePromise;
364371

365372
// Wait for the sink.end to be called
366-
await waitForSinkEnd();
373+
await waitFor(() => {
374+
expect(sinkEnd).toHaveBeenCalledTimes(1);
375+
});
367376

368377
// Verify that only valid JSON objects were processed
369378
expect(handleSpy).toHaveBeenCalledTimes(1);
@@ -446,11 +455,13 @@ describe('incremental render NDJSON endpoint', () => {
446455
// Track processed chunks to verify immediate processing
447456
const processedChunks: unknown[] = [];
448457

458+
const sinkAdd = jest.fn();
449459
// Create a sink that records processed chunks
450460
const sink: incremental.IncrementalRenderSink = {
451461
add: (chunk) => {
452462
console.log('Sink.add called with chunk:', chunk);
453463
processedChunks.push(chunk);
464+
sinkAdd(chunk);
454465
},
455466
end: jest.fn(),
456467
abort: jest.fn(),
@@ -511,7 +522,9 @@ describe('incremental render NDJSON endpoint', () => {
511522
req.write(`${JSON.stringify(initialObj)}\n`);
512523

513524
// Wait for the server to process the first object and set up the response
514-
await waitForProcessing(100);
525+
await waitFor(() => {
526+
expect(handleSpy).toHaveBeenCalledTimes(1);
527+
});
515528

516529
// Verify handleIncrementalRenderRequest was called
517530
expect(handleSpy).toHaveBeenCalledTimes(1);
@@ -526,7 +539,9 @@ describe('incremental render NDJSON endpoint', () => {
526539
for (const chunk of chunksToSend) {
527540
req.write(`${JSON.stringify(chunk)}\n`);
528541
// eslint-disable-next-line no-await-in-loop
529-
await waitForProcessing(10);
542+
await waitFor(() => {
543+
expect(sinkAdd).toHaveBeenCalledWith(chunk);
544+
});
530545
}
531546

532547
// End the request
@@ -558,6 +573,8 @@ describe('incremental render NDJSON endpoint', () => {
558573
expect(handleSpy).toHaveBeenCalledTimes(1);
559574
console.log('handleSpy done');
560575

561-
await waitForSinkEnd();
576+
await waitFor(() => {
577+
expect(sink.end).toHaveBeenCalled();
578+
});
562579
});
563580
});

0 commit comments

Comments
 (0)