Skip to content

Previous render sometimes leaking into next test #716

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

Closed
ljosberinn opened this issue Jun 19, 2020 · 40 comments
Closed

Previous render sometimes leaking into next test #716

ljosberinn opened this issue Jun 19, 2020 · 40 comments
Labels
bug Something isn't working

Comments

@ljosberinn
Copy link
Contributor

  • @testing-library/react version: 10.3.0
  • Testing Framework and version: jest 26.0.1
  • DOM Environment: jsdom 16.2.2

Relevant code or config:

https://codesandbox.io/s/great-dawn-s229i?file=/src/ServiceWorker.test.tsx

it('renders a toast onupdatefound triggering site reload on click', async () => {
    const mockReload = jest.fn();

    Object.defineProperty(window, 'location', {
      value: {
        reload: mockReload,
      },
      writable: true,
    });

    makeMockRegister({
      addEventListener: makeMockAddEventListener(),
      registration: jest.fn(),
      shouldReject: false,
    });

    render(<ServiceWorker />);

    const toast = await screen.findByRole('alert');

    fireEvent.click(toast);

    expect(mockReload).toHaveBeenCalledTimes(1);
  });

What you did:

Trying to write tests in the order the ServiceWorker executes.

What happened:

screen.findByRole('alert') fails because multiple elements with that query are in the DOM.

Reproduction:

https://codesandbox.io/s/great-dawn-s229i?file=/src/ServiceWorker.test.tsx

Problem description:

If I disable all other tests except that one, it works.

image

Changing the order of the tests works too; the test marked as problematic test currently is the 5th/last.

It works locally if its the 4th:
image

Sandbox does not like the 4th...
image

...but as 3rd
image

Adding cleanup or manually calling unmount() does not help at all.

@JacobMGEvans
Copy link

I remember something like this happening a long while back in codesandbox, I think they had an issue of mine closed saying they fixed it. I will dig it up and see if it's related.

@JacobMGEvans
Copy link

Nope, it was a crash occurring. codesandbox/codesandbox-client#3149

Has this only been attempted in codesandbox or in a separate environment too?

@ljosberinn
Copy link
Contributor Author

We already talked but for visibility: the sandbox is just a reproduction of what I have locally; in this project.

@r3wt
Copy link

r3wt commented Jun 24, 2020

i've had this problem as well. we just change all of our tests to const {unmount} = render(...); then call unmount() at end of test.

@ljosberinn
Copy link
Contributor Author

I tried that too and it didn't help, sadly.

@JacobMGEvans
Copy link

JacobMGEvans commented Jun 25, 2020

Do you have timers or anything async that isn't being resolved in the tests, because the test blocks will continue to execute which will cause them to leak into JSDOM when another test starts running?

example not having await before act() or waitFor() etc...

@ljosberinn
Copy link
Contributor Author

So the initial issue technically still persists. However I was able to rewrite the test entirely and I don't have this issue anymore.

I suspect it has to be something to do with using Object.defineProperty and/or my usage, because when I used it in another test, I had the same issue of previous renders leaking into another test again. Removing said Object.defineProperty solved it however.

@mboettcher
Copy link
Contributor

I'm running into the same issue that the previous render leaks into the next test. Sounds to me like a very bad thing because this could lead to false-positive tests.

@puneetgkaur
Copy link

I am running into the same issue. Version: 10.4.8.
If I run my test in isolation it succeeds, otherwise it fails.

@eps1lon
Copy link
Member

eps1lon commented Aug 21, 2020

For everybody commenting they have the same issue: Please include:

  • exact @testing-library/react version
  • exact react-dom version
  • your test code

There's definitely a potential issue with React 17 (caused by deferred effect cleanup functions). Timers that aren't properly cleaned up can also cause this issue.

Overall these kind of issues are very hard to track down so simple "I have the same issue" are not helpful.

@FernandoBasso
Copy link

FernandoBasso commented Aug 25, 2020

I have been having a similar problem. Here's a simple, reproducible case:

unit-tests-renders-leaking.zip

It is simple app that lazy loads two routes, Home and About. The tests are very short and simple too. There are only two tests, actually.

If I run only the first test in App.spec.js, then it works. If I run only the second test, it works. The problem arises when I run both, in which case it seems the render from the first test is leaking into the second one, like if they are not being run in isolation.

I tried manually calling cleanup() and even unmount() after each unit, but the results are the same.

Pasting the relevant code here:

App.spec.js:

describe("<App />", () => {
  it("should render the loading fallback", () => {
    const { container } = render(<App />);
    expect(container.textContent).toContain('Loading...');
  });

  it("should route and render the home page", async () => {
    const { debug, container } = render(<App />);
    expect(container.textContent).toContain('Loading...');

    await waitFor(() => {
      expect(container.textContent).toContain('Home Page');
      debug();
    });
  });
});

App.js

import React, { lazy, Suspense } from "react";
import { BrowserRouter as Router, Route, Switch } from "react-router-dom";
import "./styles.css";

const Home = lazy(() => import(/* webpackChunkName: 'Home' */ "./Home"));
const About = lazy(() => import(/* webpackChunkName: 'About' */ "./About"));

export default function App() {
  return (
    <div className="App">
      <Router>
        <Suspense fallback={<div>Loading...</div>}>
          <Switch>
            <Route exact path="/" component={Home} />
            <Route path="/about" component={About} />
          </Switch>
        </Suspense>
      </Router>
    </div>
  );
}

@nickserv nickserv added the bug Something isn't working label Aug 29, 2020
@MatanBobi
Copy link
Member

MatanBobi commented Sep 6, 2020

Thanks @FernandoBasso for the code sample!
From what I can see, the DOM is being cleared between tests and the elements are being unmounted. The issue lies within the dynamic import and the lazy implementation.
On the first test, React's lazy implementation calls the dynamic import statement, at that time the lazy has a -1 status means Uninitialized. At the second render the status is already Resolved meaning that the Home Component is already there and no fallback is shown.
I tried to use jest.resetModules but that didn't help.
Though I'm not sure what's the meaning of this test (to render the App component), it's just testing React's lazy mechanism isn't it?
I'm not sure this is something we can change in our code, @nickmccurdy @kentcdodds @eps1lon does any of you have an idea about this one?

@eps1lon
Copy link
Member

eps1lon commented Sep 6, 2020

I suspect that it has to do with the lazy components. In the first test React tries to resolve them but they won't resolve in time so a fallback is displayed. In the second test they're already resolved so no fallback needs to be displayed.

I guess you'd have to reset the modules before each test. I think jest has an API for that.

@MatanBobi
Copy link
Member

I suspect that it has to do with the lazy components. In the first test React tries to resolve them but they won't resolve in time so a fallback is displayed. In the second test they're already resolved so no fallback needs to be displayed.

I guess you'd have to reset the modules before each test. I think jest has an API for that.

I tried using jest.resetModules but that didn't work.. Maybe because it's dynamically imported it wasn't cleared from the modules registry? 🤔

@bigfoot31
Copy link

I think you guys are onto something. I too used lazy components in a project and had issues with react-testing library. Never managed to solve the problem. Hacked around it by writing test cases in different files. In a previous project without lazy components, I had no issues.

@r3wt
Copy link

r3wt commented Sep 6, 2020

fwiw, we use React.lazy and Suspense as well throughout all of our projects where we've had these issues.

@ljosberinn
Copy link
Contributor Author

My initial issue was related to service worker registration, which also didn't use lazy or Suspense, but only fired the notification once the promise resolved, so it's promise-related for sure.

@eps1lon
Copy link
Member

eps1lon commented Sep 7, 2020

I suspect that it has to do with the lazy components. In the first test React tries to resolve them but they won't resolve in time so a fallback is displayed. In the second test they're already resolved so no fallback needs to be displayed.
I guess you'd have to reset the modules before each test. I think jest has an API for that.

I tried using jest.resetModules but that didn't work.. Maybe because it's dynamically imported it wasn't cleared from the modules registry?

We need to also re-require the modules when resetting:

describe("<App />", () => {
  let React;
  let cleanup;
  let render;
  let waitFor;
  let App;

  beforeEach(() => {
    jest.resetModules();

    App = require("./App").default;
    React = require("react");
    ({ cleanup, render, waitFor } = require("@testing-library/react"));
  });

  afterEach(() => {
    cleanup();
  });

  it("should render the loading fallback", () => {
    const { container } = render(<App />);
    expect(container.textContent).toContain("Loading...");
  });

  it("should route and render the home page", async () => {
    const { container } = render(<App />);
    expect(container.textContent).toContain("Loading...");

    await waitFor(() => {
      expect(container.textContent).toContain("Home Page");
    });
  });
});

Alternatively you need a single test for everything that follows a lazy component being resolved.

We only need to re-evaluate App.js in this specific case but this leads to multiple React modules throughout the component tree which is why I reset all react related modules.

The problem is that the lazy components itself are created when we evaluate App.js. In our first render their state is still Uninitialized. Though after that test their state is Resolved. Now in our second test React encounters lazy components that are already resolved and no longer needs to display a fallback.

It's not an issue with @testing-library/react but react itself. If you have an issue with testing React.lazy please open an issue in the React repository explaining your use case.

@kentr
Copy link

kentr commented May 18, 2021

To help others:

I also experienced this issue when testing with renderHook(), with no use of lazy or suspense.

The workaround above resolved the issue for me.

adsezai added a commit to adsezai/shop-nextjs that referenced this issue May 22, 2021
Fixes a bug in test where a component keeps the state between two
tests.This makes the tests unpredictable. This bug is caused by lazy
loading components. See more here
testing-library/react-testing-library#716 (comment)
@devniel
Copy link

devniel commented Jul 1, 2021

I have the same problem with a component visibility, for some reason the toBeVisible assertion is detecting my component as visible when other tests were running before, maybe because same className or jsdom ? If I run the test alone or I skip the others, it's okay.

@danascheider
Copy link

danascheider commented Jul 24, 2021

I'm having the same issue and am also not using lazy or suspense. It's also not the toBeVisible matcher in my case, since the test that's failing is failing before that matcher is called. I'm using:

  • @testing-library/react version: 12.0.0
  • Testing Framework and version: jest 26.6.3
  • DOM Environment: jsdom 16.4.0

I'm using mock service workers as well, like another commenter, with msw version 0.30.0.

I've managed to narrow it down to two tests that are causing the problem. (There are several tests in my suite that will cause the second test to fail, so the one I'm giving here is just one example.) All tests pass in isolation. Things that have not worked or are not viable for my project:

  • Using unmount or cleanup
  • Using require syntax with jest.resetMocks() (couldn't use this without changing the import syntax throughout my project, which I definitely want to avoid)
  • Using await before act in either test (this results in a warning that act is not a promise)
  • Using await before renderComponentWithMockCookies in either test
  • Passing a regex instead of a string to screen.findByText in the second test

When I run the tests in the order shown, output indicates that the content rendered for the second test, exclusive of script and style tags, is only <body><div /></body>.

The tests I'm writing are integration tests that involve nested contexts, multiple mocked API calls, and mocked cookies, so there are unfortunately several variables and I don't know how helpful this will be. The component in question is a full page. The two tests, in order, look like this:

describe('when the response indicates the user is logged out', () => {
  const server = setupServer(/* msw mock API handlers go here */)
  
  beforeAll(() => server.listen())
  beforeEach(() => server.resetHandlers())
  afterAll(() => server.close())

  // This one passes every time
  it('redirects to the login page', async () => {

    // The `renderComponentWithMockCookies` function wraps `render`, renders
    // the component in its context providers with cookies mocked, and returns the 
    // component object returned by `render`.
    const { history } = renderComponentWithMockCookies(cookies)

    const form = await screen.findByTestId('shopping-list-create-form')
    const input = await screen.findByLabelText('Title')
          
    fireEvent.change(input, { target: { value: 'Proudspire Manor' } })

    // I tried commenting out all the code in this test and uncommenting line by line
    // until I made the next one fail. When I uncommented this line, that's what made
    // the next one fail.
    fireEvent.submit(form)

    await waitFor(() => expect(history.location.pathname).toEqual('/login'))
  })
})

describe('showing and hiding the edit form', () => {
  const server = setupServer(/* msw mock API handlers go here */)
  
  beforeAll(() => server.listen())
  beforeEach(() => server.resetHandlers())
  afterAll(() => server.close())

  it('displays the form without toggling the list items when you click the edit icon', async () => {
    renderComponentWithMockCookies(cookies, games[0].id)

    // Get the data associated with this component from an array of test data
    const list = allShoppingLists.filter(list => list.game_id === games[0].id)[1]

    // It is failing here, saying that there is no element with this text on the screen. The element
    // appears in my development environment and the whole test passes in isolation.
    const listTitleEl = await screen.findByText(list.title)
    const listEl = listTitleEl.closest('.root')
    const editIcon = await within(listEl).findByTestId(`edit-shopping-list`)

    fireEvent.click(editIcon)

    await waitFor(() => expect(within(listEl).queryByText(list.list_items[0].description)).not.toBeVisible())
    await waitFor(() => expect(within(listEl).queryByText(list.list_items[1].description)).not.toBeVisible())
    await waitFor(() => expect(within(listEl).queryByDisplayValue(list.title)).toBeVisible())
  })
})

I'll come back with an update on what worked if I'm able to work this out on my own - I hope this is helpful to get to the bottom of things.

Update

It isn't exactly a solution to this problem, but since my test file was getting a little unwieldy anyway, I refactored these tests into multiple suites and they now all pass consistently. There are several tests in one of the suite that have essentially the same setup as the first test I gave here, so I'm not sure why they don't make each other fail. It'd still be good to figure out why this is happening in case it comes up again.

Update 2

Just a little extra info - I've had this issue come up again, and again it's been a fireEvent.submit line that turns out to be the issue. If I comment that out, the next test passes.

@bobbyhadz
Copy link

@danascheider same problem here, also using MSW, all tests pass in isolation and break when in a group. Did you figure out what the cause is?

@danascheider
Copy link

danascheider commented Aug 30, 2021

@bobbyhadz I don't think the problem is with MSW, I think it's that renders are not being cleaned up properly between tests for some reason. I don't know if my JS chops are up to mining the source to figure out specifically what that reason is. I used the workaround of breaking down the suite into multiple suites and haven't had issues with that, but I'd love to find a solution that allowed me to consolidate them again because the number of files is getting ridiculous.

@Serzhs
Copy link

Serzhs commented Sep 7, 2021

I have the same issue, one test fires toast, and snapshot from another test fails because toast appeared there.

RTL: 12.0.0
react-dom: ^17.0.2
react: ^17.0.2

hope that there will be a fix soon 🙏🏼🙏🏼🙏🏼

@michalstrzelecki
Copy link

I've just had the same issue with my stack. I am using MSW with React Toolkit. I just had to rest API state slice after each test has finished.
See:

I hope it helps someone one day.

@missbruni
Copy link

missbruni commented Oct 27, 2021

For those having issues with RTL and SWR, swr cache.clear() after every test handles a global cache and I think this paired with RTL isn't working always as expected which leads to cached responses to be used across tests. However, the new version of SWR handles cache on tests differently and you can now have a cache for each rendered component separately. So this has fixed for me:

previous swr solution (does not clean cache correctly between rtl tests)

import {cache, SWRConfig} from 'swr';

beforeEach(() => cache.clear())

<SWRConfig value={{ dedupingInterval: 0 }}>
      <Component {...props} />
 </SWRConfig>

new swr 1.0 solution:

import {SWRConfig} from 'swr';

 <SWRConfig value={{ provider: () => new Map(), dedupingInterval: 0 }}>
      <Component {...props} />
 </SWRConfig>

This fixed for me as each component has its own cache so there is no more caching issues between tests.
I hope it helps someone!

@eelkevdbos
Copy link

eelkevdbos commented Feb 3, 2022

I've encountered the same issue with my stack, while not using suspense or lazy features.

In my specific case, I was using axios-hooks, which keeps an LRU cache by default that contaminates responses between tests.

Disabling it via useAxios.configure({cache: false}) in my test suite "beforeAll" setup solved my issue.

@patrickp2
Copy link

patrickp2 commented Feb 23, 2022

I had this problem but mine was caused due to the URL params persisting between tests. In my project react ran queries based on the URL params provided.

To fix it I cleared the params between tests

afterEach(() => {
  // clean query strings between each test
  const { href } = window.location
  const url = href.split('?')[0]
  window.history.pushState(null, document.title, url)
})

Hopefully this helps anyone with this niche problem I had

@johnvincentio
Copy link

johnvincentio commented Mar 14, 2022

I also had this problem, similar to @patrickp2. The URL is persisting between tests. I had to reset it.

afterEach(() => {
  window.history.pushState(null, document.title, '/')
})

@TheNotary
Copy link

TheNotary commented Mar 4, 2023

I was hit by this bug because I was using useContext and that context was being initialized before my test could run and perform its assertions on the uninitialized context. Once I sorted out what was going on I changed my userContext to make sure when it was created, it did so by cloning a closure object used for representing it's data. That way the "leakages" didn't impact the consistency of my contexts.

ie

(bad-UserContext.js)

const ctx = { initialized: false };

export const createUserContext = () => {
  return ctx;
};

export const UserContext = React.createContext(ctx);

(good-UserContext.js)

const ctx = { initialized: false };

export const createUserContext = () => {
  return {...ctx};
};

export const UserContext = React.createContext({...ctx});

I hope this is the same bug, in my case I was noticing that the imports were re-firing console.log() commands at the module level a second time if a prior test had used a method from the imported module. jest.resetModules(); didn't help and might actually contain the bug producing this problem. The issue for me wasn't that it failed to resetModules, but that the old closures that the modules were created were being re-used on the second import.

@danascheider
Copy link

OK fam, after a year and a half I've finally found a solution to this that works. I am using JSDOM 21.1.1, TypeScript 4.9.5, and @testing-library/react 14.0.0 with Vitest 0.29.2. You could almost certainly modify this solution to work with slightly different toolsets.

The Solution

In my setupTests.tsx file (this could be called anything and you could use JS/JSX if you're not using TypeScript):

import { type ReactElement } from 'react' // TypeScript only
import { JSDOM } from 'jsdom'
import { render as originalRender } from '@testing-library/react'

// This is only needed for TypeScript

declare global {
  namespace NodeJS {
    interface Global {
      document: Document
      window: Window
    }
  }
}

const setDom = () => {
  const dom = new JSDOM('<!doctype html><html><body></body></html>', { /* options */ })

  // `as` and anything after it are TypeScript-specific
  global.window = dom.window as unknown as Window & typeof globalThis
  global.document = dom.window.document
}

// Or just `(ui) => {` if you aren't using TypeScript
export const render = (ui: ReactElement) => {
  setDom()
  return originalRender(ui)
}

Then, in my test files, instead of this:

import { render } from '@testing-library/react'

I import the function from my setupTests.tsx file:

import { render } from '../../setupTests' // or whatever the relative path is

Now, you will also need to scope your expectations to the rendered wrapper. For example, instead of this:

render(<MyComponent />)

expect(screen.getByText('My Component')).toBeTruthy()

You will now need to do this:

const wrapper = render(<MyComponent />)

expect(wrapper.getByText('My Component')).toBeTruthy()

The Explanation

The way we've all been doing this, render attaches the rendered component to the base, global document object. Because the cleanup function doesn't work quite like we all have conceptualised it, what ends up happening is that when we call render again in another test, the newly rendered component is added to the same DOM with the old one, resulting in the test leakage we are seeing.

With my approach, we are creating a whole new DOM with each render and scoping expectations to only look for elements within that DOM.

There are likely variations or specific use cases where this approach won't work as I've written it, but I am now able to write all my tests in a single test file without having issues with leakage. Hopefully if this exact code doesn't work for you you'll be able to tweak it so the same basic approach solves the problem for you as well.

Hope this helps!

@SanaaAfifi
Copy link

I also had this problem, similar to @patrickp2. The URL is persisting between tests. I had to reset it.

afterEach(() => {
  window.history.pushState(null, document.title, '/')
})

After A lot of research this one solved my issue thank you

@elchin-atob
Copy link

I had this issue too, and for me the reason was that I had instances of userEvent and fireEvent not being awaited on, and wrapped in act. I removed the act wrapper and awaited on userEvent directly, and it worked.

@yaswanthmaddula-sureify

If you are using jest.spyOn, remove that. That fixed the issue for me.

@gotdibbs
Copy link

Stumbled across this just now, and for us it was that we were using react-query. We had to invoke queryClient.clear() in order to "clear" this test pollution issue up.

Thanks to everyone for their tips! Helped us narrow it down.

@eudoroolivares2016
Copy link

I seem to also have this problem with lazy and suspense. It can be fixed by the order I assert things but, that isn't really great

@DalerAsrorov
Copy link

I also had this problem, similar to @patrickp2. The URL is persisting between tests. I had to reset it.

afterEach(() => {
  window.history.pushState(null, document.title, '/')
})

I didn't expect it but this is the only solution that worked for me.

@noahboekelman
Copy link

noahboekelman commented Sep 30, 2024

Adding the following fixed this for me:

afterEach(() => {
    jest.resetAllMocks();
});

@konrazem
Copy link

konrazem commented Feb 7, 2025

For those using reactive variable cache type in Apollo Client

Please check if after any action done in any test your reactive variable was changed. It has impact on another tests as it is based on the same state of reactive variable.

If thats the case you can reset reactive variable before each test the same way as you would reset this variable in browser environment.

For example if your reactive variable is

const myReactiveVarName = makeVar(new Map())

In your test env remember to reset (or not if you want to test prev result)

  test('My test after previous test where ractive var was changed i.e by fireEvent...', () => {
    myReactiveVarName (new Map());
    ...
  
   expect(...)
  }

I do not see anything in their documentation (I think there should be dedicated section like this)

@adolfoscheidt
Copy link

In my case, I was testing a hook with states that get their initial value from localStorage:

const storedState = localStorage.getItem('state')
const [state, setState] = useState(storedState);

and had an effect for setting new value on local storage whenever it changed:

  useEffect(() => {
    if (state !== storedState) {
      localStorage.setItem('state', state);
    }
  }, [state]);

This was messing up my tests because the value set in one test was being persisted to the other ones through localStorage ! (I'm using jsdom as test environment). Therefore, clearing local storage after each test run fixed it:

    afterEach(() => {
      localStorage.clear()
    });

I took a while to figure this out and didn't find other answer here with the same scenario, so I'm posting here in case it helps anyone in the future!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests