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

Error testing a single component's propTypes with checkPropTypes multiple times #91

Closed
brucewpaul opened this issue Jul 18, 2017 · 3 comments

Comments

@brucewpaul
Copy link

While going through and updating to use the external prop-types package, I noticed that some of our tests were failing and couldn't figure out why. Eventually we found that the oddity in the tests were coming from the fact that the warning issued by the package is only called once, even if testing the component's propTypes with checkPropTypes multiple times. Take for example the following tests:

it('returns error if props[propName] is not passed', () => {
    const propValue = null;
    expect(() => PropTypes.checkPropTypes(Component.propTypes, { [propName]: propValue }, propName, componentName)).to.throw(Error);
});

it('returns error if any of the props[propName] is not a PropType.node', () => {
    const propValue = shallow(<div>{{}}</div>).props().children;
    expect(() => PropTypes.checkPropTypes(Component.propTypes, { [propName]: propValue }, propName, componentName)).to.throw(Error);
});

In this case, the first test passes as the package does warn (and we override console.warn/console.error to throw an error), but the second test fails as there isn't a warning, and thus an error isn't thrown.

We know that this is the case because if we change componentName to be a unique string in each case, the warning is displayed for each as expected.

The trouble here is that with using this external package, we're unsure as to how many false positives we have in our test suite due to this.

Should there a be a way to have the warning's call each time so that tests can test this more accurately?

@ljharb
Copy link
Collaborator

ljharb commented Jul 18, 2017

This would be very helpful - in other words, either a way to disable the memoization, or a way to reset it.

@uniphil
Copy link

uniphil commented Jul 19, 2017

Related: #28

At ratehub we're maintaining a helper package for testing, check-prop-types. It mirrors this project's checkPropTypes function, but errors are returned (or thrown for assertPropTypes) instead of logged, and memoization is removed.

Alternately, there is a neat PR here using a callback to signal errors.

@ljharb
Copy link
Collaborator

ljharb commented Feb 8, 2019

This has been resolved by #178.

@ljharb ljharb closed this as completed Feb 8, 2019
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

3 participants