Skip to content

Commit 8170fd8

Browse files
AbanoubGhadbanAbanoubGhadban
authored
Fix incremental render tests (#2032)
Fix tests and refactor fixtures after NDJSON renderer changes - Fix request_spec.rb, handleRenderRequest, and serverRenderRSCReactComponent tests for new response structure - Separate incremental render fixtures from base test fixtures to prevent interference - Remove obsolete promise-based incremental render tests - Refactor VM bundle creation to use serverBundleCachePath - Clean up unneeded buildConfig call
1 parent 8a391e7 commit 8170fd8

File tree

13 files changed

+217
-306
lines changed

13 files changed

+217
-306
lines changed

‎react_on_rails_pro/packages/node-renderer/src/worker/checkProtocolVersionHandler.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ function normalizeVersion(version: string): string {
3434
return normalized;
3535
}
3636

37-
interface RequestBody {
37+
export interface RequestBody {
3838
protocolVersion?: string;
3939
gemVersion?: string;
4040
railsEnv?: string;

‎react_on_rails_pro/packages/node-renderer/src/worker/requestPrechecks.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,10 +3,10 @@
33
* @module worker/requestPrechecks
44
*/
55
import type { ResponseResult } from '../shared/utils';
6-
import { checkProtocolVersion, type ProtocolVersionBody } from './checkProtocolVersionHandler';
6+
import { checkProtocolVersion, type RequestBody } from './checkProtocolVersionHandler';
77
import { authenticate, type AuthBody } from './authHandler';
88

9-
export interface RequestPrechecksBody extends ProtocolVersionBody, AuthBody {
9+
export interface RequestPrechecksBody extends RequestBody, AuthBody {
1010
[key: string]: unknown;
1111
}
1212

‎react_on_rails_pro/packages/node-renderer/tests/fixtures/bundle-incremental.js‎

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
const { PassThrough } = require('stream');
2+
3+
global.ReactOnRails = {
4+
dummy: { html: 'Dummy Object' },
5+
6+
// Get or create stream
7+
getStreamValues: function () {
8+
if (!sharedExecutionContext.has('stream')) {
9+
const stream = new PassThrough();
10+
sharedExecutionContext.set('stream', { stream });
11+
}
12+
return sharedExecutionContext.get('stream').stream;
13+
},
14+
15+
// Add value to stream
16+
addStreamValue: function (value) {
17+
if (!sharedExecutionContext.has('stream')) {
18+
// Create the stream first if it doesn't exist
19+
ReactOnRails.getStreamValues();
20+
}
21+
const { stream } = sharedExecutionContext.get('stream');
22+
stream.write(value);
23+
return value;
24+
},
25+
26+
endStream: function () {
27+
if (sharedExecutionContext.has('stream')) {
28+
const { stream } = sharedExecutionContext.get('stream');
29+
stream.end();
30+
}
31+
},
32+
33+
// Clear all stream values
34+
clearStreamValues: function () {
35+
if (sharedExecutionContext.has('stream')) {
36+
const { stream } = sharedExecutionContext.get('stream');
37+
stream.destroy();
38+
sharedExecutionContext.delete('stream');
39+
}
40+
},
41+
};

‎react_on_rails_pro/packages/node-renderer/tests/fixtures/bundle.js‎

Lines changed: 0 additions & 54 deletions
Original file line numberDiff line numberDiff line change
@@ -1,57 +1,3 @@
1-
const { PassThrough } = require('stream');
2-
31
global.ReactOnRails = {
42
dummy: { html: 'Dummy Object' },
5-
6-
// Get or create async value promise
7-
getAsyncValue: function() {
8-
debugger;
9-
if (!sharedExecutionContext.has('asyncPromise')) {
10-
const promiseData = {};
11-
const promise = new Promise((resolve, reject) => {
12-
promiseData.resolve = resolve;
13-
promiseData.reject = reject;
14-
});
15-
promiseData.promise = promise;
16-
sharedExecutionContext.set('asyncPromise', promiseData);
17-
}
18-
return sharedExecutionContext.get('asyncPromise').promise;
19-
},
20-
21-
// Resolve the async value promise
22-
setAsyncValue: function(value) {
23-
debugger;
24-
if (!sharedExecutionContext.has('asyncPromise')) {
25-
ReactOnRails.getAsyncValue();
26-
}
27-
const promiseData = sharedExecutionContext.get('asyncPromise');
28-
promiseData.resolve(value);
29-
},
30-
31-
// Get or create stream
32-
getStreamValues: function() {
33-
if (!sharedExecutionContext.has('stream')) {
34-
const stream = new PassThrough();
35-
sharedExecutionContext.set('stream', { stream });
36-
}
37-
return sharedExecutionContext.get('stream').stream;
38-
},
39-
40-
// Add value to stream
41-
addStreamValue: function(value) {
42-
if (!sharedExecutionContext.has('stream')) {
43-
// Create the stream first if it doesn't exist
44-
ReactOnRails.getStreamValues();
45-
}
46-
const { stream } = sharedExecutionContext.get('stream');
47-
stream.write(value);
48-
return value;
49-
},
50-
51-
endStream: function() {
52-
if (sharedExecutionContext.has('stream')) {
53-
const { stream } = sharedExecutionContext.get('stream');
54-
stream.end();
55-
}
56-
},
573
};

‎react_on_rails_pro/packages/node-renderer/tests/fixtures/secondary-bundle-incremental.js‎

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,40 @@
1+
const { PassThrough } = require('stream');
2+
3+
global.ReactOnRails = {
4+
dummy: { html: 'Dummy Object from secondary bundle' },
5+
6+
// Get or create stream
7+
getStreamValues: function () {
8+
if (!sharedExecutionContext.has('secondaryStream')) {
9+
const stream = new PassThrough();
10+
sharedExecutionContext.set('secondaryStream', { stream });
11+
}
12+
return sharedExecutionContext.get('secondaryStream').stream;
13+
},
14+
15+
// Add value to stream
16+
addStreamValue: function (value) {
17+
if (!sharedExecutionContext.has('secondaryStream')) {
18+
// Create the stream first if it doesn't exist
19+
ReactOnRails.getStreamValues();
20+
}
21+
const { stream } = sharedExecutionContext.get('secondaryStream');
22+
stream.write(value);
23+
},
24+
25+
endStream: function () {
26+
if (sharedExecutionContext.has('secondaryStream')) {
27+
const { stream } = sharedExecutionContext.get('secondaryStream');
28+
stream.end();
29+
}
30+
},
31+
32+
// Clear all stream values
33+
clearStreamValues: function () {
34+
if (sharedExecutionContext.has('secondaryStream')) {
35+
const { stream } = sharedExecutionContext.get('secondaryStream');
36+
stream.destroy();
37+
sharedExecutionContext.delete('secondaryStream');
38+
}
39+
},
40+
};

‎react_on_rails_pro/packages/node-renderer/tests/fixtures/secondary-bundle.js‎

Lines changed: 0 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -1,53 +1,3 @@
11
global.ReactOnRails = {
22
dummy: { html: 'Dummy Object from secondary bundle' },
3-
4-
5-
// Get or create async value promise
6-
getAsyncValue: function() {
7-
if (!sharedExecutionContext.has('secondaryAsyncPromise')) {
8-
const promiseData = {};
9-
const promise = new Promise((resolve, reject) => {
10-
promiseData.resolve = resolve;
11-
promiseData.reject = reject;
12-
});
13-
promiseData.promise = promise;
14-
sharedExecutionContext.set('secondaryAsyncPromise', promiseData);
15-
}
16-
return sharedExecutionContext.get('secondaryAsyncPromise').promise;
17-
},
18-
19-
// Resolve the async value promise
20-
setAsyncValue: function(value) {
21-
if (!sharedExecutionContext.has('secondaryAsyncPromise')) {
22-
ReactOnRails.getAsyncValue();
23-
}
24-
const promiseData = sharedExecutionContext.get('secondaryAsyncPromise');
25-
promiseData.resolve(value);
26-
},
27-
28-
// Get or create stream
29-
getStreamValues: function() {
30-
if (!sharedExecutionContext.has('secondaryStream')) {
31-
const stream = new PassThrough();
32-
sharedExecutionContext.set('secondaryStream', { stream });
33-
}
34-
return sharedExecutionContext.get('secondaryStream').stream;
35-
},
36-
37-
// Add value to stream
38-
addStreamValue: function(value) {
39-
if (!sharedExecutionContext.has('secondaryStream')) {
40-
// Create the stream first if it doesn't exist
41-
ReactOnRails.getStreamValues();
42-
}
43-
const { stream } = sharedExecutionContext.get('secondaryStream');
44-
stream.write(value);
45-
},
46-
47-
endStream: function() {
48-
if (sharedExecutionContext.has('secondaryStream')) {
49-
const { stream } = sharedExecutionContext.get('secondaryStream');
50-
stream.end();
51-
}
52-
},
533
};

‎react_on_rails_pro/packages/node-renderer/tests/handleRenderRequest.test.ts‎

Lines changed: 13 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -78,7 +78,7 @@ describe(testName, () => {
7878
],
7979
});
8080

81-
expect(result).toEqual(renderResult);
81+
expect(result.response).toEqual(renderResult);
8282
expect(
8383
hasVMContextForBundle(path.resolve(__dirname, `./tmp/${testName}/1495063024898/1495063024898.js`)),
8484
).toBeTruthy();
@@ -92,7 +92,7 @@ describe(testName, () => {
9292
bundleTimestamp: BUNDLE_TIMESTAMP,
9393
});
9494

95-
expect(result).toEqual({
95+
expect(result.response).toEqual({
9696
status: 410,
9797
headers: { 'Cache-Control': 'no-cache, no-store, max-age=0, must-revalidate' },
9898
data: 'No bundle uploaded',
@@ -108,7 +108,7 @@ describe(testName, () => {
108108
bundleTimestamp: BUNDLE_TIMESTAMP,
109109
});
110110

111-
expect(result).toEqual(renderResult);
111+
expect(result.response).toEqual(renderResult);
112112
});
113113

114114
test('If lockfile exists, and is stale', async () => {
@@ -133,7 +133,7 @@ describe(testName, () => {
133133
],
134134
});
135135

136-
expect(result).toEqual(renderResult);
136+
expect(result.response).toEqual(renderResult);
137137
expect(
138138
hasVMContextForBundle(path.resolve(__dirname, `./tmp/${testName}/1495063024898/1495063024898.js`)),
139139
).toBeTruthy();
@@ -165,7 +165,7 @@ describe(testName, () => {
165165
],
166166
});
167167

168-
expect(result).toEqual(renderResult);
168+
expect(result.response).toEqual(renderResult);
169169
expect(
170170
hasVMContextForBundle(path.resolve(__dirname, `./tmp/${testName}/1495063024898/1495063024898.js`)),
171171
).toBeTruthy();
@@ -199,7 +199,7 @@ describe(testName, () => {
199199
],
200200
});
201201

202-
expect(result).toEqual(renderResult);
202+
expect(result.response).toEqual(renderResult);
203203
// only the primary bundle should be in the VM context
204204
// The secondary bundle will be processed only if the rendering request requests it
205205
expect(
@@ -254,7 +254,7 @@ describe(testName, () => {
254254
assetsToCopy: additionalAssets,
255255
});
256256

257-
expect(result).toEqual(renderResult);
257+
expect(result.response).toEqual(renderResult);
258258

259259
// Only the primary bundle should be in the VM context
260260
// The secondary bundle will be processed only if the rendering request requests it
@@ -310,7 +310,7 @@ describe(testName, () => {
310310
dependencyBundleTimestamps: [SECONDARY_BUNDLE_TIMESTAMP],
311311
});
312312

313-
expect(result).toEqual({
313+
expect(result.response).toEqual({
314314
status: 410,
315315
headers: { 'Cache-Control': 'no-cache, no-store, max-age=0, must-revalidate' },
316316
data: 'No bundle uploaded',
@@ -328,7 +328,7 @@ describe(testName, () => {
328328
dependencyBundleTimestamps: [SECONDARY_BUNDLE_TIMESTAMP],
329329
});
330330

331-
expect(result).toEqual(renderResult);
331+
expect(result.response).toEqual(renderResult);
332332
});
333333

334334
test('rendering request can call runOnOtherBundle', async () => {
@@ -348,7 +348,7 @@ describe(testName, () => {
348348
dependencyBundleTimestamps: [SECONDARY_BUNDLE_TIMESTAMP],
349349
});
350350

351-
expect(result).toEqual(renderResultFromBothBundles);
351+
expect(result.response).toEqual(renderResultFromBothBundles);
352352
// Both bundles should be in the VM context
353353
expect(
354354
hasVMContextForBundle(path.resolve(__dirname, `./tmp/${testName}/1495063024898/1495063024898.js`)),
@@ -370,7 +370,7 @@ describe(testName, () => {
370370
bundleTimestamp: BUNDLE_TIMESTAMP,
371371
});
372372

373-
expect(result).toEqual({
373+
expect(result.response).toEqual({
374374
status: 200,
375375
headers: { 'Cache-Control': 'public, max-age=31536000' },
376376
data: renderingRequest,
@@ -402,7 +402,7 @@ describe(testName, () => {
402402
bundleTimestamp: BUNDLE_TIMESTAMP,
403403
});
404404

405-
expect(result).toEqual({
405+
expect(result.response).toEqual({
406406
status: 200,
407407
headers: { 'Cache-Control': 'public, max-age=31536000' },
408408
data: JSON.stringify('undefined'),
@@ -420,7 +420,7 @@ describe(testName, () => {
420420
dependencyBundleTimestamps: [SECONDARY_BUNDLE_TIMESTAMP],
421421
});
422422

423-
expect(result).toEqual({
423+
expect(result.response).toEqual({
424424
status: 410,
425425
headers: { 'Cache-Control': 'no-cache, no-store, max-age=0, must-revalidate' },
426426
data: 'No bundle uploaded',

0 commit comments

Comments
 (0)