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

Testing Async Components #346

Closed
bencripps opened this issue Apr 25, 2016 · 36 comments
Closed

Testing Async Components #346

bencripps opened this issue Apr 25, 2016 · 36 comments

Comments

@bencripps
Copy link

Not sure if this is an issue, or whether I'm simply missing something. I'm using enzyme mount to test a top level connected component, which goes through multiple render cycles before the component is finally loaded (to fetch data, etc.).

For my testing environment I have overridden our ajax component to simply return JSON rather than making an external call for data. This seems to work; when I inspect the component after render, the store has the correct data.

We are using Jasmine and Karma. Here is the slimmest version of our test.

    beforeEach((done) => {
        cmp = mount(component);
        setTimeout(() => {
            done();
        }, 12000);
    });

   it('should render a cmp', (done) => {
        const x = cmp;
        // at this point, x.component.findDOMNode() returns the dom node
        // and the component is fully rendered, except that the component 
        // which reflects the data shows 0 record, but the store shows 2
        done();
    });

If I step through the render cycle of my component, the data is there, and it looks like it's returning a component which reflects the fetched data, however, the enzyme mount doesn't have that in the returned dom.

Perhaps this is a race condition, but as you can see I've set the timeout to 12000 which should be more than enough. Is there a better way to test a component that fetches data/ has multiple renders before we'd like to test it?

@ljharb
Copy link
Member

ljharb commented Apr 25, 2016

what if you add cmp.update() before done() in the beforeEach? (not sure off the top of my head if that's just part of shallow or not)

@bencripps
Copy link
Author

cmp.update is a function. I added it:


  beforeEach((done) => {
        const { component } = generateComponent();
        cmp = mount(component);
        setTimeout(() => {
            cmp.update();
            done();
        }, 12000);
    });

But it looks like the rendered dom is the same :(

@bencripps
Copy link
Author

Just as a side note, after I add the cmp.update the component which renders the data doesn't go through another render cycle.

@bencripps
Copy link
Author

Closing this issue. Enzyme is behaving correctly. My component was failing silently. Once I fixed the issue, the component rendered correctly. Thanks for the help.

@danielhneville
Copy link

Ben, this sounds a lot like an issue that I'm having right now - I have a component that makes an API call in the componentDidMount lifecycle function and updates the state, and I have a jest mock function in the test that replies with data. I put a console.log in the component's render function, so I can see in the terminal that the state does change. However, Enzyme doesn't see the change.

The reason for this is that the state change happens after the test is done. I tested that by using Sinon to spy on the render function and by adding in a .update() -- calling .update() calls render again, but the state is still the initial state.

Is this the kind of silent failure that you were talking about? Would you be willing to describe the error you found in your component (if you still remember)?

@bencripps
Copy link
Author

Sure, I'd be happy to explain my issue:

In my componentDidMount function, I was calling a utility function that was async in nature, and erring out. Because the call was async -- it was taken off the call stack, so I couldn't see the error. When I put a debugger in my async method, and walked the whole lifecycle, I saw where the error was occurring.

Once I fixed it, the DOM was being correctly updated, so for me this wasn't an enzyme issue at all -- it was just difficult to debug at first, because of the async function.

@chauthai
Copy link

@cdheffner I have the same behaviour in a test. I might be able to post a sample project later today.

@chauthai
Copy link

@cdheffner I fixed my problem by directed awaiting and evaluating the async method that manipulates the component lifecycle. e.g. if you use chai-as-promised:

await expect(wrapper.instance().init()).to.eventually.be.fulfilled();

After the await the state of the component should be updated too.

@ljharb
Copy link
Member

ljharb commented Apr 19, 2017

Alternatively you can just return the promise to mocha.

@eahlberg
Copy link

@ljharb could you please elaborate on what you mean? I'm having a similar issue as described above where I do async fetching in componentDidUpdate where I set a flag upon success, and then I conditionally render a spinner. See below.

class ExampleComponent extends React.Component {
    constructor() {
      super()
      this.state = {
        dataReady: false
      }
    }

    componentDidMount () {
      fetch('example.com/data.json').then(data => {
        this.setState({ dataReady: true })
      })
    }
    
    render () {
      if (this.state.dataReady) {
        return (<div> data is ready! </div>)
      } else {
        return (<div> spinner! </div>)
      }
    }
}

How can I test this component and be able to test the case where the data has been fetched? I've tried stubbing the fetch call with a promise, and then using wrapper.update(), with no success.

@ljharb
Copy link
Member

ljharb commented May 28, 2017

@eahlberg Something like this:

it('ensures the state is set', () => {
  const promise = Promise.resolve(mockData);
  sinon.stub(global, 'fetch', () => promise);

  const wrapper = mount(<ExampleComponent />);

  return promise.then(() => {
    expect(wrapper.state()).to.have.property('dataReady', true);

    wrapper.update();
  }).then(() => {
    expect(wrapper.text()).to.contain('data is ready');
  });
});

@eahlberg
Copy link

eahlberg commented May 29, 2017 via email

@louisgv
Copy link

louisgv commented Jun 7, 2017

To those who wanted to try chai-as-promised as suggested by @chauthai, the API has changed and the code should be changed into something like this:

  it('should etc', async function () {
    ...
    await expect(wrapper.instance().componentWillMount())
      .should.be.fulfilled;
   ...
  });

@ljharb
Copy link
Member

ljharb commented Jun 7, 2017

@louisgv Note that that's using async function, which is mostly just simple sugar for promises - so the initial example will always work identically to the async/await example.

@cyrieu
Copy link

cyrieu commented Nov 23, 2017

@ljharb Thank you for your answers in this thread, they are super helpful! Sorry to revisit this, but I had a quick question about the code snippet you posted above:

it('ensures the state is set', () => {
  const promise = Promise.resolve(mockData);
  sinon.stub(global, 'fetch', () => promise);

  const wrapper = mount(<ExampleComponent />);

  return promise.then(() => {
    expect(wrapper.state()).to.have.property('dataReady', true);

    wrapper.update();
  }).then(() => {
    expect(wrapper.text()).to.contain('data is ready');
  });
});

How can you guarantee that the code inside the return promise.then(() => {...} runs after the this.setState({ dataReady: true }) code in the .then() after the stubbed fetch in the componentDidMount method posed in the original question? I tried replicating the code here and it appeared to work, but it broke after wrapping this.setState({ dataReady: true }) in a setTimeout(..., 5000), since the state was not yet updated. Would love to hear your thoughts!

@ljharb
Copy link
Member

ljharb commented Nov 23, 2017

@cyrieu #346 (comment) was in response to #346 (comment); in which fetch is used - thus, stubbing it out with promise, and the order of operations being that the setState .then callback is queued up before the final return promise.then, the Promise spec itself guarantees that.

When using setTimeout, your only option is to either a) guess how long to wait (don't do this), or b) use something like sinon's mock clock, and manually tick the clock forward until after the timeout has executed. It's best, however, to avoid use of setTimeout whenever possible.

@cyrieu
Copy link

cyrieu commented Nov 23, 2017

@ljharb Thanks for the response! I guess I'm slightly puzzled as to how the the Promise spec guarantees that order, since I feel like here we have 2 .then callbacks branching from the same stubbed fetch, and doesn't that mean that they get executed arbitrarily?

My goal for testing out setTimeout in the callback was to simulate what would happen if I called another async function in the promise (maybe another fetch) and I wanted to test the component's state after that as well. In that case, would using sinon's mock clock be the best solution?

Thanks for helping me out, want to try my best to understand what's going on here!

@ljharb
Copy link
Member

ljharb commented Nov 24, 2017

@cyrieu no, they get executed deterministically in the order they were added to the promise.

If the secondary call used promises, you could mock it out the same way you've mocked out fetch - but yes, if it uses setTimeout, you'd have to use a mock clock like sinon's.

@Jack-Barry
Copy link

Jack-Barry commented Nov 27, 2017

I was having a little bit of trouble with the sinon.stub approach because the syntax is outdated as of v3.0.0. I got it to work with the new syntax, but ended up having to throw setImmediate into the mix to get the component to update as desired.

If I return the variable promise as proposed by @ljharb in the previous example, my component does not render as expected (the state of the component doesn't update). However, if I return the stubbed method (example_object.getExample() in this post) everything seems to work as planned. I tried both approaches with and without the setImmediate call but this was the only way it would work for me.

Would anyone mind taking a look at what I came up with and letting me know if there's any glaring issues with this setup or suggestions to make it any cleaner? I have some paranoia that setImmediate is a little hacky.

Thanks in advance!

Edit: The following configuration will output correctly in console.log but will not actually run the specs so it will not work

ExampleComponent.spec.js

it('sets `state.example` during ComponentDidMount', () => {
  // Stub the method in ComponentDidMount that returns a promise
  sinon.stub(example_object, 'getExample')
    .callsFake(() => Promise.resolve({ data: { id: 1, name: 'a name' } }))
  
  const wrapper = mount(<ExampleComponent exampleID="1" />)

  // Ensure the stubbed method resolves before proceeding
  return example_object.getExample()
    .then(() => {
      setImmediate(() => {
        console.log(wrapper.update().debug()) // this now renders as expected
      })
    })
})

ExampleComponent.jsx

componentDidMount() {
  // example_object.getExample is a custom method that makes use of the `axios` library
  example_object.getExample(this.props.exampleID)
    .then((response) => {
      this.setState({
        example: response.data,
      })
    })
}

@ljharb
Copy link
Member

ljharb commented Nov 27, 2017

@Jack-Barry What version of React?
(Assuming example_object is indeed the same reference throughout) I believe setState can be async; one alternative to setImmediate is .then(Promise.resolve()).then(() => {, but that does feel a bit hacky. Does that work?

@Jack-Barry
Copy link

Jack-Barry commented Nov 28, 2017

@ljharb Thanks for the advice, it turns out that the above setup was outputting correctly in console.log but was not running the assertions so no bueno there.

Here's the relevant libraries for my testing setup:

Enzyme 3.1.1
React 16.1.1
Webpack 3.8.1
Mocha 4.0.1
Mocha Webpack 1.0.1
Chai 4.1.2
Sinon 4.1.2

I finagled it a bit to where your suggestion does work. Ultimately I'm just looking for a clean, reusable pattern to set up these kinds of tests so if this is as clean as it'll get while still getting the job done I suppose that it will do, but you are right it still does feel kind of hacky, like the extra step in there is kinda poopy.

it('sets `state.example` during componentDidMount', () => {
  sinon.stub(example_object, 'getExample')
    .callsFake(() => Promise.resolve({ data: { id: 1, name: "a name", content: "some content" } }))

  const wrapper = mount(<ExampleComponent exampleID="1" />)

  return example_object.getExample()
    .then(wrapper.update())
    .then(Promise.resolve())
    .then(() => {
      expect(wrapper.state().loading).to.eq(false) // assertion passes as it should
    })
})

@Jack-Barry
Copy link

Oh my goodness, finally realized what was going on here 🤦‍♂️ The devil's in the details.

In my ExampleComponent.jsx I had a chained promise going on in real life:

componentDidMount() {
  example_object.getExample(this.props.exampleID)
    .then((response) => {
      this.setState({
        example: response.data
      })
    })
    .catch(() => {
      this.setState({
        failed_request: true
      })
    })
    .then(() => {
      this.setState({
        loading: false
      })
    })
}

This was to avoid having to duplicate the assignment of state.loading whether or not the promise resolves. If I put the assignment of loading: false into my first then block, I no longer need the extra Promise.resolve() in the spec for this.state.loading to update as expected once the stubbed promise resolves. I feel kinda schmucky but this was a good learning experience for me on what happens when promises are chained and how to ensure specs catch what's happening there.

My bad @ljharb I should have known better than to oversimplify my code for the post! Thanks a ton for your help though, I'm finally ready to rock on these babies 😎

Ultimately, for anyone looking for a simple example with the new sinon.stub syntax, here's something to get started. Also, if you have to chain some promises in your componentDidMount method, you can use the trick above with the extra .then(Promise.resolve()) to make that happen.

ExampleComponent.jsx

componentDidMount() {
  example_object.exampleMethod(this.props.exampleID)
    .then((response) => {
      this.setState({
        some_value: response.stuff,
      })
    })
    .catch()
}

ExampleComponent.spec.js

it('sets `state.some_value` during componentDidMount', () => {
  sinon.stub(example_object, 'exampleMethod')
    .callsFake(() => Promise.resolve({ stuff: 'some data' }))
    
  const wrapper = mount(<ExampleComponent />)

  return example_object.exampleMethod()
    .then(() => {
      expect(wrapper.state().some_value).to.eq('some data')
    })
})

@ljharb
Copy link
Member

ljharb commented Nov 29, 2017

aha, yes, "simplifying" example code virtually always hides the real problem :-) glad you figured it out!

@dvelitchkov
Copy link

I must be very confused about why this issue is closed and why people think their code actually works. I don't think any of the proposed solutions are really doing what people think they're doing.

it('ensures the state is set', () => {
  const promise = Promise.resolve(mockData);
  sinon.stub(global, 'fetch', () => promise);

  const wrapper = mount(<ExampleComponent />);

  return promise.then(() => {
    expect(wrapper.state()).to.have.property('dataReady', true);

    wrapper.update();
  }).then(() => {
    expect(wrapper.text()).to.contain('data is ready');
  });
});

In the example above, the promise "promise" is created and immediately resolved. So in this snippet - return promise.then(() => { , the promise continuation happens immediately, before the stubbed out "fetch" even gets a chance to fire, because "promise" is already resolved.

I have the following use case - a component that displays the version which is fetched with an AJAX api call:

This doesn't work:

let p = Promise.resolve("foo");
const fakeVer = {
  async get(){
   return p.then("bar");
  },
};

const wrapper = mount(<VersionInfo ver={fakeVer}/>);
await expect(p).resolves.toBe("bar");

Obviously, I can force it to light up green by returning p in the get(), but the actual get() call happens after the test finishes.

I see some people doing the equivalent of the following (by explicitly calling componentDidMount()):

let p = Promise.resolve("foo");
const fakeVer = {
   x: 0,
   async get(){
    console.log(`called ${++this.x} times`);
    return p.then("bar");
  },
};

const wrapper = mount(<VersionInfo ver={fakeVer}/>);
await expect(fakeVer.get()).resolves.toBe("bar");

This means that get() will be called twice - once by componentDidMount, after the test finishes, and once explicitly by me in the expectation. Also, my result is never "bar", it's always "foo".

Again, if I just returned p instead of p.then("bar"), my test would "work". But it's not really working.

Basically, I see no way to reliably and actually test async components - it's all flaky. I see no elegant solution. Or I could be missing something?

@ljharb
Copy link
Member

ljharb commented Dec 29, 2017

@dvelitchkov i think you’re misunderstanding how promises work; a .then callback can’t ever happen immediately, even with an already-resolved promise. Every .then callback executes on a future tick, which absolutely gives the fetch a chance to fire.

Separately, .then takes a function, so your example could never work. Try p.then(() => ‘bar’)?

@dvelitchkov
Copy link

dvelitchkov commented Dec 29, 2017

Yes, the last example needs to be p.then(() => 'bar') Brainfart.

let p = Promise.resolve("foo");
  const fakeVer = {
    x: 0,
    async get() {
      console.log(`called ${++this.x} times`);
      return p.then(() => "bar");
    },
  };

  const wrapper = mount(<VersionInfo ver={fakeVer} />);
  await expect(p).resolves.toBe("bar");

The p resolves to "foo", not "bar", since my await is on the original, resolved p, not whatever's returned from get. The result of get isn't even used by the time the test finishes.

But my point was more about what the console.log bit would print - "called 2 times". This was more a reply to @Jack-Barry's example, where he explicitly creates a second promise that he waits to resolve - a promise completely separate from the one created in the componentDidMount callback. The same thing is happening in @louisgv's last example - they're both chaining their assertion code pro,mise to happen after a brand new promise that is created inside the test and unrelated to the original promise that was created inside componentDidMount.

My test would also pass if I just return p; instead of p.then(....), but that doesn't mean things are wired up correctly.

My point is, their tests pass, that's great, but not because the tests are structured correctly, it's more of a happy side-effect of scheduling the "test"-created promise to resolve after the unrelated promise inside componentDidMount.

I settled on this:

 const fakeVer = {
    async get() {
      return "fakeversion";
    }
  };

  const wrapper = mount(<VersionInfo ver={fakeVer} />);

  setImmediate(() => {
    expect...
  });

My test is now super happy and green because my setImmediate call schedules my assertions callback to be enqueued after all other promises (including the one created inside componentDidMount) have finished. But that's like relying on a sleep() call to finish processing all other things - it's flaky.

@ljharb
Copy link
Member

ljharb commented Dec 29, 2017

I completely agree with you - all of the workarounds are relying on race conditions to test async things.

There’s no good solution to it in the language itself since async/await can’t be overridden by something like sinon’s fake timers.

@Jack-Barry
Copy link

Jack-Barry commented Jan 20, 2018

@dvelitchkov and @ljharb I'd like it if there were a cleaner solution and I'm open to discussion to reach one. Unfortunately @dvelitchkov I don't see an alternative or proposed solution in your responses.

I went through a lot of refactoring to ensure that the tests I was writing weren't evergreen. Here's my understanding of what's going on, and please correct me if I'm wrong. (I'm here to learn, not to be right.)

In this code in the component:

componentDidMount() {
  example_object.exampleMethod(this.props.exampleID)
    .then((response) => {
      this.setState({
        some_value: response.stuff,
      })
    })
    .catch()
}

example_object.exampleMethod() returns a Promise. When that Promise resolves, it will do some stuff with the result (set state.some_value to response.stuff), or if it is rejected it will do nothing since the .catch here is empty.

I made the minor change in this code in the spec for the component, so that the Promise chain is kicked off by Promise.resolve(wrapper) instead of example_object.exampleMethod():

it('sets `state.some_value` during componentDidMount', () => {
  sinon.stub(example_object, 'exampleMethod')
    .callsFake(() => Promise.resolve({ stuff: 'some data' }))
    
  const wrapper = mount(<ExampleComponent />)

  return Promise.resolve(wrapper)
    .then((comp) => {
      return comp.update()
    })
    .then(() => {
       expect(wrapper.state().some_value).to.eq('some data')
    })
})

Anytime example_object.exampleMethod() is called, it will return the result of calling the provided "fake" method - Promise.resolve({ stuff: 'some data' }). All this is supposed to do is make sure that if example_object.exampleMethod() is called at any time within this it block, it will resolve to { stuff: 'some data' }.

Finally, when return Promise.resolve(wrapper) is called, it is just returning the result of a Promise (which we immediately resolve to return the component being tested), then we are updating the component, then we expect that by having mounted and updated the <ExampleComponent />, example_object.exampleMethod() will have been called from within componentDidMount and thus the state of the mounted component should reflect that.

We have to mount then update() so that the rendered component that we are making assertions against has called the componentDidMount function, since it calls this function after mounting. To clarify:

return Promise.resolve(wrapper)
 // Component has mounted but componentDidMount has not been called
 .then((comp) => {
   return comp.update()
   // componentDidMount has now been called and the component rendered reflects any changes
   // This is when example_object.exampleMethod() has been called and returns the stubbed data
 })

I'll admit it's not the cleanest way to go about it, but it seems to work for my test cases (as I mentioned, not getting evergreens here) and I haven't come across any clear cut alternatives.

@ruimcf
Copy link

ruimcf commented Mar 2, 2018

I also stumbled upon this problem a few days ago, and have yet to find a good solution. I tried to used @Jack-Barry proposed solution and It works for my cases where I was using shallow. However when using mount I was not able to reproduce the behaviour.

Then I realized I was testing the result of the render after setState, and not that the state has changed. Indeed, the state in the component was updated, but the render was not updated.
So I ended up spliting the test in two.

(I am using jest and changed the promises to async/await)

The first one tests that the state is updated and uses mount

describe('when fetching succeeds' , () => {
    beforeEach(() => {
      jest.spyOn(example_object, 'example_method');
      example_object.example_method.mockReturnValue(Promise.resolve({ id: someID, content: 'some data' });
      wrapper = mount(<ExampleComponent dataID={someID} />);
    });
    afterEach(() => {
      example_object.example_method.mockClear();
    });
    test('component state is updated', async () => {
      await wrapper.update();
      expect(wrapper.state().loading).toBe(false);
      expect(wrapper.state().data).toEqual({ id: someID, content: 'some data' });
    });
  });

The second uses shallow, sets the state explicitly and tests the render:

describe('when sets the state with data' , () => {
    beforeEach(() => {
      jest.spyOn(example_object, 'example_method');
      //need to mock the method but don't care about the result
      example_object.example_method.mockReturnValue(new Promise(() => {});
      wrapper = shallow(<ExampleComponent dataID={someID} />);
    });
    afterEach(() => {
      example_object.example_method.mockClear();
    });
    test('data is rendered in the component', () => {
      wrapper.setState({ loading: false, data: { id: someID, content: 'some data' } });
      wrapper.update()
      //could do some assertions with wrapper.find('DesiredComponent') instead
      expect(wrapper).toMatchSnapshot()
    });
  });

Also, I think it is different to mount (or shallow) the component in the beforeEach() instead of at the start of the test, but I not not sure of the consequences of it.

@ljharb
Copy link
Member

ljharb commented Mar 2, 2018

wrapper.update() doesn't return a promise, so awaiting it would have no impact. Have you tried awaiting the promise that you've used to mock the return value?

@kkammili
Copy link

kkammili commented Mar 9, 2018

my Algorithmpage.js has a comp did mount method which consists of asynchronous actions and I recently upgraded from enzyme 2 to 3 and I am specifically seeing errors on these type of async actions and I am only using shallow method and my error says: TypeError: this.props.Algorithm is not a function.

componentDidMount () {
this.props.Algorithm()
this.props.Maps()
}

@ljharb
Copy link
Member

ljharb commented Mar 9, 2018

@Raju10100 please file a new issue, and include the entire component and test code

@mbrowne
Copy link

mbrowne commented Apr 10, 2018

I recently encountered a similar situation and I realized that there's no need to wrap .update() in its own then callback, since it's a synchronous method that doesn't return a promise. i.e. this code sample:

  return promise.then(() => {
    expect(wrapper.state()).to.have.property('dataReady', true);
    wrapper.update();
  }).then(() => {
    expect(wrapper.text()).to.contain('data is ready');
  })

Could be written as follows with the same effect:

  return promise.then(() => {
    expect(wrapper.state()).to.have.property('dataReady', true);
    wrapper.update();
    expect(wrapper.text()).to.contain('data is ready');
  })

(At least, as far as I can tell...)

@AndrewRayCode
Copy link

AndrewRayCode commented Aug 23, 2018

I don't know if this will help anyone, but this pattern seems to work for me:

  componentWillMount() {
    return Promise.all([this.promise1(), this.promise2()]);
  }

and in specs:

const willMount = async (ComponentClass, props) => {
  const lifecycleMethod = spy(ComponentClass.prototype, 'componentWillMount');
  const wrapper = shallow(<ComponentClass {...props} />);
  await lifecycleMethod.returnValues[0];
  lifecycleMethod.restore();
  return wrapper.update();
};
it('thing', async () => {
      const wrapper = await willMount(MyComponent, props);
      wrapper.update().find(...)
});

I'm not sure why you need the wrapper.update since I would expect a re-render to trigger, but this works

@ljharb
Copy link
Member

ljharb commented Aug 23, 2018

@AndrewRayCode i'm not sure that works how you think; in that case, willMount is a spy, not a promise, so await willMount is just "wait til the next tick". However, you could use await willMount.returnValue (i think) and then it would do what you expect.

@AndrewRayCode
Copy link

@ljharb nice good call! It looks like .returnValues[0] holds the pending promise. I updated the example

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