From 388bd20e4b61f99aff888fe6d2d875e8e17568a9 Mon Sep 17 00:00:00 2001 From: Dmitry Makhnev Date: Mon, 15 Jul 2024 13:21:02 +0200 Subject: [PATCH 1/9] ISSUE-10577: Fix the hang issue caused by failed assertions with circular values --- CHANGELOG.md | 2 + .../bigintInequality.test.ts.snap | 41 ++++++++++++++ e2e/__tests__/bigintInequality.test.ts | 54 +++++++++++++++++++ e2e/__tests__/circularInequality.test.ts | 2 +- packages/jest-worker/package.json | 2 + .../src/workers/ChildProcessWorker.ts | 5 +- .../src/workers/NodeThreadsWorker.ts | 5 +- .../workers/__tests__/WorkerEdgeCases.test.ts | 7 ++- .../workers/__tests__/processChild.test.ts | 32 +++++++++++ .../jest-worker/src/workers/messageParent.ts | 17 +++++- .../jest-worker/src/workers/processChild.ts | 17 +++++- .../src/workers/safeMessageTransferring.ts | 42 +++++++++++++++ yarn.lock | 9 ++++ 13 files changed, 227 insertions(+), 8 deletions(-) create mode 100644 e2e/__tests__/__snapshots__/bigintInequality.test.ts.snap create mode 100644 e2e/__tests__/bigintInequality.test.ts create mode 100644 packages/jest-worker/src/workers/safeMessageTransferring.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index 60d0cb5a79c0..ba649373fa16 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -77,6 +77,8 @@ - [**BREAKING**] `--testPathPattern` is now `--testPathPatterns` - [**BREAKING**] Specifying `testPathPatterns` when programmatically calling `watch` must be specified as `new TestPathPatterns(patterns)`, where `TestPathPatterns` can be imported from `@jest/pattern` - `[jest-reporters, jest-runner]` Unhandled errors without stack get correctly logged to console ([#14619](https://github.com/jestjs/jest/pull/14619)) +- `[jest-worker]` Properly handle a circular reference error when worker tries to send an assertion fails where either the expected or actual value is circular +- `[jest-worker]` Properly handle a BigInt when worker tries to send an assertion fails where either the expected or actual value is BigInt ### Performance diff --git a/e2e/__tests__/__snapshots__/bigintInequality.test.ts.snap b/e2e/__tests__/__snapshots__/bigintInequality.test.ts.snap new file mode 100644 index 000000000000..4439115ce4af --- /dev/null +++ b/e2e/__tests__/__snapshots__/bigintInequality.test.ts.snap @@ -0,0 +1,41 @@ +// Jest Snapshot v1, https://goo.gl/fbAQLP + +exports[`handles circular inequality properly 1`] = ` +"FAIL __tests__/test-1.js + ● test + + expect(received).toBe(expected) // Object.is equality + + Expected: 73n + Received: 42n + + 1 | it('test', () => { + > 2 | expect(BigInt(42)).toBe(BigInt(73)); + | ^ + 3 | }); + + at Object.toBe (__tests__/test-1.js:2:22) + +FAIL __tests__/test-2.js + ● test + + expect(received).toBe(expected) // Object.is equality + + Expected: 73n + Received: 42n + + 1 | it('test', () => { + > 2 | expect(BigInt(42)).toBe(BigInt(73)); + | ^ + 3 | }); + + at Object.toBe (__tests__/test-2.js:2:22)" +`; + +exports[`handles circular inequality properly 2`] = ` +"Test Suites: 2 failed, 2 total +Tests: 2 failed, 2 total +Snapshots: 0 total +Time: <> +Ran all test suites." +`; diff --git a/e2e/__tests__/bigintInequality.test.ts b/e2e/__tests__/bigintInequality.test.ts new file mode 100644 index 000000000000..0462ea7df268 --- /dev/null +++ b/e2e/__tests__/bigintInequality.test.ts @@ -0,0 +1,54 @@ +/** + * Copyright (c) Meta Platforms, Inc. and affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + */ + +import {tmpdir} from 'os'; +import * as path from 'path'; +import { + cleanup, + createEmptyPackage, + extractSortedSummary, + writeFiles, +} from '../Utils'; +import {runContinuous} from '../runJest'; + +const tempDir = path.resolve(tmpdir(), 'bigint-inequality-test'); + +beforeEach(() => { + createEmptyPackage(tempDir); +}); + +afterEach(() => { + cleanup(tempDir); +}); + +test('handles circular inequality properly', async () => { + const testFileContent = ` + it('test', () => { + expect(BigInt(42)).toBe(BigInt(73)); + }); + `; + + writeFiles(tempDir, { + '__tests__/test-1.js': testFileContent, + '__tests__/test-2.js': testFileContent, + }); + + const {end, waitUntil} = runContinuous( + tempDir, + ['--no-watchman', '--watch-all'], + // timeout in case the `waitUntil` below doesn't fire + {stripAnsi: true, timeout: 5000}, + ); + + await waitUntil(({stderr}) => stderr.includes('Ran all test suites.')); + + const {stderr} = await end(); + + const {summary, rest} = extractSortedSummary(stderr); + expect(rest).toMatchSnapshot(); + expect(summary).toMatchSnapshot(); +}); diff --git a/e2e/__tests__/circularInequality.test.ts b/e2e/__tests__/circularInequality.test.ts index f8ab16b7ec29..812e380a75d1 100644 --- a/e2e/__tests__/circularInequality.test.ts +++ b/e2e/__tests__/circularInequality.test.ts @@ -25,7 +25,7 @@ afterEach(() => { cleanup(tempDir); }); -test.skip('handles circular inequality properly', async () => { +test('handles circular inequality properly', async () => { const testFileContent = ` it('test', () => { const foo = {}; diff --git a/packages/jest-worker/package.json b/packages/jest-worker/package.json index ad9c71bb499d..1ff6a57a6075 100644 --- a/packages/jest-worker/package.json +++ b/packages/jest-worker/package.json @@ -20,6 +20,7 @@ }, "dependencies": { "@types/node": "*", + "@ungap/structured-clone": "^1.2.0", "jest-util": "workspace:*", "merge-stream": "^2.0.0", "supports-color": "^8.0.0" @@ -28,6 +29,7 @@ "@babel/core": "^7.11.6", "@types/merge-stream": "^1.1.2", "@types/supports-color": "^8.1.0", + "@types/ungap__structured-clone": "^1.2.0", "get-stream": "^6.0.0", "jest-leak-detector": "workspace:*", "worker-farm": "^1.6.0" diff --git a/packages/jest-worker/src/workers/ChildProcessWorker.ts b/packages/jest-worker/src/workers/ChildProcessWorker.ts index 249cfb8abd88..61a714c8e4c1 100644 --- a/packages/jest-worker/src/workers/ChildProcessWorker.ts +++ b/packages/jest-worker/src/workers/ChildProcessWorker.ts @@ -27,6 +27,7 @@ import { WorkerStates, } from '../types'; import WorkerAbstract from './WorkerAbstract'; +import {unpackMessage} from './safeMessageTransferring'; const SIGNAL_BASE_EXIT_CODE = 128; const SIGKILL_EXIT_CODE = SIGNAL_BASE_EXIT_CODE + 9; @@ -263,7 +264,7 @@ export default class ChildProcessWorker switch (response[0]) { case PARENT_MESSAGE_OK: - this._onProcessEnd(null, response[1]); + this._onProcessEnd(null, unpackMessage(response[1])); break; case PARENT_MESSAGE_CLIENT_ERROR: @@ -297,7 +298,7 @@ export default class ChildProcessWorker break; case PARENT_MESSAGE_CUSTOM: - this._onCustomMessage(response[1]); + this._onCustomMessage(unpackMessage(response[1])); break; case PARENT_MESSAGE_MEM_USAGE: diff --git a/packages/jest-worker/src/workers/NodeThreadsWorker.ts b/packages/jest-worker/src/workers/NodeThreadsWorker.ts index 111353f6c436..e5b68478ebf6 100644 --- a/packages/jest-worker/src/workers/NodeThreadsWorker.ts +++ b/packages/jest-worker/src/workers/NodeThreadsWorker.ts @@ -26,6 +26,7 @@ import { WorkerStates, } from '../types'; import WorkerAbstract from './WorkerAbstract'; +import {unpackMessage} from './safeMessageTransferring'; export default class ExperimentalWorker extends WorkerAbstract @@ -177,7 +178,7 @@ export default class ExperimentalWorker switch (response[0]) { case PARENT_MESSAGE_OK: - this._onProcessEnd(null, response[1]); + this._onProcessEnd(null, unpackMessage(response[1])); break; case PARENT_MESSAGE_CLIENT_ERROR: @@ -213,7 +214,7 @@ export default class ExperimentalWorker break; case PARENT_MESSAGE_CUSTOM: - this._onCustomMessage(response[1]); + this._onCustomMessage(unpackMessage(response[1])); break; case PARENT_MESSAGE_MEM_USAGE: diff --git a/packages/jest-worker/src/workers/__tests__/WorkerEdgeCases.test.ts b/packages/jest-worker/src/workers/__tests__/WorkerEdgeCases.test.ts index b616e61427fd..04bdfdc63589 100644 --- a/packages/jest-worker/src/workers/__tests__/WorkerEdgeCases.test.ts +++ b/packages/jest-worker/src/workers/__tests__/WorkerEdgeCases.test.ts @@ -20,7 +20,12 @@ import ThreadsWorker from '../NodeThreadsWorker'; jest.setTimeout(10_000); const root = join('../../'); -const filesToBuild = ['workers/processChild', 'workers/threadChild', 'types']; +const filesToBuild = [ + 'workers/processChild', + 'workers/threadChild', + 'workers/safeMessageTransferring', + 'types', +]; const writeDestination = join(__dirname, '__temp__'); const processChildWorkerPath = join( writeDestination, diff --git a/packages/jest-worker/src/workers/__tests__/processChild.test.ts b/packages/jest-worker/src/workers/__tests__/processChild.test.ts index 09973f5758a6..e7fa076ce349 100644 --- a/packages/jest-worker/src/workers/__tests__/processChild.test.ts +++ b/packages/jest-worker/src/workers/__tests__/processChild.test.ts @@ -41,6 +41,12 @@ beforeEach(() => { mockCount++; return { + fooCircularResult() { + const circular = {self: undefined as unknown}; + circular.self = circular; + return {error: circular}; + }, + fooPromiseThrows() { return new Promise((_resolve, reject) => { setTimeout(() => reject(mockError), 5); @@ -338,6 +344,32 @@ it('returns results when it gets resolved if function is asynchronous', async () expect(spyProcessSend).toHaveBeenCalledTimes(2); }); +it('returns results with circular references', () => { + process.emit( + 'message', + [ + CHILD_MESSAGE_INITIALIZE, + true, // Not really used here, but for type purity. + './my-fancy-worker', + ], + null, + ); + + process.emit( + 'message', + [ + CHILD_MESSAGE_CALL, + true, // Not really used here, but for type purity. + 'fooCircularResult', + [], + ], + null, + ); + + const processCallError = spyProcessSend.mock.calls[0][0][1].error; + expect(processCallError.self).toBe(processCallError.self.self); +}); + it('calls the main module if the method call is "default"', () => { process.emit( 'message', diff --git a/packages/jest-worker/src/workers/messageParent.ts b/packages/jest-worker/src/workers/messageParent.ts index 1c0697c9b37c..e4ba35e60102 100644 --- a/packages/jest-worker/src/workers/messageParent.ts +++ b/packages/jest-worker/src/workers/messageParent.ts @@ -7,6 +7,7 @@ import {isMainThread, parentPort} from 'worker_threads'; import {PARENT_MESSAGE_CUSTOM} from '../types'; +import {packMessage} from './safeMessageTransferring'; export default function messageParent( message: unknown, @@ -15,7 +16,21 @@ export default function messageParent( if (!isMainThread && parentPort != null) { parentPort.postMessage([PARENT_MESSAGE_CUSTOM, message]); } else if (typeof parentProcess.send === 'function') { - parentProcess.send([PARENT_MESSAGE_CUSTOM, message]); + try { + parentProcess.send([PARENT_MESSAGE_CUSTOM, message]); + } catch (error: unknown) { + if ( + error instanceof Error && + // if .send is a function, it's a serialization issue + !error.message.includes('.send is not a function') + ) { + // Apply specific serialization only in error cases + // to avoid affecting performance in regular cases. + parentProcess.send([PARENT_MESSAGE_CUSTOM, packMessage(message)]); + } else { + throw error; + } + } } else { throw new TypeError('"messageParent" can only be used inside a worker'); } diff --git a/packages/jest-worker/src/workers/processChild.ts b/packages/jest-worker/src/workers/processChild.ts index 15728b5b877f..2660d50c0779 100644 --- a/packages/jest-worker/src/workers/processChild.ts +++ b/packages/jest-worker/src/workers/processChild.ts @@ -21,6 +21,7 @@ import { PARENT_MESSAGE_SETUP_ERROR, type ParentMessageMemUsage, } from '../types'; +import {packMessage} from './safeMessageTransferring'; type UnknownFunction = (...args: Array) => unknown | Promise; @@ -97,7 +98,21 @@ function reportSuccess(result: unknown) { throw new Error('Child can only be used on a forked process'); } - process.send([PARENT_MESSAGE_OK, result]); + try { + process.send([PARENT_MESSAGE_OK, result]); + } catch (error: unknown) { + if ( + error instanceof Error && + // if .send is a function, it's a serialization issue + !error.message.includes('.send is not a function') + ) { + // Apply specific serialization only in error cases + // to avoid affecting performance in regular cases. + process.send([PARENT_MESSAGE_OK, packMessage(result)]); + } else { + throw error; + } + } } function reportClientError(error: Error) { diff --git a/packages/jest-worker/src/workers/safeMessageTransferring.ts b/packages/jest-worker/src/workers/safeMessageTransferring.ts new file mode 100644 index 000000000000..2361b09f488d --- /dev/null +++ b/packages/jest-worker/src/workers/safeMessageTransferring.ts @@ -0,0 +1,42 @@ +/** + * Copyright (c) Meta Platforms, Inc. and affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + */ + +import { + type SerializedRecord, + deserialize, + serialize, +} from '@ungap/structured-clone'; + +type TransferringContainer = { + __STRUCTURED_CLONE_SERIALIZED__: true; + data: SerializedRecord; +}; + +export function packMessage(message: unknown): TransferringContainer { + return { + __STRUCTURED_CLONE_SERIALIZED__: true, + data: serialize(message), + }; +} + +function isTransferringContainer( + message: unknown, +): message is TransferringContainer { + return ( + message != null && + typeof message === 'object' && + '__STRUCTURED_CLONE_SERIALIZED__' in message && + 'data' in message + ); +} + +export function unpackMessage(message: unknown): unknown { + if (isTransferringContainer(message)) { + return deserialize(message.data); + } + return message; +} diff --git a/yarn.lock b/yarn.lock index 98df7896848b..e3fb54956897 100644 --- a/yarn.lock +++ b/yarn.lock @@ -5748,6 +5748,13 @@ __metadata: languageName: node linkType: hard +"@types/ungap__structured-clone@npm:^1.2.0": + version: 1.2.0 + resolution: "@types/ungap__structured-clone@npm:1.2.0" + checksum: 5399cfc129ecae2986fbe223fdf8f443c27a628e0d24e1be0fd3d041d643e9a041be54d9b9511fdf36279e7a3cb4c9d72493f30d75fdcde52efa3ac9dc0ebb1e + languageName: node + linkType: hard + "@types/unist@npm:*, @types/unist@npm:^3.0.0": version: 3.0.2 resolution: "@types/unist@npm:3.0.2" @@ -13541,6 +13548,8 @@ __metadata: "@types/merge-stream": ^1.1.2 "@types/node": "*" "@types/supports-color": ^8.1.0 + "@types/ungap__structured-clone": ^1.2.0 + "@ungap/structured-clone": ^1.2.0 get-stream: ^6.0.0 jest-leak-detector: "workspace:*" jest-util: "workspace:*" From 7bc5ca2378de06eaaee61c9557a0d5db1f00c9e0 Mon Sep 17 00:00:00 2001 From: Dmitry Makhnev Date: Mon, 15 Jul 2024 13:32:38 +0200 Subject: [PATCH 2/9] ISSUE-10577: Update CHANGELOG.md links --- CHANGELOG.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ba649373fa16..0f1d91a826dd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -77,8 +77,8 @@ - [**BREAKING**] `--testPathPattern` is now `--testPathPatterns` - [**BREAKING**] Specifying `testPathPatterns` when programmatically calling `watch` must be specified as `new TestPathPatterns(patterns)`, where `TestPathPatterns` can be imported from `@jest/pattern` - `[jest-reporters, jest-runner]` Unhandled errors without stack get correctly logged to console ([#14619](https://github.com/jestjs/jest/pull/14619)) -- `[jest-worker]` Properly handle a circular reference error when worker tries to send an assertion fails where either the expected or actual value is circular -- `[jest-worker]` Properly handle a BigInt when worker tries to send an assertion fails where either the expected or actual value is BigInt +- `[jest-worker]` Properly handle a circular reference error when worker tries to send an assertion fails where either the expected or actual value is circular ([#15191](https://github.com/jestjs/jest/pull/15191)) +- `[jest-worker]` Properly handle a BigInt when worker tries to send an assertion fails where either the expected or actual value is BigInt ([#15191](https://github.com/jestjs/jest/pull/15191)) ### Performance From a9f6e5be1301895b38da8ef6175720bd60fd586a Mon Sep 17 00:00:00 2001 From: Dmitry Makhnev Date: Thu, 18 Jul 2024 18:53:57 +0200 Subject: [PATCH 3/9] ISSUE-10577: Make error type checking more reliable, according to the suggestion from the review --- packages/jest-worker/src/workers/messageParent.ts | 3 ++- packages/jest-worker/src/workers/processChild.ts | 3 ++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/packages/jest-worker/src/workers/messageParent.ts b/packages/jest-worker/src/workers/messageParent.ts index e4ba35e60102..a5f300316c88 100644 --- a/packages/jest-worker/src/workers/messageParent.ts +++ b/packages/jest-worker/src/workers/messageParent.ts @@ -5,6 +5,7 @@ * LICENSE file in the root directory of this source tree. */ +import {types} from 'node:util'; import {isMainThread, parentPort} from 'worker_threads'; import {PARENT_MESSAGE_CUSTOM} from '../types'; import {packMessage} from './safeMessageTransferring'; @@ -20,7 +21,7 @@ export default function messageParent( parentProcess.send([PARENT_MESSAGE_CUSTOM, message]); } catch (error: unknown) { if ( - error instanceof Error && + types.isNativeError(error) && // if .send is a function, it's a serialization issue !error.message.includes('.send is not a function') ) { diff --git a/packages/jest-worker/src/workers/processChild.ts b/packages/jest-worker/src/workers/processChild.ts index 2660d50c0779..a1b2d11e3f78 100644 --- a/packages/jest-worker/src/workers/processChild.ts +++ b/packages/jest-worker/src/workers/processChild.ts @@ -5,6 +5,7 @@ * LICENSE file in the root directory of this source tree. */ +import {types} from 'node:util'; import {isPromise} from 'jest-util'; import { CHILD_MESSAGE_CALL, @@ -102,7 +103,7 @@ function reportSuccess(result: unknown) { process.send([PARENT_MESSAGE_OK, result]); } catch (error: unknown) { if ( - error instanceof Error && + types.isNativeError(error) && // if .send is a function, it's a serialization issue !error.message.includes('.send is not a function') ) { From b6e4a5b80bbdd88208c86bc24ca0c52f96c6b8ae Mon Sep 17 00:00:00 2001 From: Dmitry Makhnev Date: Thu, 18 Jul 2024 18:58:07 +0200 Subject: [PATCH 4/9] ISSUE-10577: Fix reliability for `function` or `symbol` types in the serialization fallback case --- .../bigintInequality.test.ts.snap | 41 ---- ...ializableStructuresInequality.test.ts.snap | 215 ++++++++++++++++++ e2e/__tests__/bigintInequality.test.ts | 54 ----- ...onSerializableStructuresInequality.test.ts | 109 +++++++++ .../src/workers/safeMessageTransferring.ts | 10 +- 5 files changed, 333 insertions(+), 96 deletions(-) delete mode 100644 e2e/__tests__/__snapshots__/bigintInequality.test.ts.snap create mode 100644 e2e/__tests__/__snapshots__/nonSerializableStructuresInequality.test.ts.snap delete mode 100644 e2e/__tests__/bigintInequality.test.ts create mode 100644 e2e/__tests__/nonSerializableStructuresInequality.test.ts diff --git a/e2e/__tests__/__snapshots__/bigintInequality.test.ts.snap b/e2e/__tests__/__snapshots__/bigintInequality.test.ts.snap deleted file mode 100644 index 4439115ce4af..000000000000 --- a/e2e/__tests__/__snapshots__/bigintInequality.test.ts.snap +++ /dev/null @@ -1,41 +0,0 @@ -// Jest Snapshot v1, https://goo.gl/fbAQLP - -exports[`handles circular inequality properly 1`] = ` -"FAIL __tests__/test-1.js - ● test - - expect(received).toBe(expected) // Object.is equality - - Expected: 73n - Received: 42n - - 1 | it('test', () => { - > 2 | expect(BigInt(42)).toBe(BigInt(73)); - | ^ - 3 | }); - - at Object.toBe (__tests__/test-1.js:2:22) - -FAIL __tests__/test-2.js - ● test - - expect(received).toBe(expected) // Object.is equality - - Expected: 73n - Received: 42n - - 1 | it('test', () => { - > 2 | expect(BigInt(42)).toBe(BigInt(73)); - | ^ - 3 | }); - - at Object.toBe (__tests__/test-2.js:2:22)" -`; - -exports[`handles circular inequality properly 2`] = ` -"Test Suites: 2 failed, 2 total -Tests: 2 failed, 2 total -Snapshots: 0 total -Time: <> -Ran all test suites." -`; diff --git a/e2e/__tests__/__snapshots__/nonSerializableStructuresInequality.test.ts.snap b/e2e/__tests__/__snapshots__/nonSerializableStructuresInequality.test.ts.snap new file mode 100644 index 000000000000..2bd481d727c1 --- /dev/null +++ b/e2e/__tests__/__snapshots__/nonSerializableStructuresInequality.test.ts.snap @@ -0,0 +1,215 @@ +// Jest Snapshot v1, https://goo.gl/fbAQLP + +exports[`handles \`BigInt\` 1`] = ` +"FAIL __tests__/test-1.js + ● test + + expect(received).toBe(expected) // Object.is equality + + Expected: 73n + Received: 42n + + 1 | it('test', () => { + > 2 | expect(BigInt(42)).toBe(BigInt(73)); + | ^ + 3 | }); + + at Object.toBe (__tests__/test-1.js:2:22) + +FAIL __tests__/test-2.js + ● test + + expect(received).toBe(expected) // Object.is equality + + Expected: 73n + Received: 42n + + 1 | it('test', () => { + > 2 | expect(BigInt(42)).toBe(BigInt(73)); + | ^ + 3 | }); + + at Object.toBe (__tests__/test-2.js:2:22)" +`; + +exports[`handles \`BigInt\` 2`] = ` +"Test Suites: 2 failed, 2 total +Tests: 2 failed, 2 total +Snapshots: 0 total +Time: <> +Ran all test suites." +`; + +exports[`handles \`Map\` 1`] = ` +"FAIL __tests__/test-1.js + ● test + + expect(received).toEqual(expected) // deep equality + + - Expected - 1 + + Received + 1 + + Map { + - 1 => "3", + + 1 => "2", + } + + 1 | it('test', () => { + > 2 | expect(new Map([[1, "2"]])).toEqual(new Map([[1, "3"]])); + | ^ + 3 | }); + + at Object.toEqual (__tests__/test-1.js:2:31) + +FAIL __tests__/test-2.js + ● test + + expect(received).toEqual(expected) // deep equality + + - Expected - 1 + + Received + 1 + + Map { + - 1 => "3", + + 1 => "2", + } + + 1 | it('test', () => { + > 2 | expect(new Map([[1, "2"]])).toEqual(new Map([[1, "3"]])); + | ^ + 3 | }); + + at Object.toEqual (__tests__/test-2.js:2:31)" +`; + +exports[`handles \`Map\` 2`] = ` +"Test Suites: 2 failed, 2 total +Tests: 2 failed, 2 total +Snapshots: 0 total +Time: <> +Ran all test suites." +`; + +exports[`handles \`Symbol\` 1`] = ` +"FAIL __tests__/test-1.js + ● test + + expect(received).toEqual(expected) // deep equality + + Expected: Symbol(b) + Received: Symbol(a) + + 1 | it('test', () => { + > 2 | expect(Symbol('a')).toEqual(Symbol('b')); + | ^ + 3 | }); + + at Object.toEqual (__tests__/test-1.js:2:23) + +FAIL __tests__/test-2.js + ● test + + expect(received).toEqual(expected) // deep equality + + Expected: Symbol(b) + Received: Symbol(a) + + 1 | it('test', () => { + > 2 | expect(Symbol('a')).toEqual(Symbol('b')); + | ^ + 3 | }); + + at Object.toEqual (__tests__/test-2.js:2:23)" +`; + +exports[`handles \`Symbol\` 2`] = ` +"Test Suites: 2 failed, 2 total +Tests: 2 failed, 2 total +Snapshots: 0 total +Time: <> +Ran all test suites." +`; + +exports[`handles functions 1`] = ` +"FAIL __tests__/test-1.js + ● test + + expect(received).toEqual(expected) // deep equality + + Expected: [Function fn2] + Received: [Function fn1] + + 2 | const fn1 = () => {}; + 3 | const fn2 = () => {}; + > 4 | expect(fn1).toEqual(fn2); + | ^ + 5 | }); + + at Object.toEqual (__tests__/test-1.js:4:15) + +FAIL __tests__/test-2.js + ● test + + expect(received).toEqual(expected) // deep equality + + Expected: [Function fn2] + Received: [Function fn1] + + 2 | const fn1 = () => {}; + 3 | const fn2 = () => {}; + > 4 | expect(fn1).toEqual(fn2); + | ^ + 5 | }); + + at Object.toEqual (__tests__/test-2.js:4:15)" +`; + +exports[`handles functions 2`] = ` +"Test Suites: 2 failed, 2 total +Tests: 2 failed, 2 total +Snapshots: 0 total +Time: <> +Ran all test suites." +`; + +exports[`handles mixed structure 1`] = ` +"FAIL __tests__/test-1.js + ● test + + expect(received).toEqual(expected) // deep equality + + Expected: false + Received: {"bigInt": 42n, "fn": [Function anonymous], "map": Map {1 => "2"}, "ref": [Circular], "symbol": Symbol(asd), Symbol(qwe): Symbol(zxc)} + + 11 | method() {} + 12 | } + > 13 | expect(new Class()).toEqual(false); + | ^ + 14 | }); + + at Object.toEqual (__tests__/test-1.js:13:23) + +FAIL __tests__/test-2.js + ● test + + expect(received).toEqual(expected) // deep equality + + Expected: false + Received: {"bigInt": 42n, "fn": [Function anonymous], "map": Map {1 => "2"}, "ref": [Circular], "symbol": Symbol(asd), Symbol(qwe): Symbol(zxc)} + + 11 | method() {} + 12 | } + > 13 | expect(new Class()).toEqual(false); + | ^ + 14 | }); + + at Object.toEqual (__tests__/test-2.js:13:23)" +`; + +exports[`handles mixed structure 2`] = ` +"Test Suites: 2 failed, 2 total +Tests: 2 failed, 2 total +Snapshots: 0 total +Time: <> +Ran all test suites." +`; diff --git a/e2e/__tests__/bigintInequality.test.ts b/e2e/__tests__/bigintInequality.test.ts deleted file mode 100644 index 0462ea7df268..000000000000 --- a/e2e/__tests__/bigintInequality.test.ts +++ /dev/null @@ -1,54 +0,0 @@ -/** - * Copyright (c) Meta Platforms, Inc. and affiliates. - * - * This source code is licensed under the MIT license found in the - * LICENSE file in the root directory of this source tree. - */ - -import {tmpdir} from 'os'; -import * as path from 'path'; -import { - cleanup, - createEmptyPackage, - extractSortedSummary, - writeFiles, -} from '../Utils'; -import {runContinuous} from '../runJest'; - -const tempDir = path.resolve(tmpdir(), 'bigint-inequality-test'); - -beforeEach(() => { - createEmptyPackage(tempDir); -}); - -afterEach(() => { - cleanup(tempDir); -}); - -test('handles circular inequality properly', async () => { - const testFileContent = ` - it('test', () => { - expect(BigInt(42)).toBe(BigInt(73)); - }); - `; - - writeFiles(tempDir, { - '__tests__/test-1.js': testFileContent, - '__tests__/test-2.js': testFileContent, - }); - - const {end, waitUntil} = runContinuous( - tempDir, - ['--no-watchman', '--watch-all'], - // timeout in case the `waitUntil` below doesn't fire - {stripAnsi: true, timeout: 5000}, - ); - - await waitUntil(({stderr}) => stderr.includes('Ran all test suites.')); - - const {stderr} = await end(); - - const {summary, rest} = extractSortedSummary(stderr); - expect(rest).toMatchSnapshot(); - expect(summary).toMatchSnapshot(); -}); diff --git a/e2e/__tests__/nonSerializableStructuresInequality.test.ts b/e2e/__tests__/nonSerializableStructuresInequality.test.ts new file mode 100644 index 000000000000..aae3cf9557f9 --- /dev/null +++ b/e2e/__tests__/nonSerializableStructuresInequality.test.ts @@ -0,0 +1,109 @@ +/** + * Copyright (c) Meta Platforms, Inc. and affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + */ + +import {tmpdir} from 'os'; +import * as path from 'path'; +import { + cleanup, + createEmptyPackage, + extractSortedSummary, + writeFiles, +} from '../Utils'; +import {runContinuous} from '../runJest'; + +const tempDir = path.resolve(tmpdir(), 'bigint-inequality-test'); + +const testIn2Workers = async (testFileContent: string) => { + writeFiles(tempDir, { + '__tests__/test-1.js': testFileContent, + '__tests__/test-2.js': testFileContent, + }); + + const {end, waitUntil} = runContinuous( + tempDir, + ['--no-watchman', '--watch-all'], + // timeout in case the `waitUntil` below doesn't fire + {stripAnsi: true, timeout: 5000}, + ); + + await waitUntil(({stderr}) => stderr.includes('Ran all test suites.')); + + const {stderr} = await end(); + + return extractSortedSummary(stderr); +}; + +beforeEach(() => { + createEmptyPackage(tempDir); +}); + +afterEach(() => { + cleanup(tempDir); +}); + +test('handles `Map`', async () => { + const {summary, rest} = await testIn2Workers(` + it('test', () => { + expect(new Map([[1, "2"]])).toEqual(new Map([[1, "3"]])); + }); + `); + expect(rest).toMatchSnapshot(); + expect(summary).toMatchSnapshot(); +}); + +test('handles `BigInt`', async () => { + const {summary, rest} = await testIn2Workers(` + it('test', () => { + expect(BigInt(42)).toBe(BigInt(73)); + }); + `); + expect(rest).toMatchSnapshot(); + expect(summary).toMatchSnapshot(); +}); + +test('handles `Symbol`', async () => { + const {summary, rest} = await testIn2Workers(` + it('test', () => { + expect(Symbol('a')).toEqual(Symbol('b')); + }); + `); + expect(rest).toMatchSnapshot(); + expect(summary).toMatchSnapshot(); +}); + +test('handles functions', async () => { + const {summary, rest} = await testIn2Workers(` + it('test', () => { + const fn1 = () => {}; + const fn2 = () => {}; + expect(fn1).toEqual(fn2); + }); + `); + expect(rest).toMatchSnapshot(); + expect(summary).toMatchSnapshot(); +}); + +test('handles mixed structure', async () => { + const {summary, rest} = await testIn2Workers(` + it('test', () => { + class Class { + constructor() { + this.ref = this; + this.bigInt = BigInt(42); + this.map = new Map([[1, "2"]]); + this.symbol = Symbol('asd'); + this[Symbol('qwe')] = Symbol('zxc'); + this.fn = () => {}; + } + method() {} + } + expect(new Class()).toEqual(false); + }); + `); + expect(rest).toMatchSnapshot(); + expect(summary).toMatchSnapshot(); +}); diff --git a/packages/jest-worker/src/workers/safeMessageTransferring.ts b/packages/jest-worker/src/workers/safeMessageTransferring.ts index 2361b09f488d..e7079d1a28cd 100644 --- a/packages/jest-worker/src/workers/safeMessageTransferring.ts +++ b/packages/jest-worker/src/workers/safeMessageTransferring.ts @@ -19,7 +19,15 @@ type TransferringContainer = { export function packMessage(message: unknown): TransferringContainer { return { __STRUCTURED_CLONE_SERIALIZED__: true, - data: serialize(message), + /** + * Use the `json: true` option to avoid errors + * caused by `function` or `symbol` types. + * It's not ideal to lose `function` and `symbol` types, + * but reliability is more important. Additionally, + * information about issues with these types will be available + * in the error message. + */ + data: serialize(message, {json: true}), }; } From f4bff33c9f8787a7be55dae064cd4d11790a7b77 Mon Sep 17 00:00:00 2001 From: Dmitry Makhnev Date: Fri, 19 Jul 2024 13:14:46 +0200 Subject: [PATCH 5/9] ISSUE-10577: Fix information --- packages/jest-types/src/TestResult.ts | 5 +++++ packages/jest-worker/src/workers/safeMessageTransferring.ts | 4 +--- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/packages/jest-types/src/TestResult.ts b/packages/jest-types/src/TestResult.ts index 3fe14647f8a1..4ea07d2c3e16 100644 --- a/packages/jest-types/src/TestResult.ts +++ b/packages/jest-types/src/TestResult.ts @@ -29,6 +29,11 @@ export type AssertionResult = { * was used. */ failing?: boolean; + /** + * The raw values of the `function` or `symbol` types will be lost in some cases + * because it's not possible to serialize them correctly between workers. + * However, information about them will be available in the `failureMessages`. + */ failureDetails: Array; failureMessages: Array; fullName: string; diff --git a/packages/jest-worker/src/workers/safeMessageTransferring.ts b/packages/jest-worker/src/workers/safeMessageTransferring.ts index e7079d1a28cd..3bac0a53a517 100644 --- a/packages/jest-worker/src/workers/safeMessageTransferring.ts +++ b/packages/jest-worker/src/workers/safeMessageTransferring.ts @@ -23,9 +23,7 @@ export function packMessage(message: unknown): TransferringContainer { * Use the `json: true` option to avoid errors * caused by `function` or `symbol` types. * It's not ideal to lose `function` and `symbol` types, - * but reliability is more important. Additionally, - * information about issues with these types will be available - * in the error message. + * but reliability is more important. */ data: serialize(message, {json: true}), }; From bdafa9150e6d25674b30d8d4de88b7724003a9b6 Mon Sep 17 00:00:00 2001 From: Dmitry Makhnev Date: Mon, 22 Jul 2024 22:40:08 +0200 Subject: [PATCH 6/9] ISSUE-10577: Fix not required types in `try/catch` --- packages/jest-worker/src/workers/messageParent.ts | 2 +- packages/jest-worker/src/workers/processChild.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/jest-worker/src/workers/messageParent.ts b/packages/jest-worker/src/workers/messageParent.ts index a5f300316c88..4f7c9a547e85 100644 --- a/packages/jest-worker/src/workers/messageParent.ts +++ b/packages/jest-worker/src/workers/messageParent.ts @@ -19,7 +19,7 @@ export default function messageParent( } else if (typeof parentProcess.send === 'function') { try { parentProcess.send([PARENT_MESSAGE_CUSTOM, message]); - } catch (error: unknown) { + } catch (error) { if ( types.isNativeError(error) && // if .send is a function, it's a serialization issue diff --git a/packages/jest-worker/src/workers/processChild.ts b/packages/jest-worker/src/workers/processChild.ts index a1b2d11e3f78..97ce851f3351 100644 --- a/packages/jest-worker/src/workers/processChild.ts +++ b/packages/jest-worker/src/workers/processChild.ts @@ -101,7 +101,7 @@ function reportSuccess(result: unknown) { try { process.send([PARENT_MESSAGE_OK, result]); - } catch (error: unknown) { + } catch (error) { if ( types.isNativeError(error) && // if .send is a function, it's a serialization issue From 0b2eb637bdd8ef314838ed34afa0afe60e952b11 Mon Sep 17 00:00:00 2001 From: Dmitry Makhnev Date: Mon, 22 Jul 2024 22:42:19 +0200 Subject: [PATCH 7/9] ISSUE-10577: Add a fix for `symbols` and `functions` serialization for worker threads --- .../circularInequality.test.ts.snap | 53 --- ...ializableStructuresInequality.test.ts.snap | 338 +++++++++++++++++- e2e/__tests__/circularInequality.test.ts | 57 --- ...onSerializableStructuresInequality.test.ts | 101 ++++-- .../workers/__tests__/WorkerEdgeCases.test.ts | 1 + .../src/workers/isDataCloneError.ts | 20 ++ .../jest-worker/src/workers/messageParent.ts | 13 +- .../jest-worker/src/workers/threadChild.ts | 20 +- 8 files changed, 447 insertions(+), 156 deletions(-) delete mode 100644 e2e/__tests__/__snapshots__/circularInequality.test.ts.snap delete mode 100644 e2e/__tests__/circularInequality.test.ts create mode 100644 packages/jest-worker/src/workers/isDataCloneError.ts diff --git a/e2e/__tests__/__snapshots__/circularInequality.test.ts.snap b/e2e/__tests__/__snapshots__/circularInequality.test.ts.snap deleted file mode 100644 index 7922a6da5ced..000000000000 --- a/e2e/__tests__/__snapshots__/circularInequality.test.ts.snap +++ /dev/null @@ -1,53 +0,0 @@ -// Jest Snapshot v1, https://goo.gl/fbAQLP - -exports[`handles circular inequality properly 1`] = ` -"FAIL __tests__/test-1.js - ● test - - expect(received).toEqual(expected) // deep equality - - - Expected - 1 - + Received + 3 - - - Object {} - + Object { - + "ref": [Circular], - + } - - 3 | foo.ref = foo; - 4 | - > 5 | expect(foo).toEqual({}); - | ^ - 6 | }); - - at Object.toEqual (__tests__/test-1.js:5:15) - -FAIL __tests__/test-2.js - ● test - - expect(received).toEqual(expected) // deep equality - - - Expected - 1 - + Received + 3 - - - Object {} - + Object { - + "ref": [Circular], - + } - - 3 | foo.ref = foo; - 4 | - > 5 | expect(foo).toEqual({}); - | ^ - 6 | }); - - at Object.toEqual (__tests__/test-2.js:5:15)" -`; - -exports[`handles circular inequality properly 2`] = ` -"Test Suites: 2 failed, 2 total -Tests: 2 failed, 2 total -Snapshots: 0 total -Time: <> -Ran all test suites." -`; diff --git a/e2e/__tests__/__snapshots__/nonSerializableStructuresInequality.test.ts.snap b/e2e/__tests__/__snapshots__/nonSerializableStructuresInequality.test.ts.snap index 2bd481d727c1..d97e76639cc3 100644 --- a/e2e/__tests__/__snapshots__/nonSerializableStructuresInequality.test.ts.snap +++ b/e2e/__tests__/__snapshots__/nonSerializableStructuresInequality.test.ts.snap @@ -1,6 +1,6 @@ // Jest Snapshot v1, https://goo.gl/fbAQLP -exports[`handles \`BigInt\` 1`] = ` +exports[`processChild handles \`BigInt\` 1`] = ` "FAIL __tests__/test-1.js ● test @@ -32,7 +32,7 @@ FAIL __tests__/test-2.js at Object.toBe (__tests__/test-2.js:2:22)" `; -exports[`handles \`BigInt\` 2`] = ` +exports[`processChild handles \`BigInt\` 2`] = ` "Test Suites: 2 failed, 2 total Tests: 2 failed, 2 total Snapshots: 0 total @@ -40,7 +40,7 @@ Time: <> Ran all test suites." `; -exports[`handles \`Map\` 1`] = ` +exports[`processChild handles \`Map\` 1`] = ` "FAIL __tests__/test-1.js ● test @@ -82,7 +82,7 @@ FAIL __tests__/test-2.js at Object.toEqual (__tests__/test-2.js:2:31)" `; -exports[`handles \`Map\` 2`] = ` +exports[`processChild handles \`Map\` 2`] = ` "Test Suites: 2 failed, 2 total Tests: 2 failed, 2 total Snapshots: 0 total @@ -90,7 +90,7 @@ Time: <> Ran all test suites." `; -exports[`handles \`Symbol\` 1`] = ` +exports[`processChild handles \`Symbol\` 1`] = ` "FAIL __tests__/test-1.js ● test @@ -122,7 +122,7 @@ FAIL __tests__/test-2.js at Object.toEqual (__tests__/test-2.js:2:23)" `; -exports[`handles \`Symbol\` 2`] = ` +exports[`processChild handles \`Symbol\` 2`] = ` "Test Suites: 2 failed, 2 total Tests: 2 failed, 2 total Snapshots: 0 total @@ -130,7 +130,325 @@ Time: <> Ran all test suites." `; -exports[`handles functions 1`] = ` +exports[`processChild handles circular inequality properly 1`] = ` +"FAIL __tests__/test-1.js + ● test + + expect(received).toEqual(expected) // deep equality + + - Expected - 1 + + Received + 3 + + - Object {} + + Object { + + "ref": [Circular], + + } + + 3 | foo.ref = foo; + 4 | + > 5 | expect(foo).toEqual({}); + | ^ + 6 | }); + + at Object.toEqual (__tests__/test-1.js:5:15) + +FAIL __tests__/test-2.js + ● test + + expect(received).toEqual(expected) // deep equality + + - Expected - 1 + + Received + 3 + + - Object {} + + Object { + + "ref": [Circular], + + } + + 3 | foo.ref = foo; + 4 | + > 5 | expect(foo).toEqual({}); + | ^ + 6 | }); + + at Object.toEqual (__tests__/test-2.js:5:15)" +`; + +exports[`processChild handles circular inequality properly 2`] = ` +"Test Suites: 2 failed, 2 total +Tests: 2 failed, 2 total +Snapshots: 0 total +Time: <> +Ran all test suites." +`; + +exports[`processChild handles functions 1`] = ` +"FAIL __tests__/test-1.js + ● test + + expect(received).toEqual(expected) // deep equality + + Expected: [Function fn2] + Received: [Function fn1] + + 2 | const fn1 = () => {}; + 3 | const fn2 = () => {}; + > 4 | expect(fn1).toEqual(fn2); + | ^ + 5 | }); + + at Object.toEqual (__tests__/test-1.js:4:15) + +FAIL __tests__/test-2.js + ● test + + expect(received).toEqual(expected) // deep equality + + Expected: [Function fn2] + Received: [Function fn1] + + 2 | const fn1 = () => {}; + 3 | const fn2 = () => {}; + > 4 | expect(fn1).toEqual(fn2); + | ^ + 5 | }); + + at Object.toEqual (__tests__/test-2.js:4:15)" +`; + +exports[`processChild handles functions 2`] = ` +"Test Suites: 2 failed, 2 total +Tests: 2 failed, 2 total +Snapshots: 0 total +Time: <> +Ran all test suites." +`; + +exports[`processChild handles mixed structure 1`] = ` +"FAIL __tests__/test-1.js + ● test + + expect(received).toEqual(expected) // deep equality + + Expected: false + Received: {"bigInt": 42n, "fn": [Function anonymous], "map": Map {1 => "2"}, "ref": [Circular], "symbol": Symbol(asd), Symbol(qwe): Symbol(zxc)} + + 11 | method() {} + 12 | } + > 13 | expect(new Class()).toEqual(false); + | ^ + 14 | }); + + at Object.toEqual (__tests__/test-1.js:13:23) + +FAIL __tests__/test-2.js + ● test + + expect(received).toEqual(expected) // deep equality + + Expected: false + Received: {"bigInt": 42n, "fn": [Function anonymous], "map": Map {1 => "2"}, "ref": [Circular], "symbol": Symbol(asd), Symbol(qwe): Symbol(zxc)} + + 11 | method() {} + 12 | } + > 13 | expect(new Class()).toEqual(false); + | ^ + 14 | }); + + at Object.toEqual (__tests__/test-2.js:13:23)" +`; + +exports[`processChild handles mixed structure 2`] = ` +"Test Suites: 2 failed, 2 total +Tests: 2 failed, 2 total +Snapshots: 0 total +Time: <> +Ran all test suites." +`; + +exports[`workerThreads handles \`BigInt\` 1`] = ` +"FAIL __tests__/test-1.js + ● test + + expect(received).toBe(expected) // Object.is equality + + Expected: 73n + Received: 42n + + 1 | it('test', () => { + > 2 | expect(BigInt(42)).toBe(BigInt(73)); + | ^ + 3 | }); + + at Object.toBe (__tests__/test-1.js:2:22) + +FAIL __tests__/test-2.js + ● test + + expect(received).toBe(expected) // Object.is equality + + Expected: 73n + Received: 42n + + 1 | it('test', () => { + > 2 | expect(BigInt(42)).toBe(BigInt(73)); + | ^ + 3 | }); + + at Object.toBe (__tests__/test-2.js:2:22)" +`; + +exports[`workerThreads handles \`BigInt\` 2`] = ` +"Test Suites: 2 failed, 2 total +Tests: 2 failed, 2 total +Snapshots: 0 total +Time: <> +Ran all test suites." +`; + +exports[`workerThreads handles \`Map\` 1`] = ` +"FAIL __tests__/test-1.js + ● test + + expect(received).toEqual(expected) // deep equality + + - Expected - 1 + + Received + 1 + + Map { + - 1 => "3", + + 1 => "2", + } + + 1 | it('test', () => { + > 2 | expect(new Map([[1, "2"]])).toEqual(new Map([[1, "3"]])); + | ^ + 3 | }); + + at Object.toEqual (__tests__/test-1.js:2:31) + +FAIL __tests__/test-2.js + ● test + + expect(received).toEqual(expected) // deep equality + + - Expected - 1 + + Received + 1 + + Map { + - 1 => "3", + + 1 => "2", + } + + 1 | it('test', () => { + > 2 | expect(new Map([[1, "2"]])).toEqual(new Map([[1, "3"]])); + | ^ + 3 | }); + + at Object.toEqual (__tests__/test-2.js:2:31)" +`; + +exports[`workerThreads handles \`Map\` 2`] = ` +"Test Suites: 2 failed, 2 total +Tests: 2 failed, 2 total +Snapshots: 0 total +Time: <> +Ran all test suites." +`; + +exports[`workerThreads handles \`Symbol\` 1`] = ` +"FAIL __tests__/test-1.js + ● test + + expect(received).toEqual(expected) // deep equality + + Expected: Symbol(b) + Received: Symbol(a) + + 1 | it('test', () => { + > 2 | expect(Symbol('a')).toEqual(Symbol('b')); + | ^ + 3 | }); + + at Object.toEqual (__tests__/test-1.js:2:23) + +FAIL __tests__/test-2.js + ● test + + expect(received).toEqual(expected) // deep equality + + Expected: Symbol(b) + Received: Symbol(a) + + 1 | it('test', () => { + > 2 | expect(Symbol('a')).toEqual(Symbol('b')); + | ^ + 3 | }); + + at Object.toEqual (__tests__/test-2.js:2:23)" +`; + +exports[`workerThreads handles \`Symbol\` 2`] = ` +"Test Suites: 2 failed, 2 total +Tests: 2 failed, 2 total +Snapshots: 0 total +Time: <> +Ran all test suites." +`; + +exports[`workerThreads handles circular inequality properly 1`] = ` +"FAIL __tests__/test-1.js + ● test + + expect(received).toEqual(expected) // deep equality + + - Expected - 1 + + Received + 3 + + - Object {} + + Object { + + "ref": [Circular], + + } + + 3 | foo.ref = foo; + 4 | + > 5 | expect(foo).toEqual({}); + | ^ + 6 | }); + + at Object.toEqual (__tests__/test-1.js:5:15) + +FAIL __tests__/test-2.js + ● test + + expect(received).toEqual(expected) // deep equality + + - Expected - 1 + + Received + 3 + + - Object {} + + Object { + + "ref": [Circular], + + } + + 3 | foo.ref = foo; + 4 | + > 5 | expect(foo).toEqual({}); + | ^ + 6 | }); + + at Object.toEqual (__tests__/test-2.js:5:15)" +`; + +exports[`workerThreads handles circular inequality properly 2`] = ` +"Test Suites: 2 failed, 2 total +Tests: 2 failed, 2 total +Snapshots: 0 total +Time: <> +Ran all test suites." +`; + +exports[`workerThreads handles functions 1`] = ` "FAIL __tests__/test-1.js ● test @@ -164,7 +482,7 @@ FAIL __tests__/test-2.js at Object.toEqual (__tests__/test-2.js:4:15)" `; -exports[`handles functions 2`] = ` +exports[`workerThreads handles functions 2`] = ` "Test Suites: 2 failed, 2 total Tests: 2 failed, 2 total Snapshots: 0 total @@ -172,7 +490,7 @@ Time: <> Ran all test suites." `; -exports[`handles mixed structure 1`] = ` +exports[`workerThreads handles mixed structure 1`] = ` "FAIL __tests__/test-1.js ● test @@ -206,7 +524,7 @@ FAIL __tests__/test-2.js at Object.toEqual (__tests__/test-2.js:13:23)" `; -exports[`handles mixed structure 2`] = ` +exports[`workerThreads handles mixed structure 2`] = ` "Test Suites: 2 failed, 2 total Tests: 2 failed, 2 total Snapshots: 0 total diff --git a/e2e/__tests__/circularInequality.test.ts b/e2e/__tests__/circularInequality.test.ts deleted file mode 100644 index 812e380a75d1..000000000000 --- a/e2e/__tests__/circularInequality.test.ts +++ /dev/null @@ -1,57 +0,0 @@ -/** - * Copyright (c) Meta Platforms, Inc. and affiliates. - * - * This source code is licensed under the MIT license found in the - * LICENSE file in the root directory of this source tree. - */ - -import {tmpdir} from 'os'; -import * as path from 'path'; -import { - cleanup, - createEmptyPackage, - extractSortedSummary, - writeFiles, -} from '../Utils'; -import {runContinuous} from '../runJest'; - -const tempDir = path.resolve(tmpdir(), 'circular-inequality-test'); - -beforeEach(() => { - createEmptyPackage(tempDir); -}); - -afterEach(() => { - cleanup(tempDir); -}); - -test('handles circular inequality properly', async () => { - const testFileContent = ` - it('test', () => { - const foo = {}; - foo.ref = foo; - - expect(foo).toEqual({}); - }); - `; - - writeFiles(tempDir, { - '__tests__/test-1.js': testFileContent, - '__tests__/test-2.js': testFileContent, - }); - - const {end, waitUntil} = runContinuous( - tempDir, - ['--no-watchman', '--watch-all'], - // timeout in case the `waitUntil` below doesn't fire - {stripAnsi: true, timeout: 5000}, - ); - - await waitUntil(({stderr}) => stderr.includes('Ran all test suites.')); - - const {stderr} = await end(); - - const {summary, rest} = extractSortedSummary(stderr); - expect(rest).toMatchSnapshot(); - expect(summary).toMatchSnapshot(); -}); diff --git a/e2e/__tests__/nonSerializableStructuresInequality.test.ts b/e2e/__tests__/nonSerializableStructuresInequality.test.ts index aae3cf9557f9..c7b6f6c53fb4 100644 --- a/e2e/__tests__/nonSerializableStructuresInequality.test.ts +++ b/e2e/__tests__/nonSerializableStructuresInequality.test.ts @@ -17,7 +17,10 @@ import {runContinuous} from '../runJest'; const tempDir = path.resolve(tmpdir(), 'bigint-inequality-test'); -const testIn2Workers = async (testFileContent: string) => { +const testIn2Workers = async ( + testFileContent: string, + extraOptions: Array = [], +) => { writeFiles(tempDir, { '__tests__/test-1.js': testFileContent, '__tests__/test-2.js': testFileContent, @@ -25,7 +28,7 @@ const testIn2Workers = async (testFileContent: string) => { const {end, waitUntil} = runContinuous( tempDir, - ['--no-watchman', '--watch-all'], + ['--no-watchman', '--watch-all'].concat(extraOptions), // timeout in case the `waitUntil` below doesn't fire {stripAnsi: true, timeout: 5000}, ); @@ -45,50 +48,83 @@ afterEach(() => { cleanup(tempDir); }); -test('handles `Map`', async () => { - const {summary, rest} = await testIn2Workers(` +describe.each([ + {name: 'processChild'}, + {extraOptions: ['--workerThreads'], name: 'workerThreads'}, +])('$name', ({extraOptions}) => { + test('handles circular inequality properly', async () => { + const {summary, rest} = await testIn2Workers( + ` + it('test', () => { + const foo = {}; + foo.ref = foo; + + expect(foo).toEqual({}); + }); + `, + extraOptions, + ); + expect(rest).toMatchSnapshot(); + expect(summary).toMatchSnapshot(); + }); + + test('handles `Map`', async () => { + const {summary, rest} = await testIn2Workers( + ` it('test', () => { expect(new Map([[1, "2"]])).toEqual(new Map([[1, "3"]])); }); - `); - expect(rest).toMatchSnapshot(); - expect(summary).toMatchSnapshot(); -}); + `, + extraOptions, + ); + expect(rest).toMatchSnapshot(); + expect(summary).toMatchSnapshot(); + }); -test('handles `BigInt`', async () => { - const {summary, rest} = await testIn2Workers(` + test('handles `BigInt`', async () => { + const {summary, rest} = await testIn2Workers( + ` it('test', () => { expect(BigInt(42)).toBe(BigInt(73)); }); - `); - expect(rest).toMatchSnapshot(); - expect(summary).toMatchSnapshot(); -}); + `, + extraOptions, + ); + expect(rest).toMatchSnapshot(); + expect(summary).toMatchSnapshot(); + }); -test('handles `Symbol`', async () => { - const {summary, rest} = await testIn2Workers(` + test('handles `Symbol`', async () => { + const {summary, rest} = await testIn2Workers( + ` it('test', () => { expect(Symbol('a')).toEqual(Symbol('b')); }); - `); - expect(rest).toMatchSnapshot(); - expect(summary).toMatchSnapshot(); -}); + `, + extraOptions, + ); + expect(rest).toMatchSnapshot(); + expect(summary).toMatchSnapshot(); + }); -test('handles functions', async () => { - const {summary, rest} = await testIn2Workers(` + test('handles functions', async () => { + const {summary, rest} = await testIn2Workers( + ` it('test', () => { const fn1 = () => {}; const fn2 = () => {}; expect(fn1).toEqual(fn2); }); - `); - expect(rest).toMatchSnapshot(); - expect(summary).toMatchSnapshot(); -}); + `, + extraOptions, + ); + expect(rest).toMatchSnapshot(); + expect(summary).toMatchSnapshot(); + }); -test('handles mixed structure', async () => { - const {summary, rest} = await testIn2Workers(` + test('handles mixed structure', async () => { + const {summary, rest} = await testIn2Workers( + ` it('test', () => { class Class { constructor() { @@ -103,7 +139,10 @@ test('handles mixed structure', async () => { } expect(new Class()).toEqual(false); }); - `); - expect(rest).toMatchSnapshot(); - expect(summary).toMatchSnapshot(); + `, + extraOptions, + ); + expect(rest).toMatchSnapshot(); + expect(summary).toMatchSnapshot(); + }); }); diff --git a/packages/jest-worker/src/workers/__tests__/WorkerEdgeCases.test.ts b/packages/jest-worker/src/workers/__tests__/WorkerEdgeCases.test.ts index 04bdfdc63589..1ba69f20e059 100644 --- a/packages/jest-worker/src/workers/__tests__/WorkerEdgeCases.test.ts +++ b/packages/jest-worker/src/workers/__tests__/WorkerEdgeCases.test.ts @@ -24,6 +24,7 @@ const filesToBuild = [ 'workers/processChild', 'workers/threadChild', 'workers/safeMessageTransferring', + 'workers/isDataCloneError', 'types', ]; const writeDestination = join(__dirname, '__temp__'); diff --git a/packages/jest-worker/src/workers/isDataCloneError.ts b/packages/jest-worker/src/workers/isDataCloneError.ts new file mode 100644 index 000000000000..e3e9faf83e8e --- /dev/null +++ b/packages/jest-worker/src/workers/isDataCloneError.ts @@ -0,0 +1,20 @@ +// https://webidl.spec.whatwg.org/#datacloneerror +const DATA_CLONE_ERROR_CODE = 25; + +/** + * Unfortunately, [`util.types.isNativeError(value)`](https://nodejs.org/api/util.html#utiltypesisnativeerrorvalue) + * return `false` for `DataCloneError` error. + * For this reason, try to detect it in this way + */ +export function isDataCloneError(error: unknown): error is DOMException { + return ( + error != null && + typeof error === 'object' && + 'name' in error && + error.name === 'DataCloneError' && + 'message' in error && + typeof error.message === 'string' && + 'code' in error && + error.code === DATA_CLONE_ERROR_CODE + ); +} diff --git a/packages/jest-worker/src/workers/messageParent.ts b/packages/jest-worker/src/workers/messageParent.ts index 4f7c9a547e85..6c904f4705f2 100644 --- a/packages/jest-worker/src/workers/messageParent.ts +++ b/packages/jest-worker/src/workers/messageParent.ts @@ -8,6 +8,7 @@ import {types} from 'node:util'; import {isMainThread, parentPort} from 'worker_threads'; import {PARENT_MESSAGE_CUSTOM} from '../types'; +import {isDataCloneError} from './isDataCloneError'; import {packMessage} from './safeMessageTransferring'; export default function messageParent( @@ -15,7 +16,17 @@ export default function messageParent( parentProcess = process, ): void { if (!isMainThread && parentPort != null) { - parentPort.postMessage([PARENT_MESSAGE_CUSTOM, message]); + try { + parentPort.postMessage([PARENT_MESSAGE_CUSTOM, message]); + } catch (error) { + // Try to handle https://html.spec.whatwg.org/multipage/structured-data.html#structuredserializeinternal + // for `symbols` and `functions` + if (isDataCloneError(error)) { + parentPort.postMessage([PARENT_MESSAGE_CUSTOM, packMessage(message)]); + } else { + throw error; + } + } } else if (typeof parentProcess.send === 'function') { try { parentProcess.send([PARENT_MESSAGE_CUSTOM, message]); diff --git a/packages/jest-worker/src/workers/threadChild.ts b/packages/jest-worker/src/workers/threadChild.ts index e134615308d3..02a36cbcfcb4 100644 --- a/packages/jest-worker/src/workers/threadChild.ts +++ b/packages/jest-worker/src/workers/threadChild.ts @@ -22,6 +22,8 @@ import { PARENT_MESSAGE_SETUP_ERROR, type ParentMessageMemUsage, } from '../types'; +import {isDataCloneError} from './isDataCloneError'; +import {packMessage} from './safeMessageTransferring'; type UnknownFunction = (...args: Array) => unknown | Promise; @@ -114,11 +116,21 @@ function reportSuccess(result: unknown) { try { parentPort!.postMessage([PARENT_MESSAGE_OK, result]); - } catch (error: any) { - // Handling it here to avoid unhandled `DataCloneError` rejection + } catch (error) { + let resolvedError = error; + // Try to handle https://html.spec.whatwg.org/multipage/structured-data.html#structuredserializeinternal + // for `symbols` and `functions` + if (isDataCloneError(error)) { + try { + parentPort!.postMessage([PARENT_MESSAGE_OK, packMessage(result)]); + return; + } catch (secondTryError) { + resolvedError = secondTryError; + } + } + // Handling it here to avoid unhandled rejection // which is hard to distinguish on the parent side - // (such error doesn't have any message or stack trace) - reportClientError(error); + reportClientError(resolvedError as Error); } } From 562503ed8ad354ee3c74af8e1f8747fa6a255699 Mon Sep 17 00:00:00 2001 From: Dmitry Makhnev Date: Mon, 22 Jul 2024 22:46:08 +0200 Subject: [PATCH 8/9] ISSUE-10577: Fix file header --- packages/jest-worker/src/workers/isDataCloneError.ts | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/packages/jest-worker/src/workers/isDataCloneError.ts b/packages/jest-worker/src/workers/isDataCloneError.ts index e3e9faf83e8e..773daa7dc103 100644 --- a/packages/jest-worker/src/workers/isDataCloneError.ts +++ b/packages/jest-worker/src/workers/isDataCloneError.ts @@ -1,3 +1,10 @@ +/** + * Copyright (c) Meta Platforms, Inc. and affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + */ + // https://webidl.spec.whatwg.org/#datacloneerror const DATA_CLONE_ERROR_CODE = 25; From 19f66cc11e11283003d46e3798cbed0fdbafbd27 Mon Sep 17 00:00:00 2001 From: Simen Bekkhus Date: Tue, 23 Jul 2024 09:37:40 +0200 Subject: [PATCH 9/9] Update e2e/__tests__/nonSerializableStructuresInequality.test.ts --- e2e/__tests__/nonSerializableStructuresInequality.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/e2e/__tests__/nonSerializableStructuresInequality.test.ts b/e2e/__tests__/nonSerializableStructuresInequality.test.ts index c7b6f6c53fb4..c4d41753cce9 100644 --- a/e2e/__tests__/nonSerializableStructuresInequality.test.ts +++ b/e2e/__tests__/nonSerializableStructuresInequality.test.ts @@ -28,7 +28,7 @@ const testIn2Workers = async ( const {end, waitUntil} = runContinuous( tempDir, - ['--no-watchman', '--watch-all'].concat(extraOptions), + ['--no-watchman', '--watch-all', ...extraOptions], // timeout in case the `waitUntil` below doesn't fire {stripAnsi: true, timeout: 5000}, );