Chai: Refactor `.ownProperty` to leverage `.property`

Created on 10 Sep 2016  路  3Comments  路  Source: chaijs/chai

There's a divergence between the behavior of the .property and .ownProperty assertions due to refactoring of .property in the master branch and .ownProperty in the 4.x.x branch.

I think the thing to do here is:

  • Create a new own flag.
  • Update .property to restrict checks to own properties when the own flag is set.
  • Change .ownProperty and .haveOwnProperty into simple assertions that set the own flag and then call .property to do the rest.

That way, the code paths for .property and .ownProperty are unified and will never diverge again. It also enables chaining with .deep. For example:

expect(obj).to.have.deep.own.property('kittens', {quantity: 3});

The only breaking change to .ownProperty would be the same breaking change that happened to .property during the refactor in #744.

However, I don't think the nested and own flags mix conceptually, since the intent of nested is to test a property that's multiple levels deep, whereas the purpose of own is to test a property that is set directly on an object as opposed to being inherited. Therefore, combining these two flags would cause .property to throw an error.

I already started playing around with this on the plane. I'll go ahead and finish it if there's consensus.

@lucasfcosta @keithamus?

more-discussion-needed

Most helpful comment

I'm strongly in favor of throwing an error when nested and own is combined. I think it's important for testing libraries to be stringent about grammar and not silently fix problems., as it may lead to unexpected behavior for the user.

I should have a chance to work on this tomorrow!

All 3 comments

I totally agree! Awesome idea!
I think this is totally correct considering one of the oldest principles of the art of programming, which is: don't repeat yourself.

Doing this would make the code more modular and, as you said, would cause these two assertions to never diverge again since they basically do the same thing but have different assertion subjects.

I just think we need to have a good and self explanatory error message when nested is using alongside own.

Great suggestion 馃憤

@lucasfcosta It really seems to me that using nested alongside own does not make sense.
A nested property cannot be an own property of an object, right?

I see two options:

  1. Unset own when setting nested and unset nested when setting own.
    Something similar is done for all and any, I don't know if this is very intuitive, because unlike all and any, own and nested are not opposite of each other.
  2. Throw an error when setting the own flag while nested is already set and vice versa.

If we choose the second option I have a suggestion:

The .own property must not be used alongside .nested property.
The .nested property must not be used alongside .own property.

Yes, this is totally stolen from here

I'm strongly in favor of throwing an error when nested and own is combined. I think it's important for testing libraries to be stringent about grammar and not silently fix problems., as it may lead to unexpected behavior for the user.

I should have a chance to work on this tomorrow!

Was this page helpful?
0 / 5 - 0 ratings

Related issues

xareelee picture xareelee  路  3Comments

qbolec picture qbolec  路  5Comments

zzzgit picture zzzgit  路  3Comments

ghost picture ghost  路  4Comments

domenic picture domenic  路  4Comments