Skip to content

Return and handle promises in tests #5340

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

Merged
merged 42 commits into from
Jan 4, 2021
Merged

Return and handle promises in tests #5340

Show file tree
Hide file tree
Changes from 25 commits
Commits
Show all changes
42 commits
Select commit Hold shift + click to select a range
f4ae59c
return promise for Plotly.relayout in tests
archmoj Dec 11, 2020
67d0973
move function outside for loop
archmoj Dec 11, 2020
7b73542
return promise for Plotly.react in tests
archmoj Dec 11, 2020
6bcaa90
return promise for Plotly.plot and Plotly.newPlot in tests
archmoj Dec 11, 2020
10689cc
return promise for Plotly.restyle in tests
archmoj Dec 11, 2020
49c1cfd
fixup axis drag handles tests - call done after assert
archmoj Dec 11, 2020
0bd2a8e
fixup layout_image tests
archmoj Dec 11, 2020
ce5320a
fixup hover label tests
archmoj Dec 11, 2020
db8563e
fixup various tests - add catch blocks
archmoj Dec 11, 2020
acc6707
fixup titles tests
archmoj Dec 11, 2020
efee80b
catch more missing catch tests
archmoj Dec 11, 2020
97574f9
fixup annotations_test
archmoj Dec 14, 2020
a4285e5
do not catch failTest in beforeEach blocks
archmoj Dec 14, 2020
bc1bce4
drop some noCI flags
archmoj Dec 14, 2020
1a69837
reduce max auto retry from 5 to 1
archmoj Dec 14, 2020
e6555b0
change all catch(fail) instances to catch(failTest)
archmoj Dec 14, 2020
cdfaadf
remove unnecessary done
archmoj Dec 14, 2020
b70e728
fixup various parcoords tests to run the CI
archmoj Dec 14, 2020
c12b3e8
fixup calcdata tests - add catch blocks
archmoj Dec 17, 2020
3bccdb1
use async pattern after plot in axes test
archmoj Dec 17, 2020
892707f
handle async in plot_api tests
archmoj Dec 21, 2020
329e915
handle async in gl3d_plot_interact tests
archmoj Dec 21, 2020
5769cde
add noCI tags to some flaky mesh3d hover tests
archmoj Dec 21, 2020
ec53d29
remove duplicate catch from violin test
archmoj Dec 22, 2020
7f90732
add catch failTest to 4 places in config test
archmoj Dec 22, 2020
2c0b913
fix missing catch and done in funnelarea tests
archmoj Dec 22, 2020
2853691
fix missing catch and done in histogram2d tests
archmoj Dec 22, 2020
1694bb4
fix missing catch and done in hover_label tests
archmoj Dec 22, 2020
3a313e7
add missing catch and done in legend_scroll test
archmoj Dec 22, 2020
33b72e1
fix missing catch and done in pie tests
archmoj Dec 23, 2020
5a127f0
revert changes made to one test in legend_scroll
archmoj Dec 23, 2020
97596ae
handle async in multiple transform_aggregate tests
archmoj Dec 24, 2020
149701e
add missing done and failTest to histogram tests
archmoj Jan 4, 2021
9753fb9
drop many flaky flags
archmoj Dec 22, 2020
bbe25ef
drop strange flag
archmoj Dec 22, 2020
70ec83a
drop one more noCI flag
archmoj Dec 22, 2020
cd809a2
drop return
archmoj Jan 4, 2021
6de8082
add a flaky flag to one cartesian_interact test
archmoj Jan 4, 2021
646cb2c
Revert "drop strange flag"
archmoj Jan 4, 2021
ce51c69
drop noCI flags from parcoords tests
archmoj Jan 4, 2021
d0d14b8
Merge remote-tracking branch 'origin/master' into ensure-return-promise
archmoj Jan 4, 2021
5a3110a
maintain 5 retries for flaky containers jasmine3 and image2
archmoj Jan 4, 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
2 changes: 1 addition & 1 deletion .circleci/test.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ set +o pipefail

ROOT=$(dirname $0)/..
EXIT_STATE=0
MAX_AUTO_RETRY=5
MAX_AUTO_RETRY=1

log () {
echo -e "\n$1"
Expand Down
27 changes: 17 additions & 10 deletions test/jasmine/tests/annotations_test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -187,11 +187,13 @@ describe('annotations relayout', function() {
expect(typeof MathJax).toBe('undefined');
mockLayout.annotations[14].text = '$x+y+z$';

Plotly.plot(gd, mockData, mockLayout).then(done);

spyOn(Loggers, 'warn');
Plotly.plot(gd, mockData, mockLayout)
.then(function() {
spyOn(Loggers, 'warn');

Plotly.setPlotConfig({queueLength: 3});
return Plotly.setPlotConfig({queueLength: 3});
})
.then(done);
});

afterEach(function() {
Expand Down Expand Up @@ -327,7 +329,8 @@ describe('annotations relayout', function() {

assertText(0, 'left top');

Plotly.relayout(gd, 'annotations[0].text', 'hello').then(function() {
Plotly.relayout(gd, 'annotations[0].text', 'hello')
.then(function() {
assertText(0, 'hello');

return Plotly.relayout(gd, 'annotations[0].text', null);
Expand Down Expand Up @@ -427,11 +430,15 @@ describe('annotations relayout', function() {
{'annotations[0]': 'not an object'},
{'annotations[100]': {text: 'bad index'}}
].forEach(function(update) {
it('warns on ambiguous combinations and invalid values: ' + JSON.stringify(update), function() {
Plotly.relayout(gd, update);
expect(Loggers.warn).toHaveBeenCalled();
// we could test the results here, but they're ambiguous and/or undefined so why bother?
// the important thing is the developer is warned that something went wrong.
it('warns on ambiguous combinations and invalid values: ' + JSON.stringify(update), function(done) {
Plotly.relayout(gd, update)
.then(function() {
expect(Loggers.warn).toHaveBeenCalled();
// we could test the results here, but they're ambiguous and/or undefined so why bother?
// the important thing is the developer is warned that something went wrong.
})
.catch(failTest)
.then(done);
});
});

Expand Down
245 changes: 157 additions & 88 deletions test/jasmine/tests/axes_test.js
View file
Open in desktop

Large diffs are not rendered by default.

1,005 changes: 561 additions & 444 deletions test/jasmine/tests/calcdata_test.js
View file
Open in desktop

Large diffs are not rendered by default.

6 changes: 6 additions & 0 deletions test/jasmine/tests/carpet_test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -724,6 +724,7 @@ describe('scattercarpet hover labels', function() {
[200, 200], fig,
[['a: 0.200', 'b: 3.500', 'y: 2.900'], 'a = 0.2']
)
.catch(failTest)
.then(done);
});

Expand All @@ -736,6 +737,7 @@ describe('scattercarpet hover labels', function() {
[200, 200], fig,
[['a: 0.200', 'b: 3.500', 'y: 2.900', 'D'], 'a = 0.2']
)
.catch(failTest)
.then(done);
});

Expand All @@ -747,6 +749,7 @@ describe('scattercarpet hover labels', function() {
[200, 200], fig,
[['a: 0.200', 'y: 2.900'], null]
)
.catch(failTest)
.then(done);
});

Expand All @@ -758,6 +761,7 @@ describe('scattercarpet hover labels', function() {
[200, 200], fig,
[['b: 3.500', 'y: 2.900'], null]
)
.catch(failTest)
.then(done);
});

Expand All @@ -769,6 +773,7 @@ describe('scattercarpet hover labels', function() {
[200, 200], fig,
[['f(0.2, 3.5) = 2.900'], 'scattercarpet #5']
)
.catch(failTest)
.then(done);
});

Expand All @@ -780,6 +785,7 @@ describe('scattercarpet hover labels', function() {
[200, 200], fig,
[['f(0.2, 3.5) = 3.0'], 'pt #3']
)
.catch(failTest)
.then(done);
});
});
Expand Down
9 changes: 4 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 @@ -32,7 +32,6 @@ describe('zoom box element', function() {
mockCopy.layout.dragmode = 'zoom';

Plotly.plot(gd, mockCopy.data, mockCopy.layout)
.catch(failTest)
.then(done);
});

Expand Down Expand Up @@ -2049,12 +2048,12 @@ describe('axis zoom/pan and main plot zoom', function() {
trace1.y = [];
}

Plotly.react(gd, [trace0, trace1], layout);
return Plotly.react(gd, [trace0, trace1], layout);
});

gd.on('plotly_selected', function(d) {
selectedTracker.unshift(d);
Plotly.react(gd, [trace0], layout);
return Plotly.react(gd, [trace0], layout);
});
})
.then(_assert('base', {
Expand Down Expand Up @@ -2338,7 +2337,7 @@ describe('Event data:', function() {
expect('marker.colorbar.tickvals' in pt).toBe(false, 'marker.colorbar.tickvals');
expect('marker.colorbar.ticktext' in pt).toBe(false, 'marker.colorbar.ticktext');
})
.catch(fail)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Ah, good catch, this is a tricky one - for context, we used to call this function fail but changed it to failTest because fail is a global created by Jasmine to manually cause a test to fail. This mostly worked though it's not great to override a global. But the big problem came when we occasionally forgot to import our fail_test and ended up using the Jasmine global - which would just swallow the failure.

That said I notice there's a done.fail and in fact in v3 smarter done behavior that looks like it should make failTest obsolete - instead of .catch(failTest).then(done) we may be able to do just .then(done, done.fail) or perhaps even .then(done, done)

Definitely not something we need to do now, and it would take some experimentation to be sure this is going to fail correctly with at least as much information as failTest gives us.

.catch(failTest)
.then(done);
});

Expand Down Expand Up @@ -2366,7 +2365,7 @@ describe('Event data:', function() {
expect('marker.colorbar.tickvals' in pt).toBe(false, 'marker.colorbar.tickvals');
expect('marker.colorbar.ticktext' in pt).toBe(false, 'marker.colorbar.ticktext');
})
.catch(fail)
.catch(failTest)
.then(done);
});
});
Expand Down
19 changes: 16 additions & 3 deletions test/jasmine/tests/choropleth_test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -205,6 +205,7 @@ describe('Test choropleth hover:', function() {
fig,
['RUS\n10', 'trace 1']
)
.catch(failTest)
.then(done);
});
});
Expand All @@ -220,6 +221,7 @@ describe('Test choropleth hover:', function() {
fig,
['tpl 10', 'x']
)
.catch(failTest)
.then(done);
});
});
Expand All @@ -236,6 +238,7 @@ describe('Test choropleth hover:', function() {
fig,
['tExT', null]
)
.catch(failTest)
.then(done);
});
});
Expand All @@ -252,6 +255,7 @@ describe('Test choropleth hover:', function() {
fig,
['-text-', null]
)
.catch(failTest)
.then(done);
});
});
Expand All @@ -269,6 +273,7 @@ describe('Test choropleth hover:', function() {
fig,
['-text-', null]
)
.catch(failTest)
.then(done);
});
});
Expand All @@ -295,6 +300,7 @@ describe('Test choropleth hover:', function() {
fontFamily: 'Roboto'
}
)
.catch(failTest)
.then(done);
});
});
Expand All @@ -310,6 +316,7 @@ describe('Test choropleth hover:', function() {
fig,
['RUS', 'trace 1']
)
.catch(failTest)
.then(done);
});
});
Expand All @@ -330,15 +337,19 @@ describe('Test choropleth hover:', function() {

[false, true].forEach(function(hasCssTransform) {
it('- base case (truncate z decimals), hasCssTransform: ' + hasCssTransform, function(done) {
run(hasCssTransform, pos, base(), exp).then(done);
run(hasCssTransform, pos, base(), exp)
.catch(failTest)
.then(done);
});
});

[false, true].forEach(function(hasCssTransform) {
it('- hovertemplate case (same z truncation), hasCssTransform: ' + hasCssTransform, function(done) {
var fig = base();
fig.hovertemplate = '%{z}<extra>%{location}</extra>';
run(hasCssTransform, pos, fig, exp).then(done);
run(hasCssTransform, pos, fig, exp)
.catch(failTest)
.then(done);
});
});
});
Expand All @@ -350,7 +361,9 @@ describe('Test choropleth hover:', function() {
fig.data[0].hovertemplate = '%{properties.name}<extra>%{ct[0]:.1f} | %{ct[1]:.1f}</extra>';
fig.layout.geo.projection = {scale: 20};

run(hasCssTransform, [300, 200], fig, ['New York', '-75.1 | 42.6']).then(done);
run(hasCssTransform, [300, 200], fig, ['New York', '-75.1 | 42.6'])
.catch(failTest)
.then(done);
});
});
});
Expand Down
4 changes: 2 additions & 2 deletions test/jasmine/tests/choroplethmapbox_test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -516,7 +516,7 @@ describe('Test choroplethmapbox convert:', function() {
});
});

describe('@noCI Test choroplethmapbox hover:', function() {
describe('Test choroplethmapbox hover:', function() {
var gd;

afterEach(function(done) {
Expand Down Expand Up @@ -653,7 +653,7 @@ describe('@noCI Test choroplethmapbox hover:', function() {
});
});

describe('@noCI Test choroplethmapbox interactions:', function() {
describe('Test choroplethmapbox interactions:', function() {
var gd;

var geojson = {
Expand Down
Loading