Skip to content

Conversation

@brennj
Copy link

@brennj brennj commented Jul 7, 2019

For #52

I do need some help with the router itself, it seems to stay onto the old instance. The new test added to tests/__tests__/vue-router.js will fail unless it is first executed, the router instance is still on the about route and I'm not sure of the cause of this. Any ideas from anyone would be appreciated!

@afontcu
Copy link
Member

afontcu commented Jul 8, 2019

Hi John, and thank you for this PR!

I must say I'm a bit torn on this one. On the one hand, it makes sense to expose both router/store access to make some assertions given specific contexts. On the other, however, I feel this creates a distance between Vue Testing Library and the guiding principles that guides it.

I guess what I'm trying to say is that, given a route change, a test should assert that the page content has successfully changed, rather than checking it programmatically. Exposing both the router and (especially) the store instances are creating a simpler path to test implementation details, while creating yet another way to "test the same thing" (thus making the library a bit more complex).

What's your take on this? Can we think of some use cases that could not be covered by just checking the resulting action of navigating or interacting with the store as a user would?

Again, thank you for your work!

@brennj
Copy link
Author

brennj commented Jul 8, 2019

I can understand your sentiment @afontcu. To me, it highlights a higher level grievance of the implementation details of the library is that the store and router initialization should not be included in the render method and left in user land so that the user can achieve the use cases I want.

I.e. wanting to understand the current URL. To me there is use cases of needing to use the router methods of using the history utilities to simulate the user using the URL bar as opposed to using application navigation. But that's not possible right now since they are baked in. I agree with you more on the store. But it goes hand in hand since they are in the details of render.

@afontcu
Copy link
Member

afontcu commented Jul 10, 2019

I was just checking react testing library examples of router tests (here and here), to see how they handle this in react land. From what I've seen, they actually interact with the app and then assert that the appropiate content is visible.

However, in the example linked above, they expose a history object (with a sensible warning about avoiding impl details) and use it (in reach router tests).

Do you think you could provide a valid example of a test use case where asserting content is not enough, and where you'd rather use some internal router features? Maybe add such tests in the PR.

Thanks! 🙌

(btw, I'd love to know your take on this @dfcook)

@brennj
Copy link
Author

brennj commented Jul 21, 2019

I will get around to this, I've been trying to write some more tests for stuff in work and seeing if I'm feeling I'm missing it. The history object, as you say - is what I find myself wanting most of the time in the scenarios I want to 'look' at the url bar. I'm pretty conflicted TBH! 😅

@dfcook
Copy link
Collaborator

dfcook commented Jul 30, 2019

I'd like to keep consistency with the other testing-library implementation where possible, let us know where your tests lead you.

@afontcu afontcu added the wontfix This will not be worked on label Aug 18, 2019
@brennj brennj closed this Sep 15, 2019
@brennj
Copy link
Author

brennj commented Sep 15, 2019

Closing for now, I haven't come across needing this properly yet.

@afontcu
Copy link
Member

afontcu commented Sep 15, 2019

Great, thanks! Feel free to open up a new open (or reopen this one) if you come across a valid use case :)

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

Labels

wontfix This will not be worked on

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants