testing-library / vue-testing-library

🦎 Simple and complete Vue.js testing utilities that encourage good testing practices.
http://testing-library.com/vue
MIT License
1.09k stars 109 forks source link

[PoC] VTL@next: Update to Vue 3.0 and Vue Test Utils 2.0 #137

Closed afontcu closed 4 years ago

afontcu commented 4 years ago

This is an ongoing proof of concept to update Vue Testing Library to handle Vue 3 and Vue Test Utils 2.

Missing steps:


Some identified breaking changes:

~Also: I uploaded tsconfig.json, but I'm not 100% positive we need that?~ edit: gone

cilice commented 4 years ago

@afontcu What is your opinion on not going after VTU, but instead just using normal rendering of vue within jsdom? Similar to https://github.com/testing-library/react-testing-library/blob/master/src/pure.js, which doesn't depend on the react-test-renderer.

afontcu commented 4 years ago

@afontcu What is your opinion on not going after VTU, but instead just using normal rendering of vue within jsdom? Similar to https://github.com/testing-library/react-testing-library/blob/master/src/pure.js, which doesn't depend on the react-test-renderer.

I think VTU handles a lot of stuff internally that we can benefit from, especially given that VTU-next is being rewritten (I'm on the team) and there's some cool stuff Vue Testing Library could leverage such as plugins.

cilice commented 4 years ago

@afontcu What is your opinion on not going after VTU, but instead just using normal rendering of vue within jsdom? Similar to https://github.com/testing-library/react-testing-library/blob/master/src/pure.js, which doesn't depend on the react-test-renderer.

I think VTU handles a lot of stuff internally that we can benefit from, especially given that VTU-next is being rewritten (I'm on the team) and there's some cool stuff Vue Testing Library could leverage such as plugins.

I think I understand that, I just really like the ideological idea of having same renderer the apps use for testing and consider it highly valuable. The only thing that comes to my mind that we would really miss is the emit mixin from vtu-next, since there is no way to get emitted() otherwise, if I'm not mistaken.

lmiller1990 commented 4 years ago

If you wanted to decouple from test-utils, you could reimplement the bits you need like emitted.

That said, I don't see a huge benefit in doing so - is the VTU dependency causing problems? Any benefit to moving away from it? I see VTU as a kind of low-level lib that people can, and should, build on top of.

cilice commented 4 years ago

That said, I don't see a huge benefit in doing so - is the VTU dependency causing problems? Any benefit to moving away from it? I see VTU as a kind of low-level lib that people can, and should, build on top of.

@lmiller1990 I was under the the false impression that VTU uses some different renderer internally, but after looking at the code it all looks fine. So sorry!

I'm all good to with this approach 🙌

lmiller1990 commented 4 years ago

Cool! VTU v2 (for Vue 3) is actually a tiny library, around 800 lines, and it doesn't do too much.

I am very excited to use testing-library with Vue 3

cilice commented 4 years ago

Another idea I just had:

  • updateProps is now setProps to align with VTU.

Lots of testing libraries under the umbrella provide a rerender, that more or less do what setProps would do. Maybe it's worth aligning towards other testing libraries apis and not going after VTU in this particular case?

afontcu commented 4 years ago

Another idea I just had:

  • updateProps is now setProps to align with VTU.

Lots of testing libraries under the umbrella provide a rerender, that more or less do what setProps would do. Maybe it's worth aligning towards other testing libraries apis and not going after VTU in this particular case?

Hum, that's a good one. Not sure if "rerender" could mean more changes in the component other than updating props – what about immediately watch elements, lifecycles…? Note that this isn't bad per se – It might lead to more realistic scenarios.

The migration path would be simple, too.

lmiller1990 commented 4 years ago

Should this even have setProps? That goes against the testing library philosophy, doesn't it? Something about your tests resembling user behavior.

afontcu commented 4 years ago

Should this even have setProps? That goes against the testing library philosophy, doesn't it? Something about your tests resembling user behavior.

yeah… it was already there when I started collaborating with the library, and I didn't want to introduce a breaking change. However, as @cilice pointed out, even React Testing Lib provides a rerender method which is quite similar to setting props again.

It comes with a warning, though: "It'd probably be better if you test the component that's doing the prop updating to ensure that the props are being updated correctly"

lmiller1990 commented 4 years ago

Hm, I like setProps better than rerender, personally.

Fair enough - not against keeping this API.

cilice commented 4 years ago

To document other testing libraries with rerender:

// re-render the same component with different props rerender()

- Marko
```js
import { render } from '@marko/testing-library'
import Greeting from './greeting.marko'

const { rerender, debug } = await render(Greeting, { name: 'World' })

// re-render the same component with different props
await rerender({ name: 'Marko' })

expect(container.firstChild).toHaveTextContent('Hello World 1!') rerender({ props: { name: 'World 2' } })



But I agree that it somehow feels weird, it's possible but shouldn't be prominent, just like the testing library documentation suggests.
afontcu commented 4 years ago

I think I'm gonna open up an issue/RFC with proposals for Vue3-powered Vue Testing Lib

I'm planning on doing so but I'll wait until Vue 3 and VTU 2 releases. This way we can make sure we can iterate and align with them.

codecov[bot] commented 4 years ago

Codecov Report

Merging #137 into next will not change coverage. The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff            @@
##              next      #137   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files            1         1           
  Lines           70        53   -17     
  Branches        13         9    -4     
=========================================
- Hits            70        53   -17     
Impacted Files Coverage Δ
src/vue-testing-library.js 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update b4da088...38f53ae. Read the comment docs.

ITenthusiasm commented 4 years ago

On the comment regarding rerendering, I think some people like the idea of "never directly touching props", so to speak, to avoid the idea of touching state or props. That seems to flow with the guidelines. Additionally, it minimizes confusion by maintaining a familiar API. I just started working with VTL and almost got lost due to the difference. So I think there's value in adding the function.

It's interesting that the RTL docs have the aforementioned warning. In his series on testing JavaScript, Kent C. Dodds advocates for using the rerender function, and iirc he's the one who brought the testing library family to life.

afontcu commented 4 years ago

Hi! Yeah, it makes sense. I guess maintaining a familiar API with other testing libs make sense, as anyone coming from Vue Test Utils will still need to learn the new API. Fancy to open un a PR with the renaming? Make sure you make it point to next branch :)

ITenthusiasm commented 4 years ago

I'm not as familiar with Vue as I am with React, but I can at least try to take a crack at it.

In the meantime, there's another PR resolving a bug with the current updateProps function if anyone wants to check it out. 😏