Skip to content

docs: adding information to testing docs #1493

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

Merged
merged 7 commits into from
Feb 13, 2022

Conversation

JessicaSachs
Copy link
Contributor

Docs update for Testing Guide

This is a pretty large change for the testing guide. Here's a list of proposed changes in order of appearance.

  • Adds "Why Test?" intro section.
  • Adds "Start Testing Early!" tip to "Why Test?" intro.
  • Removes "testing concerns" in favor of describing specific issues that each testing type protects against. See the Unit, Component, and E2E sections.
  • Adds Unit Testing code example for testing an increment function.
  • Removes all mentions of "Visual testing" because that's a different testing type w/ a specific meaning (e.g. Percy or Happo snapshots).
  • Establishes "Testing Types" terminology throughout the doc.
  • Adds section about avoiding mocks in Component Testing section.
  • Swaps "Interaction concern" to "Behavioral logic".
  • Adds Component Testing example for a Stepper component.
  • Recommends against using .toMatchSnapshot() as an assertion for a component's functionality.
  • Recommends Cypress Component Testing for components that render in the browser. This is Vitest's guidance as well.
  • Adds additional information about end-to-end tests. Mainly descriptions of what kinds of bugs end-to-end tests will catch.

@netlify
Copy link

netlify bot commented Feb 7, 2022

✔️ Deploy Preview for vuejs ready!

🔨 Explore the source changes: a15b394

🔍 Inspect the deploy log: https://app.netlify.com/sites/vuejs/deploys/6208c39e0f65650007abca41

😎 Browse the preview: https://deploy-preview-1493--vuejs.netlify.app


Component tests should focus on the component's public interfaces rather than internal implementation details. Or, in other words, **test what a component does, not how it does it**.
They should not mock child components, but instead test the interactions between your component and its children by interacting with the components as a user would. For example, a component test should click on an element like a user would instead of programmatically interacting with the component.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎉

Copy link
Contributor Author

@JessicaSachs JessicaSachs Feb 7, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The advice against .toMatchSnapshot is my fave. 😂

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

question: this part about not mocking child components is not completely clear to me. Should we avoid shallow mounting that replaces child components with stubs or we're speaking about something different here?

Copy link
Contributor Author

@JessicaSachs JessicaSachs Feb 7, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, the guidance is to avoid shallowMount for component tests.

This is in-line with Testing Library and Cypress's design goals.

Only Vue Test Utils supports shallowMount as a "low level" tool. Some literature from the Testing Library author: https://kentcdodds.com/blog/why-i-never-use-shallow-rendering

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe VTU (at least at the moment) is the most used approach for component testing across the user base, and don't think we should be limiting users who prefer shallowMount as it's a valid testing approach. I have read Kent's article before and I still don't completely agree with this article personally, and I know I am not the only one.

TL;DR: I would prefer core docs not to dictate to users which approach they should take. We should allow people to choose their own approach.

.click()
.get(valueSelector)
.should('contain.text', '1')
.click() // Should still be "1" because of the max prop
Copy link
Member

@lmiller1990 lmiller1990 Feb 7, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if we want to at least include an example of how it looks with Test Utils or Testing Library (in addition), since that's something a lot of people are familiar with at this point, and most of the information you'll find on Google will reference the VTU API.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah I'm feeling weird about this, too. I actually had it written with Testing Library at first... but then I felt weird about recommending Cy, but not using Cy in the examples.

Copy link
Member

@lmiller1990 lmiller1990 Feb 7, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is a good opportunity to focus 'understand the concept, not the syntax'. Although not the most idiomatic Cypress, we can do something like:


Here's an example using Vue Test Utils, a low-level testing library that powers libraries like Testing Library and Cypress:

const valueSelector = '[data-testid=stepper-value]'
const buttonSelector = '[data-testid=increment]'

const wrapper = mount(Stepper, {
  props: {
    max: 1
  }
})

// Each of these commands does implicit assertions
// that the button and values are visible
// and that the button can be clicked.
expect(wrapper.get(valueSelector).text()).toBe('0')
wrapper.get(buttonSelector).trigger('click')
expect(wrapper.get(valueSelector).text()).toBe('1')
wrapper.get(buttonSelector).trigger('click')

// Should still be "1" because of the max prop
expect(wrapper.get(valueSelector).text()).toBe('1')

This same test can be made more readable using Cypress; notice the ideas and concepts are the same; we are testing in a similar manner to a user, by clicking and observing the HTML. The only difference is the syntax; your testing library should not change the way you choose to test, just the syntax.

mount(Stepper, {
  props: {
    max: 1
  }
})

// Each of these commands does implicit assertions
// that the button and values are visible
// and that the button can be clicked.
cy.get(valueSelector).should('contain.text', '0')
cy.get(buttonSelector).click()
cy.get(valueSelector).should('contain.text', '1')
cy.get(buttonSelector).click()

// Should still be "1" because of the max prop
cy.get(valueSelector).should('contain.text', '1'))

Far from perfect but this is what I was thinking. What do you think? It's a bit wordy and verbose, but I like the focus on ideas, not syntax and specific library APIs.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@lmiller1990 exactly described my opinion which I mentioned one comment above. As I said - we can even go further and throw away everything, except a test runner (vitest) to help this example to be more focused

We know nothing about the implementation of Stepper, only that the "input" is the `max` prop and the "output" is the state of the DOM as the user will see it.

```js
const valueSelector = '[data-testid=stepper-value]'
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should include "this example is written using Cypress" before we present it, at the very least, since the style is really different from the unit test example (eg the assertions) and that might be a little confusing/jarring.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

100000% if we keep it as a Cy example, we should annotate it. It's disjointed and jarring like "Wtf is this syntax".

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would go even further and suggest rewriting the test to Jest/having Jest code as an alternative here

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any reason why this is written in Cy? Considering vitest as a first option why not start with "just pure vitest" - this will require people not to learn any additional syntax (Cy as an example) just to understand the test

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Jest and Vitest are the test runners and aren't really "visible" in this code example.

Vitest supports both Mocha and Jasmine matchers.

The debate is really:

What component mounting strategy are we recommending?

Vue Test Utils

const wrapper = mount(Stepper, {
  props: {
    max: 10
  }
})

Cypress

mount(Stepper, {
  props: {
    max: 10
  }
})

Testing Library

render(Stepper, {
  props: {
    max: 10
  }
})
What will we recommend for interacting with components? (Vue Test Utils, Testing Library, Cypress)

Vue Test Utils

await wrapper.get('[data-testid=increment]').trigger('click')
expect(wrapper.get('[data-testid=stepper]').text()).toEqual('1')

Cypress

cy.get('[data-testid=increment]').click()
cy.get('[data-testid=stepper]').should('contain.text', '1')

Testing Library

const button = screen.getByTestId('increment')
await fireEvent.click(button)
expect(screen.queryByText('1')).toBeTruthy()
What assertion library are we recommending? (Jasmine, Mocha, Cypress's wrapped Mocha)

Jasmine

expect('whatever').toBeSomething()

Chai

expect('whatever').to.be.something

Cypress's Chai

cy.get('button').should('be.something')

In this PR we recommend Vitest as the test runner for headless components and unit tests, which comes with both Mocha and Jasmine assertions (Jasmine is camel-case, Mocha is dots) and we recommend Cypress for mounting and clicking on headed components.

If we don't want users using @vue/test-utils directly, what syntax should we pick?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added all the things in a tabbed component 😼. This should be helpful.

Screen Shot 2022-02-07 at 9 39 20 AM

Copy link
Member

@lmiller1990 lmiller1990 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cool, just some minor thoughts. I like this guide!

Copy link
Member

@NataliaTepluhina NataliaTepluhina left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@JessicaSachs great work! I've added a few comments here and there


- **User Flow**: given a sequence of user interactions that are necessary to complete an actual user task, check whether the application as a whole is working as expected.
- **Unit**: Checks that inputs to a given function, class, or composable are producing the expected output or side effects.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

note: I am not quite sure we should have a separation here between unit and component testing as component tests are considered a part of unit testing.

Copy link
Contributor Author

@JessicaSachs JessicaSachs Feb 7, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think there's two ways to test components:

  1. By whitebox testing the component, where the wrapper is the subject under test. This is what I'd consider Unit Testing your components. By using Vue Test Utils directly, you're able to perform Unit Testing.
  2. By blackbox testing the component, where the DOM is the subject under test. Testing Library and Cypress exclusively promote this blackbox-style testing (which we're referring to as "Component Testing")

The whitebox/unit method of testing components isn't documented or represented in the guide yet.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I still disagree with this definition. VTU allows you to perform proper blackbox testing if you don't assert wrapper.vm properties. Hence, the line between component and unit testing is still blurred for me.

Copy link
Contributor Author

@JessicaSachs JessicaSachs Feb 7, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you misunderstood me or I misspoke. You are correct.

Blackbox testing: DOM is the subject under test. VTU (but w/o accessing wrapper.vm or major stubbing), VTL (does not let you access internals), Cy (does not let you access internals)

Whitebox testing: Wrapper is the subject under test, DOM is sometimes used. Lots of stubbing. VTU supports whitebox via Shallow Mount. Not possible to perform with Cy or VTL.

- **User Flow**: given a sequence of user interactions that are necessary to complete an actual user task, check whether the application as a whole is working as expected.
- **Unit**: Checks that inputs to a given function, class, or composable are producing the expected output or side effects.
- **Component**: Checks that your component mounts, renders, can be interacted with, and behaves as expected. These tests import more code than unit tests, are more complex, and require more time to execute.
- **End-to-end**: Checks features that span multiple pages and make real network requests against your production-built Vue application. These tests often involve standing up a database or other backend.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

suggestion: I believe we're missing an Integration testing here

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah. I thought about that, too.

We could write a bullet about integration testing, but I wanted to include the Testing Pyramid graphic if we did that, because integration testing is best defined as a testing type that is relevant to the other testing types.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe we do that as a follow-up. I really do want to include the Testing Pyramid to explain how speed/complexity are all relative, but I don't want to rework the sections again right now.


Component tests should focus on the component's public interfaces rather than internal implementation details. Or, in other words, **test what a component does, not how it does it**.
They should not mock child components, but instead test the interactions between your component and its children by interacting with the components as a user would. For example, a component test should click on an element like a user would instead of programmatically interacting with the component.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

question: this part about not mocking child components is not completely clear to me. Should we avoid shallow mounting that replaces child components with stubs or we're speaking about something different here?


- **DO**
Component tests should focus on the component's public interfaces rather than internal implementation details. For most components, the public interface is limited to: events emitted, props, and slots. When testing, remember to **test what a component does, not how it does it**.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

suggestion: I believe we are missing side effects here like dispatching Vuex actions etc

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't believe we should encourage interacting with Vuex actions from within component tests. That's an implementation detail of the component.

A litmus test: If you replaced Vuex with Pinia, if the expected behavior of the component is the same, why should your test change?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After reading your other comments: If we're considering a section on "Unit Testing components", then executing Vuex actions makes perfect sense.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@JessicaSachs replace both Vuex/Pinia with calling a side effect like Sentry or external helper. I click the button, I want to make sure my component fires this imported helper, how do I test this?

Copy link
Contributor Author

@JessicaSachs JessicaSachs Feb 7, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's two solutions: blackbox + whitebox.

Blackbox testing:
When you open up your browser and you're developing your app, how do you know if Sentry is working properly? I imagine you look at your network tab.

Use Mock Service Worker or cy.intercept() and assert that a network request has been made to your /sentry endpoint.

This allows you to catch if your Sentry 3rd party library has broken or you are not using it correctly (maybe the base URL is wrong, etc). It couples you to the "public API" of Sentry and requires you to setup an environment that allows network stubbing, but it does not couple you to the specific files, modules, methods used.

Whitebox testing:
Use module/file mocking, stubs with a spy, etc. This couples you to the way your source code is written and the library you use. It covers up possible integration failures. BUT it requires much less infrastructure and depending on the library, will make your tests faster/simpler.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a really helpful discourse for me, btw. I hope it's not frustrating for you, because it's very helpful for me to talk through these thought experiments. Thank you. <3

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@JessicaSachs I like the discussions 😅 Sounds like a great summary! I am still not sure whether we should say that component's side effect is a part of the public component interface (because it's an internal part) but I still think we need to test these behaviors no matter what test runner we are using, so they worth a mention. What do you think?

We know nothing about the implementation of Stepper, only that the "input" is the `max` prop and the "output" is the state of the DOM as the user will see it.

```js
const valueSelector = '[data-testid=stepper-value]'
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would go even further and suggest rewriting the test to Jest/having Jest code as an alternative here


- **DON'T**

Assert the private state of a component instance or test the private methods of a component. Testing implementation details makes the tests brittle, as they are more likely to break and require updates when the implementation changes.

The component's ultimate job is rendering the correct DOM output, so tests focusing on the DOM output provide the same level of correctness assurance (if not more) while being more robust and resilient to change.

If a method needs to be tested, extract it into a standalone utility function and write a dedicated unit test for it. If it cannot be extracted cleanly, it should be tested as a part of an interaction test that invokes it.
Rely exclusively on snapshot tests. Asserting HTML strings does not describe correctness. Write tests with intentionality.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

praise: great point that if often overlooked in tests!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah it drives me nuts, lol. I remember auditing some major component libraries in our ecosystem and all of their tests were a render and a snapshot 😬.


Component tests should focus on the component's public interfaces rather than internal implementation details. Or, in other words, **test what a component does, not how it does it**.
They should not mock child components, but instead test the interactions between your component and its children by interacting with the components as a user would. For example, a component test should click on an element like a user would instead of programmatically interacting with the component.
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This line feels very confusing for me. Description of Unit-tests never explicitly mentions that components could also be tested in this way (by using shallow rendering). So basically shallow style of testing is silently ignored

Additionally this requirement feels extremely strict - considering this guide will be an intro to Vue testing for many people, it would be great to be both actionable and not biased towards such things

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, we can add copy about shallowMount into Unit Tests if we'd like. I didn't mention it, but not on purpose.

We know nothing about the implementation of Stepper, only that the "input" is the `max` prop and the "output" is the state of the DOM as the user will see it.

```js
const valueSelector = '[data-testid=stepper-value]'
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any reason why this is written in Cy? Considering vitest as a first option why not start with "just pure vitest" - this will require people not to learn any additional syntax (Cy as an example) just to understand the test

.click()
.get(valueSelector)
.should('contain.text', '1')
.click() // Should still be "1" because of the max prop
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@lmiller1990 exactly described my opinion which I mentioned one comment above. As I said - we can even go further and throw away everything, except a test runner (vitest) to help this example to be more focused

- [Vitest](https://vitest.dev/) or the other unit testing frameworks mentioned above can be used for component testing by simulating the DOM in Node.js. See [Adding Vitest to a Project](#adding-vitest-to-a-project) for more details.
- [Vitest](https://vitest.dev/) for components or composables that render headlessly. (e.g. the [`useFavicon`](https://vueuse.org/core/useFavicon/#usefavicon) function in VueUse.

- [Cypress Component Testing](https://on.cypress.io/component) for components whose expected behavior depends on properly rendering styles or triggering native DOM events.
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, I'm absolutely not familiar with how "recommendations" are being determined, but is there any reasoning to move recomendation away from testing-library for example?

I won't mention all pros and cons of cypress / cypress component testing (like running time, etc.) because I really believe everyone is aware of that but this was like a main surprise of this pull request for me - jumping from "pure vitest" to "full browser testing".

I bet this is a huge discussion point and not feeling safe to do such blind recommendation considering all possible options (promoting cypress as recommended way)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, I'm absolutely not familiar with how "recommendations" are being determined, but is there any reasoning to move recomendation away from testing-library for example?

Swapping to Cypress as the recommendation was the main point of the Pull Request and will (rightly) be a point of debate.

The short of it is that:

Vitest recommends Cypress for testing components that are rendered in the browser, so it feels weird to recommend people use Vitest and then Vitest says "No, use Cypress for testing your headed components". 🔁

@JessicaSachs
Copy link
Contributor Author

JessicaSachs commented Feb 7, 2022

Lots of comments (this is good!!!!).

So far what I'm hearing:

  1. Representation of Unit Testing a component using shallowMount in Vitest by using @vue/test-utils directly. (Testing Library's philosophy explicitly against all mocking... I guess this feels a little awkward because we're calling Vue Test Utils a "low level" library?)
  2. Addition of Integration Testing. (another PR maybe?)
  3. Rework the Component example. Perhaps we should remove it altogether? I think all choices are somewhat confusing in their own way.

See the comment I wrote here RE the syntax on the Component example.

@JessicaSachs
Copy link
Contributor Author

JessicaSachs commented Feb 7, 2022

How's this?

Screen Shot 2022-02-07 at 9 22 49 AM

I added another one...

Screen Shot 2022-02-07 at 9 39 20 AM

@JessicaSachs
Copy link
Contributor Author

JessicaSachs commented Feb 7, 2022

Last night I was thinking more about the shallowMount discussion.

I realized that Svelte and React exclusively recommend full mount/render instead of shallowMount.

https://reactjs.org/docs/testing.html
https://svelte.dev/faq#how-do-i-test-svelte-apps

I did re-write the Unit Testing section to cover shallowMount, but I believe we should remove it. Enzyme popularized shallowMount 4 years ago, but the majority of modern resources recommend against mocking the component hierarchy.

I do not think we should recommend it or mention it because the only way you can shallowMount is via VTU, and in the Component Testing section we tell users not to use VTU unless they have advanced components. It feels weird to mention something we're telling you not to use.

I'm happy to leave it in, I guess, but I feel that it's unnecessary given that the other frameworks do not recommend shallowMount.

@xanf
Copy link

xanf commented Feb 7, 2022

@JessicaSachs sorry, maybe I'm missing somethif, but I can't find any guidance againstmocking trees in both links you've provided. React router ne even mentions how one could achieve this, and svelte uses "mount" as an operation of putting xomponnent into dom.

Could you please point me to their opinion?

@JessicaSachs
Copy link
Contributor Author

JessicaSachs commented Feb 7, 2022

No problem. In general, there will always be a layer of mocking. I mean, JSDom mocks the browser, yknow? The idea is that it is kept to a minimum and is not the default behavior.

React router ne even mentions how one could achieve this

React recommends React Testing Library with Jest. React Testing Library recommends using a real React Router in their examples. https://testing-library.com/docs/example-react-router/

React Testing Library author (Kent) recommends against mocking anything in the component hierarchy. That's the opinion of the React Testing Library, and is therefor the "best practice" for testing React components (as I interpret it, via their docs).

svelte uses "mount" as an operation of putting xomponnent into dom.

Don't they use render via Svelte Testing Library? Either way, their recommendation is a blackbox test. You mount the component to the DOM and assert that the DOM contains the correct state. You don't have access to the internals of the component instance (a svelte-equivalent of wrapper.vm).

Both React and Svelte recommend blackbox style testing and a full mount with as little component hierarchy mocked as possible.

@xanf
Copy link

xanf commented Feb 7, 2022

@JessicaSachs sorry, it seems we're reading the same text, but understand it differently

I'm aware of Kent's position on mocking vs full-tree, but I can't say that for example svelte or even react socs says "as little as possible" (testing library obviously says that)

Based on my experience of maintaining gitlab as probsbly the biggest opensource app which is using vue - shallow-mounting was a huge time saver suring vue2 to vue3 migration wheb 95% of mount tests are simply red due to nuances of compat build ajd focused shallow tests allowed me to precisely determine reason of failure. That's why I'm so concerned about blindly going down the full mount path

@JessicaSachs
Copy link
Contributor Author

JessicaSachs commented Feb 7, 2022

I'm really happy Gitlab had a good experience with VTU's shallowMount. That's awesome. It's a great success story, and I'm glad that you have the debugging and development experience to use shallow mounting to its full potential.

One point of clarification: Please understand that nothing I am proposing is
done so blindly. I work full-time in the testing space. I've been building Open Source test runners full-time for years and I've worked as a Lead QA Eng building test infrastructure for a long time before that. I've worked with users and taught workshops. All of this experience gives me very good market awareness in the space. I rely on this experience when we discuss testing strategies to promote in the ecosystem. Nothing is done blindly.

Next

I think we've gone past the point of being productive.

Let's call it, include shallowMount, and revisit this later if we'd like. There's really no harm in including it and it's very important to you, and not too important to me.

I had already updated the documentation to include it. I was mainly interested in the conversation.

What else is left? How do people feel about the Testing Syntax Switcher?

@xanf
Copy link

xanf commented Feb 7, 2022

@JessicaSachs thank you for your cooperation! I really feel proposing all options to the users is really important for a health of community

Regarding your clarification: please could tou assume the same level of good intentions and awareness from others. I'm frontend testing expert in GitLab for years, i actively contribute to @vue/test-utils (both versions) and last year more than 2000 people total attended my workshops on testing and vue testing. I believe this also gives me market awareness and understanding people's pain.

I'm more than welcome for any diwcussions and I always have strong opinions weakly held, but the last message was not the friendliest one and feels more like social proofing than bringing arguments to discussion. I really believe this was not your intent and maybe the reason I read it in such way are just cultural differences, just wanted to give an honest feedback

@JessicaSachs
Copy link
Contributor Author

JessicaSachs commented Feb 7, 2022

💔 I re-wrote that message a few times and strongly considered omitting talking about my background because I felt like it detracted from the convo, but wasn't sure how to best communicate that I do think about these things all the time.

I don't question your experience and expertise at all, and I'm really happy that you're willing to engage with me so deeply on this topic. A lot of people aren't able to and it's pretty refreshing (even during semi-conflict).

Let's slay the rest of this PR and make whatever changes we need 👍🏻 🎉

@bencodezen
Copy link
Member

Thanks everyone for the spirited discussion! Clearly, people are coming from different experiences and there are a lot of different opinions in terms of what is best. That said, I think It's great to see everyone being able to share their views on "best practices" while still trying to keep in mind what's best for the community as a whole.

In terms of next steps, what do we think we need to wrap up this PR? I'd love to see us merge these changes so that people can benefit from all of this sooner rather than later. And then as the community establishes more patterns / best practice / guidance on testing, we can certainly iterate on it with another PR!

@JessicaSachs
Copy link
Contributor Author

JessicaSachs commented Feb 8, 2022

Thanks Ben, you're awesome.

I think this is where we left off after the first review and I applied the relevant feedback.

So far what I'm hearing:

  1. Representation of Unit Testing a component using shallowMount in Vitest by using @vue/test-utils directly. (Testing Library's philosophy explicitly against all mocking... I guess this feels a little awkward because we're calling Vue Test Utils a "low level" library?)

We finished this discussion and I rewrote the Unit Testing section and Component Testing section

  1. Addition of Integration Testing. (another PR maybe?)

Another PR please.

  1. Rework the Component example. Perhaps we should remove it altogether? I think all choices are somewhat confusing in their own way.

I created UI/UX to solve this. Please let me know if the tabbed "switcher" is good enough for now and I will push up the source for it.
#1493 (comment)

image

What I need:

  • Confirmation that we can skip Integration Testing for now (No. 2)
  • Approval for the tabbed mount helpers (No. 3)

@NataliaTepluhina
Copy link
Member

@bencodezen @JessicaSachs

I believe multiple approaches are a good idea. However, I'd prefer pure VTU example over testing library there.

I'll bring a few more points within one comments so that the discussion is not spread:

  • Tests that are "Whitebox tests" are aware of the implementation details and dependencies of a component. Components be unit tested by using @vue/test-utils's shallowMount command instead of the mount command.

    I believe this definition is not completely correct. We can perform whitebox testing with mount and shallowMount both, if we're checking internal component properties:

    expect(wrapper.vm.something).toBe('Hello World')

    This (IMO) should be discouraged disregarding the mounting method.

    At the same time, we can perform blackbox testing with both mount and shallowMount:

    expect(wrapper.find('#some-html-element').text()).tobe('Hello World')

    This does not correlate directly with the mounting method.

  • Vitest is relatively new and is still undergoing rapid development

In all fairness, I am not sure that the only default recommended option should be something under active development 🤔

  • We recommend using @testing-library/vue for testing components in applications, as its focus aligns better with the testing priorities of applications. Use @vue/test-utils only if you are building advanced components that require testing Vue-specific internals.

    It's not completely clear to me why the second case is more advanced 🤔 Maybe we need some examples here to understand why can't we achieve the same goal with VTU and why we recommend @testing-library/vue over it

Honestly, after trying to fit so many points within one guide, I start thinking that maybe we should limit this section to recommended testing frameworks/libraries with a short explanation and then let users read the comprehensive guides on the mentioned libraries docs. But this is just my 2 cents

@lmiller1990
Copy link
Member

lmiller1990 commented Feb 10, 2022

I looked at how React solves this problem. They have three pages:

  1. Testing https://reactjs.org/docs/testing.html
  2. Recipes https://reactjs.org/docs/testing-recipes.html
  3. Frameworks https://reactjs.org/docs/testing-environments.html

Here's what they say.

  1. Testing - extremely short. "Here are some tools that we know people use and enjoy, check their docs". We'd link to VTU, Cypress, Testing Library. It's about a page long.
  2. Recipes. They have some short examples, without too much ceremony. Basically they go "here is a scenario", "here's an example test". This is where we'd have the tabs, like we do for Options and Composition, if we want to include multiple frameworks. If we do this, I'd not include too much philosophy and opinions. You could link to external blog posts, talks, etc, if you want.
  3. Environments. This page does not even say the world "React", you could literally copy and paste it here and it would be the same.

I wonder if we can do something similar here? The way they've done it in React does not recommend too specifically, it just presents information (kind of like the rest of the Vue docs - we don't recommend Options or Composition, we just list some of the trade-offs) and leave the philosophy with the user. I like this, since that's what everyone is talking about here - the trade-offs for each of the approaches.

@JessicaSachs
Copy link
Contributor Author

@lmiller1990 FYI this is the first paragraph in the first page of the react docs about testing.

Screen Shot 2022-02-12 at 7 03 53 PM

Like React, Vue does have recommendations. We do recommend Vitest as the default test runner for Vue and will continue to do so, because it's what makes sense for our recommended bundler: Vite.

Removing all recommendations and thought leadership will push the work onto the user to sift through 3rd party blog posts.

So far, we've added inclusive wording and a non-biased writeup of Whitebox/Blackbox testing and how to unit test components... I feel like we've rewritten the doc such that we're presenting all the information while actually writing content instead of just presenting links.

Aside from using Cypress's code example by default (I think it's out of place on its own), I'm happy with the content. Can we add the tabbed UI for the diff frameworks (just like we have for Composition vs Options) and merge?

@NataliaTepluhina
Copy link
Member

@JessicaSachs I believe the points from #1493 (comment) are still not addressed (especially the connection between shallow/non-shallow mounting and whitebox/blackbox testing). Addressing this and adding tabbed UI remove the last blockers for merging the PR 👍🏻

@JessicaSachs
Copy link
Contributor Author

Okay I thiiiink I did it. Just give edits to the copy if it's not right -- I tried to adjust correctly.

Also, the Testing API Switcher should probably just be a Code Group inside of https://github.com/vuejs/theme. It's looking pretty good though, I think!

@JessicaSachs
Copy link
Contributor Author

Screen Shot 2022-02-13 at 3 30 40 AM

Screen Shot 2022-02-13 at 3 31 01 AM

Screen Shot 2022-02-13 at 3 31 28 AM

@NataliaTepluhina
Copy link
Member

@JessicaSachs thank you for the patience and for implementing all the changes and tweaks! I believe the testing API switcher looks great now so let's merge this PR and make any changes involving Code Group (if necessary) in the follow-ups 👍🏻

@NataliaTepluhina NataliaTepluhina merged commit f7e8d6e into vuejs:main Feb 13, 2022
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

Successfully merging this pull request may close these issues.

5 participants