From 30970a34873ab81eaa051d460d50b3e3d608db0a Mon Sep 17 00:00:00 2001 From: Andrew Mee Date: Thu, 21 Sep 2017 14:50:25 +0100 Subject: [PATCH] Update to use pa11y 5 beta and async/await (#33) * Update to use pa11y 5 beta and async/await * Correct package version to alpha version, update linting package * Remove filtering of non-errors as pa11y 5 no longer returns them by default --- .eslintrc.js | 2 +- .travis.yml | 6 +-- lib/pa11y-ci.js | 45 +++++++++++----------- package.json | 6 +-- test/integration/cli-erroring.js | 4 +- test/integration/cli-json.js | 2 +- test/integration/cli-mixed.js | 2 +- test/unit/lib/pa11y-ci.js | 65 ++++++++++++++------------------ test/unit/mock/pa11y.mock.js | 9 ++--- 9 files changed, 65 insertions(+), 76 deletions(-) diff --git a/.eslintrc.js b/.eslintrc.js index 1f67508..ac3db81 100644 --- a/.eslintrc.js +++ b/.eslintrc.js @@ -1,3 +1,3 @@ 'use strict'; -module.exports = require('pa11y-lint-config/eslint/es6'); +module.exports = require('pa11y-lint-config/eslint/es2017'); diff --git a/.travis.yml b/.travis.yml index 252fd05..272c0be 100644 --- a/.travis.yml +++ b/.travis.yml @@ -5,13 +5,11 @@ matrix: include: # Run linter once - - node_js: '6' + - node_js: '8' env: LINT=true # Run tests - - node_js: '4' - - node_js: '5' - - node_js: '6' + - node_js: '8' # Global environment variables env: diff --git a/lib/pa11y-ci.js b/lib/pa11y-ci.js index 88ee5c1..d4b85ff 100644 --- a/lib/pa11y-ci.js +++ b/lib/pa11y-ci.js @@ -48,7 +48,6 @@ function pa11yCi(urls, options) { delete options.log; // Create a Pa11y test function and an async queue - const pa11yTest = pa11y(options); const taskQueue = queue(testRunner, options.concurrency); taskQueue.drain = testRunComplete; @@ -67,32 +66,39 @@ function pa11yCi(urls, options) { // This is the actual test runner, which the queue will // execute on each of the URLs - function testRunner(config, done) { - const url = (typeof config === 'string' ? config : config.url); + async function testRunner(config) { + let url; + if (typeof config === 'string') { + url = config; + config = options; + } else { + url = config.url; + config = defaults({}, config, options); + } // Run the Pa11y test on the current URL and add // results to the report object - pa11yTest.run(config, (error, results) => { - if (error) { - log.error(` ${chalk.cyan('>')} ${url} - ${chalk.red('Failed to run')}`); - report.results[url] = [error]; - return done(); - } - const errors = results.filter(filterNonErrors); + + try { + const results = await pa11y(url, config); + let message = ` ${chalk.cyan('>')} ${url} - `; - if (errors.length) { - message += chalk.red(`${errors.length} errors`); + if (results.issues.length) { + message += chalk.red(`${results.issues.length} errors`); log.error(message); - report.results[url] = errors; - report.errors += errors.length; + report.results[url] = results.issues; + report.errors += results.issues.length; } else { - message += chalk.green(`${errors.length} errors`); + message += chalk.green(`${results.issues.length} errors`); log.info(message); report.results[url] = []; report.passes += 1; } - done(); - }); + } catch (error) { + log.error(` ${chalk.cyan('>')} ${url} - ${chalk.red('Failed to run')}`); + report.results[url] = [error]; + } + } // This function is called once all of the URLs in the @@ -137,8 +143,3 @@ function pa11yCi(urls, options) { }); } - -// Just a utility function to filter out non errors -function filterNonErrors(result) { - return result.type === 'error'; -} diff --git a/package.json b/package.json index 0386bf4..87e47a0 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "pa11y-ci", - "version": "1.3.1", + "version": "2.0.0-alpha", "description": "Pa11y CI is a CI-centric accessibility test runner, built using Pa11y", "keywords": [], "author": "Team Pa11y", @@ -25,7 +25,7 @@ "globby": "^6.1.0", "lodash": "^4.17.4", "node-fetch": "^1.7.0", - "pa11y": "^4.11.0", + "pa11y": "beta", "protocolify": "^2.0.0", "wordwrap": "^1.0.0" }, @@ -34,7 +34,7 @@ "mocha": "^3.4.2", "mockery": "^2.0.0", "nyc": "^11.0.1", - "pa11y-lint-config": "^1.0.0", + "pa11y-lint-config": "^1.2.0", "proclaim": "^3.4.4", "sinon": "^2.3.2" }, diff --git a/test/integration/cli-erroring.js b/test/integration/cli-erroring.js index 7d7032b..d5ead69 100644 --- a/test/integration/cli-erroring.js +++ b/test/integration/cli-erroring.js @@ -22,7 +22,7 @@ describe('pa11y-ci (with a single erroring URL)', () => { it('outputs error information', () => { assert.include(global.lastResult.output, 'Errors in http://notahost:8090/erroring-1'); - assert.include(global.lastResult.output, 'Error opening url "http://notahost:8090/erroring-1" : Host notahost not found'); + assert.include(global.lastResult.output, 'Failed to navigate: http://notahost:8090/erroring-1'); }); it('outputs a total erroring notice', () => { @@ -51,7 +51,7 @@ describe('pa11y-ci (with multiple erroring URLs)', () => { it('outputs error information', () => { assert.include(global.lastResult.output, 'Errors in http://notahost:8090/erroring-1'); - assert.include(global.lastResult.output, 'Error opening url "http://notahost:8090/erroring-1" : Host notahost not found'); + assert.include(global.lastResult.output, 'Failed to navigate: http://notahost:8090/erroring-1'); assert.include(global.lastResult.output, 'Errors in http://localhost:8090/timeout'); assert.include(global.lastResult.output, 'timed out'); }); diff --git a/test/integration/cli-json.js b/test/integration/cli-json.js index 8d633f1..84663ca 100644 --- a/test/integration/cli-json.js +++ b/test/integration/cli-json.js @@ -22,7 +22,7 @@ describe('pa11y-ci (with the `--json` flag set)', () => { results: { 'http://notahost:8090/erroring-1': [ { - message: 'Error opening url "http://notahost:8090/erroring-1" : Host notahost not found' + message: 'Failed to navigate: http://notahost:8090/erroring-1' } ], 'http://localhost:8090/failing-1': [ diff --git a/test/integration/cli-mixed.js b/test/integration/cli-mixed.js index 9182ab5..93dd075 100644 --- a/test/integration/cli-mixed.js +++ b/test/integration/cli-mixed.js @@ -24,7 +24,7 @@ describe('pa11y-ci (with erroring, failing, and passing URLs)', () => { it('outputs error information', () => { assert.include(global.lastResult.output, 'Errors in http://notahost:8090/erroring-1'); - assert.include(global.lastResult.output, 'Error opening url "http://notahost:8090/erroring-1" : Host notahost not found'); + assert.include(global.lastResult.output, 'Failed to navigate: http://notahost:8090/erroring-1'); assert.include(global.lastResult.output, 'Errors in http://localhost:8090/failing-1'); assert.include(global.lastResult.output, 'html element should have a lang'); assert.notInclude(global.lastResult.output, 'Errors in http://notahost:8090/passing-1'); diff --git a/test/unit/lib/pa11y-ci.js b/test/unit/lib/pa11y-ci.js index 9a43318..87e003b 100644 --- a/test/unit/lib/pa11y-ci.js +++ b/test/unit/lib/pa11y-ci.js @@ -84,24 +84,20 @@ describe('lib/pa11y-ci', () => { }; pa11yError = new Error('Pa11y Error'); - pa11yResults = [ - { - type: 'error', - message: 'Pa11y Result Error', - selector: '', - context: '' - }, - { - type: 'warning', - message: 'Pa11y Result Warning', - selector: '', - context: '' - } - ]; + pa11yResults = { + issues: [ + { + type: 'error', + message: 'Pa11y Result Error', + selector: '', + context: '' + } + ] + }; - pa11y.mockTestRunner.run.withArgs('foo-url').yieldsAsync(null, []); - pa11y.mockTestRunner.run.withArgs('bar-url').yieldsAsync(null, pa11yResults); - pa11y.mockTestRunner.run.withArgs('baz-url').yieldsAsync(pa11yError); + pa11y.withArgs('foo-url').resolves({issues: []}); + pa11y.withArgs('bar-url').resolves(pa11yResults); + pa11y.withArgs('baz-url').rejects(pa11yError); returnedPromise = pa11yCi(userUrls, userOptions); }); @@ -130,11 +126,6 @@ describe('lib/pa11y-ci', () => { assert.isUndefined(defaults.firstCall.returnValue.log); }); - it('creates a Pa11y test runner with the expected options', () => { - assert.calledOnce(pa11y); - assert.calledWithExactly(pa11y, defaults.firstCall.returnValue); - }); - it('creates an Async.js queue with the expected concurrency', () => { assert.calledOnce(queue); assert.isFunction(queue.firstCall.args[0]); @@ -146,10 +137,10 @@ describe('lib/pa11y-ci', () => { }); it('Runs the Pa11y test runner on each of the URLs', () => { - assert.callCount(pa11y.mockTestRunner.run, 3); - assert.calledWith(pa11y.mockTestRunner.run, 'foo-url'); - assert.calledWith(pa11y.mockTestRunner.run, 'bar-url'); - assert.calledWith(pa11y.mockTestRunner.run, 'baz-url'); + assert.callCount(pa11y, 3); + assert.calledWith(pa11y, 'foo-url'); + assert.calledWith(pa11y, 'bar-url'); + assert.calledWith(pa11y, 'baz-url'); }); it('logs that the tests have started running', () => { @@ -196,7 +187,7 @@ describe('lib/pa11y-ci', () => { assert.isArray(report.results['bar-url']); assert.lengthEquals(report.results['bar-url'], 1); - assert.strictEqual(report.results['bar-url'][0], pa11yResults[0]); + assert.strictEqual(report.results['bar-url'][0], pa11yResults.issues[0]); assert.isArray(report.results['baz-url']); assert.lengthEquals(report.results['baz-url'], 1); @@ -214,10 +205,10 @@ describe('lib/pa11y-ci', () => { log.error = sinon.spy(); log.info = sinon.spy(); - pa11y.mockTestRunner.run.reset(); - pa11y.mockTestRunner.run.withArgs('foo-url').yieldsAsync(null, []); - pa11y.mockTestRunner.run.withArgs('bar-url').yieldsAsync(null, []); - pa11y.mockTestRunner.run.withArgs('baz-url').yieldsAsync(null, []); + pa11y.reset(); + pa11y.withArgs('foo-url').resolves({issues: []}); + pa11y.withArgs('bar-url').resolves({issues: []}); + pa11y.withArgs('baz-url').resolves({issues: []}); returnedPromise = pa11yCi(userUrls, userOptions); }); @@ -258,12 +249,14 @@ describe('lib/pa11y-ci', () => { userUrls = [ { url: 'qux-url', - bar: 'baz' + bar: 'baz', + concurrency: 4, + wrapWidth: 80 } ]; - pa11y.mockTestRunner.run.reset(); - pa11y.mockTestRunner.run.withArgs(userUrls[0]).yieldsAsync(null, []); + pa11y.reset(); + pa11y.withArgs('qux-url', userUrls[0]).resolves({issues: []}); returnedPromise = pa11yCi(userUrls, userOptions); }); @@ -279,8 +272,8 @@ describe('lib/pa11y-ci', () => { }); it('Runs the Pa11y test runner on each of the URLs with configurations', () => { - assert.callCount(pa11y.mockTestRunner.run, 1); - assert.calledWith(pa11y.mockTestRunner.run, userUrls[0]); + assert.callCount(pa11y, 1); + assert.calledWith(pa11y, 'qux-url', userUrls[0]); }); it('correctly logs the number of errors for the URL', () => { diff --git a/test/unit/mock/pa11y.mock.js b/test/unit/mock/pa11y.mock.js index 6e90f1e..253a4d9 100644 --- a/test/unit/mock/pa11y.mock.js +++ b/test/unit/mock/pa11y.mock.js @@ -4,11 +4,8 @@ const sinon = require('sinon'); const pa11y = module.exports = sinon.stub(); -const mockTestRunner = module.exports.mockTestRunner = { - run: sinon.stub() +const mockResults = module.exports.mockResults = { + issues: [] }; -const mockResults = module.exports.mockResults = []; - -mockTestRunner.run.yieldsAsync(null, mockResults); -pa11y.returns(mockTestRunner); +pa11y.resolves(mockResults);