Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix ignore url handling #536

Closed
wants to merge 5 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 17 additions & 7 deletions lib/pa11y.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ module.exports = pa11y;
/**
* Run accessibility tests on a web page.
* @public
* @param {String} url - The URL to run tests against.
* @param {(String|Object)} url - The URL to run tests against, or options object.
* @param {Object} [options={}] - Options to change the way tests run.
* @param {Function} [callback] - An optional callback to use instead of promises.
* @returns {Promise} Returns a promise which resolves with a results object.
Expand All @@ -34,6 +34,11 @@ async function pa11y(url, options = {}, callback) {
// Verify that the given options are valid
verifyOptions(options);

if (!url && !options.ignoreUrl) {
// eslint-disable-next-line max-len
throw new Error('A url must be defined via the `url` parameter or `options.url`, or `options.ignoreUrl` must be set to true.');
}

// Call the actual Pa11y test runner with
// a timeout if it takes too long
pa11yResults = await promiseTimeout(
Expand All @@ -58,7 +63,7 @@ async function pa11y(url, options = {}, callback) {
/**
* Parse arguments from the command-line to properly identify the url, options, and callback
* @private
* @param {String} url - The URL to run tests against.
* @param {(String|Object)} url - The URL to run tests against, or options object.
* @param {Object} [options={}] - Options to change the way tests run.
* @param {Function} [callback] - An optional callback to use instead of promises.
* @returns {Array} the new values of url, options, and callback
Expand All @@ -68,11 +73,13 @@ function parseArguments(url, options, callback) {
callback = options;
options = {};
}
if (typeof url !== 'string') {
options = url;
url = options.url;
if (url) {
if (typeof url !== 'string') {
options = url;
url = options.url;
}
url = sanitizeUrl(url);
}
url = sanitizeUrl(url);
options = defaultOptions(options);

return [url,
Expand Down Expand Up @@ -109,7 +116,9 @@ function defaultOptions(options) {
*/
async function runPa11yTest(url, options, state) {

options.log.info(`Running Pa11y on URL ${url}`);
if (url) {
options.log.info(`Running Pa11y on URL ${url}`);
}

await setBrowser(options, state);

Expand Down Expand Up @@ -513,6 +522,7 @@ pa11y.defaults = {
info: noop
},
method: 'GET',
page: null,
postData: null,
rootElement: null,
rules: [],
Expand Down
62 changes: 34 additions & 28 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

82 changes: 82 additions & 0 deletions test/unit/issue/535.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,82 @@
'use strict';

const assert = require('proclaim');
const mockery = require('mockery');
const extend = require('node.extend');
const sinon = require('sinon');


/**
* Note: this test is designed to replicate issue 535
*
* When setting a default of `ignoreUrl: true`, the `pa11y()` function shouldn't
* require a url parameter
*
* https://github.com/pa11y/pa11y/issues/535
*/
describe('lib/pa11y', () => {
let pa11y;
let pa11yResults;
let puppeteer;

beforeEach(() => {

pa11yResults = {
mockResults: true
};
/* eslint-disable no-underscore-dangle */
global.window = {
__pa11y: {
run: sinon.stub().returns(pa11yResults)
}
};
/* eslint-enable no-underscore-dangle */

puppeteer = require('../mock/puppeteer.mock');
mockery.registerMock('puppeteer', puppeteer);

puppeteer.mockPage.evaluate.resolves(pa11yResults);

pa11y = require('../../../lib/pa11y');

});

afterEach(() => {
/* eslint-disable no-underscore-dangle */
delete global.window;
/* eslint-enable no-underscore-dangle */
});

describe('Issue #535', () => {
let page;
let pa11yError;

beforeEach(async () => {
puppeteer.launch.resetHistory();
puppeteer.mockBrowser.newPage.resetHistory();
puppeteer.mockBrowser.close.resetHistory();
puppeteer.mockPage.close.resetHistory();
puppeteer.mockPage.goto.resetHistory();

page = puppeteer.mockPage;

pa11y.defaults = extend({}, pa11y.defaults, {
browser: puppeteer.mockBrowser,
page,
ignoreUrl: true
});

try {
await pa11y();
} catch (error) {
pa11yError = error;
}
});

it('ignoring url in defaults does not throw error', () => {
assert.isNotInstanceOf(pa11yError, Error);
assert.notCalled(page.goto);
});
});

});
58 changes: 58 additions & 0 deletions test/unit/lib/pa11y.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,60 @@ describe('lib/pa11y', () => {
assert.isFunction(pa11y);
});

describe('pa11y', () => {
beforeEach(() => {
puppeteer.launch.resetHistory();
puppeteer.mockBrowser.newPage.resetHistory();
puppeteer.mockBrowser.close.resetHistory();
puppeteer.mockPage.close.resetHistory();
puppeteer.mockPage.goto.resetHistory();
});

it('throws error when called without a url and `defaults.ignoreUrl` is not defined', async () => {
let pa11yError;

pa11y.defaults = extend({}, pa11y.defaults, {
browser: puppeteer.mockBrowser,
page: puppeteer.mockPage
});

try {
await pa11y();
} catch (error) {
pa11yError = error;
}

assert.instanceOf(pa11yError, Error);
assert.strictEqual(pa11yError.message, 'A url must be defined via the `url` parameter or `options.url`, or `options.ignoreUrl` must be set to true.');
});

it('does not call page.goto when `defaults.ignoreUrl` is set to true', async () => {
const page = puppeteer.mockPage;

pa11y.defaults = extend({}, pa11y.defaults, {
browser: puppeteer.mockBrowser,
page: puppeteer.mockPage,
ignoreUrl: true
});

await pa11y();

assert.notCalled(page.goto);
});

it('does not call page.goto when `options.ignoreUrl` is set to true', async () => {
const page = puppeteer.mockPage;

await pa11y({
browser: puppeteer.mockBrowser,
page: puppeteer.mockPage,
ignoreUrl: true
});

assert.notCalled(page.goto);
});
});

describe('pa11y(url)', () => {
let resolvedValue;

Expand Down Expand Up @@ -1021,6 +1075,10 @@ describe('lib/pa11y', () => {
assert.deepEqual(pa11y.defaults.method, 'GET');
});

it('has a `page` property', () => {
assert.isNull(pa11y.defaults.page);
});

it('has a `postData` property', () => {
assert.isNull(pa11y.defaults.postData);
});
Expand Down