Skip to content
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

react: Testing thrown errors #1060

Open
perymimon opened this issue May 16, 2022 · 7 comments
Open

react: Testing thrown errors #1060

perymimon opened this issue May 16, 2022 · 7 comments

Comments

@perymimon
Copy link

perymimon commented May 16, 2022

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

@eps1lon
Copy link
Member

eps1lon commented May 16, 2022

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

@eps1lon eps1lon transferred this issue from testing-library/react-testing-library May 16, 2022
@eps1lon eps1lon changed the title save throwing error on result.error when renderHook render a hook that throw error react: Testing thrown errors May 16, 2022
@perymimon
Copy link
Author

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

@ckknight
Copy link

ckknight commented Jun 7, 2022

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 };

@TadeuA
Copy link

TadeuA commented Aug 1, 2022

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.

@eps1lon
Copy link
Member

eps1lon commented Aug 9, 2022

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').

@erosenberg
Copy link

erosenberg commented Mar 13, 2023

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')
})

@joebobmiles
Copy link

@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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants