Merge pull request #1569 from plotly/better-image-test-logs

Better image test logs

Author: etpinard

.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`.

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

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