Skip to content

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

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 12 commits into from
Mar 9, 2017
Merged

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

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
0e3c61f
merge karma ciconf with (regular) conf
etpinard Mar 9, 2017
aa1aaef
comment & lint in karma conf
etpinard Mar 9, 2017
62d33b9
rm hasWebGLSupport early return in jasmine specs
etpinard Mar 9, 2017
100b993
rm unnecessary quotes in yml
etpinard Mar 9, 2017
14621e1
revamp circle.yml for ubuntu 14.04 with 2x parallel containers !!!
etpinard Mar 9, 2017
b42f021
use `karma-jasmine-spec-tags` to skip a few test on CI
etpinard Mar 9, 2017
8d7171f
add karma-verbose-reporter in dev deps
etpinard Mar 9, 2017
b86bf06
add `karma-spec-reporter` to log info about skipped specs
etpinard Mar 9, 2017
631533d
tweak precision to make tests pass on CI w/ Chrome 54
etpinard Mar 9, 2017
dbc6ca1
lock uglify-js to 2.7.x
etpinard Mar 9, 2017
e52f020
add no_citest script
etpinard Mar 9, 2017
33ae963
use (plain) npm run test-jasmine on CI
etpinard Mar 9, 2017
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
27 changes: 11 additions & 16 deletions circle.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
general:
artifacts:
- "build/test_images/"
- "build/test_images_diff/"
- build/test_images/
- build/test_images_diff/

machine:
node:
Expand All @@ -12,21 +12,16 @@ machine:
- docker

dependencies:
pre:
- eval $(node tasks/docker.js pull)
post:
- eval $(node tasks/docker.js run)
- npm run cibuild
override:
- npm install && npm dedupe && npm prune && npm install
Copy link
Contributor Author

Choose a reason for hiding this comment

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

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

cc @bpostlethwaite @scjody

- npm ls || true
- npm run docker -- pull
- npm run pretest
- eval $(node tasks/docker.js setup)
- npm prune && npm ls
- npm run docker -- run
- npm run cibuild
- npm run docker -- setup
Copy link
Contributor Author

@etpinard etpinard Mar 9, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

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


test:
override:
- npm run test-image
- npm run test-image-gl2d
- npm run test-export
- npm run citest-jasmine
- npm run test-bundle
- npm run test-syntax
- eslint .
- ./tasks/ci_test.sh:
parallel: true
Copy link
Contributor Author

Choose a reason for hiding this comment

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

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

7 changes: 4 additions & 3 deletions package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,12 +28,12 @@
"build": "npm run preprocess && npm run bundle && npm run header && npm run stats",
"cibuild": "npm run preprocess && node tasks/cibundle.js",
"watch": "node tasks/watch.js",
"lint": "eslint --version && eslint . || true",
"lint-fix": "eslint . --fix",
"lint": "eslint --version && eslint .",
"lint-fix": "eslint . --fix || true",
"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",
Copy link
Contributor Author

@etpinard etpinard Mar 9, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

TODO:

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

Copy link
Contributor

Choose a reason for hiding this comment

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

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?)

Copy link
Contributor Author

@etpinard etpinard Mar 9, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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

"test-image": "node tasks/test_image.js",
"test-image-gl2d": "node tasks/test_image.js gl2d_* --queue",
"test-export": "node tasks/test_export.js",
Expand Down Expand Up @@ -119,6 +119,7 @@
"karma-coverage": "^1.0.0",
"karma-firefox-launcher": "^1.0.0",
"karma-jasmine": "^1.1.0",
"karma-jasmine-spec-tags": "^1.0.1",
"madge": "^1.6.0",
"node-sass": "^4.5.0",
"npm-link-check": "^1.2.0",
Expand Down
22 changes: 22 additions & 0 deletions tasks/ci_test.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
#!/bin/bash

EXIT_STATE=0

case $CIRCLE_NODE_INDEX in

0)
npm run test-image || EXIT_STATE=$?
npm run test-image-gl2d || EXIT_STATE=$?
npm run test-export || EXIT_STATE=$?
npm run test-syntax || EXIT_STATE=$?
npm run lint || EXIT_STATE=$?
exit $EXIT_STATE
Copy link
Contributor Author

@etpinard etpinard Mar 9, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

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?

Copy link
Contributor

Choose a reason for hiding this comment

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

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

;;

1)
npm run citest-jasmine || EXIT_STATE=$?
npm run test-bundle || EXIT_STATE=$?
exit $EXIT_STATE
;;

esac
11 changes: 2 additions & 9 deletions tasks/docker.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,12 +46,5 @@ switch(arg) {
break;
}

// Log command string on CircleCI, to then `eval` them,
// which appears to be more reliable then calling `child_process.exec()`
if(isCI) {
console.log(cmd);
}
else {
console.log(msg);
common.execCmd(cmd, cb, errorCb);
}
console.log(msg);
common.execCmd(cmd, cb, errorCb);
4 changes: 3 additions & 1 deletion tasks/test_export.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,4 +14,6 @@ var cmd = containerCommands.getRunCmd(
);

console.log(msg);
common.execCmd(cmd);
common.execCmd(containerCommands.ping, function() {
common.execCmd(cmd);
});
4 changes: 3 additions & 1 deletion tasks/test_image.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,4 +14,6 @@ var cmd = containerCommands.getRunCmd(
);

console.log(msg);
common.execCmd(cmd);
common.execCmd(containerCommands.ping, function() {
common.execCmd(cmd);
});
2 changes: 0 additions & 2 deletions tasks/util/container_commands.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,6 @@ containerCommands.setup = [
containerCommands.injectEnv,
containerCommands.restart,
'sleep 1',
containerCommands.ping,
'echo '
].join(' && ');

containerCommands.dockerRun = [
Expand Down
18 changes: 0 additions & 18 deletions test/jasmine/assets/has_webgl_support.js
View file
Open in desktop

This file was deleted.

42 changes: 0 additions & 42 deletions test/jasmine/karma.ciconf.js
View file
Open in desktop

This file was deleted.

56 changes: 32 additions & 24 deletions test/jasmine/karma.conf.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ var constants = require('../../tasks/util/constants');

var arg = process.argv[4];

var isCI = !!process.env.CIRCLECI;
var testFileGlob = arg ? arg : 'tests/*_test.js';
var isSingleSuiteRun = (arg && arg.indexOf('bundle_tests/') === -1);
var isRequireJSTest = (arg && arg.indexOf('bundle_tests/requirejs') !== -1);
Expand Down Expand Up @@ -53,13 +54,14 @@ func.defaultConfig = {

// frameworks to use
// available frameworks: https://npmjs.org/browse/keyword/karma-adapter
frameworks: ['jasmine', 'browserify'],
frameworks: ['jasmine', 'jasmine-spec-tags', 'browserify'],

// list of files / patterns to load in the browser
//
// N.B. this field is filled below
files: [],

// list of files / pattern to exclude
exclude: [],

// preprocess matching files before serving them to the browser
Expand All @@ -84,14 +86,23 @@ func.defaultConfig = {
colors: true,

// enable / disable watching file and executing tests whenever any file changes
autoWatch: true,
autoWatch: !isCI,

// if true, Karma captures browsers, runs the tests and exits
singleRun: isCI,

// how long will Karma wait for a message from a browser before disconnecting (30 ms)
browserNoActivityTimeout: 30000,

// start these browsers
// available browser launchers: https://npmjs.org/browse/keyword/karma-launcher
browsers: ['Chrome_WindowSized'],

// custom browser options
//
// window-size values came from observing default size
//
// '--ignore-gpu-blacklist' allow to test WebGL on CI (!!!)
customLaunchers: {
Chrome_WindowSized: {
base: 'Chrome',
Expand All @@ -103,58 +114,55 @@ func.defaultConfig = {
}
},

// Continuous Integration mode
// if true, Karma captures browsers, runs the tests and exits
singleRun: false,

browserify: {
transform: ['../../tasks/util/shortcut_paths.js'],
extensions: ['.js'],
watch: true,
watch: !isCI,
debug: true

// unfortunately a few tests don't behave well on CI
// using `karma-jasmine-spec-tags`
// add @noCI to the spec description to skip a spec on CI
client: {
tagPrefix: '@',
skipTags: isCI ? 'noCI' : null
},

}
};


// Add lib/index.js to single-suite runs,
// to avoid import conflicts due to plotly.js
// circular dependencies.
if(isSingleSuiteRun) {
func.defaultConfig.files = [
func.defaultConfig.files.push(
pathToJQuery,
pathToMain,
testFileGlob
];
pathToMain
);

func.defaultConfig.preprocessors[pathToMain] = ['browserify'];
func.defaultConfig.preprocessors[testFileGlob] = ['browserify'];
}
else if(isRequireJSTest) {
func.defaultConfig.files = [
constants.pathToRequireJS,
constants.pathToRequireJSFixture,
testFileGlob
constants.pathToRequireJSFixture
];
}
else if(isIE9Test) {
// load ie9_mock.js before plotly.js+test bundle
// to catch reference errors that could occur
// when plotly.js is first loaded.

func.defaultConfig.files = [
'./assets/ie9_mock.js',
testFileGlob
];

func.defaultConfig.files.push('./assets/ie9_mock.js');
func.defaultConfig.preprocessors[testFileGlob] = ['browserify'];
}
else {
func.defaultConfig.files = [
pathToJQuery,
testFileGlob
];

func.defaultConfig.files.push(pathToJQuery);
func.defaultConfig.preprocessors[testFileGlob] = ['browserify'];
}

// lastly, load test file glob
func.defaultConfig.files.push(testFileGlob);

module.exports = func;
5 changes: 1 addition & 4 deletions test/jasmine/tests/gl2d_click_test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ var Lib = require('@src/lib');
var createGraphDiv = require('../assets/create_graph_div');
var destroyGraphDiv = require('../assets/destroy_graph_div');
var customMatchers = require('../assets/custom_matchers');
var hasWebGLSupport = require('../assets/has_webgl_support');

// cartesian click events events use the hover data
// from the mousemove events and then simulate
Expand All @@ -17,9 +16,7 @@ Plotly.register([
require('@lib/contourgl')
]);

describe('Test hover and click interactions', function() {

if(!hasWebGLSupport('gl2d_click_test')) return;
describe('@noCI Test hover and click interactions', function() {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done in #1459


var mock = require('@mocks/gl2d_14.json');
var mock2 = require('@mocks/gl2d_pointcloud-basic.json');
Expand Down
4 changes: 0 additions & 4 deletions test/jasmine/tests/gl2d_date_axis_render_test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,14 +1,10 @@
var PlotlyInternal = require('@src/plotly');

var hasWebGLSupport = require('../assets/has_webgl_support');

var createGraphDiv = require('../assets/create_graph_div');
var destroyGraphDiv = require('../assets/destroy_graph_div');

describe('date axis', function() {

if(!hasWebGLSupport('axes_test date axis')) return;

var gd;

beforeEach(function() {
Expand Down
6 changes: 0 additions & 6 deletions test/jasmine/tests/gl_plot_interact_test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,12 +11,6 @@ var mouseEvent = require('../assets/mouse_event');
var selectButton = require('../assets/modebar_button');
var customMatchers = require('../assets/custom_matchers');

/*
* WebGL interaction test cases fail on the CircleCI
* most likely due to a WebGL/driver issue
*
*/

var MODEBAR_DELAY = 500;


Expand Down
7 changes: 1 addition & 6 deletions test/jasmine/tests/mapbox_test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ var supplyLayoutDefaults = require('@src/plots/mapbox/layout_defaults');
var d3 = require('d3');
var createGraphDiv = require('../assets/create_graph_div');
var destroyGraphDiv = require('../assets/destroy_graph_div');
var hasWebGLSupport = require('../assets/has_webgl_support');
var mouseEvent = require('../assets/mouse_event');
var customMatchers = require('../assets/custom_matchers');
var failTest = require('../assets/fail_test');
Expand Down Expand Up @@ -176,8 +175,6 @@ describe('mapbox defaults', function() {
describe('mapbox credentials', function() {
'use strict';

if(!hasWebGLSupport('mapbox credentials')) return;

var dummyToken = 'asfdsa124331wersdsa1321q3';
var gd;

Expand Down Expand Up @@ -279,11 +276,9 @@ describe('mapbox credentials', function() {
}, LONG_TIMEOUT_INTERVAL);
});

describe('mapbox plots', function() {
describe('@noCI, mapbox plots', function() {
'use strict';

if(!hasWebGLSupport('mapbox plots')) return;

var mock = require('@mocks/mapbox_0.json'),
gd;

Expand Down
Loading