enzymejs / enzyme

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

Improve shallow rendering for use with higher-order components #539

Open lencioni opened 8 years ago

lencioni commented 8 years ago

When using shallow rendering with a component that is wrapped by a higher order component, you need to take some extra steps to make shallow rendering give you the structure you expect. One way is to shallow render the wrapped component, find the unwrapped component within this structure, and then shallow render that. e.g.

const wrapper = shallow(<Foo />).find('Foo').shallow();

Of course, you can also export the unwrapped component as a named export, and use that to find by reference as well.

import Foo, { Foo as UnwrappedFoo } from './Foo';

...

const wrapper = shallow(<Foo />).find(UnwrappedFoo).shallow();

This bums me out a little, because if you have a bunch of tests for a component, and then later decide to wrap that component in a HOC, you need to go back and update all of your tests in this way. Using the string approach feels less than ideal, and the reference approach feels a bit cumbersome when applied on a large scale. I'm wondering if we can come up with a solution that will work well for this scenario. Here are some thoughts.

Do any of these sound good? Should we employ multiple strategies? Are there other possibilities?

nfcampos commented 8 years ago

You can also do

const wrapper = shallow(<Foo />).first().shallow();

I definitely feel this pain, but I'm not sure what the right solution is. But I do lean more towards defining the depth

lencioni commented 8 years ago

That's true, but I think you have to tag on another .first().shallow() for each HOC.

nfcampos commented 8 years ago

yes if you have 3 HOCs it becomes the rather terrible

const wrapper = shallow(<Foo />).first().shallow().first().shallow().first().shallow();
ljharb commented 8 years ago

The only option I like there is #250. Even with multiple HOCs, I think that would work transparently. You'd still always need to export the pure version, but I think that's just an inherent limitation in HOCs that enzyme can't gracefully overcome.

lencioni commented 8 years ago

If I understand that proposal correctly, you would need to export each level of HOC that you wrap your component in, and then pass those all as references to the expand option. I think I would rather use .find('Foo').shallow().

Edit: I think the find approach might have a similar issue actually...

aweary commented 8 years ago

Ideally, I think it would be nice to have an API would be able to find a component at any level in the potential render tree for a component, even if it was too deep to be rendered with a shallow render.

var deepWrapper = wrapper.shallowFromDeepChild('SomeDeepComponent');

Though I think that would be difficult to implement. I like #250 but if you have to export every level of HOC then it doesn't seem to bring any advantage over just testing the exported component directly. It's definitely a hard problem.

lencioni commented 8 years ago

@Aweary I think your suggestion is similar to my thought about adding an until option, which would keep going until it rendered something that matched the selector.

CurtisHumphrey commented 8 years ago

I suggest instead of a list to expand have a list of do not expand. For me and our tests, most of the time we use shallow because we have components inside of it we do not want to expand (big things maybe directly connect to redux etc). We know this list as we build the widget and tests. Then if we later wrap it with HOC that list doesn't change and nothing in the tests have to change, problem solved.

ljharb commented 8 years ago

Ideally we'd want both options I think: "only expand through these" or "expand unless it's one of these".

ljharb commented 8 years ago

re https://github.com/airbnb/enzyme/issues/539#issuecomment-239843015 - specifically, as a separate API from shallow - and I don't think we should allow any sort of "depth" number, since that tightly couples tests to JSX structure.

CurtisHumphrey commented 8 years ago

I second @ljharb on both points: both options and no depth number

oliviertassinari commented 7 years ago

For anyone looking for a temporary solution, we are using an until operator at @doctolib as we use quite some HoC:

cc @matthieuprat for the credit.

enzyme/operator/until.js ``` jsx export default function until(selector, {context} = this.options) { if (this.isEmptyRender() || typeof this.node.type === 'string') return this; const instance = this.instance(); if (instance.getChildContext) { context = { ...context, ...instance.getChildContext(), }; } return this.is(selector) ? this.shallow({context}) : this.shallow({context})::until(selector, {context}); } ```

That we use it like this:

const AppBanner = ({banner, children}) => {
  return (
    <Banner>
      {children}
    </Banner>
  );
};

export default compose(
  pure,
  connect(({
    globalMessagesBanner$,
  }) => ({
    banner: globalMessagesBanner$.startWith('key: common.error'),
  })),
)(AppBanner);
const wrapper = shallow(<AppBanner />, {context});

assert.strictEqual(
  wrapper.until('[banner]').find(Banner).props().children,
  'key: common.error'
);
matthieuprat commented 7 years ago

Here is a Gist with a bunch of tests for this until operator to make it more "graspable": https://gist.github.com/matthieuprat/5fd37abbd4a4002e6cfe0c73ae54cda8

elodszopos commented 7 years ago

Here's a little helper that I use. Works for multiple nesting too (multiple decorators / HoC wrapping).

export function renderComponent(Component, defaultProps = {}) {
  return function innerRender(props = {}, nestLevel) {
    let rendered = shallow(<Component {...defaultProps} {...props} />);

    if (nestLevel) {
      let repeat = nestLevel;

      while (repeat--) {
        rendered = rendered.first().shallow();
      }
    }

    return rendered;
  };
}

And then:

const renderer = renderComponent(Foo); // defined in describe
const wrapper = renderer({ someProp: someValue }, 1); // @connected Foo, to be used in an it block

It's a temporary solution really. I'd like to fully support this effort and if there's anything I can help with, I will.

I'm also all for what @ljharb said in his comment above. The temporary solution that I have tightly couples to JSX and is volatile. Far from ideal.

kumar303 commented 7 years ago

EDIT: Here is a more detailed explanation of this approach: https://hacks.mozilla.org/2018/04/testing-strategies-for-react-and-redux/

This is a helper that is working for me so far (here is the current code and tests). It's nice because you can wrap as many HOCs around your component as you need to without having to update any test code. It's pretty similar to @oliviertassinari's approach but takes a component class instead of a selector.

import { oneLine } from 'common-tags';
import { shallow } from 'enzyme';

/*
 * Repeatedly render a component tree using enzyme.shallow() until
 * finding and rendering TargetComponent.
 *
 * The `componentInstance` parameter is a React component instance.
 * Example: <MyComponent {...props} />
 *
 * The `TargetComponent` parameter is the React class (or function) that
 * you want to retrieve from the component tree.
 */
export function shallowUntilTarget(componentInstance, TargetComponent, {
  maxTries = 10,
  shallowOptions,
  _shallow = shallow,
} = {}) {
  if (!componentInstance) {
    throw new Error('componentInstance parameter is required');
  }
  if (!TargetComponent) {
    throw new Error('TargetComponent parameter is required');
  }

  let root = _shallow(componentInstance, shallowOptions);

  if (typeof root.type() === 'string') {
    // If type() is a string then it's a DOM Node.
    // If it were wrapped, it would be a React component.
    throw new Error(
      'Cannot unwrap this component because it is not wrapped');
  }

  for (let tries = 1; tries <= maxTries; tries++) {
    if (root.is(TargetComponent)) {
      // Now that we found the target component, render it.
      return root.shallow(shallowOptions);
    }
    // Unwrap the next component in the hierarchy.
    root = root.dive();
  }

  throw new Error(oneLine`Could not find ${TargetComponent} in rendered
    instance: ${componentInstance}; gave up after ${maxTries} tries`
  );
}

Here is an example of how I use it. In SomeComponent.js:

import React from 'react';
import { compose } from 'redux';
import { connect } from 'react-redux';

export function SomeComponentBase() {
  return <div>This is the unwrapped component that we want to test</div>;
}

// Pretend that this is a typical Redux mapper.
const mapStateToProps = (state) => ({});

export default compose(
  connect(mapStateToProps),
)(SomeComponentBase);

Example of rendering it for a test:

import SomeComponent, { SomeComponentBase } from './SomeComponent';
import { shallowUntilTarget } from './utils';

// This unwraps SomeComponent until it finds SomeComponentBase:
const root = shallowUntilTarget(<SomeComponent />, SomeComponentBase);
ljharb commented 7 years ago

Linking to #250.

seanconnollydev commented 6 years ago

I wasn't able to get the "shallow until render" approaches to work, so went with an approach that mocks the specific component I want to avoid rendering. Jest mocks are a little quirky, but they did the job. Especially helpful when testing Apollo's Query/graphql components.

https://medium.com/@sconnolly17/testing-graphql-container-components-with-react-apollo-and-jest-9706a00b4aeb

dschinkel commented 5 years ago

My opinion on this:

This bums me out a little, because if you have a bunch of tests for a component, and then later decide to wrap that component in a HOC, you need to go back and update all of your tests in this way

Honestly you've changed your design, your Component Under test is now doing more. It's no longer as simple. So you should expect your tests to break in this case. It's par for the course. When you change your design by introducing an HOC, there's no magic that's going to allow your tests from breaking. Your tests are designed to give you feedback. This is not too much fragility as it is you've changed your contract in a big way inside that component. So it's totally expected if you introduce an HOC like that.

When I TDD I get that kind of feedback early. And it forces me to think a little more about my design up front in baby steps as I go. I expect this kind of feedback from shallow.

Instead of looking for a solution that prevents your tests from breaking and more magic to get around it, think more about how your'e designing your components and when it's natural to expect your tests to break when you completely redesign the contract like that. There's no such thing as "all fragile or all not". You can have a mix of fragility, with expected contracts breaking your tests, both are different cases...so know when changing your design, you're actually getting the feedback you should be getting. If you test after well, it's kinda too late, now you have to try to go and break apart your components to try and make them simpler. That's exactly why I TDD. I don't end up in that situation that much...because I move things around and break things apart early.

I know this is a late reply, but I'm just looking through old issues in here. Thought to comment.

kumar303 commented 5 years ago

Honestly you've changed your design, your Component Under test is now doing more.

Yes but the new HOC is just an implementation detail. The interface of your component did not change so the unit tests for it should not have to change.

dschinkel commented 5 years ago

Yea I just don't think it's a big deal in those cases honestly personally plus you can use a recursive dive() helper method to solve this. But regardless I think if you're doing dive() lot, you have to wonder if maybe you're also actually over using HOCs (yes you can over use it, there are other ways to decouple) and I'm glad hooks are coming out with React as well now. Adding HOCs is actually making your component do more, and more complex. There are ways to break things apart and decouple without having to nest a bunch of HOCs or continuously keep adding HOCs often IMO. I agree with FB that HOCs and render props are really kind of a bad design pattern which is what they admitted in their hooks docs. HOCs are a form of injection but you end up making your component more complex, instead of adhereing to the 4 rules of simple design. It's a design feedback actually IMO with dive/shallow.

you especially don't want to end up with a Christmas tree: render_props-hoc-christmas-tree

dschinkel commented 5 years ago

We could add an until option to shallow that would take a reference to a component or a string reference. This would reduce some of the boilerplate of .find('Foo').shallow() just a little, but I think it has many of the same disadvantages that we currently have

@lencioni can you site what you mean by the following for a shallow until helper:

I think it has many of the same disadvantages that we currently have

plus you'd want that to dive() not shallow for a shallowUntil

lencioni commented 5 years ago

Oh boy I wrote that a long time ago and before we added dive. I don't remember exactly what I meant, but maybe I was thinking about the possibility that Enzyme could be configured to do this automatically so when you introduce or remove an HOC, you don't need to change a bunch of tests. Although, that certainly has its own set of tradeoffs and that probably would be worse in a number of other ways.

dalvallana commented 5 years ago

I'm into something similar here: MyComponent calls a class method under certain conditions in the constructor, so I want to spy on this method before shallowing MyComponent, in order to test it is called when it should. Normally I would do something like:

sinon.spy(MyComponent.prototype, 'myMethod');

But when MyComponent is wrapped in a HOC, I end up doing:

sinon.spy(MyComponent.prototype.constructor.WrappedComponent.prototype, 'myMethod');

which is frankly awful...

I honestly don't know if enzyme provides a nicer way of doing this... Maybe I'm missing something.

ljharb commented 5 years ago

@dalvallana you either have to do that, or to export the wrapped component directly - neither of which is nice. In that specific case, however, you should be able to shallow render the HOC, and before calling .dive(), do spyOn(wrapper.childAt(0).type().prototype, 'myMethod') (i believe that will work).

slikts commented 5 years ago

I wish this was solved in Enzyme instead of there being a potpourri of snippets, gists and unmaintained one-off packages to choose from. Not having official support suggests that there's something unsound about diving based on a selector, but this thread doesn't really explicate what the issues would be.

ljharb commented 5 years ago

@slikts typically you'd use one .dive() per HOC, and not require any selectors.

slikts commented 5 years ago

It's not required in the sense that there is a workaround (chaining .dive()), but the depth of nesting is an implementation detail. For example, I might refactor withABC() into withA(), withB(), etc., and the test failing would be a false negative.

seanconnollydev commented 5 years ago

@slikts I think the root of the problem is that Enzyme provides too many utilities that are dependent on the structure of the component tree rather than something closer to what the user is seeing. This is the reason why I am moving away from Enzyme and towards react-testing-library. With RTL you would solve this problem with jest mocks, similar to what I wrote about in this post:

https://blog.usejournal.com/testing-graphql-container-components-with-react-apollo-and-jest-9706a00b4aeb

ljharb commented 5 years ago

@slikts yes, that is true, but your tests for the component are testing some of the implementation details, and that's ok.

@goldenshun good testing is a balance of surface testing (like what RTL and mount provides) and unit testing (like what shallow provides). Only using one technique is always a mistake. Separately, using mocks and stubs makes tests more brittle, and should be avoided.

goldensunliu commented 5 years ago

@ljharb I think you meant to tag @goldenshun. no worries.

seanconnollydev commented 5 years ago

@ljharb https://github.com/ljharb The entire premise of this thread is that using shallow with higher order components is brittle. Mocks can be brittle too but in this case they are more stable than shallow.

On Fri, Apr 12, 2019, 11:19 PM Sitian Liu notifications@github.com wrote:

@ljharb https://github.com/ljharb I think you meant to tag @goldenshun https://github.com/goldenshun. no worries.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/airbnb/enzyme/issues/539#issuecomment-482777630, or mute the thread https://github.com/notifications/unsubscribe-auth/AAWY34NBXORCBQEC7K4BBALPQFUP5ANCNFSM4CMM3SWA .

ljharb commented 5 years ago

@goldenshun i suppose that’s subjective; i don’t find that to be the case.

Certainly there is an improvement to be made here, but the brittleness you’re referring to is “you have to match the count of HOCs with the count of dives”. In practice, this is only mildly annoying - the brittleness of mocks is that your code will fail in production but your tests won’t show that, because the mocks aren’t up to date and mask the errors.

seanconnollydev commented 5 years ago

@ljharb Your criticism of mocks is true for both jest mocks and shallow rendering. They are both forms of mocking because they both halt rendering of the component tree at a certain later of the component tree. As with any unit test, they will not detect all classes of error.

My submission is that jest mocks are a more reliable mechanism for the case at hand because they will be more resilient to changes in the component structure.

On Sat, Apr 13, 2019, 8:59 AM Jordan Harband notifications@github.com wrote:

@goldenshun https://github.com/goldenshun i suppose that’s subjective; i don’t find that to be the case.

Certainly there is an improvement to be made here, but the brittleness you’re referring to is “you have to match the count of HOCs with the count of dives”. In practice, this is only mildly annoying - the brittleness of mocks is that your code will fail in production but your tests won’t show that, because the mocks aren’t up to date and mask the errors.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/airbnb/enzyme/issues/539#issuecomment-482816828, or mute the thread https://github.com/notifications/unsubscribe-auth/AAWY34NXCKPY2RHPP4CTSYTPQHYNVANCNFSM4CMM3SWA .

ljharb commented 5 years ago

In practice, I’ve found the opposite to be true - and while changing the component structure shouldn’t change tests for other things, the tests for the component being changed shouldn’t necessarily be completely resilient to changes in structure.

dschinkel commented 5 years ago

This thread drives me ape shit.

dschinkel commented 5 years ago

https://blog.usejournal.com/testing-graphql-container-components-with-react-apollo-and-jest-9706a00b4aeb

@goldensunliu this is exactly why I hate Apollo, and is a typical example of what I've ranted about...a ton of nested HOCs in a component or in an existing HOC. It's bad design period...and is why you're having to integration test it and having such a hard time with it period. Don't ask shallow to fix bad design for you...you're stuck with this kind of prod code when you use Apollo, so you'll have to mount it, regardless of the setup if you only integration test it. But what you should be doing is mostly unit tests (not mostly integration tests) in the first place. That's a whole other discussion.

Your premise that Enzyme doesn't help here is wrong, it's that Apollo's HOC nesting is shit. Essentially Apollo code is the imperative shell, and you shouldn't mix functional core logic inside all those nested HOCs.

Boundaries by Gary Bernhardt (this says Ruby but we use the concept in ANY code, any lang so understand what he's saying here and apply that to your React Design)

So in the case of Apollo test the imperative shell is the Apollo Nested HOCs garbage that you think is "making your life easier" but really it's causing you hell because it's poor design and therefore not easily testable. Decouple your business behavior (user handler logic, container logic) out of there and move them into their own layers, keep the methods you move out small, components small, and test them with unit tests with shallow or if they are just pure functions without React stuff, test them with simple unit tests using Jest or Mocha.

hmmChase commented 5 years ago

I have a class component called IdeaCard.js that using styled-components and react-apollo that looks like:

render() {
    return (
      <sc.li>
        <Mutation
          mutation={query.DELETE_IDEA_MUTATION}
          variables={{ id: this.props.id }}
        >
          {deleteIdea => (
            <sc.deleteBtn
              type="button"
              onClick={e => this.handleClickDeleteBtn(e, deleteIdea)}
            />
          )}
        </Mutation>

        <Mutation
          mutation={query.UPDATE_IDEA_MUTATION}
          variables={{ id: this.props.id, content: this.state.content }}
        >
          {updateIdea => (
            <sc.ideaInput
              type="text"
              value={this.state.content}
              onChange={e => this.handleChangeideaInput(e, updateIdea)}
            />
          )}
        </Mutation>
      </sc.li>
    );
  }

If I do:

wrapper = shallow(
    <MockedProvider mocks={mockQueries} addTypename={false}>
        <IdeaCard {...mockProps} />
    </MockedProvider>
);

And then:

console.log(wrapper.dive().dive().dive().debug());

I get:

<IdeaCardstyle__li>
    <Mutation mutation={{...}} variables={{...}}>
        [function]
    </Mutation>
    <Mutation mutation={{...}} variables={{...}}>
        [function]
    </Mutation>
</IdeaCardstyle__li>

Is there something I can do test this, or do I need to use mount instead of shallow?

Thanks for any help.

ljharb commented 5 years ago

@hmmChase use the new wrappingComponent option to pass MockedProvider, which will save you a dive; but otherwise, what do you want to test? From that debug, you can .find Mutation and use .renderProp to test what the function returns, you can assert on the props passed to Mutation (and delegate that testing responsibility to Mutation's tests)… please file a new issue if you still have questions.

sktguha commented 4 years ago

Hi, any update on this ? are there plans to implement something like shallowuntil or like list of nodes which are to be expanded or not expanded, as discussed ?