CircleCI: run jobs in parallel

[antoinerg]

This PR is a follow-up to #3612 to try to improve the runtime of our test suite and its robustness.

Here we do so by leveraging the parallelism option offered in CircleCI. I moved out test-bundle into its own job and proceeded to run test-jasmine and test-image in parallel.

The test-image routine had to modified for it to accept a list of mocks as command-line arguments (it used to only support one globbing pattern which wasn't suitable).

I selected a parallelism of 2 for a few jobs in order for them to run in ~ 4 minutes. Because we need to build first (~ 2 minutes), we end up with a total running time of roughly 6 minutes:

[etpinard]

This is brilliant. Thanks for looking into this @antoinerg !

Before going any further, I'd like to get @bpostlethwaite 's approval.

Off this branch, we have:

that is, a max of 8 containers are running simultaneously and 4 of those containers set parallelism: 2. @bpostlethwaite is this going to be take too juice much out of our CircleCI plan?

[antoinerg]

that is, a max of 8 containers are running simultaneously and 4 of those containers set parallelism: 2. @bpostlethwaite is this going to be take too juice much out of our CircleCI plan?

@etpinard this is a very valid concern that I was going to bring up to your attention. Little correction: I think we have 3 job with a parallelism of 2 so in total we use 5*1 + 3*2 = 11 containers.

One thing that concerned me was whether CircleCI would wait for 11 containers to be available to even start or would it execute each job sequentially if we're short on containers. I assume it does the latter but we should check to make sure our tests do not get queued up for long period of times.

[bpostlethwaite]

Yes this is fine. If we need more containers we'll buy them. Please let me know if there is significant queuing!

[etpinard]

@antoinerg looks like we have the green light 🚀

Before merging this thing, I noticed that the tests that get skipped are no longer logged. For example, in test-jasmine container off master, we have:

If I remember correctly, the spec reporter

plotly.js/test/jasmine/karma.conf.js

Line 114 in 61fb58e

var reporters = (isFullSuite && !argv.tags) ? ['dots', 'spec'] : ['progress'];

is responsible for this, along with its config object:

plotly.js/test/jasmine/karma.conf.js

Lines 247 to 257 in 61fb58e

	// use 'karma-spec-reporter' to log info about skipped specs
	specReporter: {
	suppressErrorSummary: true,
	suppressFailed: true,
	suppressPassed: true,
	suppressSkipped: false,
	showSpecTiming: false,
	// use 'karma-fail-fast-reporter' to fail fast w/o conflicting
	// with other karma plugins
	failFast: false
	},

---

I'm not sure how useful logging the skipped tests is. What do you think @antoinerg? Maybe we could add a CLI argument to allow devs to switch from

var reporters = ['dots', 'spec'];

// to
var reportes = ['progress'];

Thoughts?

[etpinard]

Oh one more thing too, at present we shouldn't have to split the gl2d_* mocks from the others during npm run test-image (hmm but maybe the heatmapgl and pointcloud mocks still need their own test call).

So it would be nice, to bring all the pixel comparison tests into one container. Once that's done perhaps bump the parallelism to 3 in that container.

[antoinerg]

Oh one more thing too, at present we shouldn't have to split the gl2d_* mocks from the others during npm run test-image (hmm but maybe the heatmapgl and pointcloud mocks still need their own test call).

One big difference between them and "regular" mocks is that we render them in queue instead of in batch:

plotly.js/package.json

Lines 37 to 38 in 12ce725

	"test-image": "node tasks/test_image.js",
	"test-image-gl2d": "node tasks/test_image.js gl2d_* --queue",

Out of curiosity, what makes you think the following warning may not apply anymore?

plotly.js/test/image/compare_pixels_test.js

Line 69 in 12ce725

console.log('WARN: Running gl2d image tests in batch may lead to unwanted results\n');

Anyway, I will test it out in a separate branch and let you know the result 😸

[etpinard]

One big difference between them and "regular" mocks is that we render them in queue instead of in batch

Good eye. Yeah, we shouldn't have to run the gl2d image tests in a queue any more, but maybe the heatmapgl and pointcloud still behave weird.

[etpinard]

Out of curiosity, what makes you think the following warning may not apply anymore?

That one sounds obsolete.

[antoinerg]

Oh one more thing too, at present we shouldn't have to split the gl2d_* mocks from the others during npm run test-image (hmm but maybe the heatmapgl and pointcloud mocks still need their own test call).

So it would be nice, to bring all the pixel comparison tests into one container. Once that's done perhaps bump the parallelism to 3 in that container.

Done in 609cdf8 and seems to work fine.

[antoinerg]

I'm not sure how useful logging the skipped tests is. What do you think @antoinerg? Maybe we could add a CLI argument to allow devs to switch from

I think it is useful to show the skipped tests. I added a flag for it in b99672b.

[etpinard]

Looks like we'll have to keep this:

Running

npm run test-image

off this branch fails at gl2d_pointcloud-basic:

[antoinerg]

I could reproduce this issue locally! Nice catch 👀

With be33648, I reintroduced sortGl2dMockList and running npm run test-image completed without error 🎉

[etpinard]

works for me too 🎉

[etpinard]

⚡ - no need to set the timezone in containers that run image tests (they have their own timezone in their docker container).

[archmoj]

@etpinard would you please provide a pointer to this?

[etpinard]

Not sure what else can I say.

[etpinard]

a thing of beauty. Thanks @antoinerg - looks like you're almost there!

[etpinard]

Amazing work! 💃 💃

Feel free to ignore #3634 (review)