-
-
Notifications
You must be signed in to change notification settings - Fork 2k
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
Comments
what if you add |
cmp.update is a function. I added it:
But it looks like the rendered dom is the same :( |
Just as a side note, after I add the |
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. |
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)? |
Sure, I'd be happy to explain my issue: In my 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. |
@cdheffner I have the same behaviour in a test. I might be able to post a sample project later today. |
@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. |
Alternatively you can just return the promise to mocha. |
@ljharb could you please elaborate on what you mean? I'm having a similar issue as described above where I do async fetching in 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 |
@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');
});
}); |
ok cool, I got it working. Huge thanks!
|
To those who wanted to try
|
@louisgv Note that that's using |
@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:
How can you guarantee that the code inside the |
@cyrieu #346 (comment) was in response to #346 (comment); in which When using |
@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 My goal for testing out Thanks for helping me out, want to try my best to understand what's going on here! |
@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 |
I was having a little bit of trouble with the If I return the variable 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 Thanks in advance! Edit: The following configuration will output correctly in
|
@Jack-Barry What version of React? |
@ljharb Thanks for the advice, it turns out that the above setup was outputting correctly in Here's the relevant libraries for my testing setup:
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
})
}) |
Oh my goodness, finally realized what was going on here 🤦♂️ The devil's in the details. In my 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 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
componentDidMount() {
example_object.exampleMethod(this.props.exampleID)
.then((response) => {
this.setState({
some_value: response.stuff,
})
})
.catch()
}
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')
})
}) |
aha, yes, "simplifying" example code virtually always hides the real problem :-) glad you figured it out! |
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 - 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 I see some people doing the equivalent of the following (by explicitly calling 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 Again, if I just returned 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? |
@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, |
Yes, the last example needs to be 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 But my point was more about what the My test would also pass if I just 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 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 |
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. |
@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()
}
I made the minor change in this code in the spec for the component, so that the Promise chain is kicked off by 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 Finally, when We have to mount 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. |
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. (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. |
|
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 () { |
@Raju10100 please file a new issue, and include the entire component and test code |
I recently encountered a similar situation and I realized that there's no need to wrap
Could be written as follows with the same effect:
(At least, as far as I can tell...) |
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 |
@AndrewRayCode i'm not sure that works how you think; in that case, |
@ljharb nice good call! It looks like |
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 returnJSON
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.
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?The text was updated successfully, but these errors were encountered: