CircleCI with Ubuntu 14.04, 2x parallelism and WebGL jasmine tests #1455

etpinard · 2017-03-09T15:13:16Z

resolves #241

Big news in CircleCI land !!!

Our tests are now running on an Ubuntu 14.04 build image on CircleCI. Previously we were using a (now very outdated) Ubuntu 12.04 build image.

This build image comes with Chrome 54 by installed by default. This is big news because this Chrome version can create a WebGL context using the --ignore-gpu-blacklist (which we were setting previously, but wasn't functional). That is, our WebGL jasmine tests can now run on CI 🎉 - which for all intents and purposes close #241.

With the WebGL suites npm run test-jasmine now takes more than 5 minutes to run on CI, bringing our total CI run time to above 15 minutes which is kind of slow 🐢 . Needless to say, this was a good time to add 2x parallelism to our CI builds 🏎️ . With 2x parallelism the total build times are back to about 10 minutes. See commit 14621e1 for more info.

Unfortunately, some jasmine tests still need to be skipped namely the mapbox suites (which are very resource intensive and end up blowing up the process) and similar for some parcoords tests (cc @monfera). Luckily the karma-spec-tags plugin made skipping those few tests on CI very easy 🍸

- use process.env.CIRCLECI to determine if karma is run on CI or not - autoWatch is false on CI - singleRun is true on CI - Chrome 54 (on ubuntu 14.04) is now used on CI !!

- Chrome is now used on CI - highlight that --ignore-gpu-blacklist allow to test WebGL on CI (CircleCI already wraps the process with xfvb) - DRY files array operations

- ... as WebGL is now supported on CI 🎉

- add ci_tesh.sh that splits test command between 2 containers - make `npm run lint` exit with exit code 1 if fails - rm `eval ($node tasks/docker.js ---)` hack (no longer needed in 14.04) - ping imagetest server from outside container, as pinging from inside was failling in 14.04 - add more robust `npm i` step

- unfortunately, still some test need to be skipped on CI e.g. mapbox-gl takes too much resources ant blows up the run and similarly for some parcoords tests.

- which is very useful for debugging as discussed in CONTRIBUTING.md

- so that the CI logs show which tests were skipped - use with 'dots' reporter when not targeting an individual suite for cleaner output.

- the new 2.8.x series breaks the `cwise` transform - see #1450 (comment) - maybe we should make uglify-js a core dependencies then?

- to make it easier to run tests that are skipped on CI and easier to document.

etpinard · 2017-03-09T15:17:09Z

test/jasmine/karma.conf.js

@@ -77,7 +77,7 @@ func.defaultConfig = {
    // See note in CONTRIBUTING.md about more verbose reporting via karma-verbose-reporter:
    // https://www.npmjs.com/package/karma-verbose-reporter ('verbose')
    //
-    reporters: ['progress'],
+    reporters: isSingleSuiteRun ? ['progress'] : ['dots', 'spec'],


logs now look like:

See https://github.com/mlex/karma-spec-reporter for more info.

etpinard · 2017-03-09T15:19:08Z

tasks/noci_test.sh

+# tests that aren't run on CI
+
+# jasmine specs with @noCI tag
+npm run citest-jasmine --  tests/*_test.js --tags noCI || EXIT_STATE=$?


I'm very pleased with karma-jasmine-spec-tags. This ⏫ allow us to run allow the specs with the @noCI tag. This will make the pre-release process a lot less painful.

TODO:

update dev docs.

done in https://github.com/plotly/dev-docs/commit/17302734858a6bc967944450490ea4ffef1757bd

The link ⬆️ is broken.

See usage here: b42f021

etpinard · 2017-03-09T15:21:53Z

test/jasmine/karma.ciconf.js

-
-    func.defaultConfig.autoWatch = false;
-
-    func.defaultConfig.browsers = ['Firefox_WindowSized'];


So note that Chrome is now used by default both locally and on CI.

We those of you that prefer dev'ing in FF:

npm run test-jasmine -- tests/*_test.js --browsers Firefox_WindowSized

works.

cc @n-riesco

etpinard · 2017-03-09T15:23:47Z

circle.yml

-    - eval $(node tasks/docker.js run)
-    - npm run cibuild
+  override:
+    - npm install && npm dedupe && npm prune && npm install


This thing here is (a lot) more predictable than simply npm install

cc @bpostlethwaite @scjody

etpinard · 2017-03-09T15:27:24Z

tasks/ci_test.sh

+        npm run test-export     || EXIT_STATE=$?
+        npm run test-syntax     || EXIT_STATE=$?
+        npm run lint            || EXIT_STATE=$?
+        exit $EXIT_STATE


Important: I chose this || EXIT_STATE=$? pattern over #!/bin/bash -e because I wanted all tests commands to run even if one of them fails. Note that $? is the exit code of the previously command. Stuff to the right of || is only executed if in the command to the left exits with a non-zero code.

Does anyone know a better way to do this?

This looks good to me. Bash scripting is always a bit weird.

etpinard · 2017-03-09T15:28:21Z

package.json

@@ -131,7 +131,7 @@
    "read-last-lines": "^1.1.0",
    "requirejs": "^2.3.1",
    "through2": "^2.0.3",
-    "uglify-js": "^2.7.5",
+    "uglify-js": "~2.7.5",


see #1450 (comment)

We should probably open an issue in cwise about this.

etpinard · 2017-03-09T15:29:18Z

package.json

@@ -131,7 +131,7 @@
    "read-last-lines": "^1.1.0",
    "requirejs": "^2.3.1",
    "through2": "^2.0.3",
-    "uglify-js": "^2.7.5",
+    "uglify-js": "~2.7.5",


We should probably open an issue in cwise about this.

etpinard · 2017-03-09T15:30:06Z

circle.yml

-    - npm run test-syntax
-    - eslint .
+    - ./tasks/ci_test.sh:
+        parallel: true


More info on https://circleci.com/docs/1.0/parallel-manual-setup/#simple-example

etpinard · 2017-03-09T15:32:32Z

test/jasmine/tests/gl2d_click_test.js

@@ -16,7 +16,7 @@ Plotly.register([
    require('@lib/contourgl')
 ]);

-describe('Test hover and click interactions', function() {
+describe('@noCI Test hover and click interactions', function() {


This one here should work on CI. I'll investigate.

done in #1459

etpinard · 2017-03-09T15:35:52Z

test/jasmine/tests/parcoords_test.js

@@ -219,7 +219,7 @@ describe('parcoords initialization tests', function() {
    });
 });

-describe('parcoords', function() {
+describe('@noCI parcoords', function() {


@monfera this suite here blows the process on CI.

Maybe next time you play around with the parcoords code you could try to investigate which it(s) exactly blow(s) up the process. Or better yet, try to make the test less resource intensive. Thanks in advance!

etpinard · 2017-03-09T15:40:34Z

circle.yml

-    - npm prune && npm ls
+    - npm run docker -- run
+    - npm run cibuild
+    - npm run docker -- setup


Notice the docker commands are split between other commands. This is reduce the risk of race conditions.

etpinard · 2017-03-09T16:52:09Z

package.json

    "docker": "node tasks/docker.js",
    "pretest": "node tasks/pretest.js",
    "test-jasmine": "karma start test/jasmine/karma.conf.js",
-    "citest-jasmine": "karma start test/jasmine/karma.ciconf.js",
+    "citest-jasmine": "CIRCLECI=0 karma start test/jasmine/karma.conf.js",


TODO:

~~add cross-env to get this to work on Windows.~~

It would be better to fix whatever checks the CIRCLECI var. It's confusing to see CIRCLECI=0 on CircleCI. (If it needs to be this way, can you add a comment?)

Oh right. Thanks for checking!

npm run citest-jasmine is now effectively obsolete. But the CI and local case can now be handled via npm run test-jasmine.

I kept "citest-jasmine" to not perturb workflows for other devs. But if I get enough 👍 I will gladly 🔪 it.

Moreover, this should really be:

"citest-jasmine": "CIRCLECI=1 karma start test/jasmine/karma.conf.js",

but funny story !!"0" in JS evaluates to true, not to be confused with !!0 which is false.

Speaking of which it's probably not work bringing in cross-env for some obsolete script. 🔪

- no need to set the CIRCLECI env variable again.

Per plotly/plotly.js#1455

etpinard added 11 commits March 9, 2017 09:35

comment & lint in karma conf

aa1aaef

- Chrome is now used on CI - highlight that --ignore-gpu-blacklist allow to test WebGL on CI (CircleCI already wraps the process with xfvb) - DRY files array operations

rm hasWebGLSupport early return in jasmine specs

62d33b9

- ... as WebGL is now supported on CI 🎉

rm unnecessary quotes in yml

100b993

use karma-jasmine-spec-tags to skip a few test on CI

b42f021

- unfortunately, still some test need to be skipped on CI e.g. mapbox-gl takes too much resources ant blows up the run and similarly for some parcoords tests.

add karma-verbose-reporter in dev deps

8d7171f

- which is very useful for debugging as discussed in CONTRIBUTING.md

add karma-spec-reporter to log info about skipped specs

b86bf06

- so that the CI logs show which tests were skipped - use with 'dots' reporter when not targeting an individual suite for cleaner output.

tweak precision to make tests pass on CI w/ Chrome 54

631533d

lock uglify-js to 2.7.x

dbc6ca1

- the new 2.8.x series breaks the `cwise` transform - see #1450 (comment) - maybe we should make uglify-js a core dependencies then?

add no_citest script

e52f020

- to make it easier to run tests that are skipped on CI and easier to document.

etpinard added status: reviewable labels Mar 9, 2017

etpinard commented Mar 9, 2017

View reviewed changes

use (plain) npm run test-jasmine on CI

33ae963

- no need to set the CIRCLECI env variable again.

etpinard merged commit 10dd54a into master Mar 9, 2017

etpinard deleted the circleci-14.04-parallel branch March 9, 2017 22:57

alexcjohnson mentioned this pull request Mar 12, 2017

Allow toggling legend to show just 1 series (or group) by double clicking #1432

Merged

This was referenced Mar 13, 2017

Fast color parsing #1443

Merged

Skip gl3d hover and click test on CI #1478

Merged

etpinard mentioned this pull request Mar 23, 2017

provide benchmarks for plotly.js #1511

Closed

etpinard mentioned this pull request Jun 30, 2017

Mapbox subplots open items #815

Closed

thewtex added a commit to Kitware/itk-vtk-viewer that referenced this pull request Jul 24, 2018

ci(Travis): Add --ignore-gpu-blacklist

606ab41

Per plotly/plotly.js#1455

thewtex added a commit to Kitware/itk-vtk-viewer that referenced this pull request Jul 24, 2018

ci(Travis): Add --ignore-gpu-blacklist

8a10387

Per plotly/plotly.js#1455

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

GitHub Sponsors

CircleCI with Ubuntu 14.04, 2x parallelism and WebGL jasmine tests #1455

CircleCI with Ubuntu 14.04, 2x parallelism and WebGL jasmine tests #1455

etpinard commented Mar 9, 2017

etpinard Mar 9, 2017

etpinard Mar 9, 2017

etpinard Mar 9, 2017

etpinard Mar 9, 2017 •

edited

Loading

etpinard Mar 9, 2017

n-riesco Mar 10, 2017

etpinard Mar 9, 2017

etpinard Mar 9, 2017

etpinard Mar 9, 2017

etpinard Mar 9, 2017 •

edited

Loading

scjody Mar 9, 2017

etpinard Mar 9, 2017

etpinard Mar 9, 2017

etpinard Mar 9, 2017

etpinard Mar 9, 2017

etpinard Mar 9, 2017

etpinard Mar 9, 2017

etpinard Mar 9, 2017

etpinard Mar 9, 2017 •

edited

Loading

etpinard Mar 9, 2017 •

edited

Loading

scjody Mar 9, 2017

etpinard Mar 9, 2017 •

edited

Loading

etpinard Mar 9, 2017


		func.defaultConfig.autoWatch = false;

		func.defaultConfig.browsers = ['Firefox_WindowSized'];

CircleCI with Ubuntu 14.04, 2x parallelism and WebGL jasmine tests #1455

CircleCI with Ubuntu 14.04, 2x parallelism and WebGL jasmine tests #1455

Conversation

etpinard commented Mar 9, 2017

Big news in CircleCI land !!!

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

etpinard Mar 9, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

etpinard Mar 9, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

etpinard Mar 9, 2017 • edited Loading

Choose a reason for hiding this comment

etpinard Mar 9, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

etpinard Mar 9, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

etpinard Mar 9, 2017 •

edited

Loading

etpinard Mar 9, 2017 •

edited

Loading

etpinard Mar 9, 2017 •

edited

Loading

etpinard Mar 9, 2017 •

edited

Loading

etpinard Mar 9, 2017 •

edited

Loading