Skip to content

Commit

Permalink
Fail tests if toWarnDev() does not wrap warnings in array (facebook#1…
Browse files Browse the repository at this point in the history
…3227)

* Fail tests if toWarn() does not wrap warnings in array

* Fix newly failing tests

* Another fix
  • Loading branch information
gaearon authored Jul 18, 2018
1 parent acbb4f9 commit 236f608
Show file tree
Hide file tree
Showing 5 changed files with 73 additions and 10 deletions.
8 changes: 4 additions & 4 deletions packages/react-dom/src/__tests__/ReactChildReconciler-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -80,8 +80,8 @@ describe('ReactChildReconciler', () => {
'Keys should be unique so that components maintain their identity ' +
'across updates. Non-unique keys may cause children to be ' +
'duplicated and/or omitted — the behavior is unsupported and ' +
'could change in a future version.',
' in div (at **)\n' +
'could change in a future version.\n' +
' in div (at **)\n' +
' in Component (at **)\n' +
' in Parent (at **)\n' +
' in GrandParent (at **)',
Expand Down Expand Up @@ -127,8 +127,8 @@ describe('ReactChildReconciler', () => {
'Keys should be unique so that components maintain their identity ' +
'across updates. Non-unique keys may cause children to be ' +
'duplicated and/or omitted — the behavior is unsupported and ' +
'could change in a future version.',
' in div (at **)\n' +
'could change in a future version.\n' +
' in div (at **)\n' +
' in Component (at **)\n' +
' in Parent (at **)\n' +
' in GrandParent (at **)',
Expand Down
5 changes: 3 additions & 2 deletions packages/react-dom/src/__tests__/ReactDOMFiber-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -980,8 +980,9 @@ describe('ReactDOMFiber', () => {
expect(() => ReactDOM.render(<Example />, container)).toWarnDev(
'Expected `onClick` listener to be a function, instead got `false`.\n\n' +
'If you used to conditionally omit it with onClick={condition && value}, ' +
'pass onClick={condition ? value : undefined} instead.\n',
' in div (at **)\n' + ' in Example (at **)',
'pass onClick={condition ? value : undefined} instead.\n' +
' in div (at **)\n' +
' in Example (at **)',
);
});

Expand Down
8 changes: 4 additions & 4 deletions packages/react-dom/src/__tests__/ReactMultiChild-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -209,8 +209,8 @@ describe('ReactMultiChild', () => {
'Keys should be unique so that components maintain their identity ' +
'across updates. Non-unique keys may cause children to be ' +
'duplicated and/or omitted — the behavior is unsupported and ' +
'could change in a future version.',
' in div (at **)\n' +
'could change in a future version.\n' +
' in div (at **)\n' +
' in WrapperComponent (at **)\n' +
' in div (at **)\n' +
' in Parent (at **)',
Expand Down Expand Up @@ -269,8 +269,8 @@ describe('ReactMultiChild', () => {
'Keys should be unique so that components maintain their identity ' +
'across updates. Non-unique keys may cause children to be ' +
'duplicated and/or omitted — the behavior is unsupported and ' +
'could change in a future version.',
' in div (at **)\n' +
'could change in a future version.\n' +
' in div (at **)\n' +
' in WrapperComponent (at **)\n' +
' in div (at **)\n' +
' in Parent (at **)',
Expand Down
46 changes: 46 additions & 0 deletions scripts/jest/matchers/__tests__/toWarnDev-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -169,5 +169,51 @@ describe('toWarnDev', () => {
}).toWarnDev('Hi');
}).toThrow('Received more than one component stack for a warning');
});

it('fails if multiple strings are passed without an array wrapper', () => {
expect(() => {
expect(() => {
console.error('Hi \n in div');
}).toWarnDev('Hi', 'Bye');
}).toThrow(
'toWarnDev() second argument, when present, should be an object'
);
expect(() => {
expect(() => {
console.error('Hi \n in div');
console.error('Bye \n in div');
}).toWarnDev('Hi', 'Bye');
}).toThrow(
'toWarnDev() second argument, when present, should be an object'
);
expect(() => {
expect(() => {
console.error('Hi \n in div');
console.error('Wow \n in div');
console.error('Bye \n in div');
}).toWarnDev('Hi', 'Bye');
}).toThrow(
'toWarnDev() second argument, when present, should be an object'
);
expect(() => {
expect(() => {
console.error('Hi \n in div');
console.error('Wow \n in div');
console.error('Bye \n in div');
}).toWarnDev('Hi', 'Wow', 'Bye');
}).toThrow(
'toWarnDev() second argument, when present, should be an object'
);
});

it('fails on more than two arguments', () => {
expect(() => {
expect(() => {
console.error('Hi \n in div');
console.error('Wow \n in div');
console.error('Bye \n in div');
}).toWarnDev('Hi', undefined, 'Bye');
}).toThrow('toWarnDev() received more than two arguments.');
});
}
});
16 changes: 16 additions & 0 deletions scripts/jest/matchers/toWarnDev.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,22 @@ const createMatcherFor = consoleMethod =>
`but was given ${typeof expectedMessages}.`
);
}
if (
options != null &&
(typeof options !== 'object' || Array.isArray(options))
) {
throw new Error(
'toWarnDev() second argument, when present, should be an object. ' +
'Did you forget to wrap the messages into an array?'
);
}
if (arguments.length > 3) {
// `matcher` comes from Jest, so it's more than 2 in practice
throw new Error(
'toWarnDev() received more than two arguments. ' +
'Did you forget to wrap the messages into an array?'
);
}

const withoutStack = options.withoutStack;
const warningsWithoutComponentStack = [];
Expand Down

0 comments on commit 236f608

Please sign in to comment.