-
Notifications
You must be signed in to change notification settings - Fork 29
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
'to satisfy' with circular data causes "Maximum call stack size exceeded" #694
Comments
Did a bit of debugging and turns out the same happens with any circular structure ( it('fails for circular structures with "Maximum call stack size exceeded"', () => {
const circular = {};
circular.loop = circular;
const circular2 = {};
circular2.loop = circular2;
expect(circular, 'to satisfy', circular2);
}); Not sure what unexpected is expected to do with circular structures, the docs don't say anything about this. |
Hmm, yeah, we don't guard against circularity in the right hand-side of It works fine with a circular subject: const expect = require('unexpected');
const circular = {};
circular.a = circular;
expect(circular, 'to satisfy', {a: {a: {a:{}}}}); // :+1:
expect(circular, 'to satisfy', {a: {a: {b:{}}}});
We can probably fix this, but I'm wondering what your use case is? 🤔 |
Hi @papandreou, thanks for the quick response! I am honored to be the first person throwing a circular structure on the right side of an expectation ;) My use case was verifying a piece of code that was doing something like this: const form = new FormData()
form.append('some', 'value')
fetch('some/url', {method: 'post', body: form}) using an expectation roughly like: const expectedForm = new FormData()
expectedForm.append('some', 'value')
expect(fetchOptions, 'to satisfy', {method: 'post', body: expectedForm}) Turns out FormData is internally implemented in a circular way (at least in jsdom), entries having a reference to the FormData instance or something like that. (This was unknown to me.) As someone not particularly familiar with unexpected and contributing to a codebase with it already in place, I expected this assertion to either work or shout at me for FormData not being supported. Shouting at me would have not surprised me, I understand supporting any type all the supported JS engines have built in is not feasible. As this might be a usage edge case fixing should also not be urgent. I did poke around in the source but saw no obvious fix (but have never seen unexpected "from inside" before). Also, now that I understand what the problem is I can come up with other ways of verifying behavior in my use case. |
We don't support this in in |
Upssy, you can't get at the fields. Let me brew something up for you. |
Try to add the following type to unexpected: expect.addType({
name: 'FormData',
base: 'wrapperObject',
identify(value) {
return value && value instanceof FormData;
},
prefix: (output, value) => output.jsFunctionName('FormData').text('('),
suffix: (output, value) => output.text(')'),
unwrap: form => {
const result = {};
Array.from(form.entries()).forEach(([key, value]) => {
if (typeof result[key] === 'undefined') {
result[key] = value;
} else if (typeof result[key] === 'string') {
result[key] = [result[key], value];
} else {
result[key].push(value);
}
});
return result;
}
}); Then something like this: const a = new FormData();
a.append('foo', 'bar');
a.append('foo', 'quz');
a.append('bar', 'baz');
expect(a, 'to satisfy', {
foo: ['bar', 'qux'],
bar: 'baz'
}); will fail with a diff like this: |
You can also simplify it to just return the entries as an array if the order matters to you. |
@sunesimonsen That looks quite neat, thanks for the suggestion. 👍
If there is an easy fix to avoid "Maximum call stack size exceeded" and fail with a helpful message on circular data, that would be nice. |
@salomvary I agree that it would be a good idea to detect circular data or the right hand side of the to satisfy assertion. |
Given this added to test/unexpected.spec.js:
npx jest -- test/unexpected.spec.js
on the latestmaster
of this repo fails with an exception:Using Node.js v12.14.1.
The text was updated successfully, but these errors were encountered: