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

router.push could be a simple mock functions by default? #41

Closed
cexbrayat opened this issue Jan 6, 2021 · 8 comments · Fixed by #53
Closed

router.push could be a simple mock functions by default? #41

cexbrayat opened this issue Jan 6, 2021 · 8 comments · Fixed by #53

Comments

@cexbrayat
Copy link
Contributor

What problem is this solving

For a component doing a navigation with router.push({ name: 'home' }); on an action, our associated unit test usually does a simple assertion like the following:

const mockRouter = createRouter({ history: createWebHistory(), routes: [] });
jest.spyOn(mockRouter, 'push').mockResolvedValue();
const wrapper = mount(App, {   global: { plugins: [mockRouter] } });
// do the action
expect(mockRouter.push).toHaveBeenCalledWith({ name: 'home' });

Using vue-router-mock, we can simplify the test a bit by using createRouterMock:

// do the action
expect(getRouter().push).toHaveBeenCalledWith({ name: 'home' });

but the mock router created tries to really resolve the navigation and throws:

Error: Uncaught [Error: No match for {"name":"home","params":{}}]

Proposed solution

Would it be possible to have a simpler router mock that just have mock functions by default?
Most use cases would probably not need a real navigation to be resolved and triggered.
Or maybe that could be an option?

Describe alternatives you've considered

It's of course possible to spy on getRouter().push but I feel that this should be the default.

@posva
Copy link
Owner

posva commented Jan 6, 2021

Most use cases would probably not need a real navigation to be resolved and triggered.

I disagree with this one. I often need query params updated on the url.

What if we could by default ignore named route locations so query params and hash are still populated? Meaning that the vue router mock push method calls internally resolve to check if it's possible to navigate to the location and if it isn't strips out the name

@cexbrayat
Copy link
Contributor Author

I disagree with this one. I often need query params updated on the URL.

Can you give me an example, I'm not sure I understand what you mean?
If a component navigates to a route with some params, couldn't we just check if router.push was called with the right name or path or params and fake the navigation?

@posva
Copy link
Owner

posva commented Jan 6, 2021

An example is a search page with search params in url as query params, the params are used in the search but are an implementation detail and should not get in the way of testing the search component.

@cexbrayat
Copy link
Contributor Author

Do you mean when you test the search component? In that case, wouldn't simple utils like setQuery() be enough to set the context, instead of triggering a real navigation?

Even if I'm misunderstanding your use-case, wouldn't most unit tests be simple mount(Something) and check if router.push was properly called (without the hassle of mocking the next component dependencies)?

@posva
Copy link
Owner

posva commented Jan 7, 2021

The push is done internally. The test clicks on the button to search

@cexbrayat
Copy link
Contributor Author

Ok so let's go back to what you were proposing

What if we could by default ignore named route locations so query params and hash are still populated? Meaning that the vue router mock push method calls internally resolve to check if it's possible to navigate to the location and if it isn't strips out the name

Sounds good to me. The resolve phase could even be skipped if the mock router has been created with no routes?

@posva
Copy link
Owner

posva commented Jan 7, 2021

It shouldn't be skipped because it normalizes the route location and also to simulate how the router changes the current route by affecting the whole route also, and this should be respected because it won't trigger the same watchers, making a test fail while the actual code works

cexbrayat added a commit to cexbrayat/vue-router-mock that referenced this issue Jan 27, 2021
Adds a try/catch around the route resolution to simplify testing scenarios where the developers use `router.push({name: 'hey'})` and do not want to declare the corresponding route, as they just want to check if `push` was called.

Fixes posva#41
@cexbrayat
Copy link
Contributor Author

I followed your idea and opened #53

I tried a patched version in our projects, and it does simplify our tests quite a bit 👍

posva pushed a commit to cexbrayat/vue-router-mock that referenced this issue Mar 30, 2021
Adds a try/catch around the route resolution to simplify testing scenarios where the developers use `router.push({name: 'hey'})` and do not want to declare the corresponding route, as they just want to check if `push` was called.

Fixes posva#41
@posva posva closed this as completed in #53 Mar 30, 2021
posva added a commit that referenced this issue Mar 30, 2021
Co-authored-by: Eduardo San Martin Morote <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants