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

API for testing emitted events #6

Closed
Austio opened this issue Jun 7, 2017 · 9 comments
Closed

API for testing emitted events #6

Austio opened this issue Jun 7, 2017 · 9 comments

Comments

@Austio
Copy link
Contributor

Austio commented Jun 7, 2017

Related to TODO in #4

Some ideas

// Creates listener on the component
wrapper.listenTo('event', spy) 

//  returns array of events that have been emitted
wrapper.events
@Austio
Copy link
Contributor Author

Austio commented Jun 8, 2017

I'll go ahead and begin implementing this tomorrow. @eddyerburgh could you set as in progress in that todo list when you have a sec so that others don't duplicate effort.

@yyx990803
Copy link
Member

What about making it wrapper.on to be consistent with vm.$on? It seems to be just an alias.

@eddyerburgh
Copy link
Member

eddyerburgh commented Jun 8, 2017

I think there needs to be more discussion on this before we build anything.

Personally, for methods like vm.$on, vm.$destroy, maybe even vm.$emit, we should encourage using the wrapper vm.

wrapper.vm.$on
wrapper.on

wrapper.vm.$emit
wrapper.emit

wrapper.vm.$destroy
wrapper.destroy

Otherwise we'll end up with a huge API.

Although maybe on and emit are such common use cases that we should include them as methods.

@Austio
Copy link
Contributor Author

Austio commented Jun 9, 2017

Thought about this a little more. I can make a good case for either one of these directions.

What do you think about keeping the number of things the library does to a minimum initially and going with encouraging using wrapper.vm. Doing that may alleviate confusion for having the wrapper only implementing some of the methods available.

@codebryo codebryo mentioned this issue Jun 23, 2017
36 tasks
@Austio Austio closed this as completed Jun 29, 2017
@Austio
Copy link
Contributor Author

Austio commented Jun 29, 2017

Closing: can reopen and implement after we have more information on what we are needing.

@yyx990803
Copy link
Member

yyx990803 commented Sep 27, 2017

Reopen as I am revisiting this feature. After having to manually create spies for all the events I wanted to assert, it felt quite tedious. I am proposing an API that requires minimal setup by simply auto-recording all emitted events in a wrapper.emitted object.

Say you emit the following events:

wrapper.vm.$emit('foo', 1)
wrapper.vm.$emit('foo', 2, 3)
wrapper.vm.$emit('bar')

Then wrapper.emitted would look like this:

{
  foo: [[1], [2, 3]],
  bar: [[]]
}

This simple data structure allows all kinds of assertions:

// assert event emitted
expect(wrapper.emitted.foo).toBeTruthy()

// assert event count
expect(wrapper.emitted.foo.length).toBe(2)

// assert event payload
expect(wrapper.emitted.foo[0]).toEqual([1])

The best part is you don't need to do any wiring or setup, you can just assert it after you trigger the events.

/cc @eddyerburgh @codebryo

@yyx990803 yyx990803 reopened this Sep 27, 2017
@eddyerburgh
Copy link
Member

I think this is an elegant solution, and we should implement it. Are you able to make a PR @yyx990803? I'm happy to if you're not.

@codebryo
Copy link
Contributor

That absolutely looks like a very clean approach on the events.

I support @eddyerburgh view regarding keeping the API minimal and encourage using the vm as longterm it allows/teaches users to also start using functionality that may be introduced with upcoming Vue versions, while it won't be necessary to update the test utils right away.

On the comment from @yyx990803 I think that's a piece of API that brings the test-utils forward.
It provides an API that adds value on top of what using single vm.$on events couldn't do.
I'm all for it :)

@eddyerburgh
Copy link
Member

Closed with 0033c4a

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