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

How do I simulate drag and drop in enzyme testcase? #1357

Closed
saudar82 opened this issue Nov 13, 2017 · 22 comments
Closed

How do I simulate drag and drop in enzyme testcase? #1357

saudar82 opened this issue Nov 13, 2017 · 22 comments
Labels

Comments

@saudar82
Copy link

saudar82 commented Nov 13, 2017

I am trying to create a test for simulating a dragging of an image into a div. I am clueless on how to achieve this. Any help is much appreciated.

it('test drag and drop', () => {
    const app = mount(<App />);
    const loco = app.find('.app img');
    const dropTarget = app.find('.drop-target');
    let mockEvent = { dataTransfer: { type: 'test' } };
    loco.simulate('drag', mockEvent);
    //loco.simulate('dragover', dropTarget, mockEvent);
    //loco.simulate('drop', dropTarget, mockEvent);
});

I am trying to simulate the drop event, not sure how to do as 2 elements are involved in it.

@ljharb
Copy link
Member

ljharb commented Nov 13, 2017

In general, I'd discourage any use of simulate. Instead, I'd explicitly call the function props you want to invoke.

@jonny-no1
Copy link
Contributor

Hi @ljharb I noticed your Owner badge after reading your comment discouraging the use of .simulate. Could you please elaborate? I couldn't find any such notes in the docs chapters .simulate or Future Work.

@ljharb
Copy link
Member

ljharb commented Dec 4, 2017

It's my personal opinion, and isn't documented in the repo anywhere.

Basically, it's that the name "simulate" implies that it's actually simulating how events in the browser work; however, events don't bubble the same way, and simulating "change" doesn't fire all the key/focus/blur events leading up to it, etc. I think a much simpler and cleaner approach is to explicitly extract the prop function, and invoke it manually.

@jonny-no1
Copy link
Contributor

jonny-no1 commented Dec 4, 2017

That's a fair point. I've been working working with simulate a lot today and it can be quite misleading. Another example is simulating a click... some might assume that a click will entail mouse down and mouse up.

I do think that it should be mentioned in the docs that calling the function explicitly is a good alternative as simulate doesn't fully simulate.

@ljharb
Copy link
Member

ljharb commented Dec 4, 2017

Sounds like a good PR :-D

@saudar82 I'm going to close this; I think explicitly invoking props is the proper solution here, since simulate doesn't actually simulate things faithfully.

@Anupheaus
Copy link

Hi both, nothing personal but I'd have to disagree with you. When you are testing, you are generally testing a specific bit of functionality, not the whole thing, in fact most of the time you specifically don't want to test the whole thing in one test. Using simulate, yes you aren't getting the surrounding events, but I think this is right. If you were to simulate a "click" event, why should you get mouse down and mouse up events? What if you are testing touch screen? You wouldn't want them in that case and by making a simulate click do those events you are starting to make big assumptions about what is actually needed.

If you want to properly unit test something then:

  1. You should simulate and/or mock everything that piece of functionality that you are testing needs, i.e. if you need a mouse down, followed by a click followed by a mouse up (or whatever the exact order is) then you should simulate each event in that order.
  2. You should also be testing edge or erroneous cases too, i.e. what if you just get a click and no mouse down and mouse up events? How does your component or app respond to this? This in turn starts you thinking "well, how could that happen? Ah, I haven't thought about how this might work on a tablet or phone". All good thoughts in my opinion.

Personally, I don't think invoking props is the proper solution either, how often can a user do that? Never. If you aren't testing an API I think you should be testing it how the user would be using it or as close as is feasible to do so. Otherwise you begin to make assumptions about how the browser would call that method and then all you are testing is how you think a browser would call your code, not how it actually would (of course, assuming that the simulate is as a browser would call it).

If you want more than one event to happen in a particular order then I'd recommend making some test helper functions to alleviate any repetitiveness. For example; simulateClick({ includeMouseDownAndUp: true ... }); - it is concise, explicit and readable.

I'm not trying to cause any offence here and I hope that I have not got the wrong idea about what you guys are saying, but I wanted to offer a different opinion and perspective on this subject.

@ljharb
Copy link
Member

ljharb commented Jul 10, 2018

@Anupheaus the problem is that .simulate('click') does not actually simulate a click event - if it were doing that, it'd be great, but it doesn't work that way.

Your tests don't need to test that react and the DOM are working properly - that's for React, and browsers, to test. Your tests should be testing your code.

In other words, I think we're somewhat agreeing - a proper simulate would be most useful - but since one doesn't exist, the current broken one should be avoided.

@Anupheaus
Copy link

@ljharb Ah sorry, from the issue comments above it implies that it is only "simulating" the surrounding events that is not working, not simulate itself - my mistake. If simulate does not work at all then why has it not yet been removed from the repo?

I agree that you don't need to test react and the DOM and I wasn't suggesting that. I also agree that your tests primary purpose should only be to test your own code. I guess it just doesn't sit right with me that you are exposing a prop purely for testing purposes.

My advice would be; rather than expose a prop purely for testing, do this in your test instead:

const component = enzyme.mount(<Component />);
const div = component.find('div');
simulateEvent(div, 'mousedown', { clientX: 10, clientY: 10 });

where simulateEvent is the following method in a test helpers file somewhere:

function simulateEvent(component, eventName, eventData) {
  const eventType = (() => {
    if (['mousedown', 'mousemove', 'mouseup'].includes(eventName)) { return 'MouseEvent'; }
  })();
  const event = new window[eventType](eventName, eventData);
  component.getDOMNode().dispatchEvent(event);
}

Of course, the simulateEvent method makes a number of assumptions and is definitely not complete but I've only just created it and I only needed the events I've added so far (but it can easily be enhanced with more events).

@ljharb
Copy link
Member

ljharb commented Jul 12, 2018

@Anupheaus we didn't realize simulate was bad until v2; we didn't remove it in v3 so as to minimize the pain of migrating from v2 to v3. Whenever a v4 comes out, simulate will be gone, with prejudice.

@fabb
Copy link

fabb commented Oct 30, 2018

Does this mean libs like react-dropzone need to change their implementation in order to become testable again?

@ljharb
Copy link
Member

ljharb commented Oct 30, 2018

It means their testing strategy might be unique, but consumers of those libs are already testable as-is, they just have to explicitly invoke props as needed.

@fabb
Copy link

fabb commented Oct 30, 2018

Hm, my particular problem with react-dropzone is that it allows a function as a child in order to be able to show different content/text when dragging files over it. (e.g. „Some of the files you‘re about to drop are not supported“).
I‘m not sure how to test the logic properly without simulate.

@ljharb
Copy link
Member

ljharb commented Oct 30, 2018

.prop(‘children’)(), just like you’d test any function prop.

@fabb
Copy link

fabb commented Oct 30, 2018

Oh cool! Thanks for helping out a noob!

@fabb
Copy link

fabb commented Oct 30, 2018

I need to know the parameter names, but a bit of greybox testing is ok i guess.

@fabb
Copy link

fabb commented Oct 30, 2018

Hm, I still need to figure out how to access the children property of the decendant component. My component I want to test looks a bit like this:

class MyDropzoneComponent extends React.Component {
  render() {
    return (
        <div className="dropzone">
          <Dropzone>
            {({ isDragAccept, isDragReject, acceptedFiles, rejectedFiles }) => {
              return "Some logic specific to MyDropzoneComponent that I want to test.";
            }}
          </Dropzone>
        </div>
    );
  }
}

@ljharb
Copy link
Member

ljharb commented Oct 30, 2018

wrapper.find(Dropzone).prop(‘children’)?

@fabb
Copy link

fabb commented Oct 31, 2018

Works, thanks a lot!

@Raj002
Copy link

Raj002 commented Jun 19, 2019

@fabb How did you resolve this issue? Can you provide solution for this?

@fabb
Copy link

fabb commented Jun 20, 2019

@Raj002 exactly like @ljharb suggested:

    const myDropzoneComponentWrapper = mount(< MyDropzoneComponent someParameter={someValue} />)
    const dropzoneChildrenFunc = myDropzoneComponentWrapper.find(Dropzone).prop('children')
    const content = mount(
        dropzoneChildrenFunc({
            isDragActive: true,
            isDragAccept: true,
            isDragReject: false,
            acceptedFiles: [{ name: 'someFile' }],
            rejectedFiles: [],
            getRootProps: () => {},
            getInputProps: () => {},
        })
    )
    expect(content.text()).toEqual(someValue) // or whatever is expected of your provided content func

@mvasin
Copy link
Contributor

mvasin commented Mar 12, 2020

@Anupheaus we didn't realize simulate was bad until v2; we didn't remove it in v3 so as to minimize the pain of migrating from v2 to v3. Whenever a v4 comes out, simulate will be gone, with prejudice.

Should we then mark simulate as depricated in the current v3?

@ljharb
Copy link
Member

ljharb commented Mar 12, 2020

Yes, see #2173.

victorborg3s added a commit to victorborg3s/SortableCategorySelector that referenced this issue Mar 20, 2020
... without '.simulate()' event, as seen [here](enzymejs/enzyme#1357 (comment)) that it will be discontinued.

- Lift the state of elements from SortableCategorySelector to parent;
- Configures lint to not conflict with 'pretier/react';
breville added a commit to code-dot-org/code-dot-org that referenced this issue Apr 16, 2020
This required a different approach than using Enzme's simulate() function which is apparently not the most useful: enzymejs/enzyme#1357
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

7 participants