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

Add convenience methods to encourage use for data-testid #1500

Closed
afontcu opened this issue Apr 9, 2020 · 8 comments
Closed

Add convenience methods to encourage use for data-testid #1500

afontcu opened this issue Apr 9, 2020 · 8 comments

Comments

@afontcu
Copy link
Member

afontcu commented Apr 9, 2020

As suggested here, I'm opening up this issue to suggest the addition of methods that simplify the usage of test data attributes.

Using data-testid (or data-test, the name specifics is not the goal of the issue) might become one of the suggested ways to query components in VTU. In short, with a test-related data attribute we make the relationship between test and code under test resilient and more explicit (among other benefits).

What problem does this feature solve?

However, right now it is quite hard to type, prone to errors, and even slow to read:

wrapper.find('[data-testid="fooBar"]')

Not to mention how this would look if we needed to add some variables and backticks to the mix.

What does the proposed API look like?

I see two main options to avoid this problem, and thus make the recommended way a bit more comfortable:

  1. To provide a different method: findByTestId('fooBar').
    Is easy to grasp and self-explanatory, but splits the find() API in two. This is the approach used in the Testing Library family (source).

  2. To make data-testid the default behavior for find, so that find('fooBar') becomes find('[data-testid="fooBar"]') internally in VTU.
    This would mean that find('button') would no longer return a button element, but rather the element with data-testid="button". This might be undesirable.

Thoughts?

@lmiller1990
Copy link
Member

lmiller1990 commented Apr 9, 2020

Another alternative is find for data-testid and a new method, query, for querySelector. Those feel nice to write.

We also have get. How does this fit in? Maybe we don't need get.

Someone in #1498 suggested a single method with options. Eg find('foo', { 'testid' }). find('button'). You still need to get approx the same amount of characters. It could work.

I think there is some benefit to having a single method (eg, find). But I could live with (1) as well - I think it promotes good practice. It is a breaking change, though. If we do this, people can get querySelector via element.querySelector if they need it.

@SergiCL
Copy link
Contributor

SergiCL commented Apr 9, 2020

I like the idea of using data-testid as the default option, but change completely how finds works seems like a dangerous path to me. Many tests could be broken.
On the other hand, findByTestId seems a safer option, but it can grow the API in a disorganized way.

What do you think about making find a container for all the find sub-methods?
Something like this:

find.byTestId('...')
find.byClass('...')
find.byQuerySelector('...')

This way is more organized and readable, and find (as a function) can be used as before until it's deprecated.
The focus on data-testid is a little lost compared to the first option, but that can be fixed by prioritizing the use in the documentation.

@dobromir-hristov
Copy link
Contributor

dobromir-hristov commented Apr 9, 2020

Personally, the only change I would do, is to have predictable return types. How I find components should be my choice, as for example my current project has very strict rules on what attributes and classes get applied, so most of the searching is done by Components. This is not my choice.

So if you query for Vue Instance (which you want to drop) you get that. (You could do complex assertions on props, things that are not in the DOM at all.)

If you query for DOM, you get DOM.

wrapper.find(query: string): DOMWrapper {}
wrapper.findComponent(query: string | ref | name): VueWrapper {}

PS:
As for data-testid, ppl use data-test, data-test-id, data-unit, and so on. I had a helper for this, which I used like find(dti('whatever')) and it got the job done. Enforcing this is just wrong.

@lmiller1990
Copy link
Member

Maybe just encouraging people to use a dti helper is adequate. I agree that breaking changes is never a fun thing.

Regarding findByComponent, if we can support it for Vue 3, we should - the expectation is after finding a component you can access the vm though, which doesn’t look like it’s possible at this point, at least not in the way as v2.

Does your current project do find(Foo).vm/props frequently, or is it more just find(Foo).exists() @dobromir-hristov ? Is this likely to be difficult to migrate to Vue 3?

@lmiller1990
Copy link
Member

lmiller1990 commented Apr 10, 2020

I thought about this a bit more. I am leaning towards @dobromir-hristov's argument.

As a developer, I personally like libraries that push you to develop using certain styles, practices etc. I'd rather have someone smarter than me make the important decisions, like how best to find a DOM element. testing-library is a good example of this in action - Kent has strong opinions (which I mostly agree with) and the show in his library.

I'm not sure VTU should be this opinionated, at least by default. Some options are:

  • new method findByTestId. Longer to type, but who cares, we have IDEs, etc.
  • we maintain some extensions outside of core. These could include things like findByTestId. We could architecture the library in a way that others could easily add their own methods, too.
  • we add optional args to find. find("foo", { testId: true }).

@Stoom
Copy link
Contributor

Stoom commented Apr 13, 2020

@lmiller1990 I like the idea of extendability, like how Chai has their plugin system... extend the assertion type and a using method to add it on. I guess this would be similar to the Vue.use paradise as well.

@lmiller1990
Copy link
Member

lmiller1990 commented Apr 14, 2020

Yep, giving users ability to extend and build on what we provide seems like a good idea. Everyone has their own version of readable tests... we have some discussion for vtu next about this topic here: vuejs/test-utils#55.

@lmiller1990
Copy link
Member

lmiller1990 commented Apr 22, 2020

Ok, so we will have some kind of plugin system, see

vuejs/test-utils#82
vuejs/test-utils#55

The idea of a repo with community plugins with a data-testid find function seems like a good solution for this. For now, we will not introduce new methods, but encourage people to mold the library how they like with plugins and good docs.

For those who like this practice (like me) you can make an extension, or you can use the awesome testing-library which supports this out of the box.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants