Skip to content

Add a getProp and getAttribute methods #27

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

Closed
eddyerburgh opened this issue Aug 10, 2017 · 4 comments
Closed

Add a getProp and getAttribute methods #27

eddyerburgh opened this issue Aug 10, 2017 · 4 comments

Comments

@eddyerburgh
Copy link
Member

Currently we have a hasAttribute, hasProp and hasStyle methods. These aren't great for assertions, as they return booleans. If tests fail, they will fail because they were expecting true but received false.

For example, when asserting attributes with hasAttribute:

expect(wrapper.hasAttribute('attribute', 'value')).toBe(true) // expected false to be true

An alternative would be to have a getAttribute function:

expect(wrapper.getAttribute('attribute')).toEqual('value') // expected 'wrong value' to equal 'value'

This makes debugging a lot easier, and provides a better testing experience.

I think we should add get methods for props and attributes. We could do the same with style, but this won't be ideal - because the styles we return will be the computed styles:

expect(wrapper.getStyle('background')).toEqual('red') // expected 'rgb(250, 251, 252) none repeat scroll 0% 0% / auto padding-box border-box' to equal 'red'

I think we should keep hasAttribute, hasProp and hasStyle, but add getAttribute and getProp

@defnorep
Copy link

defnorep commented Aug 18, 2017

Happened across this issue since you are asking for input on it, so here's my input:

This API makes sense to me. At first I wondered why we don't just use .attribute() instead of .getAttribute(), but it is more explicit what the method does when it lives alongside .hasAttribute().

Where things start to get dicey (in my opinion) is when we stop looking at attributes and start looking at Vue instance properties such as props, data object properties, computed properties, methods, injected dependencies, etc. All of these things are accessed homogeneously and it's up to the developer to have an understanding of what each one is.

What that means to your proposed API is that we have to start considering if props are different from those other Vue instance properties. Do we create methods for each type? Or do we embrace Vue's harmonized instance member access somehow with a differently named API? Attributes are sufficiently different from props to warrant different treatment.

Option 1: .hasAttribute() .getAttribute() .hasProp() .getProp() .getData() .hasData()

Option 2: .hasAttribute() .getAttribute() .has() .get()

Maybe the type or location of the instance member is worth testing. Someone might want to ensure that a provided service is available on a descendant component using .getInject(), or that something is known as a data object property as opposed to a prop, for instance. Or, maybe they don't care.

It's a tough API to nail down.

@riophae
Copy link

riophae commented Aug 18, 2017

Hi, @Daekano.
Though Vue forces us to make each member name unique among all props & data, it's still good to be explicit to tell what we are going to test.

@eddyerburgh
Copy link
Member Author

eddyerburgh commented Sep 1, 2017

So on reflection, getProps or get is not a good idea. It will return an unbound value, so if the prop/ method/ data/ whatever is a function that references this it won't behave as expected.

@eddyerburgh
Copy link
Member Author

I'm going to close this for the moment. We should encourage users to use getAttribute on the Wrapper element:

wrapper.element.getAttribute('attribute')

And for props/data:

wrapper.vm.propName

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

3 participants