to.have.attr passes when actual is undefined

[marcodejongh]

The following assertion passes:

            it('Reproduction path', () => {
                const wrapper = shallow(<div />);
                expect(undefined).to.have.attr('key', 'somekey');
            });

It also passes on to.have.prop

[ayrton]

This certainly feels like unexpected results, both assertions actually pass:

expect(undefined).to.have.attr('key', 'somekey');
expect(undefined).to.not.have.attr('key', 'somekey');

[marcodejongh]

Yes this is a particulary nasty bug, I've found that I had a lot of cases passing even tho they should fail.
Because I passed in wrapper.children().get(1) instead of wrapper.children().at(1).
The assertion only seems to work correctly if you expect on a enzyme wrapper

[ljharb]

At the very least, an enzyme expectation should always fail if the "actual" is not a non-array non-function object.

[marcodejongh]

Ok to summarise: the proposed fix will be to have all the chai-enzyme assertions throw an error if the "actual" is not a enzyme warpper

[vesln]

yeah... that's a bit problematic. the thing is... we use overwrite* in order to play nice with other chai.js plugins that may have the same syntax (such as chai-jquery for example).

to me this seems more or less like a chai.js issue, rather than chai-enzyme. @keithamus do you have any thoughts on this?

[vesln]

/cc @logicalparadox

[keithamus]

Hey @vesln thanks for bringing me in on this.

TL;DR: This will be "fixed" in version 4.0.0 of chai, but you shouldn't use overwriteMethod unless the method actually exists. (Which, I know, is a pain).

---

This is kind of a chai bug, and kind of a bug with this lib. For context read chaijs/chai#467, but it boils down to:

1. Calling overwriteMethod gives you the super method
2. If you're overwriting a method that doesn't exist then super is an empty function (which coincidentally means its a passing assertion).
3. attr doesn't exist in chai core, so if you're just using this plugin then you get an empty function as super
4. It looks as though your lib calls super when the wrap function presumably rejects the assertion object based on some criteria which I assume undefined is part of.

So whats happening:

1. expect(undefined).to.have.attr('key', 'value') is called
2. wrap() (your lib internals) rejects undefined as something it can assert on
3. Your lib calls super with the object
4. Chai doesn't have a super, so just calls an empty function
5. Assertion "passes"

The "fix" for this is in Chai 4.x.x branch - it makes the default behaviour of super throw if there is no underlying method. Having said that, this could be fixed on your side by discontinuing use of overwriteMethod unless you absolutely know that the underlying method exists. overwriteMethod, IMO, is for edge cases - not the common case. If you'd like to get involved with how chai intends to fix this long term, take a look at chaijs/chai#585

[vesln]

@keithamus thanks a ton!

in this case, @marcodejongh, i'd suggest to change the overwrite* methods where possible (eg. won't be doable for contain) and throw if the passed objects are not an enzyme wrapper

[keithamus]

You can check to see if a method exists by looking at chai.Assertion.prototype.*. This will let you programatically determine whether you should use overwriteMethod or addMethod. Of course, it depends largely on the plugins you've loaded as to whether you call overwrite or add.

[ayrton]

it('fails when the actual is not an enzyme wrapper'), () => {})

[ayrton]

Looking good @marcodejongh 🙌, let's wait for @vesln and @andreasklinger to chime in before merging.