Update tests to not rely on key iteration order
Andrew Clark committed
Dec 13, 2016 at 20:09 UTC
496512f8b8cb81cbeae1a4ffc7a53f515b5b72d7
2 files changed
+72
-37
src/renderers/shared/fiber/ReactFiberScheduler.js
+2
-1
@@ -390,7 +390,8 @@ module.exports = function<T, P, I, TI, C, CX>(config : HostConfig<T, P, I, TI, C
390
function resetWorkPriority(workInProgress : Fiber) {
391
let newPriority = NoWork;
392
393
- // Check for pending update priority
393
+ // Check for pending update priority. This is usually null so it shouldn't
394
+ // be a perf issue.
395
const queue = workInProgress.updateQueue;
396
if (queue) {
397
newPriority = getPendingPriority(queue);
src/renderers/shared/fiber/__tests__/ReactIncrementalUpdates-test.js
+70
-36
@@ -21,15 +21,6 @@ describe('ReactIncrementalUpdates', () => {
21
ReactNoop = require('ReactNoop');
22
});
23
24
- function div(...children) {
25
- children = children.map(c => typeof c === 'string' ? { text: c } : c);
26
- return { type: 'div', children, prop: undefined };
27
- }
28
-
29
- function span(prop) {
30
- return { type: 'span', children: [], prop };
31
- }
32
-
24
it('applies updates in order of priority', () => {
25
let state;
26
class Foo extends React.Component {
@@ -50,8 +41,10 @@ describe('ReactIncrementalUpdates', () => {
41
}
42
43
ReactNoop.render(<Foo />);
44
+ ReactNoop.flushDeferredPri(25);
45
+ expect(state).toEqual({ a: 'a' });
46
ReactNoop.flush();
54
- expect(Object.keys(state)).toEqual(['a', 'b', 'c']);
47
+ expect(state).toEqual({ a: 'a', b: 'b', c: 'c' });
48
});
49
50
it('applies updates with equal priority in insertion order', () => {
@@ -72,20 +65,26 @@ describe('ReactIncrementalUpdates', () => {
65
66
ReactNoop.render(<Foo />);
67
ReactNoop.flush();
75
- expect(Object.keys(state)).toEqual(['a', 'b', 'c']);
68
+ expect(state).toEqual({ a: 'a', b: 'b', c: 'c' });
69
});
70
71
it('only drops updates with equal or lesser priority when replaceState is called', () => {
72
let instance;
73
+ let ops = [];
74
const Foo = React.createClass({
75
getInitialState() {
76
return {};
77
},
78
+ componentDidMount() {
79
+ ops.push('componentDidMount');
80
+ },
81
+ componentDidUpdate() {
82
+ ops.push('componentDidUpdate');
83
+ },
84
render() {
85
+ ops.push('render');
86
instance = this;
86
- return (
87
- <span prop={Object.keys(this.state).join('')} />
88
- );
87
+ return <div />;
88
},
89
});
90
@@ -104,28 +103,45 @@ describe('ReactIncrementalUpdates', () => {
103
ReactNoop.flushAnimationPri();
104
// Even though a replaceState has been already scheduled, it hasn't been
105
// flushed yet because it has low priority.
107
- expect(ReactNoop.getChildren()).toEqual([span('ab')]);
106
+ expect(instance.state).toEqual({ a: 'a', b: 'b' });
107
+ expect(ops).toEqual([
108
+ 'render',
109
+ 'componentDidMount',
110
+ 'render',
111
+ 'componentDidUpdate',
112
+ ]);
113
+
114
+ ops = [];
115
116
ReactNoop.flush();
117
// Now the rest of the updates are flushed.
111
- expect(ReactNoop.getChildren()).toEqual([span('cd')]);
118
+ expect(instance.state).toEqual({ c: 'c', d: 'd' });
119
+ expect(ops).toEqual([
120
+ 'render',
121
+ 'componentDidUpdate',
122
+ ]);
123
});
124
125
it('can abort an update, schedule additional updates, and resume', () => {
126
let instance;
127
+ let ops = [];
128
class Foo extends React.Component {
129
state = {};
130
+ componentDidUpdate() {
131
+ ops.push('componentDidUpdate');
132
+ }
133
render() {
134
+ ops.push('render');
135
instance = this;
120
- return (
121
- <span prop={Object.keys(this.state).join('')} />
122
- );
136
+ return <div />;
137
}
138
}
139
140
ReactNoop.render(<Foo />);
141
ReactNoop.flush();
142
143
+ ops = [];
144
+
145
let progressedUpdates = [];
146
function createUpdate(letter) {
147
return () => {
@@ -141,9 +157,14 @@ describe('ReactIncrementalUpdates', () => {
157
instance.setState(createUpdate('c'));
158
159
// Do just enough work to begin the update but not enough to flush it
144
- ReactNoop.flushDeferredPri(20);
145
- expect(ReactNoop.getChildren()).toEqual([span('')]);
160
+ ReactNoop.flushDeferredPri(15);
161
+ // expect(ReactNoop.getChildren()).toEqual([span('')]);
162
+ expect(ops).toEqual(['render']);
163
expect(progressedUpdates).toEqual(['a', 'b', 'c']);
164
+ expect(instance.state).toEqual({ a: 'a', b: 'b', c: 'c' });
165
+
166
+ ops = [];
167
+ progressedUpdates = [];
168
169
instance.setState(createUpdate('f'));
170
ReactNoop.performAnimationWork(() => {
@@ -152,21 +173,35 @@ describe('ReactIncrementalUpdates', () => {
173
});
174
instance.setState(createUpdate('g'));
175
155
- // Updates a, b, and c were aborted, so they should be applied first even
156
- // though they have low priority. Update f was scheduled after the render
157
- // was aborted, so it should come after d and e, which have higher priority.
176
+ ReactNoop.flushAnimationPri();
177
+ expect(ops).toEqual([
178
+ // Flushes animation work (d and e)
179
+ 'render',
180
+ 'componentDidUpdate',
181
+ ]);
182
+ ops = [];
183
ReactNoop.flush();
159
- expect(ReactNoop.getChildren()).toEqual([span('abcdefg')]);
160
- expect(progressedUpdates).toEqual(['a', 'b', 'c', 'd', 'e', 'f', 'g']);
184
+ expect(ops).toEqual([
185
+ // Flushes deferred work (f and g)
186
+ 'render',
187
+ 'componentDidUpdate',
188
+ ]);
189
+ expect(progressedUpdates).toEqual(['d', 'e', 'a', 'b', 'c', 'f', 'g']);
190
+ expect(instance.state).toEqual({ a: 'a', b: 'b', c: 'c', d: 'd', e: 'e', f: 'f', g: 'g' });
191
});
192
193
it('can abort an update, schedule a replaceState, and resume', () => {
194
let instance;
195
+ let ops = [];
196
const Foo = React.createClass({
197
getInitialState() {
198
return {};
199
},
200
+ componentDidUpdate() {
201
+ ops.push('componentDidUpdate');
202
+ },
203
render() {
204
+ ops.push('render');
205
instance = this;
206
return (
207
<span prop={Object.keys(this.state).join('')} />
@@ -176,6 +211,7 @@ describe('ReactIncrementalUpdates', () => {
211
212
ReactNoop.render(<Foo />);
213
ReactNoop.flush();
214
+ ops = [];
215
216
let progressedUpdates = [];
217
function createUpdate(letter) {
@@ -193,9 +229,11 @@ describe('ReactIncrementalUpdates', () => {
229
230
// Do just enough work to begin the update but not enough to flush it
231
ReactNoop.flushDeferredPri(20);
196
- expect(ReactNoop.getChildren()).toEqual([span('')]);
232
+ expect(ops).toEqual(['render']);
233
expect(progressedUpdates).toEqual(['a', 'b', 'c']);
234
+ expect(instance.state).toEqual({ a: 'a', b: 'b', c: 'c' });
235
236
+ ops = [];
237
progressedUpdates = [];
238
239
instance.setState(createUpdate('f'));
@@ -205,14 +243,10 @@ describe('ReactIncrementalUpdates', () => {
243
});
244
instance.setState(createUpdate('g'));
245
208
- // Updates a, b, and c were aborted, so they should be applied first even
209
- // though they have low priority. Update f was scheduled after the render
210
- // was aborted, so it should come after d and e, which have higher priority.
211
- // Because e is a replaceState, d gets dropped.
246
ReactNoop.flush();
213
- expect(ReactNoop.getChildren()).toEqual([span('efg')]);
247
// Ensure that updater function d is never called.
248
expect(progressedUpdates).toEqual(['e', 'f', 'g']);
249
+ expect(instance.state).toEqual({ e: 'e', f: 'f', g: 'g' });
250
});
251
252
it('does not call callbacks that are scheduled by another callback until a later commit', () => {
@@ -259,7 +293,7 @@ describe('ReactIncrementalUpdates', () => {
293
render() {
294
ops.push('render');
295
instance = this;
262
- return <span prop={Object.keys(this.state).join('')} />;
296
+ return <div />;
297
}
298
}
299
ReactNoop.render(<Foo />);
@@ -273,7 +307,7 @@ describe('ReactIncrementalUpdates', () => {
307
});
308
ReactNoop.flush();
309
276
- expect(ReactNoop.getChildren()).toEqual([span('ab')]);
310
+ expect(instance.state).toEqual({ a: 'a', b: 'b' });
311
expect(ops).toEqual([
312
'componentWillReceiveProps',
313
'render',
@@ -290,7 +324,7 @@ describe('ReactIncrementalUpdates', () => {
324
render() {
325
ops.push('render');
326
instance = this;
293
- return <span prop={Object.keys(this.state).join('')} />;
327
+ return <div />;
328
}
329
}
330
@@ -304,7 +338,6 @@ describe('ReactIncrementalUpdates', () => {
338
});
339
340
ReactNoop.flush();
307
- expect(ReactNoop.getChildren()).toEqual([span('ab')]);
341
expect(ops).toEqual([
342
// Initial render
343
'render',
@@ -313,6 +346,7 @@ describe('ReactIncrementalUpdates', () => {
346
// be flushed in the same commit.
347
'render',
348
]);
349
+ expect(instance.state).toEqual({ a: 'a', b: 'b' });
350
351
expectDev(console.error.calls.count()).toBe(1);
352
console.error.calls.reset();