Add loose equals and loose contains functions on ShallowWrapper

[mathieuancelin]

This PR add two functions on the ShallowWrapper to perform loose equality and loose contains check on a tree. The functions are named .rightContains() and .rightEquals() because the equality is based on the expected element (the right one) instead of the wrapped element (the left one). The function names are not very good and I'll be happy to change it for something more relevant and meaningful.

I added these functions because it is really really handy to write clean tests that check just the necessary parts of the tree for the test, i.e. "I want to check if that particular node is present in my tree, but I don't care about its style or onClick handler".

It works by checking if at least, all the props (including children) of the expected element are present on the wrapper element and equal to each other. It is based on the Utils.nodeEqual function with a little tweak on the props length comparison.

To use it, you just have to write something like :

const MyComponent = React.createClass({
  handleClick() { ...  },
  render() {
    return (
      <div>
        <div onClick={this.handleClick} className="foo bar">Hello</div>
      </div>
    );
  }
});

const wrapper = shallow(<MyComponent />);

// here we don't need to check if onClick is present on the node, so we just skip it
// same for the className
expect(wrapper.rightEquals(
  <div>
    <div>Hello</div>
  </div>
)).to.equal(true);

// here we don't need to check if onClick is present on the node, so we just skip it
// same for the className
expect(wrapper.rightContains(
  <div>Hello</div>
)).to.equal(true);

// here we don't need to check if onClick is present on the node, so we just skip it
expect(wrapper.rightContains(
  <div className="foo bar">Hello</div>
)).to.equal(true);

As the contributors guide suggest, I added some documentation and some tests that should pass with all versions of react.

These kind of feature are suggested on some issues like #204 or #273

[aweary]

I like this, with the exception of the API name 😄. Maybe something like looseContains/looseEquals would be more accurate/clear?

[aweary]

should be "its" not "it's"

[ljharb]

I definitely am not a fan of the names.

Does this comparison ignore all props, or just style and onClick? I think we'd need to very explicitly define which things are compared and which are ignored, and ideally the names of the functions took this into account.

[aweary]

@ljharb I believe it ignores all props that aren't present on the expression passed into rightContains/rightEquals. So something like rightContains('<div><div/></div>') would match any structure that contains an empty nested div, is that right @mathieuancelin?

[ljharb]

so something like matchesElement, and containsMatchingElement?

[mathieuancelin]

@aweary @ljharb I'm not a fan of the name either, but first I wanted to write something that works, see if you guys are interested in such feature and then discuss the name, I know it can be tricky to agree on public API names ;-)

[ljharb]

"looks like" here should be in curly quotes, not backticks, since it's not code

[aweary]

If you're going for emphasis, try italics 😄

[mathieuancelin]

@ljharb yes, @aweary is right, these function just check props that are present on the elements passed into rightContains/rightEquals so you can check style or onClick if you want to, or you can just ignore them because it's not relevant for you test

[ljharb]

can we be very very explicit about whether this has some or every semantics?

I almost would prefer three separate methods: containsMatchingElement, containsAllMatchingElements, and containsAnyMatchingElements - taking a node, an array, and an array, respectively - rather than risking any confusion about what conditions cause true or false to be returned. Thoughts?

[mathieuancelin]

It actually just work like wrapper.contains(nodeOrNodes) except the "source of truth" is inverted. It's not A.contains(B) anymore but B.contains(A) instead.

[ljharb]

I don't think that's accurate. shallow(<A a><B b /></A>).containsMatchingElement(<B />) isn't the same thing as shallow(<B />).contains(<A a><B b /></A>).

I see this as simply "contains", but using the element-to-be-found's prop list rather than the element-being-searched.

[mathieuancelin]

I see this as simply "contains", but using the element-to-be-found's prop list rather than the element-being-searched.

Yes, that's what I was trying to explain :-)

[mathieuancelin]

Hey @ljharb and @aweary,

I've just pushed some new code with the modifications that you suggested.
Let me know if something's wrong.

[ljharb]

Nothing immediately jumps out at me. Is this perhaps something we also want exposed on mount? Both mount and shallow's wrappers have contains.

[mathieuancelin]

Yeah, I thought exactly the same thing. I will take a look at mount today

[mathieuancelin]

@ljharb I just added the contains* function to the ReactWrapper with some tests and the documentation

[mathieuancelin]

@ljharb it should be good now :-)

[aweary]

@lelandrichardson @ljharb do you think we can get this merged soon?

[ljharb]

@aweary it needs to be rebased on latest master :-)

[aweary]

Yeah, I meant more in terms of whether you guys will be accepting it. If that's the case then @mathieuancelin it'd be great if you could rebase 😄

[mathieuancelin]

@aweary it's rebased, just waiting on CI :-)

[aweary]

Maybe it's just me, but it says it's still out-of-date with master 😬

[mathieuancelin]

@aweary it seems good now, but travis seems a bit overloaded at the moment

[aweary]

I believe @ljharb is going to want that last merge commit (3394025) to be squashed as well

[mathieuancelin]

@aweary It should be okay now. I don't even know where does this merge come from. Anyway, it seems to be fixed

[lelandrichardson]

I had considered a similar API before, but never ended up writing it. I think this is quite useful in practice, though it always feels a little messy to me in my head. I think it's a reasonable addition though.

What do we think about the names fuzzyContains and fuzzyEquals? cc @ljharb

[ljharb]

I like the methods but I don't like the name "fuzzy" - the current names were my suggestion and atm I still find them to be the clearest.

[lelandrichardson]

The problem I have with the current names is that they don't seem to indicate to me that they are not "strict". using the "fuzzy" prefix makes it obvious that this is not strict equality.

[ljharb]

To me, "matching" is a non-strict thing - since in every test framework, "to match" generally means "is kind of the same as". Another common word to describe non-strict-equality is "loose equality". "Fuzzy" might imply that { foo: 3 } and { Foo: 3 } are the same - because you don't know to what depth "fuzzy" refers to.

[lelandrichardson]

Considering test libraries use the match nomenclature, i'll go ahead and agree with the current naming :)

OK. Let's do this! We've got some fresh conflicts. Mind rebasing and fixing? Thanks!

[mathieuancelin]

@lelandrichardson it should be good now :-)

[mathieuancelin]

Thanks @lelandrichardson :-)