Skip to content

Commit 7c23e2e

Browse files
committed
#30a PR feedback: adding test case with partially set camera and solving it; test case refactor for multiple mocks, fewer globals and explicit async steps
1 parent 4975417 commit 7c23e2e

File tree

2 files changed

+48
-35
lines changed

2 files changed

+48
-35
lines changed

src/plots/gl3d/scene.js

+1-1
Original file line numberDiff line numberDiff line change
@@ -624,7 +624,7 @@ proto.saveCamera = function saveCamera(layout) {
624624
function same(x, y, i, j) {
625625
var vectors = ['up', 'center', 'eye'],
626626
components = ['x', 'y', 'z'];
627-
return x[vectors[i]][components[j]] === y[vectors[i]][components[j]];
627+
return y[vectors[i]] && (x[vectors[i]][components[j]] === y[vectors[i]][components[j]]);
628628
}
629629

630630
if(cameraDataLastSave === undefined) hasChanged = true;

test/jasmine/tests/gl_plot_interact_test.js

+47-34
Original file line numberDiff line numberDiff line change
@@ -213,52 +213,65 @@ describe('Test gl plot interactions', function() {
213213

214214
describe('gl3d plots', function() {
215215

216-
var mock = require('@mocks/gl3d_scatter3d-connectgaps.json'),
217-
relayoutCallback;
216+
// Expected shape of projection-related data
217+
var cameraStructure = {
218+
up: {x: jasmine.any(Number), y: jasmine.any(Number), z: jasmine.any(Number)},
219+
center: {x: jasmine.any(Number), y: jasmine.any(Number), z: jasmine.any(Number)},
220+
eye: {x: jasmine.any(Number), y: jasmine.any(Number), z: jasmine.any(Number)}
221+
};
222+
223+
function makePlot(mock) {
224+
return Plotly.plot(createGraphDiv(), mock.data, mock.layout);
225+
}
218226

219-
beforeEach(function(done) {
220-
gd = createGraphDiv();
227+
function addEventCallback(graphDiv) {
228+
var relayoutCallback = jasmine.createSpy('relayoutCallback');
229+
graphDiv.on('plotly_relayout', relayoutCallback);
230+
return {graphDiv: graphDiv, relayoutCallback: relayoutCallback};
231+
}
221232

222-
Plotly.plot(gd, mock.data, mock.layout).then(function() {
233+
function verifyInteractionEffects(tuple) {
223234

224-
relayoutCallback = jasmine.createSpy('relayoutCallback');
235+
// One 'drag': simulating fairly thoroughly as the mouseup event is also needed here
236+
mouseEvent('mousemove', 400, 200);
237+
mouseEvent('mousedown', 400, 200);
238+
mouseEvent('mousemove', 320, 320, {buttons: 1});
239+
mouseEvent('mouseup', 320, 320);
225240

226-
gd.on('plotly_relayout', relayoutCallback);
241+
// Check event emission count
242+
expect(tuple.relayoutCallback).toHaveBeenCalledTimes(1);
227243

228-
delay(done);
229-
});
230-
});
244+
// Check structure of event callback value contents
245+
expect(tuple.relayoutCallback).toHaveBeenCalledWith(jasmine.objectContaining({scene: cameraStructure}));
231246

232-
it('should respond to drag interactions', function(done) {
247+
// Check camera contents on the DIV layout
248+
var divCamera = tuple.graphDiv.layout.scene.camera;
233249

234-
// Expected shape of projection-related data
235-
var cameraStructure = {
236-
up: {x: jasmine.any(Number), y: jasmine.any(Number), z: jasmine.any(Number)},
237-
center: {x: jasmine.any(Number), y: jasmine.any(Number), z: jasmine.any(Number)},
238-
eye: {x: jasmine.any(Number), y: jasmine.any(Number), z: jasmine.any(Number)}
239-
};
250+
expect(divCamera).toEqual(cameraStructure);
240251

241-
setTimeout(function() {
242-
243-
// One 'drag': simulating fairly thoroughly as the mouseup event is also needed here
244-
mouseEvent('mousemove', 400, 200);
245-
mouseEvent('mousedown', 400, 200);
246-
mouseEvent('mousemove', 320, 320, {buttons: 1});
247-
mouseEvent('mouseup', 320, 320);
248-
249-
// Check event emission count
250-
expect(relayoutCallback).toHaveBeenCalledTimes(1);
252+
return tuple.graphDiv;
253+
}
251254

252-
// Check structure of event callback value contents
253-
expect(relayoutCallback).toHaveBeenCalledWith(jasmine.objectContaining({scene: cameraStructure}));
255+
function markForTeardown(graphDiv) {
256+
// TODO consider changing this test file such that the teardown needs no communication via a global variable
257+
gd = graphDiv;
258+
}
254259

255-
// Check camera contents on the DIV layout
256-
var divCamera = gd.layout.scene.camera;
257-
expect(divCamera).toEqual(cameraStructure);
260+
function testEvents(plot, done) {
261+
plot.then(function(graphDiv) {
262+
var tuple = addEventCallback(graphDiv);
263+
verifyInteractionEffects(tuple);
264+
markForTeardown(graphDiv);
265+
done();
266+
});
267+
}
258268

259-
delay(done);
269+
it('should respond to drag interactions with mock of unset camera', function(done) {
270+
testEvents(makePlot(require('@mocks/gl3d_scatter3d-connectgaps.json')), done);
271+
});
260272

261-
}, MODEBAR_DELAY);
273+
it('should respond to drag interactions with mock of partially set camera', function(done) {
274+
testEvents(makePlot(require('@mocks/gl3d_errorbars_zx.json')), done);
262275
});
263276
});
264277

0 commit comments

Comments
 (0)