From 3408b1ec365e0dec100e300451fe13598e275da1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89tienne=20T=C3=A9treault-Pinard?= Date: Tue, 29 Nov 2016 18:11:57 -0500 Subject: [PATCH 1/9] mapbox: don't try to emit 'plotly_relayout' after map.remove() - see http://codepen.io/etpinard/pen/ZBabVW for explanation - this removes the 'gd.emit is not a function' failures on 'npm run citest-jasmine' --- src/plots/mapbox/mapbox.js | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/plots/mapbox/mapbox.js b/src/plots/mapbox/mapbox.js index 0a7edcfdb38..a645fd97351 100644 --- a/src/plots/mapbox/mapbox.js +++ b/src/plots/mapbox/mapbox.js @@ -126,6 +126,8 @@ proto.createMap = function(calcData, fullLayout, resolve, reject) { // keep track of pan / zoom in user layout and emit relayout event map.on('moveend', function(eventData) { + if(!self.map) return; + var view = self.getView(); opts._input.center = opts.center = view.center; @@ -358,7 +360,10 @@ proto.updateLayers = function() { }; proto.destroy = function() { - if(this.map) this.map.remove(); + if(this.map) { + this.map.remove(); + this.map = null; + } this.container.removeChild(this.div); }; From 51f873ec93aaab5c9ef1bd28d13d1fc6e54b4499 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89tienne=20T=C3=A9treault-Pinard?= Date: Wed, 30 Nov 2016 12:26:46 -0500 Subject: [PATCH 2/9] lint --- test/jasmine/tests/mapbox_test.js | 48 +++++++++++++++++--------- test/jasmine/tests/updatemenus_test.js | 7 ++-- 2 files changed, 35 insertions(+), 20 deletions(-) diff --git a/test/jasmine/tests/mapbox_test.js b/test/jasmine/tests/mapbox_test.js index 34eafbc05dd..32f9b3f1d35 100644 --- a/test/jasmine/tests/mapbox_test.js +++ b/test/jasmine/tests/mapbox_test.js @@ -215,10 +215,12 @@ describe('mapbox credentials', function() { lat: [10, 20, 30] }], {}, { mapboxAccessToken: dummyToken - }).catch(function(err) { + }) + .catch(function(err) { cnt++; expect(err).toEqual(new Error(constants.mapOnErrorMsg)); - }).then(function() { + }) + .then(function() { expect(cnt).toEqual(1); done(); }); @@ -263,10 +265,12 @@ describe('mapbox credentials', function() { } }, { mapboxAccessToken: '' - }).catch(function(err) { + }) + .catch(function(err) { cnt++; expect(err).toEqual(new Error(msg)); - }).then(function() { + }) + .then(function() { expect(cnt).toEqual(1); done(); }); @@ -310,22 +314,26 @@ describe('mapbox plots', function() { expect(gd._fullLayout.mapbox).toBeUndefined(); return Plotly.restyle(gd, 'visible', true); - }).then(function() { + }) + .then(function() { expect(countVisibleTraces(gd, modes)).toEqual(2); return Plotly.restyle(gd, 'visible', 'legendonly', [1]); - }).then(function() { + }) + .then(function() { expect(countVisibleTraces(gd, modes)).toEqual(1); return Plotly.restyle(gd, 'visible', true); - }).then(function() { + }) + .then(function() { expect(countVisibleTraces(gd, modes)).toEqual(2); var mockCopy = Lib.extendDeep({}, mock); mockCopy.data[0].visible = false; return Plotly.newPlot(gd, mockCopy.data, mockCopy.layout); - }).then(function() { + }) + .then(function() { expect(countVisibleTraces(gd, modes)).toEqual(1); done(); @@ -348,7 +356,8 @@ describe('mapbox plots', function() { }; return Plotly.addTraces(gd, [trace]); - }).then(function() { + }) + .then(function() { expect(countVisibleTraces(gd, modes)).toEqual(2); var trace = { @@ -359,11 +368,13 @@ describe('mapbox plots', function() { }; return Plotly.addTraces(gd, [trace]); - }).then(function() { + }) + .then(function() { expect(countVisibleTraces(gd, modes)).toEqual(3); return Plotly.deleteTraces(gd, [0, 1, 2]); - }).then(function() { + }) + .then(function() { expect(gd._fullLayout.mapbox).toBeUndefined(); done(); @@ -462,28 +473,32 @@ describe('mapbox plots', function() { assertLayout('Mapbox Dark', [0, 0], 1.234, [80, 100, 908, 270]); return Plotly.relayout(gd, 'mapbox.zoom', '6'); - }).then(function() { + }) + .then(function() { expect(restyleCnt).toEqual(0); expect(relayoutCnt).toEqual(2); assertLayout('Mapbox Dark', [0, 0], 6, [80, 100, 908, 270]); return Plotly.relayout(gd, 'mapbox.style', 'light'); - }).then(function() { + }) + .then(function() { expect(restyleCnt).toEqual(0); expect(relayoutCnt).toEqual(3); assertLayout('Mapbox Light', [0, 0], 6, [80, 100, 908, 270]); return Plotly.relayout(gd, 'mapbox.domain.x', [0, 0.5]); - }).then(function() { + }) + .then(function() { expect(restyleCnt).toEqual(0); expect(relayoutCnt).toEqual(4); assertLayout('Mapbox Light', [0, 0], 6, [80, 100, 454, 270]); return Plotly.relayout(gd, 'mapbox.domain.y[0]', 0.5); - }).then(function() { + }) + .then(function() { expect(restyleCnt).toEqual(0); expect(relayoutCnt).toEqual(5); @@ -669,7 +684,8 @@ describe('mapbox plots', function() { }; return Plotly.extendTraces(gd, update, [0, 1]); - }).then(function() { + }) + .then(function() { assertDataPts([5, 5]); done(); diff --git a/test/jasmine/tests/updatemenus_test.js b/test/jasmine/tests/updatemenus_test.js index c415b0ccbd3..cf632adff10 100644 --- a/test/jasmine/tests/updatemenus_test.js +++ b/test/jasmine/tests/updatemenus_test.js @@ -225,8 +225,8 @@ describe('update menus buttons', function() { mockCopy.layout.updatemenus[1].x = 1; allMenus = mockCopy.layout.updatemenus; - buttonMenus = allMenus.filter(function(opts) {return opts.type === 'buttons';}); - dropdownMenus = allMenus.filter(function(opts) {return opts.type !== 'buttons';}); + buttonMenus = allMenus.filter(function(opts) { return opts.type === 'buttons'; }); + dropdownMenus = allMenus.filter(function(opts) { return opts.type !== 'buttons'; }); Plotly.plot(gd, mockCopy.data, mockCopy.layout).then(done); }); @@ -244,12 +244,11 @@ describe('update menus buttons', function() { // Count the *total* number of buttons we expect for this mock: var buttonCount = 0; - buttonMenus.forEach(function(menu) {buttonCount += menu.buttons.length;}); + buttonMenus.forEach(function(menu) { buttonCount += menu.buttons.length; }); assertNodeCount('.' + constants.buttonClassName, buttonCount); done(); - }); function assertNodeCount(query, cnt) { From b43cba4691dd0a79fdf37f6c1787ba992e0d10ae Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89tienne=20T=C3=A9treault-Pinard?= Date: Wed, 30 Nov 2016 12:27:33 -0500 Subject: [PATCH 3/9] pass TIMEOUT_INTERVAL to mapbox cases - see https://github.com/jasmine/jasmine/commit/68ba5b6d48624c063b4eeae1bdd63c5aa84a3177 --- test/jasmine/tests/mapbox_test.js | 31 ++++++++++++++++--------------- 1 file changed, 16 insertions(+), 15 deletions(-) diff --git a/test/jasmine/tests/mapbox_test.js b/test/jasmine/tests/mapbox_test.js index 32f9b3f1d35..b9146242e3c 100644 --- a/test/jasmine/tests/mapbox_test.js +++ b/test/jasmine/tests/mapbox_test.js @@ -14,6 +14,7 @@ var customMatchers = require('../assets/custom_matchers'); var MAPBOX_ACCESS_TOKEN = require('@build/credentials.json').MAPBOX_ACCESS_TOKEN; var TRANSITION_DELAY = 500; var MOUSE_DELAY = 100; +var LONG_TIMEOUT_INTERVAL = 5 * jasmine.DEFAULT_TIMEOUT_INTERVAL; var noop = function() {}; @@ -204,7 +205,7 @@ describe('mapbox credentials', function() { lat: [10, 20, 30] }]); }).toThrow(new Error(constants.noAccessTokenErrorMsg)); - }); + }, LONG_TIMEOUT_INTERVAL); it('should throw error if token is invalid', function(done) { var cnt = 0; @@ -224,7 +225,7 @@ describe('mapbox credentials', function() { expect(cnt).toEqual(1); done(); }); - }); + }, LONG_TIMEOUT_INTERVAL); it('should use access token in mapbox layout options if present', function(done) { var cnt = 0; @@ -246,7 +247,7 @@ describe('mapbox credentials', function() { expect(gd._fullLayout.mapbox.accesstoken).toEqual(MAPBOX_ACCESS_TOKEN); done(); }); - }); + }, LONG_TIMEOUT_INTERVAL); it('should bypass access token in mapbox layout options when config points to an Atlas server', function(done) { var cnt = 0; @@ -274,7 +275,7 @@ describe('mapbox credentials', function() { expect(cnt).toEqual(1); done(); }); - }); + }, LONG_TIMEOUT_INTERVAL); }); describe('mapbox plots', function() { @@ -338,7 +339,7 @@ describe('mapbox plots', function() { done(); }); - }); + }, LONG_TIMEOUT_INTERVAL); it('should be able to delete and add traces', function(done) { var modes = ['line', 'circle']; @@ -379,7 +380,7 @@ describe('mapbox plots', function() { done(); }); - }); + }, LONG_TIMEOUT_INTERVAL); it('should be able to restyle', function(done) { var restyleCnt = 0, @@ -436,7 +437,7 @@ describe('mapbox plots', function() { ]); }) .then(done); - }); + }, LONG_TIMEOUT_INTERVAL); it('should be able to relayout', function(done) { var restyleCnt = 0, @@ -506,7 +507,7 @@ describe('mapbox plots', function() { done(); }); - }); + }, LONG_TIMEOUT_INTERVAL); it('should be able to add, update and remove layers', function(done) { var mockWithLayers = require('@mocks/mapbox_layers'); @@ -638,7 +639,7 @@ describe('mapbox plots', function() { done(); }); - }); + }, LONG_TIMEOUT_INTERVAL); it('should be able to update the access token', function(done) { Plotly.relayout(gd, 'mapbox.accesstoken', 'wont-work').catch(function(err) { @@ -652,7 +653,7 @@ describe('mapbox plots', function() { expect(gd._promises.length).toEqual(0); done(); }); - }); + }, LONG_TIMEOUT_INTERVAL); it('should be able to update traces', function(done) { function assertDataPts(lengths) { @@ -690,7 +691,7 @@ describe('mapbox plots', function() { done(); }); - }); + }, LONG_TIMEOUT_INTERVAL); it('should display to hover labels on mouse over', function(done) { function assertMouseMove(pos, len) { @@ -704,7 +705,7 @@ describe('mapbox plots', function() { assertMouseMove(blankPos, 0).then(function() { return assertMouseMove(pointPos, 1); }).then(done); - }); + }, LONG_TIMEOUT_INTERVAL); it('should respond to hover interactions by', function(done) { var hoverCnt = 0, @@ -752,7 +753,7 @@ describe('mapbox plots', function() { done(); }); - }); + }, LONG_TIMEOUT_INTERVAL); it('should respond drag / scroll interactions', function(done) { var relayoutCnt = 0, @@ -811,7 +812,7 @@ describe('mapbox plots', function() { // TODO test scroll - }); + }, LONG_TIMEOUT_INTERVAL); it('should respond to click interactions by', function(done) { var ptData; @@ -844,7 +845,7 @@ describe('mapbox plots', function() { }); }) .then(done); - }); + }, LONG_TIMEOUT_INTERVAL); function getMapInfo(gd) { var subplot = gd._fullLayout.mapbox._subplot, From 8433627cb0288b62e44724f9bb08bf216638d9f0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89tienne=20T=C3=A9treault-Pinard?= Date: Wed, 30 Nov 2016 12:28:24 -0500 Subject: [PATCH 4/9] replace beforeEach-afterEach TIMEOUT_INTERVAL hack by it-arg - see https://github.com/jasmine/jasmine/commit/68ba5b6d48624c063b4eeae1bdd63c5aa84a3177 --- test/jasmine/tests/download_test.js | 18 ++++++------------ 1 file changed, 6 insertions(+), 12 deletions(-) diff --git a/test/jasmine/tests/download_test.js b/test/jasmine/tests/download_test.js index 1e29234a4dc..bcb17a6c354 100644 --- a/test/jasmine/tests/download_test.js +++ b/test/jasmine/tests/download_test.js @@ -3,10 +3,11 @@ var createGraphDiv = require('../assets/create_graph_div'); var destroyGraphDiv = require('../assets/destroy_graph_div'); var textchartMock = require('@mocks/text_chart_arrays.json'); +var LONG_TIMEOUT_INTERVAL = 2 * jasmine.DEFAULT_TIMEOUT_INTERVAL; + describe('Plotly.downloadImage', function() { 'use strict'; var gd; - var originalTimeout; // override click handler on createElement // so these tests will not actually @@ -27,16 +28,10 @@ describe('Plotly.downloadImage', function() { beforeEach(function() { gd = createGraphDiv(); - - // downloadImage can take a little longer - // so give it a little more time to finish - originalTimeout = jasmine.DEFAULT_TIMEOUT_INTERVAL; - jasmine.DEFAULT_TIMEOUT_INTERVAL = 10000; }); afterEach(function() { destroyGraphDiv(); - jasmine.DEFAULT_TIMEOUT_INTERVAL = originalTimeout; }); it('should be attached to Plotly', function() { @@ -45,11 +40,11 @@ describe('Plotly.downloadImage', function() { it('should create link, remove link, accept options', function(done) { downloadTest(gd, 'jpeg', done); - }); + }, LONG_TIMEOUT_INTERVAL); it('should create link, remove link, accept options', function(done) { downloadTest(gd, 'png', done); - }); + }, LONG_TIMEOUT_INTERVAL); it('should create link, remove link, accept options', function(done) { checkWebp(function(supported) { @@ -59,12 +54,11 @@ describe('Plotly.downloadImage', function() { done(); } }); - - }); + }, LONG_TIMEOUT_INTERVAL); it('should create link, remove link, accept options', function(done) { downloadTest(gd, 'svg', done); - }); + }, LONG_TIMEOUT_INTERVAL); }); From 133d71bb719e3feb2637485eb6e5123bd7b927bc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89tienne=20T=C3=A9treault-Pinard?= Date: Wed, 30 Nov 2016 12:28:55 -0500 Subject: [PATCH 5/9] bump event max listeners to remove console warnings - in update menu cases --- test/jasmine/tests/updatemenus_test.js | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/test/jasmine/tests/updatemenus_test.js b/test/jasmine/tests/updatemenus_test.js index cf632adff10..8864efcb59f 100644 --- a/test/jasmine/tests/updatemenus_test.js +++ b/test/jasmine/tests/updatemenus_test.js @@ -4,6 +4,7 @@ var constants = require('@src/components/updatemenus/constants'); var d3 = require('d3'); var Plotly = require('@lib'); var Lib = require('@src/lib'); +var Events = require('@src/lib/events'); var createGraphDiv = require('../assets/create_graph_div'); var destroyGraphDiv = require('../assets/destroy_graph_div'); var TRANSITION_DELAY = 100; @@ -220,6 +221,10 @@ describe('update menus buttons', function() { beforeEach(function(done) { gd = createGraphDiv(); + // bump event max listeners to remove console warnings + Events.init(gd); + gd._internalEv.setMaxListeners(20); + // move update menu #2 to click on them separately var mockCopy = Lib.extendDeep({}, mock); mockCopy.layout.updatemenus[1].x = 1; From 86e0303c252b9783295bccd3f71c1a01a4b300ee Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89tienne=20T=C3=A9treault-Pinard?= Date: Wed, 30 Nov 2016 12:29:38 -0500 Subject: [PATCH 6/9] set window size when running jasmine tests in FF --- test/jasmine/karma.ciconf.js | 2 +- test/jasmine/karma.conf.js | 6 +++++- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/test/jasmine/karma.ciconf.js b/test/jasmine/karma.ciconf.js index 6ba65949842..7b782e60cea 100644 --- a/test/jasmine/karma.ciconf.js +++ b/test/jasmine/karma.ciconf.js @@ -33,7 +33,7 @@ function func(config) { func.defaultConfig.autoWatch = false; - func.defaultConfig.browsers = ['Firefox']; + func.defaultConfig.browsers = ['Firefox_WindowSized']; config.set(func.defaultConfig); } diff --git a/test/jasmine/karma.conf.js b/test/jasmine/karma.conf.js index 4af5b9df14f..28f046bc961 100644 --- a/test/jasmine/karma.conf.js +++ b/test/jasmine/karma.conf.js @@ -82,11 +82,15 @@ func.defaultConfig = { browsers: ['Chrome_WindowSized'], // custom browser options + // window-size values came from observing default size customLaunchers: { Chrome_WindowSized: { base: 'Chrome', - // window-size values came from observing default size flags: ['--window-size=1035,617', '--ignore-gpu-blacklist'] + }, + Firefox_WindowSized: { + base: 'Firefox', + flags: ['--width=1035', '--height=617'] } }, From 60573f6b008c8e26b06ccff08580a20748fa6616 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89tienne=20T=C3=A9treault-Pinard?= Date: Wed, 30 Nov 2016 12:30:40 -0500 Subject: [PATCH 7/9] adjust hover label test to work in both Chrome and FF --- test/jasmine/tests/hover_label_test.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/test/jasmine/tests/hover_label_test.js b/test/jasmine/tests/hover_label_test.js index e89d92f8a43..618551cdaa1 100644 --- a/test/jasmine/tests/hover_label_test.js +++ b/test/jasmine/tests/hover_label_test.js @@ -726,9 +726,9 @@ describe('hover on fill', function() { mock.data.forEach(function(trace) { trace.hoveron = 'fills'; }); Plotly.plot(createGraphDiv(), mock.data, mock.layout).then(function() { - return assertLabelsCorrect([242, 142], [249.175, 133.8], 'trace 2'); + return assertLabelsCorrect([242, 142], [252, 133.8], 'trace 2'); }).then(function() { - return assertLabelsCorrect([242, 292], [231.125, 210], 'trace 1'); + return assertLabelsCorrect([242, 292], [233, 210], 'trace 1'); }).then(function() { return assertLabelsCorrect([147, 252], [158.925, 248.1], 'trace 0'); }).then(done); @@ -749,7 +749,7 @@ describe('hover on fill', function() { }).then(function() { return assertLabelsCorrect([237, 218], [266.75, 265], 'trace 1'); }).then(function() { - return assertLabelsCorrect([237, 251], [247.7, 254], 'trace 0'); + return assertLabelsCorrect([237, 240], [247.7, 254], 'trace 0'); }).then(function() { // zoom in to test clipping of large out-of-viewport shapes return Plotly.relayout(gd, { From 6886efbbbc37cf47cfd5b0a327eef90fc92db245 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89tienne=20T=C3=A9treault-Pinard?= Date: Thu, 1 Dec 2016 12:11:54 -0500 Subject: [PATCH 8/9] fix typo in heatmapgl attributes --- src/traces/heatmapgl/attributes.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/traces/heatmapgl/attributes.js b/src/traces/heatmapgl/attributes.js index 23617d4f332..d2f8fc0f8c3 100644 --- a/src/traces/heatmapgl/attributes.js +++ b/src/traces/heatmapgl/attributes.js @@ -9,7 +9,7 @@ 'use strict'; -var heatmapAttrs = require('../scatter/attributes'); +var heatmapAttrs = require('../heatmap/attributes'); var colorscaleAttrs = require('../../components/colorscale/attributes'); var colorbarAttrs = require('../../components/colorbar/attributes'); From caea87ad2c06e5cfe0fca76aad02386b721efeac Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89tienne=20T=C3=A9treault-Pinard?= Date: Thu, 1 Dec 2016 18:18:17 -0500 Subject: [PATCH 9/9] bump tolerance (for Ricky's computer) --- test/jasmine/tests/updatemenus_test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/jasmine/tests/updatemenus_test.js b/test/jasmine/tests/updatemenus_test.js index 8864efcb59f..ef2128feaa2 100644 --- a/test/jasmine/tests/updatemenus_test.js +++ b/test/jasmine/tests/updatemenus_test.js @@ -698,7 +698,7 @@ describe('update menus interactions', function() { // must compare with a tolerance as the exact result // is browser/font dependent (via getBBox) - expect(Math.abs(actualWidth - width)).toBeLessThan(11); + expect(Math.abs(actualWidth - width)).toBeLessThan(12); // height is determined by 'fontsize', // so no such tolerance is needed