feat(jest-runner): switch workers to advanced serialization

[alex3d]

Summary

PR would switch workers from current JSON serialization to advanced (structuredClone) serialization.
This would fix problems with BigInt(#11617), circular references (#10577) and probably some other problems.

Test plan

Enable already existing BigInt test in e2e/__tests__/complexItemsInErrors.ts and test in e2e/__tests__/circularInequality.test.ts

[facebook-github-bot]

Hi @alex3d!

Thank you for your pull request and welcome to our community.

Action Required

In order to merge any pull request (code, docs, etc.), we require contributors to sign our Contributor License Agreement, and we don't seem to have one on file for you.

Process

In order for us to review and merge your suggested changes, please sign at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need to sign the corporate CLA.

Once the CLA is signed, our tooling will perform checks and validations. Afterwards, the pull request will be tagged with CLA signed. The tagging process may take up to 1 hour after signing. Please give it that time before contacting us about it.

If you have received this in error or have any questions, please contact us at cla@meta.com. Thanks!

[alex3d]

cc @SimenB
It seems you have had plans to switch to advanced serialization after Node 12 EOL: #11467 (comment)

I've added basic worker result sanitization to pass https://github.com/facebook/jest/blob/b38d7d345a81d97d1dc3b68b8458b1837fbf19be/e2e/__tests__/complexItemsInErrors.ts#L31
Probably this should be removed along with test to make child_process workers more in line with new experimental worker_threads runner.

[facebook-github-bot]

Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Meta Open Source project. Thanks!

[mrazauskas]

Thanks!

It is worth to consider landing the sanitizeTestResult() function separately, because it is fixing one of the bugs I have hit while using workerThreads worker.

[mrazauskas]

Better add it as a function:

Suggested change

	const sanitizeTestResult = (result: TestResult) => {
	function sanitizeTestResult(result: TestResult) {

[mrazauskas]

By the way, I was testing this branch locally. In the Node docs it is mentioned that structure clone algorithm:

supports more built-in JavaScript object types, such as BigInt, Map and Set, ArrayBuffer and TypedArray, Buffer, Error, RegExp etc.

However, this format is not a full superset of JSON, and e.g. properties set on objects of such built-in types will not be passed on through the serialization step.

How to understand this? Does it mean that extra properties set on an Error object can lead to some bugs in assertion error messages? I was trying to figure this out, but everything worked smooth.

[alex3d]

Yeah, node docs are right: extra properties of Error are stripped after structuredClone. But result.testResults[0].failureDetails[0] actually is a plain object. I'm not familiar enough with jest internals to tell why it is not an Error.

It is worth to consider landing the sanitizeTestResult() function separately, because it is fixing one of the bugs I have hit while using workerThreads worker.

Should I open new PR?
Was that workerThreads bug something like expect(someFunction).eq(someOtherFunction)?
Also, I was thinking about more JSON.stringify like approach of deleting function properties from TestResult recursively, but jest test suite didn't show up any cases when function properties could appear in other places.

[mrazauskas]

Should I open new PR? Was that workerThreads bug something like expect(someFunction).eq(someOtherFunction)?

The output in console was similar to "handles functions that close over outside variables" test. It was appearing while I was refactoring code base with huge objects, but leaving some mistakes in tests. In a way tests were supposed to fail, only the message was odd. Since object were huge, I did not manage to reproduce the error so far.

Not sure about separate PR. Let’s wait for maintainer’s feedback.

[github-actions]

This PR is stale because it has been open 90 days with no activity. Remove stale label or comment or this will be closed in 30 days.

[github-actions]

This PR was closed because it has been stalled for 30 days with no activity. Please open a new PR if the issue is still relevant, linking to this one.

[github-actions]

This PR was closed because it has been stalled for 30 days with no activity. Please open a new PR if the issue is still relevant, linking to this one.

[github-actions]

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.
Please note this issue tracker is not a help forum. We recommend using StackOverflow or our discord channel for questions.