Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Discussion: Framework "Recommendations" #472

Open
blainekasten opened this issue Jun 27, 2016 · 28 comments
Open

Discussion: Framework "Recommendations" #472

blainekasten opened this issue Jun 27, 2016 · 28 comments

Comments

@blainekasten
Copy link
Contributor

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:

  • A seperate library for making library specific connections easier. e.g., enzyme-redux, enzyme-coflux, enzyme-mobx.
    • This would provide helper utilities to make it simple (maybe there are already libraries for this?).
  • Docs on our recommendations

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
Copy link

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

@aweary
Copy link
Collaborator

aweary commented Jun 27, 2016

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.

@aweary aweary added the docs label Jun 27, 2016
@jacobrask
Copy link

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
Copy link

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
Copy link

@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
Copy link
Contributor Author

@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
Copy link
Contributor

gaearon commented Jun 30, 2016

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

@blainekasten
Copy link
Contributor Author

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
Copy link

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
Copy link

@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
Copy link

@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(<Comp.WrappedComponent />);

Works like a charm.

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

console.log(Comp);

@Satyam
Copy link

Satyam commented Jan 30, 2017

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
Copy link
Member

ljharb commented Jan 30, 2017

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

@Satyam
Copy link

Satyam commented Jan 30, 2017

@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
Copy link
Member

ljharb commented Jan 30, 2017

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
Copy link

@Satyam what error do you get.

@Satyam
Copy link

Satyam commented Jan 31, 2017

@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
Copy link

hellqvist commented Jan 31, 2017

@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
Copy link

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
Copy link
Member

ljharb commented Feb 2, 2017

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

@naomiajacobs
Copy link

@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
Copy link

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
Copy link
Member

ljharb commented Feb 12, 2017

@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
Copy link

valentinvoilean commented Feb 12, 2017

@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
Copy link

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
Copy link
Member

ljharb commented Feb 13, 2017

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
Copy link

skynode commented Apr 2, 2017

@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
Copy link

Satyam commented Apr 2, 2017

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests