From 7a492cfbc635127b4bc076119e6f8bf8ae7dea67 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89tienne=20T=C3=A9treault-Pinard?= Date: Mon, 3 Apr 2017 17:37:11 -0400 Subject: [PATCH 1/4] generalize docker-run cmd --- tasks/util/container_commands.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tasks/util/container_commands.js b/tasks/util/container_commands.js index 938cf694993..39c96676186 100644 --- a/tasks/util/container_commands.js +++ b/tasks/util/container_commands.js @@ -29,7 +29,7 @@ containerCommands.dockerRun = [ '--name', constants.testContainerName, '-v', constants.pathToRoot + ':' + constants.testContainerHome, '-p', constants.testContainerPort + ':' + constants.testContainerPort, - 'plotly/testbed:latest' + constants.testContainerImage ].join(' '); containerCommands.getRunCmd = function(isCI, commands) { From 3ebe7976d5d54ec00372bd4a6b984ba9781bfe3f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89tienne=20T=C3=A9treault-Pinard?= Date: Fri, 7 Apr 2017 16:17:14 -0400 Subject: [PATCH 2/4] robustify error handling in px comparison test - add handle for request errors, to catch connections errors that can happen when the image server blows up - log error messages directly to stdout, so that they are written to the stdout as they happen in a child process exec block. --- test/image/compare_pixels_test.js | 20 ++++++++++++++++---- 1 file changed, 16 insertions(+), 4 deletions(-) diff --git a/test/image/compare_pixels_test.js b/test/image/compare_pixels_test.js index ce210f88f88..5e053bb8949 100644 --- a/test/image/compare_pixels_test.js +++ b/test/image/compare_pixels_test.js @@ -212,6 +212,10 @@ function comparePixels(mockName, cb) { imagePaths = getImagePaths(mockName), saveImageStream = fs.createWriteStream(imagePaths.test); + function log(msg) { + process.stdout.write('Error for', mockName + ':', msg); + } + function checkImage() { // baseline image must be generated first @@ -253,8 +257,8 @@ function comparePixels(mockName, cb) { function onEqualityCheck(err, isEqual) { if(err) { common.touch(imagePaths.diff); - console.error(err); - return; + log(err) + return cb(false, mockName); } if(isEqual) { fs.unlinkSync(imagePaths.diff); @@ -266,12 +270,20 @@ function comparePixels(mockName, cb) { // 525 means a plotly.js error function onResponse(response) { if(+response.statusCode === 525) { - console.error('plotly.js error while generating', mockName); - cb(false, mockName); + log('plotly.js error') + return cb(false, mockName); } } + // this catches connection errors + // e.g. when the image server blows up + function onError(err) { + log(err) + return cb(false, mockName); + } + request(requestOpts) + .on('error', onError) .on('response', onResponse) .pipe(saveImageStream) .on('close', checkImage); From d58a7b5c3fa5d9c9ca5b97fcd3f57344ac46b888 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89tienne=20T=C3=A9treault-Pinard?= Date: Fri, 7 Apr 2017 16:22:19 -0400 Subject: [PATCH 3/4] add info about 'Allow edits from maintainers' option in PR template --- .github/PULL_REQUEST_TEMPLATE.md | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/.github/PULL_REQUEST_TEMPLATE.md b/.github/PULL_REQUEST_TEMPLATE.md index 52270074d0b..c026625bd46 100644 --- a/.github/PULL_REQUEST_TEMPLATE.md +++ b/.github/PULL_REQUEST_TEMPLATE.md @@ -4,8 +4,9 @@ Developers are strongly encouraged to first make a PR to their own plotly.js for Before opening a pull request, developer should: -- `git rebase` their local branch off the latest `master` -- make sure to **not** `git add` the `dist/` folder (the `dist/` is updated only on verion bumps) -- write an overview of what the PR attempts to do. +- `git rebase` their local branch off the latest `master`, +- make sure to **not** `git add` the `dist/` folder (the `dist/` is updated only on verion bumps), +- write an overview of what the PR attempts to do, +- select the _Allow edits from maintainers_ option (see this [article](https://help.github.com/articles/allowing-changes-to-a-pull-request-branch-created-from-a-fork/) for more details). Note that it is forbidden to force push (i.e. `git push -f`) to remote branches associated with opened pull requests. Force pushes make it hard for maintainers to keep track of updates. Therefore, if required, please `git merge master` into your PR branch instead of `git rebase master`. From b55e41185124810c23d7d3760e700500d2b79048 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89tienne=20T=C3=A9treault-Pinard?= Date: Fri, 7 Apr 2017 17:21:49 -0400 Subject: [PATCH 4/4] ; :rage: --- test/image/compare_pixels_test.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/test/image/compare_pixels_test.js b/test/image/compare_pixels_test.js index 5e053bb8949..3a51c9182cd 100644 --- a/test/image/compare_pixels_test.js +++ b/test/image/compare_pixels_test.js @@ -257,7 +257,7 @@ function comparePixels(mockName, cb) { function onEqualityCheck(err, isEqual) { if(err) { common.touch(imagePaths.diff); - log(err) + log(err); return cb(false, mockName); } if(isEqual) { @@ -270,7 +270,7 @@ function comparePixels(mockName, cb) { // 525 means a plotly.js error function onResponse(response) { if(+response.statusCode === 525) { - log('plotly.js error') + log('plotly.js error'); return cb(false, mockName); } } @@ -278,7 +278,7 @@ function comparePixels(mockName, cb) { // this catches connection errors // e.g. when the image server blows up function onError(err) { - log(err) + log(err); return cb(false, mockName); }