use new jasmine pattern for catching errors in async code

[archmoj]

Addressing #5340 (comment)
The first pass to retire failTest.

@plotly/plotly_js

[alexcjohnson]

Have you verified that done.fail gives similar feedback to what we got from failTest when an exception is thrown? And is there a reason that pattern is preferable to .then(done, done) that the docs imply should do the same thing in Jasmine 3?

[archmoj]

Have you verified that done.fail gives similar feedback to what we got from failTest when an exception is thrown? And is there a reason that pattern is preferable to .then(done, done) that the docs imply should do the same thing in Jasmine 3?

RE1:
I verified and it prints out the error in case of exception.

RE2:
It looks like we could use the .then(don, done) instead but then this case would be confusing:

plotly.js/test/jasmine/tests/animate_test.js

Line 183 in 444be40

Plotly.animate(gd, ['foobar'], animOpts).then(failTest).then(done, done);

So it seems preferable to keep done.fail as is.

[alexcjohnson]

True, done.fail is more explicit. Also possibly .then(done, done) would look like a pass if we (or any dep) ever throw a non-error, ie throw 'whoops!' rather than throw new Error('whoops!')

So great, let's keep done.fail.

That Plotly.animate test is certainly confusing in any case, and it makes me wonder whether we're really rejecting the way we want to. Looks like it's this one:

plotly.js/src/plot_api/plot_api.js

Lines 3471 to 3474 in 444be40

	if(frame.type === 'byname' && !trans._frameHash[frame.data.name]) {
	Lib.warn('animate failure: frame not found: "' + frame.data.name + '"');
	reject();
	return;

Most people seem to recommend rejecting with an Error which seems like it would be more useful, but if we did that we'd need to change the test to something like:

Plotly.animate(gd, ['foobar'], animOpts).then(done.fail, () => { done() });

Anyway not worth spending time on now, Plotly.animate is no longer something we're recommending people use anymore. But if we have a future need to reject a promise I think we should do so with an Error.

[alexcjohnson]

After this PR are there any uses of failTest left, or can we delete it entirely?

[archmoj]

After this PR are there any uses of failTest left, or can we delete it entirely?

There are just a few cases. But let's attempt drop failTest in another PR.

[alexcjohnson]

💃