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

Add support for deep property checking #9

Closed

Conversation

mitchellsimoens
Copy link
Contributor

For issue #4

Adds the deep modifier as in chai. You can use dot or bracket notation for objects and arrays.

@mitchellsimoens
Copy link
Contributor Author

@dongryphon looks like the yarn install in the travis run is borked

@dongryphon
Copy link
Collaborator

Thanks for the heads up - I'll take a look at this tonight (hopefully)

@@ -362,37 +363,86 @@ class Assert {

property: {
evaluate (object, property, value) {
let only = this._modifiers.only;
//TODO deep properties "foo.bar.baz"
const { deep, only, own } = this._modifiers;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could be a good deal simpler / perfier in this form:

    let obj = object;
    let prop = property;

    if (deep) {
        let parts = property.split(/.../);
        prop = parts.pop();

        for (let part of parts) {
            if (!obj) {
                return false;
            }
            obj = obj[part]; // ish
        }
    }

    // pretty much as before using obj / prop instead of object / property

?

@dongryphon
Copy link
Collaborator

Looks like the tests are running now but there are some failures

@mitchellsimoens
Copy link
Contributor Author

Odd, new failures do no appear with node v6 but do with node v8... will fix for v8 and back-test with v6. Wouldn't think there would be a difference between the two with simple object/array lookups but ok :)

@mitchellsimoens
Copy link
Contributor Author

So the issue here, and it's an annoying one, was that [Object] was now going to print out to [Array]:

screen shot 2017-06-19 at 8 44 17 am

So I used A.print to get the same printed message. Don't like using values that aren't static strings but this was only way to get tests to pass on 6 and 8.

Assert.js Outdated
return false;
}
}
}
}
else if (!(property in object)) {
else if (typeof obj === 'object' && !(prop in obj)) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if this check is to prevent in from throwing... won't we have the same issue in the for loop below? Would this handle both?

    if (typeof obj !== 'object' || !(prop in obj)) {

return false;
}

obj = obj[part]; // ish
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just realized we should probably check only here as well.. So that

    expect({ a: { b: 2}, c: true }).to.only.have.deep.property('a.b', 2);

Is reported as a failure.

Agree?

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

Successfully merging this pull request may close these issues.

2 participants