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.

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
27 changes: 11 additions & 16 deletions circle.yml
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

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.

11 changes: 7 additions & 4 deletions package.json
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=1 karma start test/jasmine/karma.conf.js",
"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,9 @@
"karma-coverage": "^1.0.0",
"karma-firefox-launcher": "^1.0.0",
"karma-jasmine": "^1.1.0",
"karma-jasmine-spec-tags": "^1.0.1",
"karma-spec-reporter": "0.0.30",
"karma-verbose-reporter": "0.0.6",
"madge": "^1.6.0",
"node-sass": "^4.5.0",
"npm-link-check": "^1.2.0",
Expand All @@ -128,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",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We should probably open an issue in cwise about this.

"watchify": "^3.9.0",
"xml2js": "^0.4.16"
}
Expand Down
22 changes: 22 additions & 0 deletions tasks/ci_test.sh
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

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 test-jasmine || EXIT_STATE=$?
npm run test-bundle || EXIT_STATE=$?
exit $EXIT_STATE
;;

esac
11 changes: 2 additions & 9 deletions tasks/docker.js
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);
16 changes: 16 additions & 0 deletions tasks/noci_test.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
#! /bin/bash

EXIT_STATE=0

# tests that aren't run on CI

# jasmine specs with @noCI tag
npm run citest-jasmine -- tests/*_test.js --tags noCI || EXIT_STATE=$?
Copy link
Contributor Author

Choose a reason for hiding this comment

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

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.

Copy link
Contributor Author

@etpinard etpinard Mar 9, 2017

Choose a reason for hiding this comment

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

TODO:

  • update dev docs.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Contributor

Choose a reason for hiding this comment

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

The link ⬆️ is broken.

See usage here: b42f021


# mapbox image tests take too much resources on CI
npm run test-image -- mapbox_* || EXIT_STATE=$?

# run gl2d image test again (some mocks are skipped on CI)
npm run test-image-gl2d || EXIT_STATE=$?

exit $EXIT_STATE
4 changes: 3 additions & 1 deletion tasks/test_export.js
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
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
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

This file was deleted.

42 changes: 0 additions & 42 deletions test/jasmine/karma.ciconf.js

This file was deleted.

67 changes: 42 additions & 25 deletions test/jasmine/karma.conf.js
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 @@ -75,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'],
Copy link
Contributor Author

Choose a reason for hiding this comment

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

logs now look like:

image

Copy link
Contributor Author

Choose a reason for hiding this comment

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


// web server port
port: 9876,
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,64 @@ 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
},

// use 'karma-spec-reporter' to log info about skipped specs
specReporter: {
suppressErrorSummary: true,
suppressFailed: true,
suppressPassed: true,
suppressSkipped: false,
showSpecTiming: false,
failFast: false
}
};


// 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;
7 changes: 5 additions & 2 deletions test/jasmine/tests/annotations_test.js
Original file line number Diff line number Diff line change
Expand Up @@ -514,23 +514,26 @@ describe('annotations autorange', function() {

function assertRanges(x, y, x2, y2, x3, y3) {
var fullLayout = gd._fullLayout;
var PREC = 1;

var PREC = 1;
// xaxis2 need a bit more tolerance to pass on CI
// this most likely due to the different text bounding box values
// on headfull vs headless browsers.
// but also because it's a date axis that we've converted to ms
var PRECX2 = -10;
// yaxis2 needs a bit more now too...
var PRECY2 = 0.2;
// and xaxis3 too...
var PRECX3 = 0.2;

var dateAx = fullLayout.xaxis2;

expect(fullLayout.xaxis.range).toBeCloseToArray(x, PREC, '- xaxis');
expect(fullLayout.yaxis.range).toBeCloseToArray(y, PREC, '- yaxis');
expect(Lib.simpleMap(dateAx.range, dateAx.r2l))
.toBeCloseToArray(Lib.simpleMap(x2, dateAx.r2l), PRECX2, 'xaxis2 ' + dateAx.range);
expect(fullLayout.yaxis2.range).toBeCloseToArray(y2, PRECY2, 'yaxis2');
expect(fullLayout.xaxis3.range).toBeCloseToArray(x3, PREC, 'xaxis3');
expect(fullLayout.xaxis3.range).toBeCloseToArray(x3, PRECX3, 'xaxis3');
expect(fullLayout.yaxis3.range).toBeCloseToArray(y3, PREC, 'yaxis3');
}

Expand Down
Loading