Skip to content
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

Prevent matcher failures from printing large deeply nested objects #2306

Open
jchurchill opened this issue Oct 21, 2020 · 2 comments
Open

Prevent matcher failures from printing large deeply nested objects #2306

jchurchill opened this issue Oct 21, 2020 · 2 comments

Comments

@jchurchill
Copy link

jchurchill commented Oct 21, 2020

Is your feature request related to a problem? Please describe.
We frequently use matchers to assert that a particular set of fields on a POJO matches certain values. For example:

const actual = { v1: "1", v2: "2", v3: "3", /* ... */ };
sinon.assert.match(actual, { v1: "1" }); // just make sure v1 has the right value

This comes up most frequently when making assertions about how a function was invoked when the args are combined into a single POJO (which is a somewhat common pattern).

// I plan to spy on this and use it in `sinon.assert.calledWithMatch`
const myFunc = namedArgs => {
    const { v1, v2, v3 } = namedArgs;
    // ... do something with v1, v2, v3 ...
};

However, if any of the values is a very large, deeply nested object, then a matcher failure will attempt to print the entire object deeply, regardless of whether the match failure was present in that object. When the object is large enough, this can take long enough that it's unclear what's going on and what you've done wrong.

Describe the solution you'd like
The matcher should make an attempt to give feedback only on the parts of the object that were mismatched. For example:

const deeplyNested = /* ... */;
const obj = { simple: "A", deeplyNested };
sinon.assert.match(obj, { simple: "B", deeplyNested });

should print something like

Match failure:
{
-   simple: "B",
+   simple: "A",
    deeplyNested: (matched)
}

Describe alternatives you've considered
I've discovered that the problem can be avoided by adding a toString method to the large deeply nested object. This helps, but is a losing battle; I can't anticipate all the large objects out there that might cause this problem.

Additional context
I'm using Sinon 9.2.0.

Example repro:

import sinon from "sinon";

const makeDeepObject = (numFields, nestDepth) => {
    const deepObject = {};
    if (nestDepth === 0) {
        return deepObject;
    }
    for (let i = 0; i < numFields; i++) {
        deepObject[`field_${i}`] = makeDeepObject(numFields, nestDepth - 1);
    }
    return deepObject;
}

/**
 * Function to assert spy behavior on
 * @param {{ value1: string, value2: object }} namedArgs
 */
const testStub = sinon.stub();
const deepObject = makeDeepObject(10, 4); // make a pretty large object

testStub({ value1: "Foo", deepObject });

// Successful match - is fast
sinon.assert.calledWithMatch(testStub, { value1: "Foo" });

// Failed match - is slow, presumably due to printing deep object
sinon.assert.calledWithMatch(testStub, { value1: "Bar" });
@mantoni
Copy link
Member

mantoni commented Oct 22, 2020

Thank you for taking the time to explain the issue with this level of detail. I have experienced the same problem before, in my case with http request objects on node.

It's not a trivial change though. I think we'd have to create a deep copy of the actual object, using the matchers as selectors for what to copy. I'd really like to see this happen though. I think it's worth trying.

Should you be interested in implementing this, I'm here to help. Otherwise I'll get to it eventually.

@stale
Copy link

stale bot commented Dec 25, 2020

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants