Skip to content

Commit

Permalink
fix: support errors with circular dependencies in object values with …
Browse files Browse the repository at this point in the history
…--parallel (#5212)

* fix: support errors with circular dependencies in object values with --parallel

* Also handle nested non-writable getters
  • Loading branch information
JoshuaKGoldberg authored Oct 29, 2024
1 parent f44f71b commit ba0fefe
Show file tree
Hide file tree
Showing 6 changed files with 68 additions and 5 deletions.
8 changes: 7 additions & 1 deletion lib/nodejs/serializer.js
Original file line number Diff line number Diff line change
Expand Up @@ -262,9 +262,15 @@ class SerializableEvent {
breakCircularDeps(result.error);

const pairs = Object.keys(result).map(key => [result, key]);

const seenPairs = new Set();
let pair;

while ((pair = pairs.shift())) {
if (seenPairs.has(pair[1])) {
continue;
}

seenPairs.add(pair[1]);
SerializableEvent._serialize(pairs, ...pair);
}

Expand Down
4 changes: 3 additions & 1 deletion lib/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -675,7 +675,9 @@ exports.breakCircularDeps = inputObj => {

seen.add(obj);
for (const k in obj) {
if (Object.prototype.hasOwnProperty.call(obj, k)) {
const descriptor = Object.getOwnPropertyDescriptor(obj, k);

if (descriptor && descriptor.writable) {
obj[k] = _breakCircularDeps(obj[k], k);
}
}
Expand Down
15 changes: 15 additions & 0 deletions test/integration/fixtures/parallel/circular-error-object.mjs
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
import {describe,it} from "../../../../index.js";

describe('test1', () => {
it('test', () => {
const errorA = {};
const objectB = {toA: errorA};
errorA.toB = objectB;

const error = new Error("Oh no!");
error.error = errorA;
error.values = [errorA];

throw error;
});
});
15 changes: 15 additions & 0 deletions test/integration/fixtures/parallel/getter-error-object.mjs
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
import {describe, it} from '../../../../index.js';

describe('test1', () => {
it('test', async () => {
const error = new Error('Oh no!');

error.nested = {
get inner() {
return 'abc';
}
};

throw error;
});
});
31 changes: 28 additions & 3 deletions test/integration/parallel.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,8 @@ describe('parallel run', () => {
assert.strictEqual(result.stats.passes, 3);
});

it('should correctly handle circular references in an exception', async () => {
const result = await runMochaJSONAsync('parallel/circular-error.mjs', [
it('should correctly handle circular array references in an exception', async () => {
const result = await runMochaJSONAsync('parallel/circular-error-array.mjs', [
'--parallel',
'--jobs',
'2',
Expand All @@ -45,7 +45,7 @@ describe('parallel run', () => {
});

it('should correctly handle an exception with retries', async () => {
const result = await runMochaJSONAsync('parallel/circular-error.mjs', [
const result = await runMochaJSONAsync('parallel/circular-error-array.mjs', [
'--parallel',
'--jobs',
'2',
Expand All @@ -58,4 +58,29 @@ describe('parallel run', () => {
assert.strictEqual(result.failures[0].err.message, 'Foo');
assert.strictEqual(result.failures[0].err.foo.props[0], '[Circular]');
});

it('should correctly handle circular object references in an exception', async () => {
const result = await runMochaJSONAsync('parallel/circular-error-object.mjs', [
'--parallel',
'--jobs',
'2',
require.resolve('./fixtures/parallel/testworkerid1.mjs')
]);
assert.strictEqual(result.stats.failures, 1);
assert.strictEqual(result.stats.passes, 1);
assert.strictEqual(result.failures[0].err.message, 'Oh no!');
assert.deepStrictEqual(result.failures[0].err.values, [ { toB: { toA: '[Circular]' } } ]);
});

it('should correctly handle a non-writable getter reference in an exception', async () => {
const result = await runMochaJSONAsync('parallel/getter-error-object.mjs', [
'--parallel',
'--jobs',
'2',
require.resolve('./fixtures/parallel/testworkerid1.mjs')
]);
assert.strictEqual(result.stats.failures, 1);
assert.strictEqual(result.stats.passes, 1);
assert.strictEqual(result.failures[0].err.message, 'Oh no!');
});
});

0 comments on commit ba0fefe

Please sign in to comment.