react-testing-library icon indicating copy to clipboard operation
react-testing-library copied to clipboard

Validate fireEvent.change

Open WilliamPriorielloGarda opened this issue 5 years ago • 6 comments

  • @testing-library/react version: "@testing-library/react": "^10.4.8",
  • Testing Framework and version: "jest": "^26.3.0", "ts-jest": "^26.2.0",
  • DOM Environment: "@testing-library/jest-dom": "^5.11.2",
"react": "^16.13.0",

Relevant code or config:

FULL CODE SANDBOX: https://codesandbox.io/s/silly-hertz-1v1uq

The test file: 'BasicSearchElement.test.tsx'

import React, { SyntheticEvent } from "react";
import {
  BasicSearchElement,
  BasicSearchElementProps
} from "../src/BasicSearchElement";
import { render, RenderResult, fireEvent } from "@testing-library/react";

describe("BasicSearchElement with props", () => {
  test("this test, which does not pass a value to 'renderBasicSearchElement', passes", async () => {
    const mockInputValue = "mockInputValue";
    const handleChange = jest.fn();
    const renderedComponent: RenderResult = renderBasicSearchElement({
      onChange: handleChange
      //value: mockInputValue THIS IS OMITTED IN THE PASSING TEST
    });

    const input = await renderedComponent.findByTestId("BasicSearchElement");

    const mockTypingEvent: Partial<SyntheticEvent> = {
      target: { value: mockInputValue }
    };
    fireEvent.change(input, mockTypingEvent);
    //handleChange is called once when we don't pass a value in renderBasicSearchElement
    expect(handleChange).toHaveBeenCalledTimes(1);
  });
  test("this test, which DOES pass a value to 'renderBasicSearchElement', FAILS", async () => {
    const mockInputValue = "mockInputValue";
    const handleChange = jest.fn();
    const renderedComponent: RenderResult = renderBasicSearchElement({
      onChange: handleChange,
      value: mockInputValue //HERE WE PASS A VALUE
    });

    const input = await renderedComponent.findByTestId("BasicSearchElement");

    const mockTypingEvent: Partial<SyntheticEvent> = {
      target: { value: "mockInputValue" }
    };
    fireEvent.change(input, mockTypingEvent);
    //handleChange is called once when we don't pass a value in renderBasicSearchElement
    expect(handleChange).toHaveBeenCalledTimes(1);
  });
});

function renderBasicSearchElement(
  props: Partial<BasicSearchElementProps> = {}
) {
  // @ts-ignore
  return render(<BasicSearchElement {...props} />);
}

What you did:

There are two tests defined in the testing file. In the first test, we do NOT pass a value to the input component. See line 14 //value: mockInputValue THIS IS OMITTED IN THE PASSING TEST

In the second test, we DO pass a value to the input component. See line 31 value: mockInputValue //HERE WE PASS A VALUE

What happened:

The first test passes the condition on line 24 (expect(handleChange).toHaveBeenCalledTimes(1);). That is, the handleChange callback is called once after we call fireEvent.change(input, mockTypingEvent);

The second test fails the same condition, but on line 41. That is, the handleChange callback is NOT called after our fireEvent.change(input, mockTypingEvent);. The official failure message is: expect(jest.fn()).toHaveBeenCalledTimes(expected) Expected number of calls: 1 Received number of calls: 0

The only difference between the tests is whether or not we decide to pass a value to our input element (lines 14 and 31). That is, if we specify a "value" prop for the input element, the handleChange callback is not executed

Reproduction:

FULL CODE SANDBOX: https://codesandbox.io/s/silly-hertz-1v1uq

Problem description:

The handleChange callback should be called (after a call to fireEvent.change) even if we pass a value to the input element

Suggested solution:

I don't have time right now to investigate this more.

WilliamPriorielloGarda avatar Aug 11 '20 19:08 WilliamPriorielloGarda

If you don't dispatch a change event with a changed value no onChange will be called. You have to pass a different value:

const mockTypingEvent: Partial<SyntheticEvent> = {
-  target: { value: "mockInputValue" }
+  target: { value: "changed-value" }
};
fireEvent.change(input, mockTypingEvent);
//handleChange is called once when we don't pass a value in renderBasicSearchElement
expect(handleChange).toHaveBeenCalledTimes(1);

The first test is passing because you do call it with a changed value.

I hope that helps.

eps1lon avatar Aug 11 '20 20:08 eps1lon

@eps1lon Figured it was something I was doing. Thanks a lot!

WilliamPriorielloGarda avatar Aug 11 '20 20:08 WilliamPriorielloGarda

@eps1lon point really helped me after hitting my head against the wall. Here's the vanilla JS working spec (extracted from Typescript example):

test('onChange fires', async () => {
  const changeHandler = jest.fn()
  const { getByRole } = render(<Input label="label is require prop haha" uniqueId="myUniqId" onChange={changeHandler} />);
  const input = getByRole('textbox')
  const mockTypingEvent = {
    target: {
      value: "changed-value"
    }
  };
  await fireEvent.change(input, mockTypingEvent)
  expect(changeHandler).toHaveBeenCalledTimes(1)
})

I feel like this should be documented more closely to what actually works but maybe it was me skipping over the docs that are already there I dunno ¯_(ツ)_/¯

roblevintennis avatar Oct 21 '20 15:10 roblevintennis

I think there are some pre-flight checks we could do:

  • check if the value is actually different
  • check if the value makes sense for that input[type]

We can probably catch the most common issues while neglecting more complext components like <div contentEditable />.

I'm re-opening until we've done some investigation whether this is viable because, as far as I can tell, fireEvent.change is a common footgun.

eps1lon avatar Oct 21 '20 15:10 eps1lon

Is the solution here to have fireEvent.change throw an error if the value is the same and the existing value?

radfahrer avatar Oct 21 '20 16:10 radfahrer

I think warning the user is the best approach here. An error could potentially break tests that are written with this change cancellation in mind, and React doesn't error about this in production so it could be surprising. But it's also something I could see as confusing if it happened by accident.

nickserv avatar Jul 11 '21 23:07 nickserv