handleChange works unexpectedly when input is inside of a component being tested

[nooruddin]

I am trying to figure out why following line of code would always work no matter what number I put.
waitForElement(() => expect(onChange).toHaveBeenCalledTimes(3));

I am trying to test that input has been called x number of times however input is controlled within the component itself. No matter what number provide it will always pass the test.
complete-react-testing.zip

With fireEvent, I expect it to be
waitForElement(() => expect(onChange).toHaveBeenCalledTimes(1));

With userEvent, I expect it to be
waitForElement(() => expect(onChange).toHaveBeenCalledTimes(5));

Here is my Component:

import React, { useState } from "react";
import axios from "axios";
import "./Pokemon.css";

const getPokemonByColor = (color) =>
  `https://pokeapi.co/api/v2/pokemon-color/${color}/`;

const Pokemon = () => {
  const [pokemons, setPokemons] = useState([]);
  const [error, setError] = useState(null);
  const [search, setSearch] = useState("");
  const handleFetch = async (event) => {
    event.preventDefault();
    let result;

    try {
      result = await axios.get(getPokemonByColor(search));
      setPokemons(result.data.pokemon_species.slice(0, 5));
    } catch (error) {
      setError(error);
    }
  };

  const handleChange = (event) => {
    setSearch(event.target.value);
  };

  return (
    <div>
      {error && <span>Something went wrong ...</span>}
      <form onSubmit={handleFetch}>
        <div>
          <input
            id="search"
            type="text"
            value={search}
            onChange={handleChange}
            placeholder="Pokemon Color"
            className="search"
          />
        </div>
        <button className="search-button" type="submit" data-testid="button">
          Fetch Pokemons
        </button>
      </form>
      <ul className="pokemons">
        {pokemons.length > 0 &&
          pokemons.map((pokemon) => (
            <li className="pokemon-item" key={pokemon.name}>
              <a className="pokemon-link" href={pokemon.url}>
                {pokemon.name}
              </a>
            </li>
          ))}
      </ul>
    </div>
  );
};

export default Pokemon;

Here are my tests:

import React from "react";
import axios from "axios";
import {
  render,
  screen,
  waitForElement,
  fireEvent,
  cleanup,
} from "@testing-library/react";
import userEvent from "@testing-library/user-event";

import Pokemon from "./Pokemon";

jest.mock("axios");

afterEach(cleanup);
describe("search input tests", () => {
  test("calls the onChange callback handler", () => {
    const onChange = jest.fn();

    const { getByRole } = render(<Pokemon />);
    const input = getByRole("textbox");
    expect(input.value).toBe("");
    fireEvent.change(input, {
      target: { value: "black" },
    });
    expect(input.value).toBe("black");
    waitForElement(() => expect(onChange).toHaveBeenCalledTimes(3));
  });

  test("calls the onChange callback handler", async () => {
    const onChange = jest.fn();

    const { getByRole } = render(<Pokemon />);

    const input = getByRole("textbox");
    expect(input.value).toBe("");
    await userEvent.type(input, {
      target: { value: "black" },
    });
    waitForElement(() => expect(input.value).toBe("black"));
    waitForElement(() => expect(onChange).toHaveBeenCalledTimes(7));
  });
});

describe("api tests", () => {
  test("fetches pokemons from an API and display them", async () => {
    const pokemons = [
      {
        name: "snorlax",
        url: "https://pokeapi.co/api/v2/pokemon-species/143/",
      },
      {
        name: "murkrow",
        url: "https://pokeapi.co/api/v2/pokemon-species/198/",
      },
      {
        name: "unown",
        url: "https://pokeapi.co/api/v2/pokemon-species/201/",
      },
      {
        name: "sneasel",
        url: "https://pokeapi.co/api/v2/pokemon-species/215/",
      },
    ];

    axios.get.mockImplementationOnce(() =>
      Promise.resolve({ data: { pokemon_species: pokemons } })
    );

    render(<Pokemon />);
    // screen.debug();
    userEvent.click(screen.getByRole("button"));
    expect(await screen.findAllByRole("listitem")).toHaveLength(4);
    // screen.debug();
  });

  test("fetches stories from an API and fails", async () => {
    axios.get.mockImplementationOnce(() => Promise.reject(new Error()));

    render(<Pokemon />);

    userEvent.click(screen.getByRole("button"));

    const message = await screen.findByText(/Something went wrong/);

    expect(message).toBeInTheDocument();
  });
});

[marcosvega91]

Hi @nooruddin
I think that you are using the wrong function. You need to use waitFor instead of waitForElement.

Look at here

waitForElement is used to wait for element to appear or disappear but is now deprecated.

waitFor is used to wait for an expectation to pass

[nooruddin]

@marcosvega91 waitFor is only available in 10.x onwards. Correct? create-react-app gives ^9.5.0 at this time.

[nooruddin]

@marcosvega91 Bumping default version to latest from npm (10.x), resulted in MutationObserver is not a function error.

I tried installing jest-environment-jsdom-sixteen as suggested in #477. No luck.

It seems like if I am coming from create-react-app route, I will have to all testing library dependency and peer dependencies to be the latest one.

Is there a way I can fix this, without doing any upgrades?

[marcosvega91]

Hi @nooruddin you can use wait

wait(() => expect(onChange).toHaveBeenCalledTimes(1));

[samtsai]

Created a sandbox to further debug the issue:
https://codesandbox.io/s/delicate-pine-irxx8?file=/src/pokemon/Pokemon.test.js

[samtsai]

That being said, I'm having mixed results in Codesandbox, which I think might be more them then the code.

But you're also missing an await since this is an async call and might be why you're getting the success.

[samtsai]

Few other things:

• add the await
• your test for onChange will not work unless you receive it as a prop in Pokemon.js, so you're current spies are never going to pass
• for the second test that uses userEvent, I think it'd be better to do: await userEvent.type(input, "black"); (I'm not sure the previous passing the event object would work, that seems more like the api for fireEvent)
• as mentioned above: wait() would be a better choice since you already have the element:

// Test 1
await wait(() => expect(onChange).toHaveBeenCalledTimes(1));
// Test 2
await wait(() => expect(input.value).toBe("black"));

[nooruddin]

@samtsai tried all these approaches. No luck still. also sandbox is behaving weird where it would pass some test and fail on the next run. Also, jest.mock("axios") won't work on sandbox.
Also, I don't think await is necessary here however I did try and it would actually fail the test that are working.

I also followed all the best practices listed here.

Here is my resulting test so far. All tests will pass even the ones which is testing for wait(() => expect(onChange).toHaveBeenCalledTimes(3)); or wait(() => expect(onChange).toHaveBeenCalledTimes(7));

import React from "react";
import axios from "axios";
import {
  render,
  screen,
  wait,
  fireEvent,
  waitForElement,
} from "@testing-library/react";
import userEvent from "@testing-library/user-event";

import Pokemon from "../Pokemon";

jest.mock("axios");

describe("search input tests", () => {
  test("calls the onChange callback handler", () => {
    const onChange = jest.fn();

    render(<Pokemon />);
    const input = screen.getByRole("textbox");
    expect(input.value).toBe("");
    fireEvent.change(input, {
      target: { value: "black" },
    });
    expect(input.value).toBe("black");
    wait(() => expect(onChange).toHaveBeenCalledTimes(3));
  });

  test("calls the onChange callback handler", () => {
    const onChange = jest.fn();

    render(<Pokemon />);

    const input = screen.getByRole("textbox");
    expect(input.value).toBe("");
    userEvent.type(input, {
      target: { value: "black" },
    });
    waitForElement(() => expect(input.value).toBe("black"));
    wait(() => expect(onChange).toHaveBeenCalledTimes(7));
  });
});

describe("api tests", () => {
  test("fetches pokemons from an API and display them", async () => {
    const pokemons = [
      {
        name: "snorlax",
        url: "https://pokeapi.co/api/v2/pokemon-species/143/",
      },
      {
        name: "murkrow",
        url: "https://pokeapi.co/api/v2/pokemon-species/198/",
      },
      {
        name: "unown",
        url: "https://pokeapi.co/api/v2/pokemon-species/201/",
      },
      {
        name: "sneasel",
        url: "https://pokeapi.co/api/v2/pokemon-species/215/",
      },
    ];

    axios.get.mockImplementationOnce(() =>
      Promise.resolve({ data: { pokemon_species: pokemons } })
    );

    render(<Pokemon />);
    // screen.debug();
    userEvent.click(screen.getByRole("button"));
    expect(await screen.findAllByRole("listitem")).toHaveLength(4);
    // screen.debug();
  });

  test("fetches stories from an API and fails", async () => {
    axios.get.mockImplementationOnce(() => Promise.reject(new Error()));

    render(<Pokemon />);

    userEvent.click(screen.getByRole("button"));

    const message = await screen.findByText(/Something went wrong/);

    expect(message).toBeInTheDocument();
  });
});

[samtsai]

@nooruddin can you try adding async and await:

test("calls the onChange callback handler",  async () => {

await wait(() => expect(onChange).toHaveBeenCalledTimes(3));

[samtsai]

Also it doesn't look like you've added onChange as a prop to Pokemon if that's how you intend to use it and test it.

[nooruddin]

@samtsai yeah I had onChange prop but it was not working either and I guess it makes sense because input is controlled by component itself and is not expecting any props as such.

I have also tried adding async/await which would result in error.

Expected number of calls: 3
Received number of calls: 0

I have also tried removing wait as such:
await expect(onChange).toHaveBeenCalledTimes(3); but that would result in same error as well.

I have attached updated zip file in case if you want to try couple of things and see if you can get it working.
complete-react-testing.zip

Also, I have not installed any packages separately. Everything I have in package.json minus axios was installed as part of create-react-app.

[MatanBobi]

@samtsai yeah I had onChange prop but it was not working either and I guess it makes sense because input is controlled by component itself and is not expecting any props as such.

I'm trying to understand this one, if you're not using the onChange prop inside the component and not passing it in your test, the onChange mocked function won't be called..
Since you're trying to test a controlled component, mocking the onChange inside the component isn't a best practice IMO since it's an implementation detail, you should be testing the output of that onChange handler (in the case of the Pokemon component, you can see that the value of the input was indeed changed).
I may have been missing something here so sorry if I joined the discussion at a late stage.

[samtsai]

@MatanBobi thanks for the input, I agree with what you're saying. 😄

[nooruddin]

@MatanBobi Yes that explanation totally makes sense. However, I am curious as to why I can test for the value of the text input to be that text but I can't test for the onChange being called for each character being typed. If an input that is controlled can be tested for the result (in this case the text being typed), why can't it be tested for it's actual onChange functionality.

I might be completely off here and not really understand the internal workings of this library. So please forgive that. 😄

[WretchedDade]

Hi @nooruddin, I'll try to answer you question. The onChange event and it's corresponding handler are contained inside of your component. Because of this, there is no way to access it from outside without a refactor to accept the onChange handler as a prop. However, as the others have said, this isn't something that needs to be tested as it is partially out of your control and only how the result is achieved rather than what was achieved.

I hope that helps! 😄

[nooruddin]

@WretchedDade in that sense, can't we say that the value property of controlled input should also be not accessible outside the component and hence it should not be possible to test it?

I guess value property works because we do specify it using target property while testing?
Having said that, is there any other way to test onChange handler while still keeping the input controlled inside component? If not, I think we can close this issue. 😃

[WretchedDade]

@nooruddin The value property is made available when the component is rendered to the dom/virtual dom. This is how you can do things like expect(input).toHaveValue('Test Value'). This works by getting a reference to the element in the dom/virtual dom. The only way to assert against the onChange handler would be to get a reference to it somehow and do your assert on that. However, the onChange handler doesn't exist in the dom/virtual dom the same way the value does.

I haven't tried to get a reference to an event handler for my component before but I can't think of any good ways to do it at the moment.

[MatanBobi]

Just following what @WretchedDade wrote and emphasizing one important thing, when using RTL we should test a component the way the user uses it.
Kent usually says: "The more your tests resemble the way your software is used, the more confidence they can give you."
Counting the number of times an onChange handler was called is an implementation detail since if this behavior will change (by browsers for example), all of your tests will break, but the user of your component won't notice it since the component will still work.

[eps1lon]

Looks like the issue has been resolved. If this is still an issue please update the original issue description and ping me so that it's easier for maintainers to understand what exactly the issue is.