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

react: Testing thrown errors

Open perymimon opened this issue 3 years ago • 10 comments

Describe the feature you'd like:

When I want to test a situation that my tested hook throws an expected error I like I have the option to test it without hacking the test code. It will be greeted if the implementation that actually done on that subject on react-hooks-testing-library will be implemented on @testing-library/react

Suggested implementation:

As describe here result.error should be hold the throwing object and accessing to result.current will should throw the error again

perymimon avatar May 16 '22 21:05 perymimon

We definitely want to document a good pattern for testing errors. But this should cover components as well not just hooks.

eps1lon avatar May 16 '22 22:05 eps1lon

It is nice, but implantation of already made work not should be delayed by feature work on components

perymimon avatar May 17 '22 09:05 perymimon

To get the functionality that was previously available in @testing-library/react-hook, I came up with this chunk of code:

(it's basically the same code as renderHook, with the extra error-handling)

import type { RenderHookOptions } from '@testing-library/react';
import { render } from '@testing-library/react';
import * as React from 'react';

function customRenderHook<TProps, TResult>(
  renderCallback: (initialProps: TProps) => TResult,
  options: RenderHookOptions<TProps> = {},
) {
  const { initialProps, wrapper } = options;
  const result = {
    current: undefined as TResult | undefined,
    error: undefined as unknown,
  };

  function TestComponent({
    renderCallbackProps,
  }: {
    renderCallbackProps: TProps | undefined;
  }) {
    let pendingResult: TResult | undefined;
    let pendingError: unknown;
    try {
      pendingResult = renderCallback(renderCallbackProps!);
    } catch (error) {
      pendingError = error;
    }
    React.useEffect(() => {
      result.current = pendingResult;
      result.error = pendingError;
    });

    return null;
  }

  const { rerender: baseRerender, unmount } = render(
    <TestComponent renderCallbackProps={initialProps} />,
    {
      wrapper,
    },
  );

  function rerender(rerenderCallbackProps?: TProps) {
    baseRerender(<TestComponent renderCallbackProps={rerenderCallbackProps} />);
  }

  return {
    result,
    rerender,
    unmount,
  };
}

export { customRenderHook as renderHook };

ckknight avatar Jun 07 '22 19:06 ckknight

I agree with perymimon, you don't need to wait to have a component error resolution to raise the hooks error resolution.

Now with the arrival of React 18 and the non-update of the lib @testing-library/react-hooks, I would be very grateful if this could be implemented as soon as possible.

TadeuA avatar Aug 01 '22 18:08 TadeuA

I agree with perymimon, you don't need to wait to have a component error resolution to raise the hooks error resolution.

The problem is that without understanding the problemspace fully we might create an interface that only works with hooks but not components. If we rush hooks implementation and later realize that it's not a good interface for testing errors in components, we have to either release another breaking change or fragment the testing landscape. Neither of these scenarios is ideal.

We already have issues by letting /react-hooks proliferate. This library encouraged testing practices that are not in line with the Testing Library mentality and caused bigger migration issues when going from 17 and 18. A scenario which we want to avoid and something we wanted to fix by using Testing Library.

To elaborate more on the issues I see with saving the last error:

  • it's no longer apparent from your test what caused the error (e.g. in "render -> fireEvent -> fireEvent -> assert on result.error" it's not clear what caused the error. Something that might be important).
  • only the last error is saved. Is only the last error relevant or should you assert on every single one?
  • what should we do with console.error that React might add as well (e.g. "The above error occured in ..."). Some testing setups throw on console.error calls so we might want to ensure that no spurious console.error goes through when you asserted on a error

From my experience it's probably better to move this to matchers i.e. provide an API along the lines of expect(() => renderHook()).toThrow('some error message').

eps1lon avatar Aug 09 '22 17:08 eps1lon

For anyone this may help, I wrote a very simple helper function to catch just an error thrown by a hook. I'm sure it can be improved, but I wanted to write a reusable function that didn't override renderHook but ran it through a try/catch as was suggested in other threads.

import { renderHook } from '@testing-library/react' // v14.0.0

// This suppresses console.error from cluttering the test output.
export const mockConsoleError = () => {
  jest.spyOn(console, 'error').mockImplementation(() => {})
}

export const restoreConsoleError = () => {
  if (console.error.mockRestore !== undefined) {
    console.error.mockRestore()
  }
}

export const catchHookError = (...args) => {
  let error
  mockConsoleError()

  try {
    renderHook(...args)
  } catch (e) {
    error = e
  }

  restoreConsoleError()
  return error
}

Here is an example of it in use:

hooks/useCustomHook.js

export const useCustomHook = (name) => {
  if (!name) {
    throw new Error('Please provide a name')
  }
}

hooks/useCustomHook.test.js

it('should throw an error', () => {
  const error = catchHookError(() => useCustomHook())
  expect(error).toEqual('Please provide a name')
})

erosenberg avatar Mar 13 '23 18:03 erosenberg

@eps1lon So.... we are letting documentation for a feature that doesn't exist yet block the implementation of said feature?

Sounds a little like putting the cart before the horse to, as you put in this comment, "start with a documented pattern first before building any API around it."

While it isn't the end of the world for me that React Testing Library can't handle errors (I have only one test in y-react that relies on this), it is annoying to see this get shoved under the rug.

joebobmiles avatar Jun 28 '23 16:06 joebobmiles