search

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.08k stars 110 forks source link

Broken test isolation of VueRouter instances #210

Closed maxarndt closed 3 years ago

maxarndt commented 3 years ago

Describe the bug A clear and concise description of what the bug is.

VueRouter instances are not erased properly after completing a test case.

To Reproduce Steps to reproduce the behavior:

git clone git@github.com:maxarndt/vue-testing-library.git
npm i
npm test

Expected behavior

All tests should pass.

Screenshots

 FAIL  src/__tests__/vue-router.js
  ✓ full app rendering/navigating (51 ms)
  ✕ another app rendering/navigating (13 ms)
  ✓ setting initial route (6 ms)

  ● another app rendering/navigating

    TestingLibraryElementError: Unable to find an element with the text: /you are home/i. This could be because the text is broken up by multiple elements. In this case, you can provide a function for your text matcher to make your matcher more flexible.

    <body>
      <div>
        <div>
          <a
            class="router-link-active"
            data-testid="home-link"
            href="#/"
          >

        Home

          </a>

          <a
            aria-current="page"
            class="router-link-exact-active router-link-active"
            data-testid="about-link"
            href="#/about"
          >

        About

          </a>

          <div>
            You are on the about page
          </div>

          <div
            data-testid="location-display"
          >
            /about
          </div>
        </div>
      </div>
    </body>

      29 |   const {getByTestId, getByText} = render(App, {routes})
      30 |
    > 31 |   expect(getByText(/you are home/i)).toBeInTheDocument()
         |          ^
      32 |   expect(getByTestId('location-display')).toHaveTextContent('/')
      33 |
      34 |   await fireEvent.click(getByTestId('about-link'))

      at Object.getElementError (node_modules/@testing-library/dom/dist/config.js:37:19)
      at node_modules/@testing-library/dom/dist/query-helpers.js:90:38
      at node_modules/@testing-library/dom/dist/query-helpers.js:62:17
      at getByText (node_modules/@testing-library/dom/dist/query-helpers.js:111:19)
      at _callee2$ (src/__tests__/vue-router.js:31:10)
      at tryCatch (node_modules/regenerator-runtime/runtime.js:63:40)
      at Generator.invoke [as _invoke] (node_modules/regenerator-runtime/runtime.js:293:22)
      at Generator.next (node_modules/regenerator-runtime/runtime.js:118:21)
      at asyncGeneratorStep (node_modules/@babel/runtime/helpers/asyncToGenerator.js:3:24)
      at _next (node_modules/@babel/runtime/helpers/asyncToGenerator.js:25:9)
      at node_modules/@babel/runtime/helpers/asyncToGenerator.js:32:7
      at Object.<anonymous> (node_modules/@babel/runtime/helpers/asyncToGenerator.js:21:12)

Related information:

Additional context

I just added two assertions to an already existent test case and duplicated the test to be executed twice in a row (see https://github.com/maxarndt/vue-testing-library/commit/306550fa8557c43adcdc5cf763143430b791ad30).

Both tests should pass if the testing environment is cleaned up properly. Unfortunately this is not the case.

ph-fritsche commented 3 years ago

What I've found out so far:

render creates a new VueRouter. If you inspect the currentRoute on that VueRouter it is / as expected. If you push a route onto it, everything behaves as expected. It seems localVue passes down the route from the test before while VueRouter is initialized with a new history.

We should try to break down the behavior of VueRouter inside createLocalVue and check with them if the carry over from one local vue instance to another is by design.

afontcu commented 3 years ago

Hi!

Yeah, this mostly looks like a bug on VTU' localVue… mind opening an issue there?

Thanks for this!

kerryboyko commented 3 years ago

render creates a new VueRouter. If you inspect the currentRoute on that VueRouter it is / as expected. If you push a route onto it, everything behaves as expected.

Can you explain how to push a route on to the VueRouter created by render()? I'm trying to test a component that gets a route parameter and it currently is not working.

blimmer commented 3 years ago

@maxarndt - I opened up #238 as a proposed solution to fix this issue. I also tested it against your fork and confirmed that this fix resolves the problem you surfaced.

Alternatively, as a quicker solution, you can set up a global beforeEach (via setupFilesAfterEnv) that resets the hash route before each test. e.g.,

// jest.config.js
module.exports = {
  setupFilesAfterEnv: ['./jest.setup.js'],
};
// ./jest.setup.js
beforeEach(() => {
  window.location.replace('http://localhost/');
});
github-actions[bot] commented 3 years ago

:tada: This issue has been resolved in version 5.8.0 :tada:

The release is available on:

Your semantic-release bot :package::rocket:

blimmer commented 3 years ago

@maxarndt - I ran into something weird today with the workaround I posted above. It appears that Vue Router adds a popstate listener to the global JSDOM window in hash mode. These popstate listeners are not cleaned up between tests, which caused additional "leaking" behaviors between tests.

Because of this, I recommend passing an instantiated router that uses abstract mode instead of resetting the window.location.

maxarndt commented 3 years ago

@blimmer thank you for your hard work on this!

I tried to use an instantiated router that uses abstract mode for my tests and stumbled across https://github.com/vuejs/vue-router/issues/729.

In abstract mode, when the application boots it will be in a "nowhere" state until you explicitly tell the router where it is.

So I ended up using a custom render function to ensure my app behaves as using history or hash router mode.

import { render } from '@testing-library/vue'
import VueRouter from 'vue-router'

import { routes } from '@/src/router'
import App from '@/src/App'

export async function renderApp () {
  const router = new VueRouter({
    mode: 'abstract',
    routes,
  })

  const renderResult = render(App, {
    routes: router,
  })

  await router.push('/')
  return renderResult
}

Do you think it's worth mentioning in the FAQ section that abstract mode comes with this pitfall?

blimmer commented 3 years ago

No problem! I agree that the initial push is useful for the FAQ. I opened https://github.com/testing-library/testing-library-docs/pull/881 for that improvement. Additionally - I tagged you in that PR with a contribution opportunity if you have the time!

github-actions[bot] commented 2 years ago

:tada: This issue has been resolved in version 6.6.0 :tada:

The release is available on:

Your semantic-release bot :package::rocket: