Warn about unsafe toWarnDev() nesting in tests (#12457)
* Add lint run to warn about improperly nested toWarnDev matchers * Updated tests to avoid invalid nesting
Brian Vaughn committed
Aug 21, 2018 at 07:43 UTC
e106b8c44f9a81058f250c7ee37b61ae0094455e
8 files changed
+160
-84
.eslintrc.js
+1
@@ -65,6 +65,7 @@ module.exports = {
65
// CUSTOM RULES
66
// the second argument of warning/invariant should be a literal string
67
'react-internal/no-primitive-constructors': ERROR,
68
+ 'react-internal/no-to-warn-dev-within-to-throw': ERROR,
69
'react-internal/warning-and-invariant-args': ERROR,
70
},
71
packages/react-dom/src/__tests__/ReactCompositeComponent-test.js
+27
-12
@@ -428,12 +428,13 @@ describe('ReactCompositeComponent', () => {
428
expect(() => {
429
expect(() => {
430
ReactDOM.render(<ClassWithRenderNotExtended />, container);
431
- }).toWarnDev(
432
- 'Warning: The <ClassWithRenderNotExtended /> component appears to have a render method, ' +
433
- "but doesn't extend React.Component. This is likely to cause errors. " +
434
- 'Change ClassWithRenderNotExtended to extend React.Component instead.',
435
- );
436
- }).toThrow(TypeError);
431
+ }).toThrow(TypeError);
432
+ }).toWarnDev(
433
+ 'Warning: The <ClassWithRenderNotExtended /> component appears to have a render method, ' +
434
+ "but doesn't extend React.Component. This is likely to cause errors. " +
435
+ 'Change ClassWithRenderNotExtended to extend React.Component instead.',
436
+ {withoutStack: true},
437
+ );
438
439
// Test deduplication
440
expect(() => {
@@ -1728,11 +1729,18 @@ describe('ReactCompositeComponent', () => {
1729
expect(() => {
1730
expect(() => {
1731
ReactTestUtils.renderIntoDocument(<RenderTextInvalidConstructor />);
1731
- }).toWarnDev(
1732
+ }).toThrow();
1733
+ }).toWarnDev(
1734
+ [
1735
+ // Expect two errors because invokeGuardedCallback will dispatch an error event,
1736
+ // Causing the warning to be logged again.
1737
'Warning: RenderTextInvalidConstructor(...): No `render` method found on the returned component instance: ' +
1738
'did you accidentally return an object from the constructor?',
1734
- );
1735
- }).toThrow();
1739
+ 'Warning: RenderTextInvalidConstructor(...): No `render` method found on the returned component instance: ' +
1740
+ 'did you accidentally return an object from the constructor?',
1741
+ ],
1742
+ {withoutStack: true},
1743
+ );
1744
});
1745
1746
it('should return error if render is not defined', () => {
@@ -1741,10 +1749,17 @@ describe('ReactCompositeComponent', () => {
1749
expect(() => {
1750
expect(() => {
1751
ReactTestUtils.renderIntoDocument(<RenderTestUndefinedRender />);
1744
- }).toWarnDev(
1752
+ }).toThrow();
1753
+ }).toWarnDev(
1754
+ [
1755
+ // Expect two errors because invokeGuardedCallback will dispatch an error event,
1756
+ // Causing the warning to be logged again.
1757
'Warning: RenderTestUndefinedRender(...): No `render` method found on the returned ' +
1758
'component instance: you may have forgotten to define `render`.',
1747
- );
1748
- }).toThrow();
1759
+ 'Warning: RenderTestUndefinedRender(...): No `render` method found on the returned ' +
1760
+ 'component instance: you may have forgotten to define `render`.',
1761
+ ],
1762
+ {withoutStack: true},
1763
+ );
1764
});
1765
});
packages/react-dom/src/__tests__/ReactDOMTextarea-test.js
+8
-8
@@ -359,10 +359,10 @@ describe('ReactDOMTextarea', () => {
359
{'there'}
360
</textarea>,
361
),
362
- ).toWarnDev(
363
- 'Use the `defaultValue` or `value` props instead of setting children on <textarea>.',
364
- );
365
- }).toThrow();
362
+ ).toThrow('<textarea> can only have at most one child');
363
+ }).toWarnDev(
364
+ 'Use the `defaultValue` or `value` props instead of setting children on <textarea>.',
365
+ );
366
367
let node;
368
expect(() => {
@@ -373,10 +373,10 @@ describe('ReactDOMTextarea', () => {
373
<strong />
374
</textarea>,
375
)),
376
- ).toWarnDev(
377
- 'Use the `defaultValue` or `value` props instead of setting children on <textarea>.',
378
- );
379
- }).not.toThrow();
376
+ ).not.toThrow();
377
+ }).toWarnDev(
378
+ 'Use the `defaultValue` or `value` props instead of setting children on <textarea>.',
379
+ );
380
381
expect(node.value).toBe('[object Object]');
382
});
packages/react-dom/src/__tests__/ReactServerRendering-test.js
+7
-6
@@ -646,12 +646,13 @@ describe('ReactDOMServer', () => {
646
expect(() => {
647
expect(() =>
648
ReactDOMServer.renderToString(<ClassWithRenderNotExtended />),
649
- ).toWarnDev(
650
- 'Warning: The <ClassWithRenderNotExtended /> component appears to have a render method, ' +
651
- "but doesn't extend React.Component. This is likely to cause errors. " +
652
- 'Change ClassWithRenderNotExtended to extend React.Component instead.',
653
- );
654
- }).toThrow(TypeError);
649
+ ).toThrow(TypeError);
650
+ }).toWarnDev(
651
+ 'Warning: The <ClassWithRenderNotExtended /> component appears to have a render method, ' +
652
+ "but doesn't extend React.Component. This is likely to cause errors. " +
653
+ 'Change ClassWithRenderNotExtended to extend React.Component instead.',
654
+ {withoutStack: true},
655
+ );
656
657
// Test deduplication
658
expect(() => {
scripts/eslint-rules/__tests__/no-to-warn-dev-within-to-throw-test.internal.js
new
+32
@@ -0,0 +1,32 @@
1
+/**
2
+ * Copyright (c) 2015-present, Facebook, Inc.
3
+ *
4
+ * This source code is licensed under the MIT license found in the
5
+ * LICENSE file in the root directory of this source tree.
6
+ *
7
+ * @emails react-core
8
+ */
9
+
10
+'use strict';
11
+
12
+const rule = require('../no-to-warn-dev-within-to-throw');
13
+const RuleTester = require('eslint').RuleTester;
14
+const ruleTester = new RuleTester();
15
+
16
+ruleTester.run('eslint-rules/no-to-warn-dev-within-to-throw', rule, {
17
+ valid: [
18
+ 'expect(callback).toWarnDev("warning");',
19
+ 'expect(function() { expect(callback).toThrow("error") }).toWarnDev("warning");',
20
+ ],
21
+ invalid: [
22
+ {
23
+ code:
24
+ 'expect(function() { expect(callback).toWarnDev("warning") }).toThrow("error");',
25
+ errors: [
26
+ {
27
+ message: 'toWarnDev() matcher should not be nested',
28
+ },
29
+ ],
30
+ },
31
+ ],
32
+});
scripts/eslint-rules/index.js
+1
@@ -3,6 +3,7 @@
3
module.exports = {
4
rules: {
5
'no-primitive-constructors': require('./no-primitive-constructors'),
6
+ 'no-to-warn-dev-within-to-throw': require('./no-to-warn-dev-within-to-throw'),
7
'warning-and-invariant-args': require('./warning-and-invariant-args'),
8
},
9
};
scripts/eslint-rules/no-to-warn-dev-within-to-throw.js
new
+33
@@ -0,0 +1,33 @@
1
+/**
2
+ * Copyright (c) 2015-present, Facebook, Inc.
3
+ *
4
+ * This source code is licensed under the MIT license found in the
5
+ * LICENSE file in the root directory of this source tree.
6
+ *
7
+ * @emails react-core
8
+ */
9
+
10
+'use strict';
11
+
12
+module.exports = function(context) {
13
+ return {
14
+ Identifier(node) {
15
+ if (node.name === 'toWarnDev') {
16
+ let current = node;
17
+ while (current.parent) {
18
+ if (current.type === 'CallExpression') {
19
+ if (
20
+ current &&
21
+ current.callee &&
22
+ current.callee.property &&
23
+ current.callee.property.name === 'toThrow'
24
+ ) {
25
+ context.report(node, 'toWarnDev() matcher should not be nested');
26
+ }
27
+ }
28
+ current = current.parent;
29
+ }
30
+ }
31
+ },
32
+ };
33
+};
scripts/jest/matchers/__tests__/toWarnDev-test.js
+51
-58
@@ -54,166 +54,159 @@ describe('toWarnDev', () => {
54
});
55
56
if (__DEV__) {
57
+ // Helper methods avoids invalid toWarn().toThrow() nesting
58
+ // See no-to-warn-dev-within-to-throw
59
+ const expectToWarnAndToThrow = (expectBlock, expectedErrorMessage) => {
60
+ let caughtError;
61
+ try {
62
+ expectBlock();
63
+ } catch (error) {
64
+ caughtError = error;
65
+ }
66
+ expect(caughtError).toBeDefined();
67
+ expect(caughtError.message).toContain(expectedErrorMessage);
68
+ };
69
+
70
it('fails if a warning does not contain a stack', () => {
58
- expect(() => {
71
+ expectToWarnAndToThrow(() => {
72
expect(() => {
73
console.error('Hello');
74
}).toWarnDev('Hello');
62
- }).toThrow(
63
- 'Received warning unexpectedly does not include a component stack'
64
- );
75
+ }, 'Received warning unexpectedly does not include a component stack');
76
});
77
78
it('fails if some warnings do not contain a stack', () => {
68
- expect(() => {
79
+ expectToWarnAndToThrow(() => {
80
expect(() => {
81
console.error('Hello\n in div');
82
console.error('Good day\n in div');
83
console.error('Bye');
84
}).toWarnDev(['Hello', 'Good day', 'Bye']);
74
- }).toThrow(
75
- 'Received warning unexpectedly does not include a component stack'
76
- );
77
- expect(() => {
85
+ }, 'Received warning unexpectedly does not include a component stack');
86
+ expectToWarnAndToThrow(() => {
87
expect(() => {
88
console.error('Hello');
89
console.error('Good day\n in div');
90
console.error('Bye\n in div');
91
}).toWarnDev(['Hello', 'Good day', 'Bye']);
83
- }).toThrow(
84
- 'Received warning unexpectedly does not include a component stack'
85
- );
86
- expect(() => {
92
+ }, 'Received warning unexpectedly does not include a component stack');
93
+ expectToWarnAndToThrow(() => {
94
expect(() => {
95
console.error('Hello\n in div');
96
console.error('Good day');
97
console.error('Bye\n in div');
98
}).toWarnDev(['Hello', 'Good day', 'Bye']);
92
- }).toThrow(
93
- 'Received warning unexpectedly does not include a component stack'
94
- );
95
- expect(() => {
99
+ }, 'Received warning unexpectedly does not include a component stack');
100
+ expectToWarnAndToThrow(() => {
101
expect(() => {
102
console.error('Hello');
103
console.error('Good day');
104
console.error('Bye');
105
}).toWarnDev(['Hello', 'Good day', 'Bye']);
101
- }).toThrow(
102
- 'Received warning unexpectedly does not include a component stack'
103
- );
106
+ }, 'Received warning unexpectedly does not include a component stack');
107
});
108
109
it('fails if warning is expected to not have a stack, but does', () => {
107
- expect(() => {
110
+ expectToWarnAndToThrow(() => {
111
expect(() => {
112
console.error('Hello\n in div');
113
}).toWarnDev('Hello', {withoutStack: true});
111
- }).toThrow('Received warning unexpectedly includes a component stack');
112
- expect(() => {
114
+ }, 'Received warning unexpectedly includes a component stack');
115
+ expectToWarnAndToThrow(() => {
116
expect(() => {
117
console.error('Hello\n in div');
118
console.error('Good day');
119
console.error('Bye\n in div');
120
}).toWarnDev(['Hello', 'Good day', 'Bye'], {withoutStack: true});
118
- }).toThrow('Received warning unexpectedly includes a component stack');
121
+ }, 'Received warning unexpectedly includes a component stack');
122
});
123
124
it('fails if expected stack-less warning number does not match the actual one', () => {
122
- expect(() => {
125
+ expectToWarnAndToThrow(() => {
126
expect(() => {
127
console.error('Hello\n in div');
128
console.error('Good day');
129
console.error('Bye\n in div');
130
}).toWarnDev(['Hello', 'Good day', 'Bye'], {withoutStack: 4});
128
- }).toThrow(
129
- 'Expected 4 warnings without a component stack but received 1'
130
- );
131
+ }, 'Expected 4 warnings without a component stack but received 1');
132
});
133
134
it('fails if withoutStack is invalid', () => {
134
- expect(() => {
135
+ expectToWarnAndToThrow(() => {
136
expect(() => {
137
console.error('Hi');
138
}).toWarnDev('Hi', {withoutStack: null});
138
- }).toThrow('Instead received object');
139
- expect(() => {
139
+ }, 'Instead received object');
140
+ expectToWarnAndToThrow(() => {
141
expect(() => {
142
console.error('Hi');
143
}).toWarnDev('Hi', {withoutStack: {}});
143
- }).toThrow('Instead received object');
144
- expect(() => {
144
+ }, 'Instead received object');
145
+ expectToWarnAndToThrow(() => {
146
expect(() => {
147
console.error('Hi');
148
}).toWarnDev('Hi', {withoutStack: 'haha'});
148
- }).toThrow('Instead received string');
149
+ }, 'Instead received string');
150
});
151
152
it('fails if the argument number does not match', () => {
152
- expect(() => {
153
+ expectToWarnAndToThrow(() => {
154
expect(() => {
155
console.error('Hi %s', 'Sara', 'extra');
156
}).toWarnDev('Hi', {withoutStack: true});
156
- }).toThrow('Received 2 arguments for a message with 1 placeholders');
157
+ }, 'Received 2 arguments for a message with 1 placeholders');
158
158
- expect(() => {
159
+ expectToWarnAndToThrow(() => {
160
expect(() => {
161
console.error('Hi %s');
162
}).toWarnDev('Hi', {withoutStack: true});
162
- }).toThrow('Received 0 arguments for a message with 1 placeholders');
163
+ }, 'Received 0 arguments for a message with 1 placeholders');
164
});
165
166
it('fails if stack is passed twice', () => {
166
- expect(() => {
167
+ expectToWarnAndToThrow(() => {
168
expect(() => {
169
console.error('Hi %s%s', '\n in div', '\n in div');
170
}).toWarnDev('Hi');
170
- }).toThrow('Received more than one component stack for a warning');
171
+ }, 'Received more than one component stack for a warning');
172
});
173
174
it('fails if multiple strings are passed without an array wrapper', () => {
174
- expect(() => {
175
+ expectToWarnAndToThrow(() => {
176
expect(() => {
177
console.error('Hi \n in div');
178
}).toWarnDev('Hi', 'Bye');
178
- }).toThrow(
179
- 'toWarnDev() second argument, when present, should be an object'
180
- );
181
- expect(() => {
179
+ }, 'toWarnDev() second argument, when present, should be an object');
180
+ expectToWarnAndToThrow(() => {
181
expect(() => {
182
console.error('Hi \n in div');
183
console.error('Bye \n in div');
184
}).toWarnDev('Hi', 'Bye');
186
- }).toThrow(
187
- 'toWarnDev() second argument, when present, should be an object'
188
- );
189
- expect(() => {
185
+ }, 'toWarnDev() second argument, when present, should be an object');
186
+ expectToWarnAndToThrow(() => {
187
expect(() => {
188
console.error('Hi \n in div');
189
console.error('Wow \n in div');
190
console.error('Bye \n in div');
191
}).toWarnDev('Hi', 'Bye');
195
- }).toThrow(
196
- 'toWarnDev() second argument, when present, should be an object'
197
- );
198
- expect(() => {
192
+ }, 'toWarnDev() second argument, when present, should be an object');
193
+ expectToWarnAndToThrow(() => {
194
expect(() => {
195
console.error('Hi \n in div');
196
console.error('Wow \n in div');
197
console.error('Bye \n in div');
198
}).toWarnDev('Hi', 'Wow', 'Bye');
204
- }).toThrow(
205
- 'toWarnDev() second argument, when present, should be an object'
206
- );
199
+ }, 'toWarnDev() second argument, when present, should be an object');
200
});
201
202
it('fails on more than two arguments', () => {
210
- expect(() => {
203
+ expectToWarnAndToThrow(() => {
204
expect(() => {
205
console.error('Hi \n in div');
206
console.error('Wow \n in div');
207
console.error('Bye \n in div');
208
}).toWarnDev('Hi', undefined, 'Bye');
216
- }).toThrow('toWarnDev() received more than two arguments.');
209
+ }, 'toWarnDev() received more than two arguments.');
210
});
211
}
212
});