Add UprightDiff support and render pages serially

tstarling · tstarling · commit 35e3024a046f · 2015-12-04T16:52:53.000+11:00
* Render pages one after the other, rather than sending navigation
  commands immediately, to prevent Phantom navigating away from the
  page being viewed before the screenshot is done. This is not too
  inefficient since images are cached between the two views.
* Add a configurable delay between the page load event and the
  screenshot, to allow scripts to run on load.
* Start Phantom in debug mode, since that gives lots of useful output.
* Allow UprightDiff to be used, instead of resemble
diff --git a/bin/gen.visual_diff.js b/bin/gen.visual_diff.js
@@ -30,11 +30,6 @@ if (opts !== null) {
 		if (!err) {
 			// analysis stats
 			console.error("STATS: " + JSON.stringify(data));
-
-			// Save the base64 data
-			var png_data = data.getImageDataUrl("").replace(/^data:image\/png;base64,/, '');
-			var png_buffer = new Buffer(png_data, 'base64');
-			fs.writeFileSync(opts.diffFile, png_buffer);
 		}
 	});
 }
diff --git a/diffserver/diffserver.settings.js.example b/diffserver/diffserver.settings.js.example
@@ -27,10 +27,19 @@ if (typeof module === 'object') {
 			stylesYamlFile: '../lib/parsoid.custom_styles.yaml',
 			injectJQuery: true,
 		},
+
+		// Engine for image diffs, may be resemble or uprightdiff
+		diffEngine: 'resemble',
+
 		// resemblejs options
-		outputSettings: {
+		resembleSettings: {
 			errorType: 'movement', // Better error display for making sense of diffs
 			largeImageThreshold: 0 // Cleaner diff without the skipped pixels grid
 		},
+
+		// UprightDiff options
+		uprightDiffSettings: {
+			binary: '/usr/bin/uprightdiff'
+		},
 	};
 }
diff --git a/lib/differ.js b/lib/differ.js
@@ -1,10 +1,11 @@
 'use strict';
 
-var p = require('phantom');
+var phantom = require('phantom');
 var resemble = require('resemble').resemble;
 var fs = require('fs');
 var yaml = require('libyaml');
 var Util = require('./differ.utils.js').Util;
+var child_process = require('child_process');
 
 // Export the differ module
 var VisualDiffer = {};
@@ -18,10 +19,11 @@ function testCompletion(browser, cb, opts) {
 	if (html1.err || html2.err) {
 		browser.exit();
 		cb(html1.err || html2.err);
-	}
-	if (html1.done && html2.done) {
+	} else if (html1.done && html2.done) {
 		browser.exit();
 		cb();
+	} else {
+		cb();
 	}
 }
 
@@ -86,6 +88,7 @@ VisualDiffer.takeScreenShot = function(browser, logger, cb, opts, htmlOpts) {
 						}
 
 						// Save the screenshot
+						console.log("Rendering", htmlOpts.screenShot);
 						page.render(htmlOpts.screenShot, function() {
 							logger(htmlOpts.name + ' done!');
 							htmlOpts.done = true;
@@ -97,11 +100,17 @@ VisualDiffer.takeScreenShot = function(browser, logger, cb, opts, htmlOpts) {
 				);
 			};
 
-			if (htmlOpts.injectJQuery) {
-				page.includeJs('http://ajax.googleapis.com/ajax/libs/jquery/1.9.1/jquery.min.js', processPage);
-			} else {
-				processPage();
-			}
+			setTimeout(
+				function() {
+					if (htmlOpts.injectJQuery) {
+						page.includeJs('http://ajax.googleapis.com/ajax/libs/jquery/1.9.1/jquery.min.js', processPage);
+					} else {
+						processPage();
+					}
+				},
+				opts.screenShotDelay
+			);
+
 		});
 	});
 };
@@ -128,12 +137,17 @@ VisualDiffer.takeScreenshots = function(opts, logger, cb) {
 	// Phantom doesn't like protocols in its proxy ips
 	// But, node.js request wants http:// proxies ... so dance around all that.
 	var proxy = (process.env.HTTP_PROXY_IP_AND_PORT || '').replace(/^https?:\/\//, '');
-	p.create('--ssl-protocol=TLSv1', '--proxy=' + proxy, function (browser) {
-		// HTML1 screenshot
-		self.takeScreenShot(browser, logger, cb, opts, opts.html1);
+	phantom.create('--debug=true', '--ssl-protocol=TLSv1', '--proxy=' + proxy, function (browser) {
+		function ss1(cb1) {
+			// HTML1 screenshot
+			self.takeScreenShot(browser, logger, cb1, opts, opts.html1);
+		}
 
-		// HTML2 screenshot
-		self.takeScreenShot(browser, logger, cb, opts, opts.html2);
+		function ss2() {
+			// HTML2 screenshot
+			self.takeScreenShot(browser, logger, cb, opts, opts.html2);
+		}
+		ss1(ss2);
 	});
 };
 
@@ -146,14 +160,45 @@ VisualDiffer.genVisualDiff = function(opts, logger, cb) {
 			if (logger) {
 				logger('--screenshotting done--');
 			}
-			if (opts.outputSettings) {
-				resemble.outputSettings(opts.outputSettings);
+			if (opts.diffEngine == "uprightdiff") {
+				VisualDiffer.compareWithUprightDiff(opts, logger, cb);
+			} else {
+				VisualDiffer.compareWithResemble(opts, logger, cb);
 			}
-			resemble(opts.html1.screenShot).compareTo(opts.html2.screenShot).
-				ignoreAntialiasing(). // <-- muy importante
-				onComplete(function(data){
-					cb(null, data);
-				});
 		}
 	});
 };
+
+VisualDiffer.compareWithResemble = function(opts, logger, cb) {
+	if (opts.outputSettings) {
+		resemble.outputSettings(opts.outputSettings);
+	}
+
+
+	resemble(opts.html1.screenShot).compareTo(opts.html2.screenShot).
+		ignoreAntialiasing(). // <-- muy importante
+		onComplete(function(data){
+			var png_data = data.getImageDataUrl("").replace(/^data:image\/png;base64,/, '');
+			var png_buffer = new Buffer(png_data, 'base64');
+			fs.writeFileSync(opts.diffFile, png_buffer);
+			cb(null, data);
+		});
+};
+
+VisualDiffer.compareWithUprightDiff = function(opts, logger, cb) {
+	child_process.execFile('/usr/local/bin/uprightdiff',
+		[
+			'--format=json',
+			opts.html1.screenShot, opts.html2.screenShot, opts.diffFile
+		],
+		function (error, stdout, stderr) {
+			if (error && error.code !== 0) {
+				error.stderr = stderr;
+				logger("UprightDiff exited with error: " + JSON.stringify(error));
+				cb(error, null);
+			} else {
+				cb(null, JSON.parse(stdout));
+			}
+		}
+	);
+};
diff --git a/lib/differ.utils.js b/lib/differ.utils.js
@@ -138,6 +138,14 @@ var standardOpts = {
 		description: 'flat OR movement (Pass-through option to resemble.js -- see resemblejs options)',
 		'default': "movement",
 	},
+	'diffEngine': {
+		description: "The diff engine to use, may be resemble or uprightdiff",
+		'default': 'resemble',
+	},
+	'screenShotDelay': {
+		description: "The amount of time (in seconds) to wait after the page load event before capturing a screenshot",
+		'default': 2,
+	},
 };
 
 

Original file line number	Diff line number	Diff line change
`@@ -30,11 +30,6 @@ if (opts !== null) {`
`30`	`30`	`if (!err) {`
`31`	`31`	`// analysis stats`
`32`	`32`	`console.error("STATS: " + JSON.stringify(data));`
`33`		`-`
`34`		`- // Save the base64 data`
`35`		`- var png_data = data.getImageDataUrl("").replace(/^data:image\/png;base64,/, '');`
`36`		`- var png_buffer = new Buffer(png_data, 'base64');`
`37`		`- fs.writeFileSync(opts.diffFile, png_buffer);`
`38`	`33`	`}`
`39`	`34`	`});`
`40`	`35`	`}`