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

Strange snapshot output with High Order Components #42

Closed
james-s-turner opened this issue Jan 12, 2017 · 6 comments
Closed

Strange snapshot output with High Order Components #42

james-s-turner opened this issue Jan 12, 2017 · 6 comments

Comments

@james-s-turner
Copy link

james-s-turner commented Jan 12, 2017

I'm trying to snapshot a routed redux app, my test code is as follows:

const app = mount(renderApp());
const tree = enzymeToJson(app);
const snapshotFileName = path.join(__dirname, "App.enzyme.js.snap");
const snapshotName = "Enzyme App renders correctly";
expect(tree).to.matchSnapshot(snapshotFileName, snapshotName);

I'm using chai-jest-snapshot to do the snapshotting. The output I see in the snapshot file looks like this:

exports[`Enzyme App renders correctly`] = `
<Provider
  store={
    Object {
      "@@reduxReactRouter/routerStateSelector": [Function routerStateSelector],
      "dispatch": [Function anonymous],
      "getState": [Function getState],
      "history": Object {
        "__v2_compatible__": true,
        "createHref": [Function createHref],
        "createKey": [Function createKey],
        "createLocation": [Function createLocation],
        ...

Snapshot testing fails because some key values differ on each test execution.

However when I use react-test-renderer the snapshot output is just HTML and the snapshot comparisons work fine:

exports[`Jest App renders correctly`] = `
<div
  id="app-container">
  <nav
    className="PageHeader navbar navbar-inverse navbar-fixed-top">
    <div
      className="container-fluid">
      <div
        className="navbar-header">
        <span
          className="navbar-brand">
          <div
            className="PageHeader-logo" />
          <span
          ...

The code I ran to produce this is:

 const tree = renderer.create(renderApp()).toJSON();
 const snapshotFileName = path.join(__dirname, "App.jest.js.snap");
 const snapshotName = "Jest App renders correctly";
 expect(tree).to.matchSnapshot(snapshotFileName, snapshotName);

So the only difference appears to be calling the enzymeToJson() method rather than using the react-test-renderer equivalent.

Any help gratefully received.

@adriantoine
Copy link
Owner

Hi!

Yeah react-test-renderer and enzyme-to-json are working differently as react-test-renderer will display the output DOM elements. Have you tried using the Enzyme render wrapper? It should produce something closer to react-test-renderer, and in any case, you can use react-test-renderer if this is what you need.

@adriantoine
Copy link
Owner

I'll mark this as invalid for now because the output looks like what's expected to me, feel free to comment if you need help.

@esimons
Copy link

esimons commented Mar 14, 2017

Not to be a pest, but I happen to side with @james-s-turner on this issue --- to my mind, the value of snapshots is that they are like freebie pinning tests, allowing you to refactor code in confidence that you haven't broken expected outputs (i.e. HTML). But if the serializer output includes internal details like Component names and prop values, it's no longer useful for refactoring, since those things are exactly what you'd be changing.

I also take your point that the render wrapper gives us the output we're looking for, but if we can't use mount we're missing out on some of the most powerful functionality that enzyme provides.

Any chance you'd be willing to reconsider this issue?

@adriantoine
Copy link
Owner

Hi @esimons,

My goal is to give output that looks like what you get using the Enzyme debug() helper. I unfortunately can't dig deeper in Enzyme wrappers. It looks as expected to me and I don't want to change the behaviour of these methods as I think it's a good thing not to go deeper in most cases.

For example I have components like that:

function SubComponent(props) {
  return <span>The value is {props.value}</span>;
}

function MyComponent(props) {
  return <SubComponent value={props.value}/>;
}

If I want to unit test MyComponent, I prefer having this as a snapshot:

<SubComponent value={1}/>

than this:

<span>The value is 1</span>

The reason is that I don't want MyComponent snapshots to fail when SubComponent gets changed. All I want to know is that SubComponents are provided with the right props, then if the output is not right, SubComponent's unit tests should fail, not MyComponent's ones.

Let's say that I have 100 snapshots for MyComponent, 100 snapshots for SubComponent and SubComponent is changed, we need to replace the span by a div. In that case, we should update SubComponent's snapshots but why should MyComponent's snapshots fail?

I can hardly see a use case where you would like to do that, and if you do, the render wrapper should work fine. If you want to unit-test MyComponent, it's enough to check that SubComponents get the right props. Then you should test SubComponent and make sure it always renders the right elements given any values.

You can also do something like that:

expect(wrapper.find('SubComponent').mount()).toMatchSnapshot();

which will show what SubComponent is rendering.

@esimons
Copy link

esimons commented Mar 14, 2017

Hey @adriantoine thanks for responding so quickly!

You make a great point, and I think you're right that in most cases we should be treating these snapshots as unit tests, rather than large-scoped tests of multiple components. In that sense shallow works wonders and will give you exactly the output you're looking for. And, as you pointed out earlier, if you wanted a larger-scoped test, you could use render to test actual HTML output.

My issue was that I was trying to use mount, which sits in an awkward place somewhere between those two poles and succeeds at neither, because it's basically outputting the combined data that shallow and render would give you (i.e. it renders out all child descendents, all the way down to leaf nodes, while also including custom component wrappers and props information between). So I would still kinda argue that the output of mount is not especially useful.

I can dig that we're not able to mess with the Enzyme wrappers here, though. I think I'll just have to change up my approach a bit. Thanks again, much appreciated!

@DzoQiEuoi
Copy link

I agree with @esimons and @james-s-turner.

If a function produces the same output the test should pass regardless of implementation details.

When you change a sub-component, the parent component's unit tests should fail because it's now doing something different.

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

No branches or pull requests

4 participants