enzymejs / enzyme

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

Discussion: Framework "Recommendations" #472

Open blainekasten opened 8 years ago

blainekasten commented 8 years ago

I think it'd be good to have a discussion to build a doc for our "recommendation" for testing framework wrapped components (redux). This comes up a lot. The real answer is that it's not this libraries scope to handle framework specifics. But it comes up a lot so we should have a discussion about how to handle it.

Personally I think there are a couple good options here:

As far as docs go. It seems like the easiest recommendation that keeps coming up is to suggest exporting the non-framework-wrapped version of a component.

export class PrivateComp { }

export const Component = frameworkWrapper(PrivateComponent, ...);
// or
export default frameworkWrapper(PrivateComponent, ...);

Looking to hear from the community on how they handle testing framework wrapped components. Extra curious in MobX users as it's got a lot of attention lately.

jacobrask commented 8 years ago

Note that Redux already exposes the unconnected component as the WrappedComponent property on the connected component.

aweary commented 8 years ago

This is a problem for anyone who uses higher-order components, if anything we might want to document how to handle those in the general case, and then maybe use Redux or something as a specific example.

jacobrask commented 8 years ago

I think there are 2 issues. The first is to test the a component wrapped in a higher-order component. In that case I think the WrappedComponent approach works well and can be encouraged.

The other is to test a component that has a child that depends on context, like Redux.

EmilTholin commented 8 years ago

I don't want to talk for the entire MobX-community, but I personally think enzyme in conjunction with something lightweight like tape or blue-tape is all that is needed. I don't see the need for an entirely new module:

// counterFactory.js
function counterFactory() {
  let c = observable({
    value: 0
  });
  c.increment = function () {
    ++c.value;
  };
  return c;
};

// IncrementButton.js
const IncrementButton = observer(['counter'], React.createClass({
  render: function () {
    let {counter} = this.props;
    return <button onClick={counter.increment}>{counter.value}</button>;
  }
}));

// App.js
const c = counterFactory();

ReactDOM.render(
  <Provider counter={c}>
    <div>
      <IncrementButton />
    </div>
  </Provider>,
  document.getElementById('app')
);

// test.js
import {mount} from 'enzyme';
import test from 'tape';

test('that clicking the button increments the counter', function (t) {
  t.plan(2);

  let c = counterFactory();
  t.equal(c.value, 0);
  // "If a component ask a store and receives a store via a property with the
  // same name, the property takes precedence. Use this to your advantage
  // when testing!"
  let wrapper = mount(<IncrementButton counter={c} />);
  wrapper.find('button').simulate('click');
  t.equal(c.value, 1);
});
mweststrate commented 8 years ago

@EmilTholin agreed, mobx-react is tested using enzyme (since recently, made stuff a lot simpler, thanks!) and tape / tape-run (not even JSX):

var React = require('react');
var enzyme = require('enzyme');
var mount = enzyme.mount;
var mobx = require('mobx');
var observer = require('../').observer;
var Provider = require('../').Provider;
var test = require('tape');
var e = React.createElement;

test('basic context', t => {
    var C = observer(["foo"], React.createClass({
        render: function() {
            return e("div", {}, "context:" + this.props.foo);
        }
    }));

    var B = React.createClass({
        render: function() {
            return e(C, {});
        }
    });

    var A = React.createClass({
        render: function() {
            return e(Provider, { foo: "bar" }, e(B, {}))
        }
    })

    const wrapper = mount(e(A));
    t.equal(wrapper.find("div").text(), "context:bar");
    t.end();
})
blainekasten commented 8 years ago

@gaearon, do you have any docs/recommendations for redux connected component testing? @jacobrask already noted the .WrappedComponent accessor. And we've often suggested privately exporting the base component. Anything else you think worth noting?

gaearon commented 8 years ago

I’m not sure I understand the question. It depends on what you want to test, right?

blainekasten commented 8 years ago

Yes. But I think mostly you just want to test the underlying component. testing the connection is sort of like writing a test to make sure redux does it's job. So I didn't know if you had any specific recommendations.

Assume a component like this:

export function Comp({foo, bar}) {
  ...
}

export default connect(fn, fn)(Comp);

I think the 3 options to test are basically this:

import { Comp }, ConnectedComponent from './';
const props = { foo: true, bar: false };

mount(<Comp {...props} />);
// test exported component

mount(<ConnectedComponent.WrappedComponent {...props} />);
// access underlying component

mount(<MockProvider><ConnectedComponent /></MockProvider);
// some how mock the provider to provide the props properly

So when your tests are aiming to verify UI and interaction based on props/state, is there any other way you've suggested testing? Or any part of testing a component that I'm missing?

JonPSmith commented 8 years ago

Hi,

I don't know if it helps but I got rather lost on how to test nested React components and ended up writing some simple components and Unit Test to try both enzyme and ReactTestUtils.

As a result I wrote an article called Unit Testing React components that use Redux which describes how to test React components that use Redux with enzyme test utility. It also points out the problems of testing nested components that use Redux.

Hope that helps someone.

lsimmons2 commented 7 years ago

@blainekasten, did you every land on a best option for this? Could you recommend any part of the docs/any resource for learning how to best test framework-wrapped components? Thanks!

hellqvist commented 7 years ago

@lsimmons2 I've been wrestling with a similar problem with a component wrapped with polyglot/translate and simply resolved it by importing the component as usual in the test

import Comp from './Comp';

Then I tested the wrapped component (the actual component) by writing like this

shallow();

Works like a charm.

By logging Comp to the console you see that WrappedComponent is a part of it

console.log(Comp);

Satyam commented 7 years ago

Going back to @blainekasten example, assuming I have:

export function Comp({foo, bar}) {
  ...
}

export default connect(fn, fn)(Comp);

I can successfully test Comp by importing its un-connected version:

import { Comp } from './';
const props = { foo: true, bar: false };

shallow(<Comp {...props} />);

However, when I try to test the component ParentComp that uses Comp, it fails:

// ParentComp:
import Comp from './';

export function ParentComp({foo, bar}) {
   ...
}

export default connect(fn, fn)(ParentComp);

In testing, I do:

import { ParentComp } from './';
const props = { foo: true, bar: false };

shallow(<ParentComp {...props} />);

Basically, it is the same as the test code for Comp except it uses ParentComp,

The problem is that in testing ParentComp, though I use the un-connected version, I can't prevent it from importing the connected Comp. Since connect is executed on importing Comp, whether I do a shallow or a deep render the result is the same. Even if shallow would not try to render Comp, connect fails when Comp is imported.

I also tried passing a context with a mock store as the second argument to shallow, but the problem arises well before shallow is called, just by importing the un-connected ParentComp, which imports the connected Comp (instead of the un-connected one), the errors pops up.

Any ideas? Thanks in advance.

ljharb commented 7 years ago

@Satyam try shallow(<Parent />).dive()?

Satyam commented 7 years ago

@ljharb The shadow call already fails, so I can't call any method on its wrapper.

Anyway, it did give me an idea and it produced a funny error message that got me thinking. So, it did help, thanks.

One of the many things I tried was to pass shallow a second argument with a context containing an instance of mockStore. It wasn't taking it, until I realized this:

shallow(<ParentComp {...props} />, { context: { store }});

The problem was that since it had gone into JSX mode upon finding the <ParentComp, it remained in JSX mode when reaching the comma.

shallow((<ParentComp {...props} />), { context: { store }});

Adding an extra set of parenthesis makes it work.

THanks

ljharb commented 7 years ago

That doesn't sound like how JSX parsing works, but maybe you're not using the normal babel transform? At any rate, you definitely need to explicitly pass context to a component that needs it, so glad that worked 🎉

hellqvist commented 7 years ago

@Satyam what error do you get.

Satyam commented 7 years ago

@hellqvist I tried to reproduce the error and failed, which means my previous diagnostics was wrong. Good you asked so I can correct it.

So I started looking into the combinations I had made before I got it working. Instead of trying things at random I made a test for each possible combination. What I found out was that the culprit was calling method .html() on the wrapper returned by shallow. I was (and still am) trying out enzyme so I wanted to see what came out so I was sending the results to the console. When using .html() the wrapper does go deep and renders everything. If I use .text() instead, I get no error. The error message is:

Invariant Violation: Could not find "store" in either the context or props of "Connect(Comp)". 
Either wrap the root component in a <Provider>, or explicitly pass "store" as a prop to "Connect(Comp)".

where Comp is the child of ParentComp, which I am testing, enhanced by react-redux connect HoC.

Calling method .html() gives the above error whether I provide it with a store in the context option in the second argument to shallow or not. Calling .text() never produces an error, whether I provide it with a store or not.

So, the problem is with .html() which, it seems, tries to do a deep render while ignoring the context I give it.

If this is expected behavior it would be nice to have a warning on it.

Thanks and sorry for the confusion.

hellqvist commented 7 years ago

@Satyam Ok, I've had the same issue and solved it by doing as in the message, adding a Provider and pass store into it.

import Provider from 'react-redux';

const store = {
    getState: function() {
        return {};
    },
    subscribe: function() {
        return {};
    },
    dispatch: function() {
        return {};
    }
};

shallow(<Provider store={store}>
                                    <ParentComp />
                                </Provider>);

I haven't had any problems using html() this way.

naomiajacobs commented 7 years ago

I've had the same issue as @Satyam, the problem is that if we wrap our component in <Provider />, then the component we are actually testing is no longer the root component, and we lose access to many of the enzyme functions that are only available on the root.

ljharb commented 7 years ago

Use .dive() to get to the wrapped component.

naomiajacobs commented 7 years ago

@ljharb we were using mount, not shallow (and therefore didn't have access to dive) - we needed to do this because we were testing ComponentA, which defines ComponentB (which included our connected component), and renders third-party ComponentC, passing down ComponentB as a prop to ComponentC.

We wanted to test the behavior of our code with the third-party code together, so we used mount.

Solution: In our specs, we define contextTypes: { store: React.PropTypes.object } on ComponentA, and delete ComponentA.contextTypes to clean up when we are done.

valentinvoilean commented 7 years ago

Guys, any news about this issue ? Using .dive doesn't help. Neither passing a fake store as context.

As a temporary workaround, I'm mocking the child components which connect to redux.

import Checkout from './Checkout'; // the component I want to test

/* The child components that connect to the store */
jest.mock('components/Cart/Cart', () => 'Cart');
jest.mock('components/Products/Products', () => 'Products');

const wrapper =  shallow(<Checkout />);

describe('Checkout', () => {
  it('should render', () => {
    expect(wrapper.find('Cart')).toBePresent();
    expect(wrapper.find('Products')).toBePresent();
  });
});
ljharb commented 7 years ago

@valentinvoilean I'm confused - you're mocking out modules that export components with a string?

I'd recommend not mocking out anything at all.

valentinvoilean commented 7 years ago

@ljharb Doing jest.mock('components/Cart/Cart', () => 'Cart' }) will produce <Cart />

This is the output I get when I do console.log(wrapper.html()):

<div><Products></Products><Cart></Cart></div>

valentinvoilean commented 7 years ago

Just wanted to mention one more thing.. Only the children connect to the store. The parent component it is a dumb component. This means, the children are still mounted somehow since there are displayed the store context errors. And yes, I would not mock anything if the "shallow" method would work properly.

ljharb commented 7 years ago

What happens if you move the shallow call inside the it? (Generally you don't want to be invoking any production code outside of it or beforeEach in mocha)

skynode commented 7 years ago

@Satyam Thanks for this. I don't completely understand your following diagnostics testing logic, but I was also testing a Redux-connected component with enzyme (which I decided to disconnect from Redux anyway while testing) and, borrowing from what you did, this worked for me:

import {mount} from 'enzyme';
import {ManageUserPage} from './ManageUserPage'; //disconnected version of my component

const props = {
        //initialize user properties
 };
const context = {
        router: {}
 };
//this works without errors or warnings
const wrapper = mount((<ManageUserPage {...props} />), {context}); 
...
...
Satyam commented 7 years ago

@skynode As I said in the my last message on the issue, I made a wrong diagnosis so it was all pointless. Sorry again.