Material UI components complicate testing of other (general) components significantly #4664

bingomanatee · 2016-07-09T17:11:12Z

Description

Remove the dependency on muiTheme

The fact that UI components constantly require a theme context makes general testing significantly more complex.

Steps to reproduce

Here is a sample test not focused on MUI components.

import React from 'react'
import { Header } from 'components/Header/Header'
// import classes from 'components/Header/Header.scss'
// import { IndexLink, Link } from 'react-router'
import { shallow } from 'enzyme'
import muiTheme from 'themes/base';

describe('(Component) Header', () => {
  let _wrapper;

  beforeEach(() => {
    _wrapper = shallow(<Header />, {context: {muiTheme}});
  });

  describe('Navigation links...', () => {

    it('Should render a logo header', () => {
      expect(_wrapper.text()).to.equal('<IndexLink /><Connect(LoginButtonContainer) /><Connect(EatTwitterButtonContainer) />');
    });
  });
});

Consistently having to inject themes where they are not relevant for testing adds a significant overhead to the process. If the MUI classes could just neutralize style rendering where no theme is present the components would have a lot less overhead in the testing context.

Dave

nathanmarks · 2016-07-10T12:10:10Z

@bingomanatee The example above works for me without putting a theme on context, what error are you getting?

nicolasiensen · 2016-07-10T18:26:23Z

@nathanmarks I agree that the example above should work, perhaps what @bingomanatee was pointing out was that we should not have to worry about passing MUI theme in the context when testing components that relies on MUI

nathanmarks · 2016-07-10T18:28:41Z

@nicolasiensen you don't need to unless you're rendering an entire component tree

bingomanatee · 2016-07-10T18:29:52Z

the above example works; however, to have to put a muiTheme in context for EVERY unit test is a bit of a chore, when the point of the tests have nothing to do with the muiTheme. That also means, for instance that all the expectations have to look for the style node that muiTheme produces, further cluttering the test code.

My proposal is that when a mui class finds that there is no muiTheme in context, no style node should be produced at all. Its not likely to happen in production and would streamline testability a lot.

nathanmarks · 2016-07-10T18:30:29Z

@bingomanatee you don't need to put muiTheme in context for every unit test

nathanmarks · 2016-07-10T18:30:57Z

have you got a complete example of the issue with unit tests?

nathanmarks · 2016-07-10T18:34:30Z

component

import React, { Component } from 'react';
import RaisedButton from 'material-ui/RaisedButton';

export default class Home extends Component {

  render() {
    return (
      <div>
        <img src="/assets/images/logo.svg" />
        <h1>Helloooo</h1>
        <RaisedButton label="woof" />
      </div>
    );
  }
}

test

/* eslint-env mocha */
import React from 'react';
import { assert } from 'chai';
import { shallow } from 'enzyme';
import Home from './Home';

describe('Home', () => {
  it('should render', () => {
    const wrapper = shallow(<Home />);
    assert.ok(wrapper);
  });
});

result

> cross-env NODE_ENV=test babel-node test/index.js


  Home
    ✓ should render


  1 passing (12ms)

oliviertassinari · 2016-07-17T19:42:27Z

@bingomanatee I'm not sure to understand what the actual issue is.
I feel like the most actionnable thing than we can do is to put a note on the documentation regarding integration testing.
For unit tests you shouldn't need to worry about the actual implementation using context or not.

nathanmarks · 2016-07-18T21:35:20Z

@oliviertassinari In next, people will be able to use this:

https://github.com/callemall/material-ui/blob/next/test/utils/createMountWithContext.js

I'm wondering if that, (and the shallow wrapper in another file) should babelified and published under material-ui-test-utils. Or just published in a folder material-ui/test-utils

Szarko · 2016-09-14T20:05:00Z

@nathanmarks - Use mount not shallow in your example and it fails.

nathanmarks · 2016-09-15T14:24:41Z

@Szarko the example was for shallow, see my last reply RE mount as it applies to the next branch

acpower7 · 2016-12-08T01:18:52Z

^Thanks. But can you offer a solution to this branch? How do you resolve the context muitheme issue (with mount)?

gfarrell · 2017-01-10T15:23:35Z

I'm having this problem having updated MaterialUI, for example in List.js you have

const {prepareStyles} = this.context.muiTheme;

This breaks my tests unless I include some muitheme in the context, which is itself extraneous to the tests.

igorrmotta · 2017-02-06T23:57:30Z

It solves the problem.

childContextTypes:{
  muiTheme: React.PropTypes.object.isRequired,
},
getChildContext() {
  return {muiTheme: getMuiTheme(baseTheme)};
},

I don't like to have this boilerplate code into all my components. But, it is what it is.

oliviertassinari · 2017-02-07T20:09:45Z

Or just published in a folder material-ui/test-utils

@nathanmarks I really like that idea! I think that I will continue my documentation improvement effort on the next branch with a Writing Tests section. I like the way redux is documenting it.

BelfordZ · 2017-02-17T23:17:39Z

Could we please have material-ui not throw if you haven't wrapped your components in a theme?

this is pretty much a deal breaker for using material-ui. Im not adding any code to my components to make material-ui work in tests. Nor am I interested in having boiler plate in my tests.

valle-xyz · 2017-02-20T05:04:30Z

Yap, that is unnecessary. If you mount a Component, Material-UI throws an error.

valle-xyz · 2017-02-20T05:13:04Z

Just for people that need a fast solution: http://stackoverflow.com/questions/38264715/how-to-pass-context-down-to-the-enzyme-mount-method-to-test-component-which-incl

But the issue remains: This is actually unnecessary boilerplate.

PolGuixe · 2017-03-05T00:15:08Z

The approach above doesn't work when using:

import muiThemeable from 'material-ui/styles/muiThemeable';

To inject the muiTheme as a property to the component.

If you need to access muiTheme from inside the component you need to do it through the context.

@oliviertassinari mentioned that using muiThemeable is recommend over context.

PolGuixe · 2017-03-05T00:25:49Z

In my case:

Component to test:

import React, { PropTypes } from 'react';
import _ from 'lodash';
import { List, ListItem, Avatar, Subheader } from 'material-ui';
import muiThemeable from 'material-ui/styles/muiThemeable';

const getDefaultStyles = palette => ({
  price: {
    backgroundColor: 'none',
    color: palette.textColor,
  },
});


class CartList extends React.PureComponent {
  // FIXME not recomended, better to use muiThemeable, however not compatible with testing
  static contextTypes = {
    muiTheme: React.PropTypes.object.isRequired
  };

  render() {
    const { items, subheader } = this.props;

    const defaultStyles = getDefaultStyles(this.context.muiTheme.palette);

    return (
      <List>
        {subheader
          ? <Subheader>{subheader}</Subheader>
          : null}
        {_.map(items, (item, index) => (
          <ListItem
            key={index}
            primaryText={item.name}
            secondaryText={item.info}
            leftAvatar={<Avatar src={item.thumbnail} />}
            // TODO not use and avatar if we need to add more price information, discounts, etc
            rightAvatar={
              <Avatar
                backgroundColor={defaultStyles.price.backgroundColor}
                color={defaultStyles.price.color}
              >
                {item.price}
              </Avatar>
            }
          />
        ))}
      </List>
    );
  }
}

export default CartList;

Test:

import React from 'react';
import { shallow } from 'enzyme';
import chai, { expect } from 'chai';
import chaiEnzyme from 'chai-enzyme';
import { Subheader } from 'material-ui';
import getMuiTheme from 'material-ui/styles/getMuiTheme';

import CartList from '../CartList.jsx';

const { describe, it } = global;
chai.use(chaiEnzyme());

const muiTheme = getMuiTheme();
const shallowWithContext = node => shallow(node, {
  context: { muiTheme },
  childContextTypes: { muiTheme: React.PropTypes.object },
});


const productList = [
  {
    name: 'XXXX',
    info: 'ajksdnfakjsfndkjsf/akhad/kjsnka',
    thumbnail: '/images/default_caroussel.jpg',
    price: '29€',
  },
  {
    name: 'YYY',
    info: 'ajksdnfakjsfndkjsf/akhad/kjsnka',
    thumbnail: '/images/default_caroussel.jpg',
  },
  {
    name: 'ZZZ',
    info: 'ajksdnfakjsfndkjsf/akhad/kjsnka',
    thumbnail: '/images/default_caroussel.jpg',
  },
];

const subheader = 'subheader test';

describe('CartList', () => {
  it('should render with subheader', () => {
    const cartList = shallowWithContext(<CartList items={productList} subheader={subheader} />);
    expect(cartList).to.contain(<Subheader>{subheader}</Subheader>);
    expect(cartList).to.have.text(subheader);
  });
});

Error:

AssertionError: expected <CartList /> to have text 'subheader test', but it has '<List />' HTML: Not available due to: Cannot read property 'prepareStyles' of undefined

disclaimer: I am new to testing, so I might be doing something else wrong

valle-xyz · 2017-03-05T02:14:23Z

I think you are totally right. But still this is overhead an some people want their tests to be as lean as possible. PolGuixe <[email protected]> schrieb am So., 05.03.2017, 8:26:

…

In my case: Component to test: import React, { PropTypes } from 'react'; import _ from 'lodash'; import { List, ListItem, Avatar, Subheader } from 'material-ui'; import muiThemeable from 'material-ui/styles/muiThemeable'; const getDefaultStyles = palette => ({ price: { backgroundColor: 'none', color: palette.textColor, }, }); class CartList extends React.PureComponent { // FIXME not recomended, better to use muiThemeable, however not compatible with testing static contextTypes = { muiTheme: React.PropTypes.object.isRequired }; render() { const { items, subheader } = this.props; const defaultStyles = getDefaultStyles(this.context.muiTheme.palette); return ( <List> {subheader ? <Subheader>{subheader}</Subheader> : null} {_.map(items, (item, index) => ( <ListItem key={index} primaryText={item.name} secondaryText={item.info} leftAvatar={<Avatar src={item.thumbnail} />} // TODO not use and avatar if we need to add more price information, discounts, etc rightAvatar={ <Avatar backgroundColor={defaultStyles.price.backgroundColor} color={defaultStyles.price.color} > {item.price} </Avatar> } /> ))} </List> ); } } export default CartList; Test: import React from 'react'; import { shallow } from 'enzyme'; import chai, { expect } from 'chai'; import chaiEnzyme from 'chai-enzyme'; import { Subheader } from 'material-ui'; import getMuiTheme from 'material-ui/styles/getMuiTheme'; import CartList from '../CartList.jsx'; const { describe, it } = global; chai.use(chaiEnzyme()); const muiTheme = getMuiTheme(); const shallowWithContext = node => shallow(node, { context: { muiTheme }, childContextTypes: { muiTheme: React.PropTypes.object }, }); const productList = [ { name: 'XXXX', info: 'ajksdnfakjsfndkjsf/akhad/kjsnka', thumbnail: '/images/default_caroussel.jpg', price: '29€', }, { name: 'YYY', info: 'ajksdnfakjsfndkjsf/akhad/kjsnka', thumbnail: '/images/default_caroussel.jpg', }, { name: 'ZZZ', info: 'ajksdnfakjsfndkjsf/akhad/kjsnka', thumbnail: '/images/default_caroussel.jpg', }, ]; const subheader = 'subheader test'; describe('CartList', () => { it('should render with subheader', () => { const cartList = shallowWithContext(<CartList items={productList} subheader={subheader} />); expect(cartList).to.contain(<Subheader>{subheader}</Subheader>); expect(cartList).to.have.text(subheader); }); }); Error: AssertionError: expected <CartList /> to have text 'subheader test', but it has '<List />' HTML: Not available due to: Cannot read property 'prepareStyles' of undefined *disclaimer: I am new to testing, so I might be doing something else wrong* — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#4664 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AKG_Z5EI-fuFH-9QF7H80BKieg0W_bdEks5rigErgaJpZM4JIqLq> .

oliviertassinari · 2017-03-05T12:37:00Z

@PolGuixe True, the more the context is abstracted for users, the better.
@Valentin-Seehausen I only see two options going forward:

We expose the test helpers we are using internally and we document it
We make the context optional for rendering the components and we document it

I don't think that option 1. and 2. are mutually exclusive.

The option 1. allows to simplify the integration tests with the mount API, unless you also want to unit test our component, then you will need the shallow helper too.
The option 2. will make it even no brainer, users won't have to look into the documentation/issue as soon as they realize that they need something more to tests their component. However, that's code branching, they might not catch everything
I have tried that with the avatar on the next branch I only had to change one line:

-const classes = context.styleManager.render(styleSheet);
+const classes = context.styleManager ? context.styleManager.render(styleSheet) : {};

PolGuixe · 2017-03-05T15:04:48Z

@oliviertassinari where can I find the test helpers you are using? I am still having problems.

oliviertassinari · 2017-03-05T15:07:07Z

I'm referring to those two helpers.

valle-xyz · 2017-03-13T08:51:36Z

@oliviertassinari Awesome.

We could warn, when the context is not provided.

jtrein · 2017-05-25T02:33:21Z

@PolGuixe's above test worked for me using the helper he created to provide context. So, thank you for that. To be fair, I don't know a lot about React context (only a bit from React Router).

My question(s) is: why do we need the theme's context unless we're running Jest snapshot testing? I just want to do "simple" unit-testy things. Is this a React limitation of not being able to de-couple a context from a lib component? Curious.

jtrein · 2017-05-25T04:39:18Z

Here's what I'm using based on the helpful above code. I have it stored in a testUtils.js file

import { mount, shallow } from 'enzyme';
import getMuiTheme from 'material-ui/styles/getMuiTheme';
import PropTypes from 'prop-types';

// enzyme MUI Test Helpers
// - https://github.com/callemall/material-ui/issues/4664

const muiTheme = getMuiTheme();

/**
* MuiMountWithContext
*
* For `mount()` full DOM rendering in enzyme.
* Provides needed context for mui to be rendered properly
* during testing.
*
* @param {obj}    node - ReactElement with mui as root or child
* @return {obj}   ReactWrapper (http://airbnb.io/enzyme/docs/api/ReactWrapper/mount.html)
*/
export const MuiMountWithContext = node => mount(node, {
  context: { muiTheme },
  childContextTypes: { muiTheme: PropTypes.object },
});

/**
* MuiShallowWithContext
*
* For `shallow()` shallow rendering in enzyme (component only as a unit).
* Provides needed context for mui to be rendered properly
* during testing.
*
* @param {obj}     node - ReactElement with mui
* @return {obj}    ShallowWrapper (http://airbnb.io/enzyme/docs/api/ShallowWrapper/shallow.html)
*/
export const MuiShallowWithContext = node => shallow(node, {
  context: { muiTheme },
  childContextTypes: { muiTheme: PropTypes.object },
});

gnapse · 2017-06-16T16:12:23Z

I bumped into this problem recently, and hence into this thread. What's up with that next coming up? Most links in this thread to it are dead. What's the final word about how to approach this problem? Is either of these two approaches available?

We expose the test helpers we are using internally and we document it

We make the context optional for rendering the components and we document it

oliviertassinari · 2017-06-16T18:30:51Z

@gnapse Oops, I have forgotten to close the issue, we went with option 1. by exposing test helpers. We have considered option 2. but it's making the implementation more complex. We think that option 1. is a better tradeoff. Here is the new documentation section on the next branch.

gnapse · 2017-06-16T19:11:43Z

@oliviertassinari thanks a lot for the info. And actually, thanks a lot for exposing me to this new/next re-implementation and the corresponding documentation website. How stable is this next thing? I love material-ui but I'm having a hard time accepting some things we dislike in our team about it (styles-in-js being a big one) and a quick glance tells me this next thing gets away from that, right? We were just starting to integrate this into our very young app, and I'd rather work with the next version than the current master one.

oliviertassinari · 2017-06-16T19:22:51Z

How stable is this next thing?

From an API point of view, we release breaking changes from one release to another. You can have a look at the releases notes to see if you are comfortable with that.
From a code point of view, I do no longer use the master branch in production, I have moved to the next version.

right?

Right.

RobertHeim · 2017-08-13T10:02:36Z

Since our tests most often do not test the mui-wrapper but the actual component we added up a dive() to jtrein's suggestion above:

export const MuiShallowWithContext = node => shallow(node, {
  context: { muiTheme },
  childContextTypes: { muiTheme: PropTypes.object },
}).dive(); // shallow-render the actual component

oliviertassinari · 2017-08-13T10:15:52Z

The v1-beta branch do no longer require a context to be present. It's transparent now.

oliviertassinari added docs Improvements or additions to the documentation discussion labels Jul 17, 2016

oliviertassinari added test and removed discussion labels Jan 16, 2017

jasiek-net mentioned this issue Mar 6, 2017

[Dialog][Enzyme] mount not render inner components, stops on <RenderToLayer /> #6290

Closed

oliviertassinari mentioned this issue Mar 19, 2017

[test] Expose the test helpers #6383

Merged

3 tasks

danielcodes mentioned this issue Mar 25, 2017

Using material-ui components broke tests CUNYTech/JobQuest#23

Closed

mbrookes mentioned this issue Apr 11, 2017

How to test a TextField material UI element whit React Jest? #6571

Closed

Ragnar-H mentioned this issue May 12, 2017

Login integration test veritus/veritus-web#20

Merged

oliviertassinari added the next label Jun 5, 2017

oliviertassinari mentioned this issue Jun 10, 2017

[docs] Add testing section #7101

Merged

oliviertassinari closed this as completed Jun 16, 2017

oliviertassinari mentioned this issue Jun 16, 2017

Consider Excluding test-utils folder #7157

Closed

mario-zancanaro mentioned this issue Nov 2, 2017

TypeError: undefined is not an object (evaluating 'color.charAt') #8960

Closed

1 task

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Material UI components complicate testing of other (general) components significantly #4664

Material UI components complicate testing of other (general) components significantly #4664

bingomanatee commented Jul 9, 2016 •

edited by oliviertassinari

Loading

nathanmarks commented Jul 10, 2016 •

edited

Loading

nicolasiensen commented Jul 10, 2016 •

edited

Loading

nathanmarks commented Jul 10, 2016

bingomanatee commented Jul 10, 2016

nathanmarks commented Jul 10, 2016

nathanmarks commented Jul 10, 2016 •

edited

Loading

nathanmarks commented Jul 10, 2016

oliviertassinari commented Jul 17, 2016 •

edited

Loading

nathanmarks commented Jul 18, 2016 •

edited

Loading

Szarko commented Sep 14, 2016

nathanmarks commented Sep 15, 2016

acpower7 commented Dec 8, 2016

gfarrell commented Jan 10, 2017

igorrmotta commented Feb 6, 2017

oliviertassinari commented Feb 7, 2017 •

edited

Loading

BelfordZ commented Feb 17, 2017 •

edited

Loading

valle-xyz commented Feb 20, 2017

valle-xyz commented Feb 20, 2017

PolGuixe commented Mar 5, 2017

PolGuixe commented Mar 5, 2017

valle-xyz commented Mar 5, 2017 via email

oliviertassinari commented Mar 5, 2017 •

edited

Loading

PolGuixe commented Mar 5, 2017

oliviertassinari commented Mar 5, 2017

valle-xyz commented Mar 13, 2017

jtrein commented May 25, 2017

jtrein commented May 25, 2017 •

edited

Loading

gnapse commented Jun 16, 2017

oliviertassinari commented Jun 16, 2017

gnapse commented Jun 16, 2017

oliviertassinari commented Jun 16, 2017 •

edited

Loading

RobertHeim commented Aug 13, 2017

oliviertassinari commented Aug 13, 2017

Material UI components complicate testing of other (general) components significantly #4664

Material UI components complicate testing of other (general) components significantly #4664

Comments

bingomanatee commented Jul 9, 2016 • edited by oliviertassinari Loading

Description

Steps to reproduce

nathanmarks commented Jul 10, 2016 • edited Loading

nicolasiensen commented Jul 10, 2016 • edited Loading

nathanmarks commented Jul 10, 2016

bingomanatee commented Jul 10, 2016

nathanmarks commented Jul 10, 2016

nathanmarks commented Jul 10, 2016 • edited Loading

nathanmarks commented Jul 10, 2016

oliviertassinari commented Jul 17, 2016 • edited Loading

nathanmarks commented Jul 18, 2016 • edited Loading

Szarko commented Sep 14, 2016

nathanmarks commented Sep 15, 2016

acpower7 commented Dec 8, 2016

gfarrell commented Jan 10, 2017

igorrmotta commented Feb 6, 2017

oliviertassinari commented Feb 7, 2017 • edited Loading

BelfordZ commented Feb 17, 2017 • edited Loading

valle-xyz commented Feb 20, 2017

valle-xyz commented Feb 20, 2017

PolGuixe commented Mar 5, 2017

PolGuixe commented Mar 5, 2017

valle-xyz commented Mar 5, 2017 via email

oliviertassinari commented Mar 5, 2017 • edited Loading

PolGuixe commented Mar 5, 2017

oliviertassinari commented Mar 5, 2017

valle-xyz commented Mar 13, 2017

jtrein commented May 25, 2017

jtrein commented May 25, 2017 • edited Loading

gnapse commented Jun 16, 2017

oliviertassinari commented Jun 16, 2017

gnapse commented Jun 16, 2017

oliviertassinari commented Jun 16, 2017 • edited Loading

RobertHeim commented Aug 13, 2017

oliviertassinari commented Aug 13, 2017

bingomanatee commented Jul 9, 2016 •

edited by oliviertassinari

Loading

nathanmarks commented Jul 10, 2016 •

edited

Loading

nicolasiensen commented Jul 10, 2016 •

edited

Loading

nathanmarks commented Jul 10, 2016 •

edited

Loading

oliviertassinari commented Jul 17, 2016 •

edited

Loading

nathanmarks commented Jul 18, 2016 •

edited

Loading

oliviertassinari commented Feb 7, 2017 •

edited

Loading

BelfordZ commented Feb 17, 2017 •

edited

Loading

oliviertassinari commented Mar 5, 2017 •

edited

Loading

jtrein commented May 25, 2017 •

edited

Loading

oliviertassinari commented Jun 16, 2017 •

edited

Loading