prop-types icon indicating copy to clipboard operation
prop-types copied to clipboard

Add ability to throw error instead of warning in tests

Open hakunin opened this issue 8 years ago • 26 comments
trafficstars

In tests it would be nice to throw an error if the property isn't of its expected type.

It would be nice if this could be scoped to our own components and not 3rd party ones using 3rd party components, but thats not necesary.

People are already turning console.warning calls to throw errors, so this would be a nice upgrade for them.

hakunin avatar Apr 13 '17 12:04 hakunin

This would be really useful for anyone using create-react-app, as PropType errors would appear in the new error overlay

iainbeeston avatar May 23 '17 14:05 iainbeeston

Related to previous conversation here.

seanadenise avatar Jul 06 '17 16:07 seanadenise

I just published a check-prop-types package that does this. The main export returns the error instead of logging it, and a helper assertPropTypes throws it.

#54 on this repo also has an interesting approach, using a callback instead.

uniphil avatar Jul 06 '17 20:07 uniphil

I'm trying to use prop-types to validate js schema of external service response on server.
And surprised it's just generating console warnings.
How do I supposed to react to validation errors?

checkPropTypes should definitely generate some report (like suggested in #88), or at least return true/false. Anyway it would be nice for user to decide how validation results should be handled (log, throw error, etc).

art-in avatar Jul 24 '17 14:07 art-in

@artin-phares I've created typed-props library which has the same interface and produce an array of validation report issues.

rumkin avatar Jul 26 '17 09:07 rumkin

There is a workaround for throwing an error on warnings. We can use getStack argument.

PropTypes.checkPropTypes(spec, values, 'prop', name, () => {
    process.nextTick(() => {
        throw new Error('Check has failed');
    });
});

This should work even in browsers with process.nextTick polyfill.

a-ignatov-parc avatar Sep 21 '18 14:09 a-ignatov-parc

@a-ignatov-parc, won't nextTick cause it to fail after the test has already completed? Potentially even after test runner has already finished. Not very useful.

asapach avatar Sep 21 '18 14:09 asapach

nextTick will be called before the current event loop continues, so it will throw after the warning is printed and before everything else.

a-ignatov-parc avatar Sep 21 '18 14:09 a-ignatov-parc

@a-ignatov-parc Thrown error will loose the stack in that way and it will not be captured in the place where it was thrown. Also printing of PropTypes' message will not be stopped.

rumkin avatar Sep 27 '18 13:09 rumkin

I am not sure what is the current state of this issue and the project.

I am using a small wrapper to throw error like so:

PropTypes.originalCheckPropTypes = PropTypes.checkPropTypes;
PropTypes.checkPropTypes = function(propTypes, attrs, attrsName, componentName) {
  const consoleError = console.error;
  console.error = function(message) {
    throw new Error(message);
  };
  PropTypes.originalCheckPropTypes(propTypes, attrs, attrsName, componentName);
  console.error = consoleError;
};

sarbbottam avatar Jan 20 '19 05:01 sarbbottam

My solution with Jest:

jest.mock('prop-types/checkPropTypes', () => {
  return (...args) => {
    const checkPropTypes = jest.requireActual('prop-types/checkPropTypes');
    const originalConsoleError = console.error;
    console.error = function(...args) {
      process.nextTick(() => {
        throw new Error(...args);
      });
    };
    const result = checkPropTypes(...args);
    console.error = originalConsoleError;
    return result;
  };
});

typeofweb avatar Aug 15 '19 16:08 typeofweb

@mmiszy Why are you using process.nextTick?

jpenna avatar Oct 22 '19 23:10 jpenna

I don't get what's the difference between @sarbbottam's and @mmiszy's workarounds.

vince1995 avatar Jan 17 '20 10:01 vince1995

@vince1995 the second one throws the error in nextTick, which means you don't get a useful stack trace that tells you where console.error was called.

ljharb avatar Jan 17 '20 17:01 ljharb

@vince1995 @ljharb quite the opposite, actually. Throwing the error without nextTick results in the error being caught by React. It does result in a test failure, however, the stacktrace is much longer. It also includes the default React warning Consider adding an error boundary to your tree to customize error handling behaviour. Visit https://fb.me/react-error-boundaries to learn more about error boundaries.]. I found this error message to be misleading in this context.

After adding nextTick, the problems are gone. You get even more useful stracktrace without all the React functions. And the warning is not displayed. You can see for yourself:

Error message in Jest without nextTick:

  ● MyComponent › should be able to do something

    Error: Uncaught [Error: The above error occurred in one of your React components:
        in Unknown (at Database/index.js:23)
        in Unknown (created by Context.Consumer)
        in withRouter(withApiData(waitForApi(MyComponent))) (created by ConnectFunction)
        in ConnectFunction (at MyComponent.test.js:87)
        in Router (created by MemoryRouter)
        in MemoryRouter (at MyComponent.test.js:86)
        in Provider (at MyComponent.test.js:85)

    Consider adding an error boundary to your tree to customize error handling behavior.
    Visit https://fb.me/react-error-boundaries to learn more about error boundaries.]

      14 |     const originalConsoleError = console.error;
      15 |     console.error = function(...args) {
    > 16 |       throw new Error(...args);
         |             ^
      17 |     };
      18 |     const result = checkPropTypes(...args);
      19 |     console.error = originalConsoleError;

      at reportException (node_modules/jsdom/lib/jsdom/living/helpers/runtime-script-errors.js:66:24)
      at Timeout.callback [as _onTimeout] (node_modules/jsdom/lib/jsdom/browser/Window.js:680:7)
      at CustomConsole.console.error (src/setupTests.js:16:13)
      at VirtualConsole.on.e (node_modules/jsdom/lib/jsdom/virtual-console.js:29:45)
      at reportException (node_modules/jsdom/lib/jsdom/living/helpers/runtime-script-errors.js:70:28)
      at Timeout.callback [as _onTimeout] (node_modules/jsdom/lib/jsdom/browser/Window.js:680:7)

Error message in Jest with nextTick

  ● MyComponent › should be able to do something

    Warning: Failed prop type: The prop `dsadsadas` is marked as required in `MyComponent`, but its value is `undefined`.
        in MyComponent (at Database/index.js:58)
        in Unknown (at Database/index.js:23)
        in Unknown (created by Context.Consumer)
        in withRouter(withApiData(waitForApi(MyComponent))) (created by ConnectFunction)
        in ConnectFunction (at MyComponent.test.js:87)
        in Router (created by MemoryRouter)
        in MemoryRouter (at MyComponent.test.js:86)
        in Provider (at MyComponent.test.js:85)

      15 |     console.error = function(...args) {
      16 |       process.nextTick(() => {
    > 17 |         throw new Error(...args);
         |               ^
      18 |       });
      19 |     };
      20 |     const result = checkPropTypes(...args);

      at process.nextTick (src/setupTests.js:17:15)

typeofweb avatar Jan 17 '20 17:01 typeofweb

ah, you’re right, I’d forgotten that react 16 changed that behavior around error catching.

ljharb avatar Jan 17 '20 18:01 ljharb

Ok, I understand this but if I add process.nextTick to my code the process exits:

image

vince1995 avatar Jan 20 '20 09:01 vince1995

Do you have the latest React?

typeofweb avatar Jan 20 '20 10:01 typeofweb

Yes.

This is my setup-tests.js file
import { configure } from 'enzyme';
import Adapter from 'enzyme-adapter-react-16';
import MutationObserver from "mutation-observer";

import Parse from "parse";


jest.mock('prop-types/checkPropTypes', () => {
  return (...args) => {
    const checkPropTypes = jest.requireActual('prop-types/checkPropTypes');
    const originalConsoleError = console.error;
    console.error = function(...args) {
      process.nextTick(() => {
        throw new Error(...args);
      });
    };
    const result = checkPropTypes(...args);
    console.error = originalConsoleError;
    return result;
  };
});

require("jest-fetch-mock");

Parse.CoreManager.set('IS_NODE', false);

process.env.SERVER_RENDERING = true;

const parseServerInfo = {
  url: "...",
  appId: "...",
  key: "..."
}

Parse.initialize(parseServerInfo.appId, parseServerInfo.key);
Parse.serverURL = parseServerInfo.url;

global.MutationObserver = MutationObserver;

configure({ adapter: new Adapter() });


window.matchMedia = window.matchMedia || function () {
  return {
    matches: false,
    addListener: function () { },
    removeListener: function () { }
  };
};

And my tests:
    shouldCrashTests.map(testCase => {
        test("Accordion: " + testCase.name, () => {

            function shouldThrow() {
                const wrapper = render(testCase.items);
            }

            expect(shouldThrow).toThrow()

        });
    })

EDIT: I've created a new project. Still fails with nextTick. Without it works.

Maybe my node version is different to yours? Mine is v12.13.1

vince1995 avatar Jan 20 '20 14:01 vince1995

For anyone else trying to captures prop-type errors in unit tests with Jest - this setupTest.js works for us:

beforeEach(() => {
  jest.spyOn(console, 'error')
  jest.spyOn(console, 'warn')
})

afterEach(() => {
  /* eslint-disable no-console,jest/no-standalone-expect */
  expect(console.error).not.toBeCalled()
  expect(console.warn).not.toBeCalled()
})

This breaks when any test calls jest.restoreAllMocks() - for us, calling jest.clearAllMocks()` instead helped.

It also requires your app to not call console.error or console.warn for "error handling" (scare quotes, since that's usually not a good idea).

jzaefferer avatar Apr 17 '20 11:04 jzaefferer

@jzaefferer The latter does not work for me.

Is there any advance or solution is there alternatives for this pain?

vicasas avatar Jul 05 '21 16:07 vicasas

We recently upgraded Jest to latest and had to drop this. I don't know of any alternatives :/

jzaefferer avatar Jul 06 '21 10:07 jzaefferer

Neither example worked for me. So create a new npm package to solve this problem and it is compatible with the latest version of Jest (27).

https://github.com/vicasas/jest-prop-types

vicasas avatar Jan 20 '22 21:01 vicasas

fwiw, there's nothing specific to jest in there - that technique should be applied for every testing framework.

ljharb avatar Jan 20 '22 21:01 ljharb

@ljharb I don't have the same experience in other testing frameworks, I don't know how they work. If anyone wants to help you are welcome and we can rename the repo and the npm package.

vicasas avatar Jan 20 '22 21:01 vicasas

I'm currently using Jest and my solution was to mock the console, then every warning or error will throw an error when running the tests. Hope this can be useful for someone else.

global.console = {
  log: jest.fn(),
  warn: (message) => {
    throw Error(message)
  },
  error: (message, data, details) => {
    throw Error(
      message + (data ? " - " + data : "") + (details ? " - " + details : "")
    )
  },
}

lrusso avatar Jan 07 '24 17:01 lrusso