Skip to content

Commit 3e18a98

Browse files
rreusserrreusser
committed
Clean up frame API tests and corner cases
1 parent 28be237 commit 3e18a98

File tree

4 files changed

+75
-65
lines changed

4 files changed

+75
-65
lines changed

Diff for: ‎src/plot_api/plot_api.js

+9-12
Original file line numberDiff line numberDiff line change
@@ -849,7 +849,7 @@ function doCalcdata(gd, traces) {
849849
// XXX: Is this correct? Needs a closer look so that *some* traces can be recomputed without
850850
// *all* needing doCalcdata:
851851
var calcdata = new Array(fullData.length);
852-
var oldCalcdata = gd.calcdata.slice(0);
852+
var oldCalcdata = (gd.calcdata || []).slice(0);
853853
gd.calcdata = calcdata;
854854

855855
// extra helper variables
@@ -2643,7 +2643,7 @@ Plotly.addFrames = function(gd, frameList, indices) {
26432643
for(i = frameList.length - 1; i >= 0; i--) {
26442644
insertions.push({
26452645
frame: frameList[i],
2646-
index: (indices && indices[i] !== undefined) ? indices[i] : bigIndex + i
2646+
index: (indices && indices[i] !== undefined && indices[i] !== null) ? indices[i] : bigIndex + i
26472647
});
26482648
}
26492649

@@ -2659,11 +2659,12 @@ Plotly.addFrames = function(gd, frameList, indices) {
26592659
var frameCount = _frames.length;
26602660

26612661
for(i = insertions.length - 1; i >= 0; i--) {
2662-
//frame = frameList[i];
26632662
frame = insertions[i].frame;
26642663

26652664
if(!frame.name) {
2666-
while(_frames[(frame.name = 'frame ' + gd._frameData._counter++)]);
2665+
// Repeatedly assign a default name, incrementing the counter each time until
2666+
// we get a name that's not in the hashed lookup table:
2667+
while(_hash[(frame.name = 'frame ' + gd._frameData._counter++)]);
26672668
}
26682669

26692670
if(_hash[frame.name]) {
@@ -2675,24 +2676,22 @@ Plotly.addFrames = function(gd, frameList, indices) {
26752676
revops.unshift({type: 'replace', index: j, value: _frames[j]});
26762677
} else {
26772678
// Otherwise insert it at the end of the list:
2678-
idx = Math.min(insertions[i].index, frameCount);
2679+
idx = Math.max(0, Math.min(insertions[i].index, frameCount));
26792680

26802681
ops.push({type: 'insert', index: idx, value: frame});
26812682
revops.unshift({type: 'delete', index: idx});
26822683
frameCount++;
26832684
}
26842685
}
26852686

2686-
Plots.modifyFrames(gd, ops);
2687-
26882687
var undoFunc = Plots.modifyFrames,
26892688
redoFunc = Plots.modifyFrames,
26902689
undoArgs = [gd, revops],
26912690
redoArgs = [gd, ops];
26922691

26932692
if(Queue) Queue.add(gd, undoFunc, undoArgs, redoFunc, redoArgs);
26942693

2695-
return Promise.resolve();
2694+
return Plots.modifyFrames(gd, ops);
26962695
};
26972696

26982697
/**
@@ -2709,7 +2708,7 @@ Plotly.deleteFrames = function(gd, frameList) {
27092708
var ops = [];
27102709
var revops = [];
27112710

2712-
frameList = frameList.splice(0);
2711+
frameList = frameList.slice(0);
27132712
frameList.sort();
27142713

27152714
for(i = frameList.length - 1; i >= 0; i--) {
@@ -2718,16 +2717,14 @@ Plotly.deleteFrames = function(gd, frameList) {
27182717
revops.unshift({type: 'insert', index: idx, value: _frames[idx]});
27192718
}
27202719

2721-
Plots.modifyFrames(gd, ops);
2722-
27232720
var undoFunc = Plots.modifyFrames,
27242721
redoFunc = Plots.modifyFrames,
27252722
undoArgs = [gd, revops],
27262723
redoArgs = [gd, ops];
27272724

27282725
if(Queue) Queue.add(gd, undoFunc, undoArgs, redoFunc, redoArgs);
27292726

2730-
return Promise.resolve();
2727+
return Plots.modifyFrames(gd, ops);
27312728
};
27322729

27332730
/**

Diff for: ‎src/plots/plots.js

+17-5
Original file line numberDiff line numberDiff line change
@@ -1137,22 +1137,34 @@ plots.graphJson = function(gd, dataonly, mode, output, useDefaults) {
11371137
* Sequence of operations to be performed on the keyframes
11381138
*/
11391139
plots.modifyFrames = function(gd, operations) {
1140-
var i, op;
1140+
var i, op, frame;
11411141
var _frames = gd._frameData._frames;
11421142
var _hash = gd._frameData._frameHash;
11431143

11441144
for(i = 0; i < operations.length; i++) {
11451145
op = operations[i];
11461146

11471147
switch(op.type) {
1148-
case 'rename':
1149-
var frame = _frames[op.index];
1148+
// No reason this couldn't exist, but is currently unused/untested:
1149+
/*case 'rename':
1150+
frame = _frames[op.index];
11501151
delete _hash[frame.name];
11511152
_hash[op.name] = frame;
1152-
break;
1153+
frame.name = op.name;
1154+
break;*/
11531155
case 'replace':
11541156
frame = op.value;
1155-
_frames[op.index] = _hash[frame.name] = frame;
1157+
var oldName = _frames[op.index].name;
1158+
var newName = frame.name;
1159+
_frames[op.index] = _hash[newName] = frame;
1160+
1161+
if(newName !== oldName) {
1162+
// If name has changed in addition to replacement, then update
1163+
// the lookup table:
1164+
delete _hash[oldName];
1165+
_hash[newName] = frame;
1166+
}
1167+
11561168
break;
11571169
case 'insert':
11581170
frame = op.value;

Diff for: ‎test/jasmine/assets/fail_test.js

+5-1
Original file line numberDiff line numberDiff line change
@@ -18,5 +18,9 @@
1818
* See ./with_setup_teardown.js for a different example.
1919
*/
2020
module.exports = function failTest(error) {
21-
expect(error).toBeUndefined();
21+
if(error === undefined) {
22+
expect(error).not.toBeUndefined();
23+
} else {
24+
expect(error).toBeUndefined();
25+
}
2226
};

Diff for: ‎test/jasmine/tests/frame_api_test.js

+44-47
Original file line numberDiff line numberDiff line change
@@ -2,15 +2,7 @@ var Plotly = require('@lib/index');
22

33
var createGraphDiv = require('../assets/create_graph_div');
44
var destroyGraphDiv = require('../assets/destroy_graph_div');
5-
6-
// A helper function so that failed tests don't simply stall:
7-
function fail(done) {
8-
return function(err) {
9-
console.error(err.toString());
10-
expect(err.toString()).toBe(true);
11-
done();
12-
};
13-
}
5+
var fail = require('../assets/fail_test');
146

157
describe('Test frame api', function() {
168
'use strict';
@@ -34,33 +26,39 @@ describe('Test frame api', function() {
3426
});
3527

3628
it('creates an empty lookup table for frames', function() {
37-
expect(gd._frameData._frameHash).toEqual({});
38-
});
39-
40-
it('initializes a name counter to zero', function() {
4129
expect(gd._frameData._counter).toEqual(0);
4230
});
4331
});
4432

45-
describe('addFrames', function() {
33+
describe('#addFrames', function() {
4634
it('names an unnamed frame', function(done) {
4735
Plotly.addFrames(gd, [{}]).then(function() {
4836
expect(Object.keys(h)).toEqual(['frame 0']);
49-
}).then(done, fail(done));
37+
}).catch(fail).then(done);
5038
});
5139

5240
it('creates multiple unnamed frames at the same time', function(done) {
5341
Plotly.addFrames(gd, [{}, {}]).then(function() {
5442
expect(f).toEqual([{name: 'frame 0'}, {name: 'frame 1'}]);
55-
}).then(done, fail(done));
43+
}).catch(fail).then(done);
5644
});
5745

5846
it('creates multiple unnamed frames in series', function(done) {
5947
Plotly.addFrames(gd, [{}]).then(
6048
Plotly.addFrames(gd, [{}])
6149
).then(function() {
6250
expect(f).toEqual([{name: 'frame 0'}, {name: 'frame 1'}]);
63-
}).then(done, fail(done));
51+
}).catch(fail).then(done);
52+
});
53+
54+
it('avoids name collisions', function(done) {
55+
Plotly.addFrames(gd, [{name: 'frame 0'}, {name: 'frame 2'}]).then(function() {
56+
expect(f).toEqual([{name: 'frame 0'}, {name: 'frame 2'}]);
57+
58+
return Plotly.addFrames(gd, [{}, {name: 'foobar'}, {}]);
59+
}).then(function() {
60+
expect(f).toEqual([{name: 'frame 0'}, {name: 'frame 2'}, {name: 'frame 1'}, {name: 'foobar'}, {name: 'frame 3'}]);
61+
}).catch(fail).then(done);
6462
});
6563

6664
it('inserts frames at specific indices', function(done) {
@@ -70,19 +68,21 @@ describe('Test frame api', function() {
7068
frames.push({name: 'frame' + i});
7169
}
7270

73-
Plotly.addFrames(gd, frames).then(function() {
71+
function validate() {
72+
for(i = 0; i < 10; i++) {
73+
expect(f[i]).toEqual({name: 'frame' + i});
74+
}
75+
}
76+
77+
Plotly.addFrames(gd, frames).then(validate).then(function() {
7478
return Plotly.addFrames(gd, [{name: 'inserted1'}, {name: 'inserted2'}, {name: 'inserted3'}], [5, 7, undefined]);
7579
}).then(function() {
7680
expect(f[5]).toEqual({name: 'inserted1'});
7781
expect(f[7]).toEqual({name: 'inserted2'});
7882
expect(f[12]).toEqual({name: 'inserted3'});
7983

8084
return Plotly.Queue.undo(gd);
81-
}).then(function() {
82-
for(i = 0; i < 10; i++) {
83-
expect(f[i]).toEqual({name: 'frame' + i});
84-
}
85-
}).then(done, fail(done));
85+
}).then(validate).catch(fail).then(done);
8686
});
8787

8888
it('inserts frames at specific indices (reversed)', function(done) {
@@ -92,36 +92,37 @@ describe('Test frame api', function() {
9292
frames.push({name: 'frame' + i});
9393
}
9494

95-
Plotly.addFrames(gd, frames).then(function() {
95+
function validate() {
96+
for(i = 0; i < 10; i++) {
97+
expect(f[i]).toEqual({name: 'frame' + i});
98+
}
99+
}
100+
101+
Plotly.addFrames(gd, frames).then(validate).then(function() {
96102
return Plotly.addFrames(gd, [{name: 'inserted3'}, {name: 'inserted2'}, {name: 'inserted1'}], [undefined, 7, 5]);
97103
}).then(function() {
98104
expect(f[5]).toEqual({name: 'inserted1'});
99105
expect(f[7]).toEqual({name: 'inserted2'});
100106
expect(f[12]).toEqual({name: 'inserted3'});
101107

102108
return Plotly.Queue.undo(gd);
103-
}).then(function() {
104-
for(i = 0; i < 10; i++) {
105-
expect(f[i]).toEqual({name: 'frame' + i});
106-
}
107-
}).then(done, fail(done));
109+
}).then(validate).catch(fail).then(done);
108110
});
109111

110112
it('implements undo/redo', function(done) {
111-
Plotly.addFrames(gd, [{name: 'frame 0'}, {name: 'frame 1'}]).then(function() {
113+
function validate() {
112114
expect(f).toEqual([{name: 'frame 0'}, {name: 'frame 1'}]);
113115
expect(h).toEqual({'frame 0': {name: 'frame 0'}, 'frame 1': {name: 'frame 1'}});
116+
}
114117

118+
Plotly.addFrames(gd, [{name: 'frame 0'}, {name: 'frame 1'}]).then(validate).then(function() {
115119
return Plotly.Queue.undo(gd);
116120
}).then(function() {
117121
expect(f).toEqual([]);
118122
expect(h).toEqual({});
119123

120124
return Plotly.Queue.redo(gd);
121-
}).then(function() {
122-
expect(f).toEqual([{name: 'frame 0'}, {name: 'frame 1'}]);
123-
expect(h).toEqual({'frame 0': {name: 'frame 0'}, 'frame 1': {name: 'frame 1'}});
124-
}).then(done, fail(done));
125+
}).then(validate).catch(fail).then(done);
125126
});
126127

127128
it('overwrites frames', function(done) {
@@ -144,11 +145,11 @@ describe('Test frame api', function() {
144145
}).then(function() {
145146
expect(f).toEqual([{name: 'test1'}, {name: 'test2'}, {name: 'test3'}]);
146147
expect(Object.keys(h)).toEqual(['test1', 'test2', 'test3']);
147-
}).then(done, fail(done));
148+
}).catch(fail).then(done);
148149
});
149150
});
150151

151-
describe('deleteFrames', function() {
152+
describe('#deleteFrames', function() {
152153
it('deletes a frame', function(done) {
153154
Plotly.addFrames(gd, [{name: 'frame1'}]).then(function() {
154155
expect(f).toEqual([{name: 'frame1'}]);
@@ -167,7 +168,7 @@ describe('Test frame api', function() {
167168
}).then(function() {
168169
expect(f).toEqual([]);
169170
expect(Object.keys(h)).toEqual([]);
170-
}).then(done, fail(done));
171+
}).catch(fail).then(done);
171172
});
172173

173174
it('deletes multiple frames', function(done) {
@@ -177,29 +178,25 @@ describe('Test frame api', function() {
177178
frames.push({name: 'frame' + i});
178179
}
179180

180-
Plotly.addFrames(gd, frames).then(function() {
181-
return Plotly.deleteFrames(gd, [2, 8, 4, 6]);
182-
}).then(function() {
181+
function validate() {
183182
var expected = ['frame0', 'frame1', 'frame3', 'frame5', 'frame7', 'frame9'];
184183
expect(f.length).toEqual(expected.length);
185184
for(i = 0; i < expected.length; i++) {
186185
expect(f[i].name).toEqual(expected[i]);
187186
}
187+
}
188188

189+
Plotly.addFrames(gd, frames).then(function() {
190+
return Plotly.deleteFrames(gd, [2, 8, 4, 6]);
191+
}).then(validate).then(function() {
189192
return Plotly.Queue.undo(gd);
190193
}).then(function() {
191194
for(i = 0; i < 10; i++) {
192195
expect(f[i]).toEqual({name: 'frame' + i});
193196
}
194197

195198
return Plotly.Queue.redo(gd);
196-
}).then(function() {
197-
var expected = ['frame0', 'frame1', 'frame3', 'frame5', 'frame7', 'frame9'];
198-
expect(f.length).toEqual(expected.length);
199-
for(i = 0; i < expected.length; i++) {
200-
expect(f[i].name).toEqual(expected[i]);
201-
}
202-
}).then(done, fail(done));
199+
}).then(validate).catch(fail).then(done);
203200
});
204201
});
205202
});

0 commit comments

Comments
 (0)