enzymejs / enzyme

JavaScript Testing utilities for React
https://enzymejs.github.io/enzyme/
MIT License
19.95k stars 2.01k forks source link

Higher Order Component makes valid assertion fail #242

Closed gCardinal closed 8 years ago

gCardinal commented 8 years ago

To be fair, I'm not sure if it's my HOC that is badly written or if really this is a bug.

Say I have this HOC:

import React, { Component, PropTypes } from 'react';

const BaseConversationsView = ComposedComponent => class extends Component {
    static propTypes = {
        conversations: PropTypes.array.isRequired
    };

    render() {
        return <ComposedComponent {...this.props} />;
    }
};

export default BaseConversationsView;

Used by this component (left it simple on purpose):

import React, { Component, PropTypes } from 'react';
import CSSModules from 'react-css-modules';
import BaseConversationsView from 'views/BaseConversationsView';
import ConversationsList from 'components/ConversationsList/ConversationsList';
import styles from './Views.scss';

class MobileConversationView extends Component {
    render() {
        let { conversations } = this.props;
        return (
            <div styleName="conversation-view">
                <ConversationsList conversations={conversations} />
            </div>
        );
    }
}

export default BaseConversationsView(
    CSSModules(
        MobileConversationView,
        styles
    )
);

This test will fail, even though it should pass:

import React from 'react'
import 'chai'
import { shallow, ShallowWrapper } from 'enzyme'
import MobileConversationsView from 'views/MobileConversationsView'
import ConversationsList from 'components/ConversationsList/ConversationsList'

let expect = chai.expect;

describe('<MobileConversationView />', () => {
    let mobileConversationView:ShallowWrapper;

    beforeEach(() => {
        mobileConversationView = shallow(
            <MobileConversationsView conversations={[]} onMenuClick={() => {}} />
        );
    });

    it("Should display a <ConversationsList />", () => {
        expect(mobileConversationView.contains(ConversationsList)).to.equal(true);
    });
});

The error isn't very helpful, regular expected false to equal true. If I used chai-enzyme and change the way I do my assertion, I get some more info.

it("Should display a <ConversationsList />", () =>
{
    expect(mobileConversationView.find(ConversationsList)).to.exist;
});

// Error is: AssertionError: expected the node in <_class /> to exist HTML: Not available due to: This method is only meant to be run on single node. 0 found instead.

Now, I made sure the HOC worked. I can display my app no problem and as far as I can tell, the way I wrote my HOC is valid, but Enzyme still seems to have problem with it.

You'll notice I use CSSModule in my examples. Is it because Enzyme does not like this particular way of writting HOC?

ljharb commented 8 years ago

Because you're shallow rendering, the component that will render is actually the wrapped component, not ConversationsList itself - if you use mount, you should be able to locate it.

lelandrichardson commented 8 years ago

@gCardinal just an FYI, we are discussing that there may be a valid "middle ground" rendering mode that doesn't render all the way down the tree like mount would, but is configurable to go more than one level deep, which would get rid of this "issue"

AugustinLF commented 8 years ago

@gCardinal It does make sense to also export the non decorated (or without the HOC) version of you component, in order to test it by itself. This is the approach recommended by react-redux to test connected components.

You can then do

export class MobileConversationView extends Component {
// stuff
}

export default BaseConversationsView(
    CSSModules(
        MobileConversationView,
        styles
    )
);

And in your tests do a import {MobileConversationsView} from 'views/MobileConversationsView'. You'll get more atomic tests, but yes, you'll lose the "real-life" use of your component, that you can anyway test with mount, as recommended by @ljharb

gCardinal commented 8 years ago

@ljharb Oh, that actually makes a ton of sense. I had never used shallow rendering before (React's shallow renderer didn't feel mature enough to me at the time) so it never really occurred to me. I just saw it as a single component even though now that you mention it, it really isn't.

@AugustinLF Yeah that's what I'll do, thanks for the tip.

I'll close this seeing as the issue was with me, not Enzyme, but I will keep an eye out for this "deeper shallow renderer" @lelandrichardson mentioned. I feel like this is the kind of use case where it wouldn't break the philosophy being shallow rendering and help in testing those kinds of components.

kujon commented 8 years ago

Not sure what to think of it, but this behaviour seems to be inconsistent with what's returned by html method, for example:

import withStyles from 'isomorphic-style-loader/lib/withStyles';

const Input = props => <input {...props} />;

const StyledInput = withStyles({/*...*/})(Input);

shallow(<StyledInput type='text' />, {/*...*/}).html(); // > <input type="text" />

shallow(<StyledInput type='text' />, {/*...*/}).is('input'); // > false
shallow(<StyledInput type='text' />, {/*...*/}).find('input').length; // > 0
ljharb commented 8 years ago

@kujon what does shallow(<StyledInput type='text' />, {/*...*/}).debug() print out?

kujon commented 8 years ago

@ljharb <StyledInput type='text' />.

ljharb commented 8 years ago

Ah! Since StyledInput is a HOC - it's actually rendering a newly-created component, and shallow only goes down one level. You either need to do shallow(<StyledInput type='text' />, {/*...*/}).find('StyledInput').shallow(), or, use mount.

kujon commented 8 years ago

@ljharb that's awesome, thanks!