diff --git a/.babelrc b/.babelrc index 977c5cc..45a72eb 100644 --- a/.babelrc +++ b/.babelrc @@ -1,5 +1,8 @@ { "presets": [ "airbnb" + ], + "plugins": [ + "transform-object-rest-spread" ] } diff --git a/.importjs.js b/.importjs.js index d77a493..f3ee12f 100644 --- a/.importjs.js +++ b/.importjs.js @@ -8,6 +8,9 @@ module.exports = { } return []; }, + excludes: [ + './lib/**', + ], declarationKeyword: function({ pathToCurrentFile }) { if (isNodeFile(pathToCurrentFile)) { return 'const'; diff --git a/.node-version b/.node-version new file mode 100644 index 0000000..dc3829f --- /dev/null +++ b/.node-version @@ -0,0 +1 @@ +6.9.1 diff --git a/.travis.yml b/.travis.yml index b92f055..6e29839 100644 --- a/.travis.yml +++ b/.travis.yml @@ -2,13 +2,11 @@ language: node_js sudo: false node_js: - - "4" - - "5" - "6" - "7" addons: - firefox: "47.0.1" + firefox: "50.0" before_install: - "export DISPLAY=:99.0" diff --git a/README.md b/README.md index e8179b7..338166f 100644 --- a/README.md +++ b/README.md @@ -37,8 +37,9 @@ npm install -g happo You'll also need Firefox installed on the machine. Happo uses [selenium-webdriver](https://github.com/SeleniumHQ/selenium) under the hood, -and will support whatever version Selenium supports. Happo currently works best -with _Firefox 47.0.1_. +and will support the same version of Firefox as Selenium supports. Happo +currently works best with _Firefox > 50_. It uses +[geckodriver](https://github.com/mozilla/geckodriver) to control Firefox. ## Introduction @@ -317,18 +318,26 @@ the argument to `happo upload`. E.g. happo upload "https://test.example" ``` -To debug uploading, you can use the `--debug` flag which will print additional -information to `stderr`. +To debug uploading, override the `uploader` configuration option with a +debug-enabled `S3Uploader` instance. This will print additional information to +`stderr`. + +```js +const S3Uploader = require('happo/lib/server/S3Uploader'); + +module.exports = { + uploader: () => new S3Uploader({ debug: true }); +} +``` ### `happo upload-test` Uploads a small text file to an AWS S3 account. This is useful if you want to test your S3 configuration. Uses the same configuration as [`happo -upload`](#happo-upload-triggeredbyurl) does. As with `happo upload`, you can -apply a `--debug` flag here for a more verbose output. +upload`](#happo-upload-triggeredbyurl) does. ```sh -happo upload-test --debug +happo upload-test ``` ## Running in a CI environment diff --git a/geckodriver/darwin-x64/geckodriver b/geckodriver/darwin-x64/geckodriver new file mode 100755 index 0000000..ac4d5ea Binary files /dev/null and b/geckodriver/darwin-x64/geckodriver differ diff --git a/geckodriver/linux-x64/geckodriver b/geckodriver/linux-x64/geckodriver new file mode 100755 index 0000000..771dcc1 Binary files /dev/null and b/geckodriver/linux-x64/geckodriver differ diff --git a/package.json b/package.json index 757edb6..ed1745e 100644 --- a/package.json +++ b/package.json @@ -1,19 +1,19 @@ { "name": "happo", - "version": "3.0.2", + "version": "4.0.0-beta.2", "description": "Command-line tool to visually diff JavaScript components", "bin": { "happo": "./bin/happo" }, "scripts": { - "build": "npm run --silent clean && babel src/server/ -d lib/server/ --ignore __tests__ && npm run --silent webpack", + "babel": "babel src/server/ -d lib/server/ --ignore __tests__", + "build": "npm run clean && npm run babel && npm run webpack", "clean": "rimraf lib public/*.worker.js public/*.bundle.js*", "jest": "jest", - "jest:watch": "jest --watch", "lint": "eslint --ext .js,.jsx .", - "prepublish": "npm run --silent lint && npm run --silent build", - "pretest": "npm run --silent lint", - "test": "npm run --silent jest", + "prepublish": "npm run lint && npm run build", + "pretest": "npm run lint", + "test": "npm run jest", "webpack": "webpack --optimize-minimize", "webpack-dev": "webpack --devtool eval-source-map --progress --watch" }, @@ -30,8 +30,9 @@ "devDependencies": { "babel-cli": "^6.18.0", "babel-core": "^6.14.0", - "babel-jest": "^17.0.2", + "babel-jest": "^18.0.0", "babel-loader": "^6.2.5", + "babel-plugin-transform-object-rest-spread": "^6.20.2", "babel-polyfill": "^6.13.0", "babel-preset-airbnb": "^2.1.1", "eslint": "^3.10.0", @@ -39,7 +40,7 @@ "eslint-plugin-import": "^2.1.0", "eslint-plugin-jsx-a11y": "^2.2.3", "eslint-plugin-react": "^6.7.1", - "jest": "^17.0.3", + "jest": "^18.1.0", "react": "^15.4.1", "react-dom": "^15.4.1", "react-waypoint": "^4.1.0", @@ -53,9 +54,9 @@ "ejs": "^2.5.2", "express": "^4.14.0", "mkdirp": "^0.5.1", - "png-crop": "0.0.1", - "pngjs": "^2.3.1", - "selenium-webdriver": "^2.53.3", + "pngjs": "^3.0.0", + "ps-node": "^0.1.4", + "selenium-webdriver": "^3.0.1", "worker-loader": "^0.7.1" }, "jest": { diff --git a/src/HappoRunner.js b/src/HappoRunner.js index 51f26e2..e81ff25 100644 --- a/src/HappoRunner.js +++ b/src/HappoRunner.js @@ -1,3 +1,4 @@ +import { SCREENSHOT_BOX_ID } from './server/Constants'; import getFullRect from './getFullRect'; import waitForImagesToRender from './waitForImagesToRender'; @@ -170,6 +171,17 @@ window.happo = { width, } = getFullRect(rootNodes); + // We place an (invisible) box on top of the visible rectangle. We + // then use it as the screenshot target. + const screenshotBox = document.createElement('div'); + screenshotBox.style.position = 'absolute'; + screenshotBox.style.left = `${left}px`; + screenshotBox.style.top = `${top}px`; + screenshotBox.style.width = `${width}px`; + screenshotBox.style.height = `${height}px`; + screenshotBox.setAttribute('id', SCREENSHOT_BOX_ID); + document.body.appendChild(screenshotBox); + resolve({ description: currentExample.description, height, diff --git a/src/server/Constants.js b/src/server/Constants.js new file mode 100644 index 0000000..5db32d7 --- /dev/null +++ b/src/server/Constants.js @@ -0,0 +1,3 @@ +module.exports = { + SCREENSHOT_BOX_ID: 'happo-screenshot-box', +}; diff --git a/src/server/__tests__/checkBrowserVersion-test.js b/src/server/__tests__/checkBrowserVersion-test.js new file mode 100644 index 0000000..5b1039a --- /dev/null +++ b/src/server/__tests__/checkBrowserVersion-test.js @@ -0,0 +1,61 @@ +jest.mock('child_process'); +const childProcess = require('child_process'); + +jest.mock('../config'); +const config = require('../config'); + +const checkBrowserVersion = require('../checkBrowserVersion'); + +beforeEach(() => { + config.driver = 'firefox'; +}); + +describe('when firefox is new enough', () => { + beforeEach(() => { + childProcess.exec = (_, cb) => cb(null, 'Mozilla Firefox 50.0'); + }); + + it('resolves', () => + checkBrowserVersion().then(() => { + expect(true).toBe(true); + })); +}); + +describe('when firefox is too old', () => { + beforeEach(() => { + childProcess.exec = (_, cb) => cb(null, 'Mozilla Firefox 47.0'); + }); + + it('rejects', () => + checkBrowserVersion().catch((error) => { + expect(error.message).toEqual( + 'Happo requires Firefox version 50 or later. You are using Mozilla Firefox 47.0'); + })); +}); + +describe('when the version string is unrecognized', () => { + beforeEach(() => { + childProcess.exec = (_, cb) => cb(null, 'Godzilla Firefox 47.0'); + }); + + it('rejects', () => + checkBrowserVersion().catch((error) => { + expect(error.message).toEqual( + 'Failed to parse Firefox version string "Godzilla Firefox 47.0"'); + })); +}); + +describe('when using a different driver', () => { + beforeEach(() => { + config.driver = 'chrome'; + + // Just to make sure that we're not falsely succeeding because of normal + // handling, we also mock an old version of Firefox. + childProcess.exec = (_, cb) => cb(null, 'Mozilla Firefox 47.0'); + }); + + it('resolves', () => + checkBrowserVersion().then(() => { + expect(true).toBe(true); + })); +}); diff --git a/src/server/__tests__/runVisualDiffs-test.js b/src/server/__tests__/runVisualDiffs-test.js index d189127..70e9c61 100644 --- a/src/server/__tests__/runVisualDiffs-test.js +++ b/src/server/__tests__/runVisualDiffs-test.js @@ -1,13 +1,18 @@ const os = require('os'); const path = require('path'); +const jest = require('jest'); const rimraf = require('rimraf'); const { config } = require('../config'); +const closeDriver = require('../closeDriver'); const initializeWebdriver = require('../initializeWebdriver'); const runVisualDiffs = require('../runVisualDiffs'); const server = require('../server'); +jest.mock('../checkBrowserVersion'); +const checkBrowserVersion = require('../checkBrowserVersion'); + describe('runVisualDiffs', () => { let driver; let startedServer; @@ -25,11 +30,12 @@ describe('runVisualDiffs', () => { afterAll(() => { startedServer.close(); - driver.close(); + return closeDriver(driver); }); beforeEach(() => { originalConfig = { ...config }; + checkBrowserVersion.mockImplementation(() => Promise.resolve()); }); afterEach(() => { diff --git a/src/server/addGeckoDriverToPath.js b/src/server/addGeckoDriverToPath.js new file mode 100644 index 0000000..021414d --- /dev/null +++ b/src/server/addGeckoDriverToPath.js @@ -0,0 +1,8 @@ +const path = require('path'); + +const { platform, arch } = process; + +const geckodriverFolder = path.join(__dirname, + `../../geckodriver/${platform}-${arch}/`); + +process.env.PATH += path.delimiter + geckodriverFolder; diff --git a/src/server/checkBrowserVersion.js b/src/server/checkBrowserVersion.js new file mode 100644 index 0000000..2daec94 --- /dev/null +++ b/src/server/checkBrowserVersion.js @@ -0,0 +1,37 @@ +const childProcess = require('child_process'); + +const firefox = require('selenium-webdriver/firefox'); + +const config = require('./config'); + +const MINIMUM_FIREFOX_VERSION = 50.0; +const FIREFOX_VERSION_MATCHER = /Mozilla Firefox ([0-9.]+)/; + +module.exports = function checkBrowserVersion() { + if (config.driver !== 'firefox') { + // Our main browser target is Firefox. If you are using something else, you + // must know what you are doing. :) + return Promise.resolve(); + } + + return new Promise((resolve, reject) => { + new firefox.Binary().locate().then((pathToExecutable) => { + childProcess.exec(`${pathToExecutable} --version`, (error, stdout) => { + if (error) { + reject(new Error(`Failed to check Firefox version: ${error}`)); + return; + } + const match = stdout.match(FIREFOX_VERSION_MATCHER); + if (!match) { + reject(new Error(`Failed to parse Firefox version string "${stdout}"`)); + } else if (parseFloat(match[1]) < MINIMUM_FIREFOX_VERSION) { + reject(new Error( + `Happo requires Firefox version ${MINIMUM_FIREFOX_VERSION} or later. ` + + `You are using ${stdout}`)); + } else { + resolve(); + } + }); + }); + }); +}; diff --git a/src/server/cli.js b/src/server/cli.js index 1542ea0..a59a8f5 100644 --- a/src/server/cli.js +++ b/src/server/cli.js @@ -2,7 +2,9 @@ const crypto = require('crypto'); const commander = require('commander'); -const S3Uploader = require('./S3Uploader'); +const { config } = require('./config'); +const checkBrowserVersion = require('./checkBrowserVersion'); +const closeDriver = require('./closeDriver'); const constructUrl = require('./constructUrl'); const initializeWebdriver = require('./initializeWebdriver'); const runVisualDiffs = require('./runVisualDiffs'); @@ -21,19 +23,22 @@ commander.command('debug').action(() => { }); commander.command('run').action(() => { - server.start().then(() => { - initializeWebdriver().then((driver) => { + server.start() + .then(checkBrowserVersion) + .then(initializeWebdriver) + .then((driver) => { runVisualDiffs(driver) .then(() => { - driver.close(); - process.exit(0); + closeDriver(driver).then(() => { + process.exit(0); + }); }) .catch((error) => { - driver.close(); - logAndExit(error); + closeDriver(driver).then(() => { + logAndExit(error); + }); }); }); - }); }); commander.command('review').action(() => { @@ -48,9 +53,9 @@ commander.command('review-demo').action(() => { }); }); -commander.command('upload []').option('--debug').action( - (triggeredByUrl, { debug }) => { - uploadLastResult(triggeredByUrl, { debug }) +commander.command('upload []').action( + (triggeredByUrl) => { + uploadLastResult(triggeredByUrl) .then((url) => { if (url) { console.log(url); @@ -59,8 +64,8 @@ commander.command('upload []').option('--debug').action( .catch(logAndExit); }); -commander.command('upload-test').option('--debug').action(({ debug }) => { - const uploader = new S3Uploader({ debug }); +commander.command('upload-test').action(() => { + const uploader = config.uploader(); uploader.prepare() .then(() => { uploader.upload({ diff --git a/src/server/closeDriver.js b/src/server/closeDriver.js new file mode 100644 index 0000000..3c453b3 --- /dev/null +++ b/src/server/closeDriver.js @@ -0,0 +1,36 @@ +const psNode = require('ps-node'); + +function kill(pid) { + return new Promise((resolve, reject) => { + psNode.kill(pid, (err) => { + if (err) { + reject(err); + } else { + resolve(); + } + }); + }); +} + +// Works around an issue in marionette causing ghost firefox instances to remain +// running. +// +// https://bugzilla.mozilla.org/show_bug.cgi?id=1310992 +// https://github.com/mozilla/geckodriver/issues/285 +module.exports = function closeDriver(driver) { + return new Promise((resolve, reject) => { + driver.close(); + psNode.lookup({ + command: 'firefox', + arguments: '--marionette', + }, (err, externalProcesses) => { + if (err) { + reject(err); + return; + } + Promise.all(externalProcesses.map(({ pid }) => kill(pid))) + .then(resolve) + .catch(reject); + }); + }); +}; diff --git a/src/server/config.js b/src/server/config.js index 9a60290..805ac15 100644 --- a/src/server/config.js +++ b/src/server/config.js @@ -2,6 +2,8 @@ const fs = require('fs'); const path = require('path'); +const S3Uploader = require('./S3Uploader'); + const defaultConfig = { bind: 'localhost', driver: 'firefox', @@ -9,6 +11,7 @@ const defaultConfig = { snapshotsFolder: 'snapshots', resultSummaryFilename: 'resultSummary.json', publicDirectories: [], + uploader: () => new S3Uploader(), sourceFiles: [], stylesheets: [], viewports: { diff --git a/src/server/initializeWebdriver.js b/src/server/initializeWebdriver.js index b0bbbdd..f4c8e05 100644 --- a/src/server/initializeWebdriver.js +++ b/src/server/initializeWebdriver.js @@ -1,3 +1,5 @@ +require('./addGeckoDriverToPath'); + const seleniumWebdriver = require('selenium-webdriver'); const { config } = require('./config'); diff --git a/src/server/runVisualDiffs.js b/src/server/runVisualDiffs.js index 604e0b1..5ef062c 100644 --- a/src/server/runVisualDiffs.js +++ b/src/server/runVisualDiffs.js @@ -1,9 +1,10 @@ const fs = require('fs'); const path = require('path'); +const { By } = require('selenium-webdriver'); const mkdirp = require('mkdirp'); -const pngCrop = require('png-crop'); +const { SCREENSHOT_BOX_ID } = require('./Constants'); const { config } = require('./config'); const constructUrl = require('./constructUrl'); const pathToSnapshot = require('./pathToSnapshot'); @@ -86,27 +87,22 @@ function getImageFromStream(stream) { }); } -function takeCroppedScreenshot({ driver, width, height, top, left }) { +function takeCroppedScreenshot({ driver }) { return new Promise((resolve, reject) => { - driver.takeScreenshot().then((screenshot) => { - const cropConfig = { width, height, top, left }; - // TODO we might need to guard against overcropping or - // undercropping here, depending on png-crop's behavior. - - // This is deprecated in Node 6. We will eventually need to change - // this to: - // - // Buffer.from(screenshot, 'base64') - const screenshotBuffer = new Buffer(screenshot, 'base64'); - - pngCrop.cropToStream(screenshotBuffer, cropConfig, (error, outputStream) => { - if (error) { - reject(error); - return; - } - getImageFromStream(outputStream).then(resolve); - }); - }); + driver.findElement(By.id(SCREENSHOT_BOX_ID)).then((overlay) => { + overlay.takeScreenshot().then((screenshot) => { + // This is deprecated in Node 6. We will eventually need to change + // this to: + // + // Buffer.from(screenshot, 'base64') + const screenshotBuffer = new Buffer(screenshot, 'base64'); + const png = new PNG(); + png.on('parsed', function handlePngParsed() { + resolve(this); + }); + png.parse(screenshotBuffer); + }).catch(reject); + }).catch(reject); }); } diff --git a/src/server/uploadLastResult.js b/src/server/uploadLastResult.js index bcfc78d..fcf587a 100644 --- a/src/server/uploadLastResult.js +++ b/src/server/uploadLastResult.js @@ -3,7 +3,7 @@ const path = require('path'); const ejs = require('ejs'); -const S3Uploader = require('./S3Uploader'); +const { config } = require('./config'); const getLastResultSummary = require('./getLastResultSummary'); const pageTitle = require('./pageTitle'); const pathToSnapshot = require('./pathToSnapshot'); @@ -62,11 +62,10 @@ function uploadHTMLFile({ uploader, diffImages, newImages, triggeredByUrl }) { /** * @param {String} triggeredByUrl - * @param {Boolean} options.debug * @return {Promise} that resolves with a URL to the html document uploaded to * s3. */ -module.exports = function uploadLastResult(triggeredByUrl, { debug } = {}) { +module.exports = function uploadLastResult(triggeredByUrl) { return new Promise((resolve, reject) => { const { diffImages, newImages } = getLastResultSummary(); @@ -75,7 +74,7 @@ module.exports = function uploadLastResult(triggeredByUrl, { debug } = {}) { return; } - const uploader = new S3Uploader({ debug }); + const uploader = config.uploader(); uploader.prepare().then(() => { const uploadPromises = []; diffImages.forEach((image) => {