Skip to content

Improve tasks CLI #821

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 26 commits into from
Aug 5, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
45a548c
tasks: rewrite all test container sh scripts in node
etpinard Jul 19, 2016
1291888
tasks: use node script to run image/export on circleci
etpinard Jul 19, 2016
262e90d
fixup: docker pull testbed
etpinard Jul 29, 2016
0fd57ba
tasks: sub fs.existsSync for fs.statSync
etpinard Jul 19, 2016
3712e17
dry: look up container url in constants.js
etpinard Jul 19, 2016
5758129
tasks: rename watch_plotly.js -> watch.js
etpinard Jul 29, 2016
bac6685
tasks: move test syntax script to tasks/
etpinard Jul 29, 2016
f564a85
tasks: add common task util module
etpinard Jul 29, 2016
04096de
tasks: use common utils
etpinard Jul 29, 2016
7040e49
lint: group preprocess in sub-routines
etpinard Jul 29, 2016
5e04f37
lint: group stats in sub-routines
etpinard Jul 29, 2016
aca8fa0
tasks: hard code mapbox access token in constants.js
etpinard Jul 29, 2016
30c1b86
tasks: merge CI vs local run cmd logic in container command module
etpinard Jul 29, 2016
640bdb4
tasks: add docker run command
etpinard Jul 29, 2016
584e644
tasks: refactor pretest
etpinard Jul 29, 2016
0001833
tasks: use common module in container tasks
etpinard Jul 29, 2016
1c91068
rm sub-tasks on circle.yml that are now part of `pretest`
etpinard Jul 29, 2016
a793734
fixup: make container commands work on CI
etpinard Jul 29, 2016
d5cb921
tasks: a few stats.js fixups
etpinard Aug 2, 2016
f662025
tasks: merge format bundle make watchified bundle script
etpinard Aug 2, 2016
294cad0
DRY: use watchified bundle util in test dashboard server
etpinard Aug 2, 2016
c479fe6
rm useless eslintrc
etpinard Aug 2, 2016
8a4f5ee
tasks: add formatTime common util
etpinard Aug 4, 2016
e5ee342
tasks: add get time last modified util
etpinard Aug 4, 2016
7c0562f
lint & comment
etpinard Aug 5, 2016
cfda7d3
tasks: add sleep (try to make pretest make consistent)
etpinard Aug 5, 2016
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
13 changes: 4 additions & 9 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -38,26 +38,21 @@ We use the following [labels](https://github.com/plotly/plotly.js/labels) to tra

#### Step 1: Clone the plotly.js repo and install its dependencies

```
```bash
git clone https://github.com/plotly/plotly.js.git
cd plotly.js
npm install
```

#### Step 2: Setup Mapbox access token

As of `v1.13.0`, plotly.js includes a [`mapbox-gl`](https://github.com/mapbox/mapbox-gl-js) integration. Creating `mapbox-gl` graphs requires an
[`accessToken`](https://www.mapbox.com/help/define-access-token/). To make sure
that the plotly.js test suites and devtools work properly, locate your Mapbox access
token and run:
#### Step 2: Setup test environment

```bash
export MAPBOX_ACCESS_TOKEN="<your access token>" && npm run pretest
npm run pretest
```

#### Step 3: Start the test dashboard

```
```bash
npm start
```

Expand Down
9 changes: 3 additions & 6 deletions circle.yml
Original file line number Diff line number Diff line change
Expand Up @@ -9,20 +9,17 @@ machine:
services:
- docker


dependencies:
pre:
- docker pull plotly/testbed:latest
post:
- npm run cibuild
- npm run pretest
- docker run -d --name mytestbed -v $PWD:/var/www/streambed/image_server/plotly.js -p 9010:9010 plotly/testbed:latest
- sudo ./tasks/run_in_testbed.sh mytestbed "cp -f test/image/index.html ../server_app/index.html"
- wget --server-response --spider --tries=8 --retry-connrefused http://localhost:9010/ping

test:
override:
- sudo ./tasks/run_in_testbed.sh mytestbed "export CIRCLECI=1 && node test/image/compare_pixels_test.js"
- sudo ./tasks/run_in_testbed.sh mytestbed "node test/image/export_test.js"
- npm run test-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.

all these ⏫ are now part of npm run pretest

- npm run test-export
Copy link
Contributor Author

Choose a reason for hiding this comment

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

the local vs CI differences are now handled at the task-runner script level.

- npm run citest-jasmine
- npm run test-bundle
- npm run test-syntax
Expand Down
35 changes: 5 additions & 30 deletions devtools/test_dashboard/server.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,9 @@ var http = require('http');
var ecstatic = require('ecstatic');
var open = require('open');
var browserify = require('browserify');
var watchify = require('watchify');

var constants = require('../../tasks/util/constants');
var compress = require('../../tasks/util/compress_attributes');
var makeWatchifiedBundle = require('../../tasks/util/watchified_bundle');
var shortcutPaths = require('../../tasks/util/shortcut_paths');

var PORT = process.argv[2] || 3000;
Expand All @@ -20,31 +19,22 @@ var server = http.createServer(ecstatic({
gzip: true
}));

// Bundle development source code
var b = browserify(constants.pathToPlotlyIndex, {
debug: true,
standalone: 'Plotly',
transform: [compress],
cache: {},
packageCache: {},
plugin: [watchify]
// Make watchified bundle for plotly.js
var bundlePlotly = makeWatchifiedBundle(function() {
// open up browser window on first bundle callback
open('http://localhost:' + PORT + '/devtools/test_dashboard');
});
b.on('update', bundlePlotly);

// Bundle devtools code
var devtoolsPath = path.join(constants.pathToRoot, 'devtools/test_dashboard');
var devtools = browserify(path.join(devtoolsPath, 'devtools.js'), {
transform: [shortcutPaths]
});

var firstBundle = true;


// Start the server up!
server.listen(PORT);

// Build and bundle all the things!
console.log('Building the first bundle. This might take a little while...\n');
getMockFiles()
.then(readFiles)
.then(createMocksList)
Expand Down Expand Up @@ -140,21 +130,6 @@ function writeFilePromise(path, contents) {
});
}

function bundlePlotly() {
b.bundle(function(err) {
if(err) {
console.error('Error while bundling!', JSON.stringify(String(err)));
} else {
console.log('Bundle updated at ' + new Date().toLocaleTimeString());
}

if(firstBundle) {
open('http://localhost:' + PORT + '/devtools/test_dashboard');
firstBundle = false;
}
}).pipe(fs.createWriteStream(constants.pathToPlotlyBuild));
}

function bundleDevtools() {
return new Promise(function(resolve, reject) {
devtools.bundle(function(err) {
Expand Down
10 changes: 5 additions & 5 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -27,21 +27,21 @@
"stats": "node tasks/stats.js",
"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_plotly.js",
"watch": "node tasks/watch.js",
"lint": "eslint . || true",
"lint-fix": "eslint . --fix",
"pretest": "node tasks/pretest.js",
"test-jasmine": "karma start test/jasmine/karma.conf.js",
"citest-jasmine": "karma start test/jasmine/karma.ciconf.js",
"test-image": "./tasks/test_image.sh",
"test-export": "./tasks/test_export.sh",
"test-syntax": "node test/syntax_test.js",
"test-image": "node tasks/test_image.js",
"test-export": "node tasks/test_export.js",
"test-syntax": "node tasks/test_syntax.js",
"test-bundle": "node tasks/test_bundle.js",
"test": "npm run citest-jasmine && npm run test-image && npm run test-syntax && npm run test-bundle",
"start-test_dashboard": "node devtools/test_dashboard/server.js",
"start-image_viewer": "node devtools/image_viewer/server.js",
"start": "npm run start-test_dashboard",
"baseline": "./tasks/baseline.sh",
"baseline": "node tasks/baseline.js",
Copy link
Contributor

Choose a reason for hiding this comment

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

Who needs make when you have npm!

"preversion": "npm-link-check && npm dedupe",
"version": "npm run build && git add -A dist src build",
"postversion": "git push && git push --tags"
Expand Down
17 changes: 17 additions & 0 deletions tasks/baseline.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
var constants = require('./util/constants');
var common = require('./util/common');
var containerCommands = require('./util/container_commands');

var msg = [
'Generating baseline image(s) using build/plotly.js from',
common.getTimeLastModified(constants.pathToPlotlyBuild),
'\n'
].join(' ');

var cmd = containerCommands.getRunCmd(
process.env.CIRCLECI,
'node test/image/make_baseline.js ' + process.argv.slice(2).join(' ')
);

console.log(msg);
common.execCmd(cmd);
20 changes: 0 additions & 20 deletions tasks/baseline.sh

This file was deleted.

11 changes: 4 additions & 7 deletions tasks/bundle.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,10 @@ var path = require('path');
var browserify = require('browserify');
var UglifyJS = require('uglify-js');

var compressAttributes = require('./util/compress_attributes');
var constants = require('./util/constants');
var common = require('./util/common');
var compressAttributes = require('./util/compress_attributes');
var doesFileExist = common.doesFileExist;

/*
* This script takes one argument
Expand All @@ -23,18 +25,13 @@ var DEV = (arg === 'dev') || (arg === '--dev');


// Check if style and font build files are there
try {
fs.statSync(constants.pathToCSSBuild).isFile();
fs.statSync(constants.pathToFontSVGBuild).isFile();
}
catch(e) {
if(!doesFileExist(constants.pathToCSSBuild) || !doesFileExist(constants.pathToFontSVG)) {
throw new Error([
'build/ is missing one or more files',
'Please run `npm run preprocess` first'
].join('\n'));
}


// Browserify plotly.js
_bundle(constants.pathToPlotlyIndex, constants.pathToPlotlyDist, {
standalone: 'Plotly',
Expand Down
11 changes: 4 additions & 7 deletions tasks/cibundle.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,9 @@ var fs = require('fs');

var browserify = require('browserify');

var compressAttributes = require('./util/compress_attributes');
var constants = require('./util/constants');
var common = require('./util/common');
var compressAttributes = require('./util/compress_attributes');

/*
* Trimmed down version of ./bundle.js for CI testing
Expand All @@ -20,17 +21,13 @@ browserify(constants.pathToPlotlyIndex, {
standalone: 'Plotly',
transform: [compressAttributes]
})
.bundle(function(err) {
if(err) throw err;
})
.bundle(common.throwOnError)
.pipe(fs.createWriteStream(constants.pathToPlotlyBuild));


// Browserify the geo assets
browserify(constants.pathToPlotlyGeoAssetsSrc, {
standalone: 'PlotlyGeoAssets'
})
.bundle(function(err) {
if(err) throw err;
})
.bundle(common.throwOnError)
.pipe(fs.createWriteStream(constants.pathToPlotlyGeoAssetsDist));
9 changes: 3 additions & 6 deletions tasks/header.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ var falafel = require('falafel');
var glob = require('glob');

var constants = require('./util/constants');
var common = require('./util/common');

// main
addHeadersInDistFiles();
Expand All @@ -14,9 +15,7 @@ updateHeadersInSrcFiles();
// add headers to dist files
function addHeadersInDistFiles() {
function _prepend(path, header) {
prependFile(path, header + '\n', function(err) {
if(err) throw err;
});
prependFile(path, header + '\n', common.throwOnError);
}

// add header to main dist bundles
Expand Down Expand Up @@ -77,9 +76,7 @@ function updateHeadersInSrcFiles() {

var newCode = licenseSrc + '\n' + codeLines.join('\n');

fs.writeFile(file, newCode, function(err) {
if(err) throw err;
});
common.writeFile(file, newCode);
}
else {
// otherwise, throw an error
Expand Down
56 changes: 34 additions & 22 deletions tasks/preprocess.js
Original file line number Diff line number Diff line change
@@ -1,35 +1,47 @@
var fs = require('fs-extra');

var sass = require('node-sass');

var constants = require('./util/constants');
var common = require('./util/common');
var pullCSS = require('./util/pull_css');
var pullFontSVG = require('./util/pull_font_svg');
var updateVersion = require('./util/update_version');
var constants = require('./util/constants');

// main
makeBuildCSS();
makeBuildFontSVG();
copyTopojsonFiles();
updateVersion(constants.pathToPlotlyCore);
updateVersion(constants.pathToPlotlyGeoAssetsSrc);

// convert scss to css
sass.render({
file: constants.pathToSCSS,
outputStyle: 'compressed'
}, function(err, result) {
if(err) console.log('SASS error');
// convert scss to css to js
function makeBuildCSS() {
sass.render({
file: constants.pathToSCSS,
outputStyle: 'compressed'
}, function(err, result) {
if(err) throw err;

// css to js
pullCSS(String(result.css), constants.pathToCSSBuild);
});
// css to js
pullCSS(String(result.css), constants.pathToCSSBuild);
});
}

// convert font svg into js
fs.readFile(constants.pathToFontSVG, function(err, data) {
pullFontSVG(data.toString(), constants.pathToFontSVGBuild);
});
function makeBuildFontSVG() {
fs.readFile(constants.pathToFontSVG, function(err, data) {
if(err) throw err;

// copy topojson files from sane-topojson to dist/
fs.copy(constants.pathToTopojsonSrc, constants.pathToTopojsonDist,
{ clobber: true },
function(err) { if(err) throw err; }
);
pullFontSVG(data.toString(), constants.pathToFontSVGBuild);
});
}

// inject package version into source index files
updateVersion(constants.pathToPlotlyCore);
updateVersion(constants.pathToPlotlyGeoAssetsSrc);
// copy topojson files from sane-topojson to dist/
function copyTopojsonFiles() {
fs.copy(
constants.pathToTopojsonSrc,
constants.pathToTopojsonDist,
{ clobber: true },
common.throwOnError
);
}
Loading