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

Docs: Vue Router #43

Merged
merged 5 commits into from
Sep 16, 2020
Merged

Docs: Vue Router #43

merged 5 commits into from
Sep 16, 2020

Conversation

lmiller1990
Copy link
Member

Although we working on some Vue Router utils/mock router to make testing Vue Router easier (vuejs/test-utils#152), I still think it's good to illustrate how you can use a real router, as well as a mock router, in your Vue unit tests. This simple article introduces both.

@lmiller1990 lmiller1990 mentioned this pull request Jul 26, 2020
@lmiller1990 lmiller1990 requested review from afontcu and posva July 26, 2020 13:10
Copy link
Member

@afontcu afontcu left a comment

Choose a reason for hiding this comment

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

LGTM! I expect this part to be interated upon when/if we end up with custom helpers, so this is a good starting point


```js {6,7,8}
import { mount } from '@vue/test-utils'
import { createRouter, createWebHistory } from 'vue-router'
Copy link
Member

Choose a reason for hiding this comment

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

We should import router instead, am I right?

Copy link
Member

Choose a reason for hiding this comment

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

The router is referring to the instance created before.

I think to make it clearer, we should show a bit of the router instance:

const router = createRouter({
  // options omitted for brevity
})

or a different comment that sounds better in English

src/guide/vue-router.md Outdated Show resolved Hide resolved
src/guide/vue-router.md Outdated Show resolved Hide resolved
})

it('redirect an unauthenticated user to 404', () => {
// Try writing this yourself! It's a good exercise. You can reuse most of the above test.
Copy link
Member

Choose a reason for hiding this comment

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

This feels more like cookbook/course rather than guide content. Not sure if the guide should contain "exercises" like this one (even though I get your point and I'd love people to do this).

Copy link
Member Author

Choose a reason for hiding this comment

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

I will add the other test

}
})

wrapper.find('button').trigger('click')
Copy link
Member

Choose a reason for hiding this comment

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

even though it is not strictly needed here (AFAICT), I'd await the trigger method, just to keep it in sync (no pun intended) with docs.

Copy link
Member

@posva posva left a comment

Choose a reason for hiding this comment

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

This is a great start! Here are some extra thoughts:

  • I think we should avoid guiding the user through the errors as if it was the normal path, but instead talk about them and why they appear and how to solve them because people are likely to get these errors when testing things by themselves, so it's important to appear in search results

I don't know if you intentionally show this to talk about mocks but I really think we should get vuejs/test-utils#152 figured out first to provide a more out of the box experience


:)
Vue Test Utils does not provide any special functions to assist with testing components that rely on Vue Router. This article will present two ways to test an application using Vue Router - using the real Vue Router, which is more production like but also may lead to complexity when testing larger applications, and by using a mock router, allowing for more fine grained control of the testing environment.
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 explicitly recommend using the mocked router as well as putting that section first

Copy link
Member Author

Choose a reason for hiding this comment

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

Yep, we can recommend mocking the router. Regarding the order, my initial idea was to use a real router to show the complexity it introduces to otherwise simple unit tests, then show the solution. I don't have a really strong opinion on the order. I'll rework the article to discuss the mocked solution first.

Comment on lines 10 to 11
import { mount } from '@vue/test-utils'
import { createRouter, createWebHistory } from 'vue-router'
Copy link
Member

Choose a reason for hiding this comment

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

I image we could remove these imports here

}
```

The root of the app, `/`, displays a `<router-link>` leading to `/posts`, where we list the posts in an unordered list.
Copy link
Member

Choose a reason for hiding this comment

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

The root of the app is not associated with a path, so I think we should omit the , `/`, part


```js {6,7,8}
import { mount } from '@vue/test-utils'
import { createRouter, createWebHistory } from 'vue-router'
Copy link
Member

Choose a reason for hiding this comment

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

The router is referring to the instance created before.

I think to make it clearer, we should show a bit of the router instance:

const router = createRouter({
  // options omitted for brevity
})

or a different comment that sounds better in English

src/guide/vue-router.md Outdated Show resolved Hide resolved
})
```

The test is now passing! If this seems like a lot of work for such a simple, that's because is it. There is an ongoing discussion to include [some helpers](https://github.com/vuejs/vue-test-utils-next/issues/152) to simplify this, but either way, it can be useful to know how things work internally.
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 agree on something there before posting this docs haha
Ideally, there should be no reference to an ongoing discussion making this look like a WIP (even though there is never a final doc version that doesn't improve!)

Copy link
Member Author

Choose a reason for hiding this comment

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

I will remove the reference to the VTU issue for now. I thought it would be good to have something documented so people can at least have some guidance (the async routing was a little confusing for me at first, I'm sure for others, too).

We should definitely provide some utils. I will try to get some momentum going. I do think there is value in explaining how things work "under the hood" as well (maybe a blog post is better than official docs for that, though).

import { createRouter, createWebHistory } from 'vue-router'

test('routing', async () => {
router.push('/')
Copy link
Member

Choose a reason for hiding this comment

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

The reason you need this push is because the initial navigation happens when we add the router plugin.

The reason the navigation is delayed until plugin installation is to allow the user setup navigation guards before the router triggers that initial navigation.

expect(wrapper.html()).toContain('Welcome to the blogging app')

await wrapper.find('a').trigger('click')
await flushPromises()
Copy link
Member

Choose a reason for hiding this comment

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

nice, I didn't know this worked in this scenario! It still works only because there are no navigation guards. This is where having those extra router test utils would be useful. Overall I think we need a router mock (vuejs/test-utils#152) that exposes utility functions like a await router.hasNavigated()

There could still be a function that takes a router instance to wrap the helpers around it: createRouteMock(router) or something similar. I also think it's worth if this kind of behavior could be automatic in test utils to make the end experience smoother


## Using a Mock Router

You can use a mock router, instead, to avoid caring about the implementation details of Vue Router in your unit tests. Instead of using Vue Router, we can just create our own minimal mock version which only implements the features we are interested in. We can do this using a combination of `jest.mock` (if you are using Jest), and `global.components`.
Copy link
Member

Choose a reason for hiding this comment

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

I really think we should provide an official mock and not let the user do it. This is something I can do myself or we can collaboratively build. I think we should go through an RFC after vuejs/test-utils#152 to setup the API and that we could first go through the API internally

Copy link
Member Author

@lmiller1990 lmiller1990 Jul 27, 2020

Choose a reason for hiding this comment

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

Sure, let's do it. If you have some ideas feel free to start hacking :D If I can find some time in the next week or so I will try to work on something too.


You can use a mock router, instead, to avoid caring about the implementation details of Vue Router in your unit tests. Instead of using Vue Router, we can just create our own minimal mock version which only implements the features we are interested in. We can do this using a combination of `jest.mock` (if you are using Jest), and `global.components`.

When we mock out a dependency, it's usually because we are not interested in testing that dependencies behavior. In this case, we do not want to test clicking `<router-link>` (which is really just an `<a>` tag) navigates to the correct page - of course it does! In this example, we might be interested in ensuring that the `<a>` has the correct `to` attribute, though. This may seem trivial, but in a larger application with much more complex routing, it can be worth ensuring links are correct (especially if they are highly dynamic).
Copy link
Member

Choose a reason for hiding this comment

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

IMO one of the most important things is: we don't want to trigger global navigation guards. This makes me think it would be nice to allow ignoring in-component navigation guards (when testing a page component) by passing an option somewhere in the router test utils

lmiller1990 and others added 3 commits July 27, 2020 21:59
Co-authored-by: Eduardo San Martin Morote <posva@users.noreply.github.com>
Co-authored-by: Adrià Fontcuberta <afontcu@gmail.com>
@lmiller1990
Copy link
Member Author

lmiller1990 commented Jul 27, 2020

Thanks @posva and @afontcu for the code review. I made most of the style changes and updated some inaccuracies, mainly due to my lack of understanding about Vue Router.

I will try get some more momentum going around the mock router we started talking about here vuejs/test-utils#152.

I think the guide is still useful until we have that mock router available in VTU. I think we should stay in beta until we implement that (and solve this vuejs/test-utils#158, unrelated to Vue Router), then move to release candidate.

@posva
Copy link
Member

posva commented Sep 5, 2020

@lmiller1990 I forgot to answer this 😓 I will be able to help you with the mock once I'm done with docs for Vue Router next. That might be in 2 weeks or so

@lmiller1990
Copy link
Member Author

lmiller1990 commented Sep 5, 2020

No problem, I think we are all pretty busy! I am working on slots docs now. There is tons of resources on mocking vue-router anyway so I don't think its too urgent.

I might rework this to emphasis mocking more so at least we have something in the meantime - even if we provide a mock router, I think there is value is showing people how to make the most of their test runner.

@lmiller1990 lmiller1990 merged commit ac8afc6 into master Sep 16, 2020
@lmiller1990 lmiller1990 deleted the docs/vue-router-article branch September 16, 2020 13:01
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.

3 participants