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

Simulate events returning promises? #823

Closed
jdgreenberger opened this issue Feb 23, 2017 · 36 comments
Labels
Projects

Comments

@jdgreenberger
Copy link

@jdgreenberger jdgreenberger commented Feb 23, 2017

Hi there - really enjoying the library! I have a question about testing simulate events. Many of the events in my code trigger functions that wait for promises.

onClick = async (e) => {
   data = await fetchData(e.target.value)
   this.setState({data})
}

In the live react, events clearly don't return promises, but I'm not sure if enzyme's simulate is also event-driven. Is there a way that I can wait for a simulate event to return a promise? I know there is an alternative, but it's a little bit more overhead.

someFunc = async () => {
  data = await fetchData(e.target.value)
   this.setState({data})
}
onClick = (e) => {
   someFunc()
}
// test onClick calls someFunc()
// test someFunc()
@ljharb

This comment has been minimized.

Copy link
Collaborator

@ljharb ljharb commented Feb 24, 2017

No, there's no way to await the outcome of an event listener. Separately, this.setState is async and takes a callback, so for your promise to work properly it'd need to pass a callback and resolve the wrapping new Promise when setState is done.

@ljharb ljharb added the question label Feb 24, 2017
@ljharb

This comment has been minimized.

Copy link
Collaborator

@ljharb ljharb commented Feb 24, 2017

What I'd recommend is having your event listener call into an instance method; spy on the instance method to test the event listener, and then unit-test the instance method.

@yutin1987

This comment has been minimized.

Copy link

@yutin1987 yutin1987 commented Apr 1, 2017

https://github.com/airbnb/enzyme/blob/master/src/ShallowWrapper.js#L620

this code should return a Promise, and then testing
await component.find('TouchableOpacity').simulate('press');

just a idea 😄

@ljharb

This comment has been minimized.

Copy link
Collaborator

@ljharb ljharb commented Apr 1, 2017

Events are not appropriate to model as Promises. Events happen multiple times; a promise resolves once.

Also, events are synchronous in the browser.

@ljharb ljharb closed this Apr 1, 2017
@jwbay

This comment has been minimized.

Copy link
Contributor

@jwbay jwbay commented Apr 1, 2017

@jdgreenberger If you've got your fetchData mocked such that it returns an immediately resolved promise, this test approach might work for you:

it('should have updated output after an asynchronous setState', (done) => {
const wrapper = shallow(<Test />);
wrapper.find('.async-btn').simulate('click');
setImmediate(() => {
expect(wrapper.find('.show-me').length).to.equal(1);
done();
});
});

It would rely on the fact that resolved Promise callbacks fire before the setImmediate callback. This is currently a safe assumption in v8, but less so in other JS implementations.

edit: Oops, didn't see this was from over a month ago, sorry for the mention.

@yutin1987

This comment has been minimized.

Copy link

@yutin1987 yutin1987 commented Apr 2, 2017

I have a case

  onPress = async () => {
    const { client, onUpload } = this.props;

    const body = await Cropper.getPhotoFromAlbum();
    const reply = await client.fetch('/upload', {
      method: 'POST',
      body: JSON.stringify({ body }),
    });

    onUpload(body);
  }

testing

  it('renders correctly', async () => {
    const onUpload = jest.fn();
    const component = shallow(
      <Component onUpload={onUpload} client={client} />,
    );

    Cropper.getPhotoFromAlbum.mockClear();
    Cropper.getPhotoFromAlbum.mockReturnValueOnce(Promise.resolve());
    fetch.mockClear();
    fetch.mockReturnValueOnce(Promise.resolve());

    await component.find('TouchableOpacity').simulate('press');
    expect(Cropper.getPhotoFromAlbum).toHaveBeenCalledTimes(1);
    expect(fetch).toHaveBeenCalledTimes(1);

    await Promise.resolve(); // added it
    expect(onUpload).toHaveBeenCalledTimes(1);
  });

await Promise.resolve(); add it will pass, thank @jwbay

@Peeja

This comment has been minimized.

Copy link

@Peeja Peeja commented Feb 22, 2018

@ljharb:

Events are not appropriate to model as Promises. Events happen multiple times; a promise resolves once.

I don't quite follow that. A single event happens once, no? You can have several clicks over time, but a single .simulate('click') is a one-time thing.

Given that the return value of React event handlers appears to be ignored, it would be nice to be able to return a Promise which resolves when all side-effects of the event have completed, for all the use cases above. (Or, to have .simulate() return whatever the handler returns, which would let you return a Promise if you wanted to.)

@ljharb

This comment has been minimized.

Copy link
Collaborator

@ljharb ljharb commented Feb 23, 2018

@Peeja no, a single event may happen 0 times, or infinite times. A single simulate call, sure, is triggering a single event - but without cooperation with the actual user code in the component, there's no way to convert that to a promise. It's simply impossible.

Separately, there'd be no way to determine if the handler was returning a promise or not - enzyme would have to unconditionally pass the return value into Promise.resolve, so it'd be impossible to determine from the test if your simulate promise was waiting on async stuff to finish, or, if it was just waiting for Promise.resolve(undefined) and the async stuff was still happening.

@Peeja

This comment has been minimized.

Copy link

@Peeja Peeja commented Feb 23, 2018

I'm still not following how an event can happen more than once. More than one click can happen, but each click happens once.

But that's really beside the point. There's no need to pass the value into Promise.resolve or anything else. Unconditionally returning the return value of the handler would be plenty. If the handler returns a promise, it can be used as a promise. If there's something else more useful it can return, it can return that.

@ljharb

This comment has been minimized.

Copy link
Collaborator

@ljharb ljharb commented Feb 23, 2018

I understand that for those who would be very familiar with how this would work, it would be sufficient for them. However, it would result in a very confusing API whose behavior highly depended on what you were doing inside the handler.

@Peeja

This comment has been minimized.

Copy link

@Peeja Peeja commented Feb 24, 2018

Returning the value returned from the handler would be confusing? It seems like the obvious thing. I rather expected it to already do that.

To be clear: I'm not asking for Enzyme to know anything about promises or async behavior. I'm just asking for .simulate() to return the value returned from the handler.

@ljharb

This comment has been minimized.

Copy link
Collaborator

@ljharb ljharb commented Feb 24, 2018

In production, that value is ignored. It’s not a good idea to have different behavior in tests than in production.

@idanen

This comment has been minimized.

Copy link

@idanen idanen commented Mar 14, 2018

I agree with @Peeja . It would be extremely helpful if simulate would return the value from the listener.
The caller can then use that value however he wants, weather it's a promise or any other value.

@ljharb

This comment has been minimized.

Copy link
Collaborator

@ljharb ljharb commented Mar 14, 2018

If you avoid simulate, and instead explicitly invoke the prop, you can do that on your own.

@KyleBastien

This comment has been minimized.

Copy link

@KyleBastien KyleBastien commented Mar 16, 2018

@ljharb What if instead of a Promise, simulate returned an Observable instead? This would fit the model better of "many events can happen over time" and you are subscribing to that stream of events. But if you don't care about that stream, then you don't have to subscribe to it. Also, it would be a synchronous Observable emit (I think) so it would match with the event behavior in the Browser. Thoughts?

@ljharb

This comment has been minimized.

Copy link
Collaborator

@ljharb ljharb commented Mar 17, 2018

@KyleMcNutt that actually makes a ton of sense! As soon as Observables are at stage 3 of the language proposal process, then Airbnb will be able to use them, and we can look into what that would take.

Note that enzyme still wouldn't have any way to link a given event (in the Observable) to "when your arbitrary async actions were finished", so it'd still be a really weird scenario where folks who returned a promise had their tests work, and folks that didn't, had their tests potentially fail silently (ie, pass, but incorrectly).

@KyleBastien

This comment has been minimized.

Copy link

@KyleBastien KyleBastien commented Mar 19, 2018

@ljharb Would Airbnb be against having enzyme take a dependency on a Observable library for handling this scenario? RxJS being the obvious choice IMHO.

@ljharb

This comment has been minimized.

Copy link
Collaborator

@ljharb ljharb commented Mar 19, 2018

Yes, but not because it’s a dependency library - because the Observable API is still likely to change since it’s not yet at stage 3.

@kunal-mandalia

This comment has been minimized.

Copy link

@kunal-mandalia kunal-mandalia commented Mar 31, 2018

I had some joy with async await in test functions like so:

it('should set credentials and instance details after mounting', async () => {
    // assign
    const wrapper = shallow(<MyComponent />)
    
    // act
    await wrapper.update()

    // assert
    expect(httpService.getInstance).toBeCalled()
    expect(wrapper.state().credentials).toEqual(credentials)
  })

Here httpService.getInstance returns a promise. Awaiting the wrapper update allowed the setState updates to complete.

@ljharb

This comment has been minimized.

Copy link
Collaborator

@ljharb ljharb commented Apr 1, 2018

@kunal-mandalia just be aware that wrapper.update() doesn't return a promise, so your await there is just creating a race condition where the promise httpInstance.getInstance returns hopefully has the chance to be resolved before the async function resumes.

@kunal-mandalia

This comment has been minimized.

Copy link

@kunal-mandalia kunal-mandalia commented Apr 1, 2018

Thanks @ljharb for the tip. The async calls in componentDidMount (httpInstance.getInstance) have a mock implementation which return a resolved promise e.g. Promise.resolve(data), there shouldn't be race conditions in this case right?

@ljharb

This comment has been minimized.

Copy link
Collaborator

@ljharb ljharb commented Apr 1, 2018

There still is, because you’re racing between that resolved promise, and your await.

@charliecalvert

This comment has been minimized.

Copy link

@charliecalvert charliecalvert commented Apr 25, 2018

This is what worked for me:

it('renders state of File paragraph after button click', () => {
        const wrapper = shallow(<App appInit={appInit}/>);
        const statusParagraph = <p className="App-intro">status: barso</p>;
        wrapper.find('#queryServer').simulate('click');
        setImmediate(() => {
            wrapper.update();
            expect(wrapper.contains(statusParagraph)).toBe(true);
        });
});
@Glaadiss

This comment has been minimized.

Copy link

@Glaadiss Glaadiss commented May 28, 2018

We can use async-await instead of done() callback:

  it("Change state of 'muted' attribute when clicked.", async () => {
    const mutedState = wrapper.state().muted;
    await wrapper.simulate("click");
    expect(wrapper.state().muted).toBe(!mutedState);
  });
@dan-teitsort

This comment has been minimized.

Copy link

@dan-teitsort dan-teitsort commented May 31, 2018

@Glaadiss,

wrapper.simulate("click") does not return whatever is returned by the onClick handler. So, await in your example is not doing anything meaningful.

@Glaadiss

This comment has been minimized.

Copy link

@Glaadiss Glaadiss commented Jun 1, 2018

@dan-teitsort It's changing the state of wrappedComponent by resolving a promise, which is called in the onClick callback.

@octalmage

This comment has been minimized.

Copy link

@octalmage octalmage commented Jul 23, 2018

@Glaadiss if you were to log the result of wrapper.simulate("click");, I suspect you would not find a promise. Which is what @dan-teitsort was trying to say.

@sumit-sinha

This comment has been minimized.

Copy link

@sumit-sinha sumit-sinha commented Oct 1, 2018

setImmediate for some reason didn't worked for me. Not sure if what I tried is the best approach but any suggestions will help. Below is my code

const sleep = (ms) =>
  new Promise((resolve) => setTimeout(resolve, ms));

it('Should do something', async () => {
   // ...some code here

   // trigger click event
   wrapper.simulate("click");

   // wait and assuming every server call is mocked it shouldn't take too much time
   await sleep(20);
  
   expect(something).toEqual(nothing);
});
@ljharb

This comment has been minimized.

Copy link
Collaborator

@ljharb ljharb commented Oct 1, 2018

do a wrapper.update() after the "sleep"

@pietrofxq

This comment has been minimized.

Copy link

@pietrofxq pietrofxq commented Oct 25, 2018

await button.simulate('click') surprisingly worked for me, though I have no reason why

@ljharb

This comment has been minimized.

Copy link
Collaborator

@ljharb ljharb commented Oct 25, 2018

Since simulate doesn’t return a promise, the await only serves to delay a tick, which means you got lucky and won your race condition.

@pietrofxq

This comment has been minimized.

Copy link

@pietrofxq pietrofxq commented Oct 25, 2018

Yeah, though it worked no matter how many times I ran the tests, I think it's more reliable to use setImmediate

@ljharb

This comment has been minimized.

Copy link
Collaborator

@ljharb ljharb commented Oct 25, 2018

The only reliable mechanism is to stub your asynchronous action in your test, and explicitly wait for the action to complete.

@vsakaria

This comment has been minimized.

Copy link

@vsakaria vsakaria commented Nov 6, 2018

Wrap the assertion in a setTimeOut

it('should set the state to "success-response" when the reject button is clicked with a success reponse', async done => {
      const mock = new MockAdapter(axios);
      mock.onPost('http://localhost:9080/cmp/api/service/v1/workflow/reject').reply(200, { success: true });

      yesRejectButton.simulate('click');

      setTimeout(() => {
        expect(wrapper.state('modalStatus')).toBe('success-response');
        done();
      });

    });
@ljharb

This comment has been minimized.

Copy link
Collaborator

@ljharb ljharb commented Nov 7, 2018

@vsakaria #823 (comment) still applies - in other words, all that does is make it slightly more likely that you'll win your race condition.

@DmMhs

This comment has been minimized.

Copy link

@DmMhs DmMhs commented May 16, 2019

That's how you can test async eventHandlers and stop execution of setImmediate after tests passing:

it('formSubmitHandler changes the state', () => {
    const wrapper = mount(<MyComponent />);
    const instance = wrapper.instance();

    wrapper.find('.form').simulate('submit');

    const asyncCheck = setImmediate(() => {
      wrapper.update();
      expect(instance.state).toEqual({
       ...
    });
    global.clearImmediate(asyncCheck);
  });
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
simulate
Awaiting triage
You can’t perform that action at this time.