Skip to content

Commit 5cff9a1

Browse files
gaearonEthan-Arrowood
authored andcommitted
Run Jest in production mode (facebook#11616)
* Move Jest setup files to /dev/ subdirectory * Clone Jest /dev/ files into /prod/ * Move shared code into scripts/jest * Move Jest config into the scripts folder * Fix the equivalence test It fails because the config is now passed to Jest explicitly. But the test doesn't know about the config. To fix this, we just run it via `yarn test` (which includes the config). We already depend on Yarn for development anyway. * Add yarn test-prod to run Jest with production environment * Actually flip the production tests to run in prod environment This produces a bunch of errors: Test Suites: 64 failed, 58 passed, 122 total Tests: 740 failed, 26 skipped, 1809 passed, 2575 total Snapshots: 16 failed, 4 passed, 20 total * Ignore expectDev() calls in production Down from 740 to 175 failed. Test Suites: 44 failed, 78 passed, 122 total Tests: 175 failed, 26 skipped, 2374 passed, 2575 total Snapshots: 16 failed, 4 passed, 20 total * Decode errors so tests can assert on their messages Down from 175 to 129. Test Suites: 33 failed, 89 passed, 122 total Tests: 129 failed, 1029 skipped, 1417 passed, 2575 total Snapshots: 16 failed, 4 passed, 20 total * Remove ReactDOMProduction-test There is no need for it now. The only test that was special is moved into ReactDOM-test. * Remove production switches from ReactErrorUtils The tests now run in production in a separate pass. * Add and use spyOnDev() for warnings This ensures that by default we expect no warnings in production bundles. If the warning *is* expected, use the regular spyOn() method. This currently breaks all expectDev() assertions without __DEV__ blocks so we go back to: Test Suites: 56 failed, 65 passed, 121 total Tests: 379 failed, 1029 skipped, 1148 passed, 2556 total Snapshots: 16 failed, 4 passed, 20 total * Replace expectDev() with expect() in __DEV__ blocks We started using spyOnDev() for console warnings to ensure we don't *expect* them to occur in production. As a consequence, expectDev() assertions on console.error.calls fail because console.error.calls doesn't exist. This is actually good because it would help catch accidental warnings in production. To solve this, we are getting rid of expectDev() altogether, and instead introduce explicit expectation branches. We'd need them anyway for testing intentional behavior differences. This commit replaces all expectDev() calls with expect() calls in __DEV__ blocks. It also removes a few unnecessary expect() checks that no warnings were produced (by also removing the corresponding spyOnDev() calls). Some DEV-only assertions used plain expect(). Those were also moved into __DEV__ blocks. ReactFiberErrorLogger was special because it console.error()'s in production too. So in that case I intentionally used spyOn() instead of spyOnDev(), and added extra assertions. This gets us down to: Test Suites: 21 failed, 100 passed, 121 total Tests: 72 failed, 26 skipped, 2458 passed, 2556 total Snapshots: 16 failed, 4 passed, 20 total * Enable User Timing API for production testing We could've disabled it, but seems like a good idea to test since we use it at FB. * Test for explicit Object.freeze() differences between PROD and DEV This is one of the few places where DEV and PROD behavior differs for performance reasons. Now we explicitly test both branches. * Run Jest via "yarn test" on CI * Remove unused variable * Assert different error messages * Fix error handling tests This logic is really complicated because of the global ReactFiberErrorLogger mock. I understand it now, so I added TODOs for later. It can be much simpler if we change the rest of the tests that assert uncaught errors to also assert they are logged as warnings. Which mirrors what happens in practice anyway. * Fix more assertions * Change tests to document the DEV/PROD difference for state invariant It is very likely unintentional but I don't want to change behavior in this PR. Filed a follow up as facebook#11618. * Remove unnecessary split between DEV/PROD ref tests * Fix more test message assertions * Make validateDOMNesting tests DEV-only * Fix error message assertions * Document existing DEV/PROD message difference (possible bug) * Change mocking assertions to be DEV-only * Fix the error code test * Fix more error message assertions * Fix the last failing test due to known issue * Run production tests on CI * Unify configuration * Fix coverage script * Remove expectDev from eslintrc * Run everything in band We used to before, too. I just forgot to add the arguments after deleting the script.
1 parent 87fda37 commit 5cff9a1

71 files changed

Lines changed: 3707 additions & 3223 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

.eslintrc.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,6 @@ module.exports = {
5959
},
6060

6161
globals: {
62-
expectDev: true,
62+
spyOnDev: true,
6363
},
6464
};

package.json

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,7 @@
4545
"core-js": "^2.2.1",
4646
"coveralls": "^2.11.6",
4747
"create-react-class": "^15.6.2",
48+
"cross-env": "^5.1.1",
4849
"del": "^2.0.2",
4950
"derequire": "^2.0.3",
5051
"escape-string-regexp": "^1.0.5",
@@ -102,7 +103,8 @@
102103
"linc": "node ./scripts/tasks/linc.js",
103104
"lint": "node ./scripts/tasks/eslint.js",
104105
"postinstall": "node node_modules/fbjs-scripts/node/check-dev-engines.js package.json",
105-
"test": "jest",
106+
"test": "cross-env NODE_ENV=development jest",
107+
"test-prod": "cross-env NODE_ENV=production jest",
106108
"flow": "node ./scripts/tasks/flow.js",
107109
"prettier": "node ./scripts/prettier/index.js write-changed",
108110
"prettier-all": "node ./scripts/prettier/index.js write",

packages/react-dom/src/__tests__/CSSPropertyOperations-test.js

Lines changed: 74 additions & 62 deletions
Original file line numberDiff line numberDiff line change
@@ -91,15 +91,17 @@ describe('CSSPropertyOperations', () => {
9191
}
9292
}
9393

94-
spyOn(console, 'error');
94+
spyOnDev(console, 'error');
9595
var root = document.createElement('div');
9696
ReactDOM.render(<Comp />, root);
97-
expectDev(console.error.calls.count()).toBe(1);
98-
expectDev(normalizeCodeLocInfo(console.error.calls.argsFor(0)[0])).toEqual(
99-
'Warning: Unsupported style property background-color. Did you mean backgroundColor?' +
100-
'\n in div (at **)' +
101-
'\n in Comp (at **)',
102-
);
97+
if (__DEV__) {
98+
expect(console.error.calls.count()).toBe(1);
99+
expect(normalizeCodeLocInfo(console.error.calls.argsFor(0)[0])).toEqual(
100+
'Warning: Unsupported style property background-color. Did you mean backgroundColor?' +
101+
'\n in div (at **)' +
102+
'\n in Comp (at **)',
103+
);
104+
}
103105
});
104106

105107
it('should warn when updating hyphenated style names', () => {
@@ -111,7 +113,7 @@ describe('CSSPropertyOperations', () => {
111113
}
112114
}
113115

114-
spyOn(console, 'error');
116+
spyOnDev(console, 'error');
115117
var styles = {
116118
'-ms-transform': 'translate3d(0, 0, 0)',
117119
'-webkit-transform': 'translate3d(0, 0, 0)',
@@ -120,17 +122,19 @@ describe('CSSPropertyOperations', () => {
120122
ReactDOM.render(<Comp />, root);
121123
ReactDOM.render(<Comp style={styles} />, root);
122124

123-
expectDev(console.error.calls.count()).toBe(2);
124-
expectDev(normalizeCodeLocInfo(console.error.calls.argsFor(0)[0])).toEqual(
125-
'Warning: Unsupported style property -ms-transform. Did you mean msTransform?' +
126-
'\n in div (at **)' +
127-
'\n in Comp (at **)',
128-
);
129-
expectDev(normalizeCodeLocInfo(console.error.calls.argsFor(1)[0])).toEqual(
130-
'Warning: Unsupported style property -webkit-transform. Did you mean WebkitTransform?' +
131-
'\n in div (at **)' +
132-
'\n in Comp (at **)',
133-
);
125+
if (__DEV__) {
126+
expect(console.error.calls.count()).toBe(2);
127+
expect(normalizeCodeLocInfo(console.error.calls.argsFor(0)[0])).toEqual(
128+
'Warning: Unsupported style property -ms-transform. Did you mean msTransform?' +
129+
'\n in div (at **)' +
130+
'\n in Comp (at **)',
131+
);
132+
expect(normalizeCodeLocInfo(console.error.calls.argsFor(1)[0])).toEqual(
133+
'Warning: Unsupported style property -webkit-transform. Did you mean WebkitTransform?' +
134+
'\n in div (at **)' +
135+
'\n in Comp (at **)',
136+
);
137+
}
134138
});
135139

136140
it('warns when miscapitalizing vendored style names', () => {
@@ -150,23 +154,25 @@ describe('CSSPropertyOperations', () => {
150154
}
151155
}
152156

153-
spyOn(console, 'error');
157+
spyOnDev(console, 'error');
154158
var root = document.createElement('div');
155159
ReactDOM.render(<Comp />, root);
156-
// msTransform is correct already and shouldn't warn
157-
expectDev(console.error.calls.count()).toBe(2);
158-
expectDev(normalizeCodeLocInfo(console.error.calls.argsFor(0)[0])).toEqual(
159-
'Warning: Unsupported vendor-prefixed style property oTransform. ' +
160-
'Did you mean OTransform?' +
161-
'\n in div (at **)' +
162-
'\n in Comp (at **)',
163-
);
164-
expectDev(normalizeCodeLocInfo(console.error.calls.argsFor(1)[0])).toEqual(
165-
'Warning: Unsupported vendor-prefixed style property webkitTransform. ' +
166-
'Did you mean WebkitTransform?' +
167-
'\n in div (at **)' +
168-
'\n in Comp (at **)',
169-
);
160+
if (__DEV__) {
161+
// msTransform is correct already and shouldn't warn
162+
expect(console.error.calls.count()).toBe(2);
163+
expect(normalizeCodeLocInfo(console.error.calls.argsFor(0)[0])).toEqual(
164+
'Warning: Unsupported vendor-prefixed style property oTransform. ' +
165+
'Did you mean OTransform?' +
166+
'\n in div (at **)' +
167+
'\n in Comp (at **)',
168+
);
169+
expect(normalizeCodeLocInfo(console.error.calls.argsFor(1)[0])).toEqual(
170+
'Warning: Unsupported vendor-prefixed style property webkitTransform. ' +
171+
'Did you mean WebkitTransform?' +
172+
'\n in div (at **)' +
173+
'\n in Comp (at **)',
174+
);
175+
}
170176
});
171177

172178
it('should warn about style having a trailing semicolon', () => {
@@ -187,22 +193,24 @@ describe('CSSPropertyOperations', () => {
187193
}
188194
}
189195

190-
spyOn(console, 'error');
196+
spyOnDev(console, 'error');
191197
var root = document.createElement('div');
192198
ReactDOM.render(<Comp />, root);
193-
expectDev(console.error.calls.count()).toBe(2);
194-
expectDev(normalizeCodeLocInfo(console.error.calls.argsFor(0)[0])).toEqual(
195-
"Warning: Style property values shouldn't contain a semicolon. " +
196-
'Try "backgroundColor: blue" instead.' +
197-
'\n in div (at **)' +
198-
'\n in Comp (at **)',
199-
);
200-
expectDev(normalizeCodeLocInfo(console.error.calls.argsFor(1)[0])).toEqual(
201-
"Warning: Style property values shouldn't contain a semicolon. " +
202-
'Try "color: red" instead.' +
203-
'\n in div (at **)' +
204-
'\n in Comp (at **)',
205-
);
199+
if (__DEV__) {
200+
expect(console.error.calls.count()).toBe(2);
201+
expect(normalizeCodeLocInfo(console.error.calls.argsFor(0)[0])).toEqual(
202+
"Warning: Style property values shouldn't contain a semicolon. " +
203+
'Try "backgroundColor: blue" instead.' +
204+
'\n in div (at **)' +
205+
'\n in Comp (at **)',
206+
);
207+
expect(normalizeCodeLocInfo(console.error.calls.argsFor(1)[0])).toEqual(
208+
"Warning: Style property values shouldn't contain a semicolon. " +
209+
'Try "color: red" instead.' +
210+
'\n in div (at **)' +
211+
'\n in Comp (at **)',
212+
);
213+
}
206214
});
207215

208216
it('should warn about style containing a NaN value', () => {
@@ -214,16 +222,18 @@ describe('CSSPropertyOperations', () => {
214222
}
215223
}
216224

217-
spyOn(console, 'error');
225+
spyOnDev(console, 'error');
218226
var root = document.createElement('div');
219227
ReactDOM.render(<Comp />, root);
220228

221-
expectDev(console.error.calls.count()).toBe(1);
222-
expectDev(normalizeCodeLocInfo(console.error.calls.argsFor(0)[0])).toEqual(
223-
'Warning: `NaN` is an invalid value for the `fontSize` css style property.' +
224-
'\n in div (at **)' +
225-
'\n in Comp (at **)',
226-
);
229+
if (__DEV__) {
230+
expect(console.error.calls.count()).toBe(1);
231+
expect(normalizeCodeLocInfo(console.error.calls.argsFor(0)[0])).toEqual(
232+
'Warning: `NaN` is an invalid value for the `fontSize` css style property.' +
233+
'\n in div (at **)' +
234+
'\n in Comp (at **)',
235+
);
236+
}
227237
});
228238

229239
it('should not warn when setting CSS custom properties', () => {
@@ -246,16 +256,18 @@ describe('CSSPropertyOperations', () => {
246256
}
247257
}
248258

249-
spyOn(console, 'error');
259+
spyOnDev(console, 'error');
250260
var root = document.createElement('div');
251261
ReactDOM.render(<Comp />, root);
252262

253-
expectDev(console.error.calls.count()).toBe(1);
254-
expectDev(normalizeCodeLocInfo(console.error.calls.argsFor(0)[0])).toEqual(
255-
'Warning: `Infinity` is an invalid value for the `fontSize` css style property.' +
256-
'\n in div (at **)' +
257-
'\n in Comp (at **)',
258-
);
263+
if (__DEV__) {
264+
expect(console.error.calls.count()).toBe(1);
265+
expect(normalizeCodeLocInfo(console.error.calls.argsFor(0)[0])).toEqual(
266+
'Warning: `Infinity` is an invalid value for the `fontSize` css style property.' +
267+
'\n in div (at **)' +
268+
'\n in Comp (at **)',
269+
);
270+
}
259271
});
260272

261273
it('should not add units to CSS custom properties', () => {

packages/react-dom/src/__tests__/DOMPropertyOperations-test.js

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -151,7 +151,7 @@ describe('DOMPropertyOperations', () => {
151151

152152
it('should not remove attributes for special properties', () => {
153153
var container = document.createElement('div');
154-
spyOn(console, 'error');
154+
spyOnDev(console, 'error');
155155
ReactDOM.render(
156156
<input type="text" value="foo" onChange={function() {}} />,
157157
container,
@@ -164,10 +164,12 @@ describe('DOMPropertyOperations', () => {
164164
);
165165
expect(container.firstChild.getAttribute('value')).toBe('foo');
166166
expect(container.firstChild.value).toBe('foo');
167-
expect(console.error.calls.count()).toBe(1);
168-
expect(console.error.calls.argsFor(0)[0]).toContain(
169-
'A component is changing a controlled input of type text to be uncontrolled',
170-
);
167+
if (__DEV__) {
168+
expect(console.error.calls.count()).toBe(1);
169+
expect(console.error.calls.argsFor(0)[0]).toContain(
170+
'A component is changing a controlled input of type text to be uncontrolled',
171+
);
172+
}
171173
});
172174
});
173175
});

packages/react-dom/src/__tests__/EventPluginHub-test.js

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ describe('EventPluginHub', () => {
2222
});
2323

2424
it('should prevent non-function listeners, at dispatch', () => {
25-
spyOn(console, 'error');
25+
spyOnDev(console, 'error');
2626
var node = ReactTestUtils.renderIntoDocument(
2727
<div onClick="not a function" />,
2828
);
@@ -31,10 +31,12 @@ describe('EventPluginHub', () => {
3131
}).toThrowError(
3232
'Expected `onClick` listener to be a function, instead got a value of `string` type.',
3333
);
34-
expectDev(console.error.calls.count()).toBe(1);
35-
expectDev(console.error.calls.argsFor(0)[0]).toContain(
36-
'Expected `onClick` listener to be a function, instead got a value of `string` type.',
37-
);
34+
if (__DEV__) {
35+
expect(console.error.calls.count()).toBe(1);
36+
expect(console.error.calls.argsFor(0)[0]).toContain(
37+
'Expected `onClick` listener to be a function, instead got a value of `string` type.',
38+
);
39+
}
3840
});
3941

4042
it('should not prevent null listeners, at dispatch', () => {

packages/react-dom/src/__tests__/ReactBrowserEventEmitter-test.js

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -285,13 +285,15 @@ describe('ReactBrowserEventEmitter', () => {
285285
putListener(CHILD, ON_CLICK_KEY, recordIDAndReturnFalse.bind(null, CHILD));
286286
putListener(PARENT, ON_CLICK_KEY, recordID.bind(null, PARENT));
287287
putListener(GRANDPARENT, ON_CLICK_KEY, recordID.bind(null, GRANDPARENT));
288-
spyOn(console, 'error');
288+
spyOnDev(console, 'error');
289289
ReactTestUtils.Simulate.click(CHILD);
290290
expect(idCallOrder.length).toBe(3);
291291
expect(idCallOrder[0]).toBe(CHILD);
292292
expect(idCallOrder[1]).toBe(PARENT);
293293
expect(idCallOrder[2]).toBe(GRANDPARENT);
294-
expectDev(console.error.calls.count()).toEqual(0);
294+
if (__DEV__) {
295+
expect(console.error.calls.count()).toEqual(0);
296+
}
295297
});
296298

297299
/**

0 commit comments

Comments
 (0)