Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Label flaky tests and add early returns in setTimeout functions to bypass undefined fullLayouts #5482

Merged
merged 7 commits into from
Feb 9, 2021
Merged

Label flaky tests and add early returns in setTimeout functions to bypass undefined fullLayouts #5482

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
bb351c8
refactor loop over describe in transition test
archmoj Feb 8, 2021
bc903cc
make one descibe block and use transition duration in test descriptions
archmoj Feb 8, 2021
832ebc2
add flaky tags
archmoj Feb 8, 2021
27c0ac4
early return with void _subplots e.g. afterAll in transition test
archmoj Feb 9, 2021
3c6ec3a
add missing gl tag to splom select test
archmoj Feb 9, 2021
a5f2087
add early return to functions called by setTimeout when gd._fullLayou…
archmoj Feb 9, 2021
85ad874
add three more early returns for destroyed plot
archmoj Feb 9, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions src/plots/cartesian/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,7 @@ exports.finalizeSubplots = function(layoutIn, layoutOut) {
*/
exports.plot = function(gd, traces, transitionOpts, makeOnCompleteCallback) {
var fullLayout = gd._fullLayout;
if(!fullLayout._subplots) return;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's this about? When do we encounter this?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sometimes happen at the end of transition_test with an error similar to the image below:
undefinedSubplot

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Simply for reference:
https://app.circleci.com/pipelines/github/plotly/plotly.js/2974/workflows/734c1977-517c-43b8-88bf-9fe83e3c5e55/jobs/103632

var subplots = fullLayout._subplots.cartesian;
var calcdata = gd.calcdata;
var i;
Expand Down
4 changes: 2 additions & 2 deletions test/jasmine/tests/animate_test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -750,7 +750,7 @@ describe('Animating multiple axes', function() {
.then(done, done.fail);
});

it('updates ranges of secondary axes (date + category case)', function(done) {
it('@flaky updates ranges of secondary axes (date + category case)', function(done) {
Plotly.newPlot(gd, [
{x: ['2018-01-01', '2019-01-01', '2020-01-01'], y: [1, 2, 3]},
{x: ['a', 'b', 'c'], y: [1, 2, 3], xaxis: 'x2', yaxis: 'y2'}
Expand Down Expand Up @@ -950,7 +950,7 @@ describe('animating scatter traces', function() {
}).then(done, done.fail);
});

it('should animate axis ranges using the less number of steps', function(done) {
it('@flaky should animate axis ranges using the less number of steps', function(done) {
// sanity-check that scatter points and bars are still there
function _assertNodeCnt() {
var gd3 = d3Select(gd);
Expand Down
10 changes: 5 additions & 5 deletions test/jasmine/tests/cartesian_interact_test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -960,7 +960,7 @@ describe('axis zoom/pan and main plot zoom', function() {
var yr0 = [-0.211, 3.211];

var specs = [{
desc: '@flaky zoombox on xy',
desc: 'zoombox on xy',
drag: ['xy', 'nsew', 30, 30],
exp: [
[['xaxis', 'xaxis2', 'xaxis3'], [1.457, 2.328]],
Expand Down Expand Up @@ -1094,7 +1094,7 @@ describe('axis zoom/pan and main plot zoom', function() {
var msg = 'after ' + s.desc;
var msg2 = ['after dblclick on subplot', s.dblclickSubplot, msg].join(' ');

it(s.desc, function(done) {
it('@flaky ' + s.desc, function(done) {
makePlot(data, layout, s).then(function() {
assertRanges('base', [
[['xaxis', 'xaxis2', 'xaxis3'], xr0],
Expand Down Expand Up @@ -1194,7 +1194,7 @@ describe('axis zoom/pan and main plot zoom', function() {
specs.forEach(function(s) {
var msg = 'after ' + s.desc;

it(s.desc, function(done) {
it('@flaky ' + s.desc, function(done) {
makePlot(data, layout, s).then(function() {
assertRanges('base', [
[['xaxis'], xr0],
Expand Down Expand Up @@ -1331,7 +1331,7 @@ describe('axis zoom/pan and main plot zoom', function() {
specs.forEach(function(s) {
var msg = 'after ' + s.desc;

it(s.desc, function(done) {
it('@flaky ' + s.desc, function(done) {
makePlot(data, layout, s).then(function() {
assertRanges('base', [
[['yaxis', 'xaxis2'], rm0],
Expand Down Expand Up @@ -1412,7 +1412,7 @@ describe('axis zoom/pan and main plot zoom', function() {
var msg2 = ['after dblclick on subplot', s.dblclickSubplot, msg].join(' ');
var spmatch = s.dblclickSubplot.match(constants.SUBPLOT_PATTERN);

it(s.desc, function(done) {
it('@flaky ' + s.desc, function(done) {
makePlot(data, layout, s).then(function() {
assertRanges('base', [
[['xaxis', 'yaxis'], rng0.xy],
Expand Down
4 changes: 2 additions & 2 deletions test/jasmine/tests/select_test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1420,7 +1420,7 @@ describe('Test select box and lasso in general:', function() {
.then(done, done.fail);
});

it('should cleanly clear and restart selections on double click when add/subtract mode on', function(done) {
it('@flaky should cleanly clear and restart selections on double click when add/subtract mode on', function(done) {
var gd = createGraphDiv();
var fig = Lib.extendDeep({}, require('@mocks/0.json'));

Expand Down Expand Up @@ -2456,7 +2456,7 @@ describe('Test select box and lasso per trace:', function() {
});

[false].forEach(function(hasCssTransform) {
it('should work for bar traces, hasCssTransform: ' + hasCssTransform, function(done) {
it('@flaky should work for bar traces, hasCssTransform: ' + hasCssTransform, function(done) {
var assertPoints = makeAssertPoints(['curveNumber', 'x', 'y']);
var assertSelectedPoints = makeAssertSelectedPoints();
var assertRanges = makeAssertRanges();
Expand Down
2 changes: 1 addition & 1 deletion test/jasmine/tests/splom_test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1825,7 +1825,7 @@ describe('Test splom select:', function() {
.then(done, done.fail);
});

it('should be able to select and then clear using API', function(done) {
it('@gl should be able to select and then clear using API', function(done) {
function _assert(msg, exp) {
return function() {
var uid = gd._fullData[0].uid;
Expand Down
60 changes: 28 additions & 32 deletions test/jasmine/tests/transition_test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,26 +12,31 @@ var destroyGraphDiv = require('../assets/destroy_graph_div');
var delay = require('../assets/delay');
var mock = require('@mocks/animation');

function runTests(transitionDuration) {
describe('Plots.transition (duration = ' + transitionDuration + ')', function() {
'use strict';
describe('Plots.transition', function() {
'use strict';

var gd;
var gd;

beforeEach(function(done) {
gd = createGraphDiv();
beforeEach(function(done) {
gd = createGraphDiv();

var mockCopy = Lib.extendDeep({}, mock);
var mockCopy = Lib.extendDeep({}, mock);

Plotly.newPlot(gd, mockCopy.data, mockCopy.layout).then(done);
});
Plotly.newPlot(gd, mockCopy.data, mockCopy.layout).then(done);
});

afterEach(function() {
Plotly.purge(gd);
destroyGraphDiv();
});
afterEach(function() {
Plotly.purge(gd);
destroyGraphDiv();
});

it('resolves only once the transition has completed', function(done) {
// Run the whole set of tests twice: once with zero duration and once with
// nonzero duration since the behavior should be identical, but there's a
// very real possibility of race conditions or other timing issues.
//
// And of course, remember to put the async loop in a closure:
[0, 20].forEach(function(transitionDuration) {
it('with duration:' + transitionDuration + ', resolves only once the transition has completed', function(done) {
var t1 = Date.now();
var traces = plotApiHelpers.coerceTraceIndices(gd, null);

Expand All @@ -42,7 +47,7 @@ function runTests(transitionDuration) {
}).then(done, done.fail);
});

it('emits plotly_transitioning on transition start', function(done) {
it('with duration:' + transitionDuration + ', emits plotly_transitioning on transition start', function(done) {
var beginTransitionCnt = 0;
var traces = plotApiHelpers.coerceTraceIndices(gd, null);

Expand All @@ -55,7 +60,7 @@ function runTests(transitionDuration) {
}).then(done, done.fail);
});

it('emits plotly_transitioned on transition end', function(done) {
it('with duration:' + transitionDuration + ', emits plotly_transitioned on transition end', function(done) {
var trEndCnt = 0;
var traces = plotApiHelpers.coerceTraceIndices(gd, null);

Expand All @@ -68,7 +73,7 @@ function runTests(transitionDuration) {
}).then(done, done.fail);
});

it('transitions an annotation', function(done) {
it('with duration:' + transitionDuration + ', transitions an annotation', function(done) {
function annotationPosition() {
var g = gd._fullLayout._infolayer.select('.annotation').select('.annotation-text-g');
var bBox = g.node().getBoundingClientRect();
Expand Down Expand Up @@ -96,7 +101,7 @@ function runTests(transitionDuration) {
}).then(done, done.fail);
});

it('transitions an image', function(done) {
it('with duration:' + transitionDuration + ', transitions an image', function(done) {
var jsLogo = 'https://images.plot.ly/language-icons/api-home/js-logo.png';
var pythonLogo = 'https://images.plot.ly/language-icons/api-home/python-logo.png';

Expand Down Expand Up @@ -131,7 +136,7 @@ function runTests(transitionDuration) {
}).then(done, done.fail);
});

it('transitions a shape', function(done) {
it('with duration:' + transitionDuration + ', transitions a shape', function(done) {
function getPath() {
return gd._fullLayout._shapeUpperLayer.select('path').node();
}
Expand Down Expand Up @@ -189,7 +194,7 @@ function runTests(transitionDuration) {
});


it('transitions a transform', function(done) {
it('with duration:' + transitionDuration + ', transitions a transform', function(done) {
Plotly.restyle(gd, {
'transforms[0]': {
enabled: true,
Expand Down Expand Up @@ -226,7 +231,7 @@ function runTests(transitionDuration) {

// This doesn't really test anything that the above tests don't cover, but it combines
// the behavior and attempts to ensure chaining and events happen in the correct order.
it('transitions may be chained', function(done) {
it('with duration:' + transitionDuration + ', transitions may be chained', function(done) {
var currentlyRunning = 0;
var beginCnt = 0;
var endCnt = 0;
Expand Down Expand Up @@ -258,17 +263,8 @@ function runTests(transitionDuration) {
.then(done, done.fail);
});
});
}
});

for(var i = 0; i < 2; i++) {
var duration = i * 20;
// Run the whole set of tests twice: once with zero duration and once with
// nonzero duration since the behavior should be identical, but there's a
// very real possibility of race conditions or other timing issues.
//
// And of course, remember to put the async loop in a closure:
runTests(duration);
}

describe('Plotly.react transitions:', function() {
var gd;
Expand Down Expand Up @@ -844,7 +840,7 @@ describe('Plotly.react transitions:', function() {
.then(done, done.fail);
});

it('should not transition layout when axis auto-ranged value do not changed', function(done) {
it('@flaky should not transition layout when axis auto-ranged value do not changed', function(done) {
var data = [{y: [1, 2, 1]}];
var layout = {transition: {duration: 10}};

Expand Down