Skip to content

Commit acbff91

Browse files
committed
fix(worker): handle circular messages
1 parent 0ab070c commit acbff91

13 files changed

Lines changed: 330 additions & 30 deletions

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,7 @@
4848
- `[jest-transform]` [**BREAKING**] Refactor API to pass an options bag around rather than multiple boolean options ([#10753](https://github.com/facebook/jest/pull/10753))
4949
- `[jest-transform]` [**BREAKING**] Refactor API of transformers to pass an options bag rather than separate `config` and other options ([#10834](https://github.com/facebook/jest/pull/10834))
5050
- `[jest-worker]` [**BREAKING**] Use named exports ([#10623] (https://github.com/facebook/jest/pull/10623))
51+
- `[jest-worker]` Handle passing messages with circular data ([#10980] (https://github.com/facebook/jest/pull/10980))
5152
- `[pretty-format]` [**BREAKING**] Convert to ES Modules ([#10515](https://github.com/facebook/jest/pull/10515))
5253

5354
### Chore & Maintenance

e2e/Utils.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -225,7 +225,7 @@ const sortTests = (stdout: string) =>
225225
.reduce((tests: Array<Array<string>>, line) => {
226226
if (['RUNS', 'PASS', 'FAIL'].includes(line.slice(0, 4))) {
227227
tests.push([line.trimRight()]);
228-
} else if (line) {
228+
} else {
229229
tests[tests.length - 1].push(line.trimRight());
230230
}
231231
return tests;
Lines changed: 99 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,99 @@
1+
// Jest Snapshot v1, https://goo.gl/fbAQLP
2+
3+
exports[`handles circular inequality properly 1`] = `
4+
FAIL __tests__/test-1.js
5+
● test
6+
7+
expect(received).toEqual(expected) // deep equality
8+
9+
- Expected - 1
10+
+ Received + 3
11+
12+
- Object {}
13+
+ Object {
14+
+ "ref": [Circular],
15+
+ }
16+
17+
3 | foo.ref = foo;
18+
4 |
19+
> 5 | expect(foo).toEqual({});
20+
| ^
21+
6 | });
22+
7 |
23+
8 | it('test 2', () => {
24+
25+
at Object.toEqual (__tests__/test-1.js:5:15)
26+
27+
test 2
28+
29+
expect(received).toEqual(expected) // deep equality
30+
31+
- Expected - 1
32+
+ Received + 3
33+
34+
- Object {}
35+
+ Object {
36+
+ "ref": [Circular],
37+
+ }
38+
39+
10 | foo.ref = foo;
40+
11 |
41+
> 12 | expect(foo).toEqual({});
42+
| ^
43+
13 | });
44+
45+
at Object.toEqual (__tests__/test-1.js:12:15)
46+
47+
48+
FAIL __tests__/test-2.js
49+
● test
50+
51+
expect(received).toEqual(expected) // deep equality
52+
53+
- Expected - 1
54+
+ Received + 3
55+
56+
- Object {}
57+
+ Object {
58+
+ "ref": [Circular],
59+
+ }
60+
61+
3 | foo.ref = foo;
62+
4 |
63+
> 5 | expect(foo).toEqual({});
64+
| ^
65+
6 | });
66+
7 |
67+
8 | it('test 2', () => {
68+
69+
at Object.toEqual (__tests__/test-2.js:5:15)
70+
71+
test 2
72+
73+
expect(received).toEqual(expected) // deep equality
74+
75+
- Expected - 1
76+
+ Received + 3
77+
78+
- Object {}
79+
+ Object {
80+
+ "ref": [Circular],
81+
+ }
82+
83+
10 | foo.ref = foo;
84+
11 |
85+
> 12 | expect(foo).toEqual({});
86+
| ^
87+
13 | });
88+
89+
at Object.toEqual (__tests__/test-2.js:12:15)
90+
91+
`;
92+
93+
exports[`handles circular inequality properly 2`] = `
94+
Test Suites: 2 failed, 2 total
95+
Tests: 4 failed, 4 total
96+
Snapshots: 0 total
97+
Time: <<REPLACED>>
98+
Ran all test suites.
99+
`;
Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,67 @@
1+
/**
2+
* Copyright (c) Facebook, Inc. and its affiliates. All Rights Reserved.
3+
*
4+
* This source code is licensed under the MIT license found in the
5+
* LICENSE file in the root directory of this source tree.
6+
*/
7+
8+
import {tmpdir} from 'os';
9+
import * as path from 'path';
10+
import {wrap} from 'jest-snapshot-serializer-raw';
11+
import {
12+
cleanup,
13+
createEmptyPackage,
14+
extractSortedSummary,
15+
writeFiles,
16+
} from '../Utils';
17+
import {runContinuous} from '../runJest';
18+
19+
const tempDir = path.resolve(tmpdir(), 'circular-inequality-test');
20+
21+
beforeEach(() => {
22+
createEmptyPackage(tempDir);
23+
});
24+
25+
afterEach(() => {
26+
cleanup(tempDir);
27+
});
28+
29+
test('handles circular inequality properly', async () => {
30+
const testFileContent = `
31+
it('test', () => {
32+
const foo = {};
33+
foo.ref = foo;
34+
35+
expect(foo).toEqual({});
36+
});
37+
38+
it('test 2', () => {
39+
const foo = {};
40+
foo.ref = foo;
41+
42+
expect(foo).toEqual({});
43+
});
44+
`;
45+
46+
writeFiles(tempDir, {
47+
'__tests__/test-1.js': testFileContent,
48+
'__tests__/test-2.js': testFileContent,
49+
});
50+
51+
const {end, waitUntil} = runContinuous(
52+
tempDir,
53+
['--no-watchman', '--watch-all'],
54+
// timeout in case the `waitUntil` below doesn't fire
55+
{timeout: 5000},
56+
);
57+
58+
await waitUntil(({stderr}) => {
59+
return stderr.includes('Ran all test suites.');
60+
});
61+
62+
const {stderr} = await end();
63+
64+
const {summary, rest} = extractSortedSummary(stderr);
65+
expect(wrap(rest)).toMatchSnapshot();
66+
expect(wrap(summary)).toMatchSnapshot();
67+
});

packages/jest-worker/package.json

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,8 @@
1616
"dependencies": {
1717
"@types/node": "*",
1818
"merge-stream": "^2.0.0",
19-
"supports-color": "^8.0.0"
19+
"supports-color": "^8.0.0",
20+
"telejson": "^5.1.0"
2021
},
2122
"devDependencies": {
2223
"@types/merge-stream": "^1.1.2",

packages/jest-worker/src/workers/ChildProcessWorker.ts

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ import {
2323
WorkerInterface,
2424
WorkerOptions,
2525
} from '../types';
26+
import {parse} from './utils';
2627

2728
const SIGNAL_BASE_EXIT_CODE = 128;
2829
const SIGKILL_EXIT_CODE = SIGNAL_BASE_EXIT_CODE + 9;
@@ -92,6 +93,9 @@ export default class ChildProcessWorker implements WorkerInterface {
9293
} as NodeJS.ProcessEnv,
9394
// Suppress --debug / --inspect flags while preserving others (like --harmony).
9495
execArgv: process.execArgv.filter(v => !/^--(debug|inspect)/.test(v)),
96+
// default to advanced serialization in order to match worker threads
97+
// @ts-expect-error
98+
serialization: 'advanced',
9599
silent: true,
96100
...this._options.forkOptions,
97101
});
@@ -162,7 +166,7 @@ export default class ChildProcessWorker implements WorkerInterface {
162166

163167
switch (response[0]) {
164168
case PARENT_MESSAGE_OK:
165-
this._onProcessEnd(null, response[1]);
169+
this._onProcessEnd(null, parse(response[1]));
166170
break;
167171

168172
case PARENT_MESSAGE_CLIENT_ERROR:
@@ -195,7 +199,7 @@ export default class ChildProcessWorker implements WorkerInterface {
195199
this._onProcessEnd(error, null);
196200
break;
197201
case PARENT_MESSAGE_CUSTOM:
198-
this._onCustomMessage(response[1]);
202+
this._onCustomMessage(parse(response[1]));
199203
break;
200204
default:
201205
throw new TypeError('Unexpected response from worker: ' + response[0]);

packages/jest-worker/src/workers/NodeThreadsWorker.ts

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ import {
2323
WorkerInterface,
2424
WorkerOptions,
2525
} from '../types';
26+
import {parse} from './utils';
2627

2728
export default class ExperimentalWorker implements WorkerInterface {
2829
private _worker!: Worker;
@@ -141,7 +142,7 @@ export default class ExperimentalWorker implements WorkerInterface {
141142

142143
switch (response[0]) {
143144
case PARENT_MESSAGE_OK:
144-
this._onProcessEnd(null, response[1]);
145+
this._onProcessEnd(null, parse(response[1]));
145146
break;
146147

147148
case PARENT_MESSAGE_CLIENT_ERROR:
@@ -175,7 +176,7 @@ export default class ExperimentalWorker implements WorkerInterface {
175176
this._onProcessEnd(error, null);
176177
break;
177178
case PARENT_MESSAGE_CUSTOM:
178-
this._onCustomMessage(response[1]);
179+
this._onCustomMessage(parse(response[1]));
179180
break;
180181
default:
181182
throw new TypeError('Unexpected response from worker: ' + response[0]);

packages/jest-worker/src/workers/__tests__/ChildProcessWorker.test.js

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,7 @@ it('passes fork options down to child_process.fork, adding the defaults', () =>
7070
env: {...process.env, FORCE_COLOR: supportsColor.stdout ? '1' : undefined}, // Default option.
7171
execArgv: ['-p'], // Filtered option.
7272
execPath: 'hello', // Added option.
73+
serialization: 'advanced', // Default option.
7374
silent: true, // Default option.
7475
});
7576
});

packages/jest-worker/src/workers/messageParent.ts

Lines changed: 22 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -6,32 +6,35 @@
66
*/
77

88
import {PARENT_MESSAGE_CUSTOM} from '../types';
9+
import {serialize} from './utils';
910

10-
const isWorkerThread = () => {
11+
const isWorkerThread: boolean = (() => {
1112
try {
1213
// `Require` here to support Node v10
13-
const {isMainThread, parentPort} = require('worker_threads');
14-
return !isMainThread && parentPort;
14+
const {
15+
isMainThread,
16+
parentPort,
17+
} = require('worker_threads') as typeof import('worker_threads');
18+
return !isMainThread && parentPort != null;
1519
} catch {
1620
return false;
1721
}
18-
};
22+
})();
1923

20-
const messageParent = (
24+
export default function messageParent(
2125
message: unknown,
22-
parentProcess: NodeJS.Process = process,
23-
): void => {
24-
try {
25-
if (isWorkerThread()) {
26-
// `Require` here to support Node v10
27-
const {parentPort} = require('worker_threads');
28-
parentPort.postMessage([PARENT_MESSAGE_CUSTOM, message]);
29-
} else if (typeof parentProcess.send === 'function') {
30-
parentProcess.send([PARENT_MESSAGE_CUSTOM, message]);
31-
}
32-
} catch {
26+
parentProcess = process,
27+
): void {
28+
if (isWorkerThread) {
29+
// `Require` here to support Node v10
30+
const {
31+
parentPort,
32+
} = require('worker_threads') as typeof import('worker_threads');
33+
// ! is safe due to `null` check in `isWorkerThread`
34+
parentPort!.postMessage([PARENT_MESSAGE_CUSTOM, serialize(message)]);
35+
} else if (typeof parentProcess.send === 'function') {
36+
parentProcess.send([PARENT_MESSAGE_CUSTOM, serialize(message)]);
37+
} else {
3338
throw new Error('"messageParent" can only be used inside a worker');
3439
}
35-
};
36-
37-
export default messageParent;
40+
}

packages/jest-worker/src/workers/processChild.ts

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ import {
1616
PARENT_MESSAGE_OK,
1717
PARENT_MESSAGE_SETUP_ERROR,
1818
} from '../types';
19+
import {serialize} from './utils';
1920

2021
let file: string | null = null;
2122
let setupArgs: Array<unknown> = [];
@@ -64,7 +65,7 @@ function reportSuccess(result: unknown) {
6465
throw new Error('Child can only be used on a forked process');
6566
}
6667

67-
process.send([PARENT_MESSAGE_OK, result]);
68+
process.send([PARENT_MESSAGE_OK, serialize(result)]);
6869
}
6970

7071
function reportClientError(error: Error) {
@@ -86,7 +87,7 @@ function reportError(error: Error, type: PARENT_MESSAGE_ERROR) {
8687

8788
process.send([
8889
type,
89-
error.constructor && error.constructor.name,
90+
error.constructor?.name,
9091
error.message,
9192
error.stack,
9293
typeof error === 'object' ? {...error} : error,

0 commit comments

Comments
 (0)