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

Support macOS and Windows (absorb next) #678

Merged
merged 9 commits into from Oct 11, 2023
44 changes: 17 additions & 27 deletions .github/workflows/tests.yml
@@ -1,38 +1,28 @@
# This workflow will do a clean install of node dependencies, build the source code and run tests.
# For more information see: https://help.github.com/actions/language-and-framework-guides/using-nodejs-with-github-actions

name: Build and lint

on:
push:
branches: # Run actions when code is committed to these branches
- master
branches:
- main
pull_request:
branches: # Run actions when a PR is pushed based on one of these branches
- master
- 1.x
- 2.x
- 3.x
- 4.x
- 5.x
- 5.2.x

jobs:
checkout_and_test:
runs-on: ubuntu-latest
test:
name: Lint and test on ${{ matrix.os }} with Node.js ${{ matrix.node-version }}
runs-on: ${{ matrix.os }}
strategy:
fail-fast: false
matrix:
os: [ubuntu-latest, windows-latest, macOS-latest]
node-version: [18, 20]
steps:
- name: Checkout code from ${{ github.repository }}
uses: actions/checkout@v2
- name: Setup node
uses: actions/setup-node@v2
- name: Normalise line-ending handling in Git
shell: bash
run: |
git config --global core.autocrlf false
if: runner.os == 'Windows'
- uses: actions/checkout@v3
- uses: actions/setup-node@v3
with:
node-version: ${{ matrix.node-version }}
- name: Install dependencies
run: npm ci
- name: Run linter
run: npm run lint
- name: Run tests
run: npm test
- run: npm ci
- run: npm run lint
- run: npm test
14 changes: 9 additions & 5 deletions lib/option.js
Expand Up @@ -80,17 +80,21 @@ function defaultOptions(options, defaults) {
}

/**
* Sanitize a URL, ensuring it has a scheme. If the URL begins with a slash or a period
* or it is a valid path relative to the current directory, it will be resolved as a
* path against the current working directory. If the URL does begin with a scheme, it
* will be prepended with "http://".
* Sanitize a URL, ensuring it has a scheme. If the URL begins with a period or it
* is a valid path relative to the current directory, it is assumed to be a file path
* and is resolved relative to the current working directory. If it is an absolute
* file path, that file path is used. If the URL does begin with a scheme, it will
* be prepended with "http://".
* @private
* @param {String} url - The URL to sanitize.
* @returns {String} Returns the sanitized URL.
*/
function sanitizeUrl(url) {
let sanitizedUrl = url;
if (/^[./]/i.test(url) || fs.existsSync(url)) {
if (/^[.]/i.test(url) || path.isAbsolute(url) || fs.existsSync(url)) {
// This works for absolute paths since path.resolve starts at the right and
// works left, stopping once it has an absolute path. So, if the URL is an
// absolute path it is returned.
sanitizedUrl = `file://${path.resolve(process.cwd(), url)}`;
} else if (!/^(https?|file):\/\//i.test(url)) {
sanitizedUrl = `http://${url}`;
Expand Down
5 changes: 3 additions & 2 deletions lib/pa11y.js
Expand Up @@ -3,6 +3,7 @@
const runAction = require('./action');
const option = require('./option');
const fs = require('fs');
const path = require('path');
const pkg = require('../package.json');
const promiseTimeout = require('p-timeout');
const puppeteer = require('puppeteer');
Expand Down Expand Up @@ -123,7 +124,7 @@ async function setBrowser(options, state) {
if (options.browser) {
options.log.debug(
'Using a pre-configured Headless Chrome instance, ' +
'the `chromeLaunchConfig` option will be ignored'
'the `chromeLaunchConfig` option will be ignored'
);
state.browser = options.browser;
state.autoClose = false;
Expand Down Expand Up @@ -283,7 +284,7 @@ async function injectRunners(options, state) {
// We only load these files once on the first run of Pa11y as they don't
// change between runs
if (!runnersJavascript.pa11y) {
runnersJavascript.pa11y = fs.readFileSync(`${__dirname}/runner.js`, 'utf-8');
runnersJavascript.pa11y = fs.readFileSync(path.join(__dirname, 'runner.js'), 'utf-8');
}

for (const runner of options.runners) {
Expand Down
4 changes: 2 additions & 2 deletions package.json
Expand Up @@ -58,8 +58,8 @@
"lint": "eslint .",
"test-unit": "jest test/unit",
"test-coverage": "jest test/unit --coverage",
"test-integration": "MOCK_SERVER_PORT=8081 jest test/integration",
"test": "MOCK_SERVER_PORT=8081 jest --coverage"
"test-integration": "jest test/integration",
"test": "jest --coverage"
},
"files": [
"bin",
Expand Down
4 changes: 4 additions & 0 deletions test/integration/global-setup.js
Expand Up @@ -2,6 +2,10 @@

const startMockWebsite = require('./mock/website');

// Set here because this runs before setup-env and is require to start server.
// Saved as environment variable for use in setup-env in case not set.
process.env.MOCK_SERVER_PORT = process.env.MOCK_SERVER_PORT || 8081;

module.exports = async () => {
global.mockWebsite = await startMockWebsite(process.env.MOCK_SERVER_PORT);
// Global.mockWebsiteAddress = `http://localhost:${global.SERVER_PORT}`;
Expand Down
4 changes: 2 additions & 2 deletions test/integration/helper/pa11y-cli.js
Expand Up @@ -20,7 +20,7 @@ function runPa11yCli(url, options = {}) {
options.arguments.push(url);

return new Promise((resolve, reject) => {
const binPath = path.resolve(`${__dirname}/../../../bin/pa11y.js`);
const binFile = path.resolve(__dirname, '../../../bin/pa11y.js');

const response = {
exitCode: '',
Expand All @@ -30,7 +30,7 @@ function runPa11yCli(url, options = {}) {
stdout: ''
};

const pa11yProcess = spawn(binPath, options.arguments, {
const pa11yProcess = spawn('node', [binFile, ...options.arguments], {
cwd: options.workingDirectory,
env: options.environment
});
Expand Down
4 changes: 3 additions & 1 deletion test/integration/mock/website.js
Expand Up @@ -2,6 +2,7 @@

const fs = require('fs');
const http = require('http');
const path = require('path');
const parseUrl = require('url').parse;

module.exports = startMockWebsite;
Expand Down Expand Up @@ -29,7 +30,8 @@ function createMockWebsite() {
request.on('end', () => {
const url = parseUrl(request.url).pathname;
try {
let html = fs.readFileSync(`${__dirname}/html/${url}.html`, 'utf-8');
const viewPath = path.join(__dirname, 'html', `${url}.html`);
let html = fs.readFileSync(viewPath, 'utf-8');
html = html.replace('{foo-header}', request.headers.foo);
html = html.replace('{bar-header}', request.headers.bar);
html = html.replace('{method}', request.method);
Expand Down
2 changes: 1 addition & 1 deletion test/integration/setup-env.js
@@ -1,4 +1,4 @@
'use strict';

jest.setTimeout(10000);
jest.setTimeout(20000);
global.mockWebsiteAddress = `http://localhost:${process.env.MOCK_SERVER_PORT}`;
10 changes: 6 additions & 4 deletions test/unit/lib/option.test.js
@@ -1,7 +1,7 @@
'use strict';

const path = require('path');
const {parseArguments, verifyOptions} = require('../../../lib/option');
const {parseArguments, verifyOptions} = require('../../../lib/option');

describe('lib/option', () => {
const noop = () => { /* No-op */ };
Expand Down Expand Up @@ -216,13 +216,15 @@ describe('lib/option', () => {
});
});

describe('when `url` does not have a scheme and starts with a slash', () => {
describe('when `url` does not have a scheme and is an absolute path', () => {
const absolutePath = path.resolve(process.cwd(), './mock-path');

beforeEach(() => {
[url, options, callback] = parseArguments('/mock-path', {}, {});
[url, options, callback] = parseArguments(absolutePath, {}, {});
});

it('navigates to `url` with a `file` scheme added', () => {
expect(url).toEqual('file:///mock-path');
expect(url).toEqual(`file://${absolutePath}`);
});
});

Expand Down
8 changes: 5 additions & 3 deletions test/unit/lib/pa11y.test.js
Expand Up @@ -225,14 +225,16 @@ describe('lib/pa11y', () => {

});

describe('when `url` does not have a scheme and starts with a slash', () => {
describe('when `url` does not have a scheme and is an absolute path', () => {
const absolutePath = path.resolve(process.cwd(), './mock-path');

beforeEach(async () => {
await pa11y('/mock-path');
await pa11y(absolutePath);
});

it('navigates to `url` with an `file` scheme added', () => {
expect(puppeteer.mockPage.goto).toHaveBeenCalledTimes(1);
expect(puppeteer.mockPage.goto).toHaveBeenCalledWith('file:///mock-path', expect.anything());
expect(puppeteer.mockPage.goto).toHaveBeenCalledWith(`file://${absolutePath}`, expect.anything());
});

});
Expand Down
3 changes: 2 additions & 1 deletion test/unit/lib/runners/htmlcs.test.js
@@ -1,5 +1,6 @@
'use strict';

const path = require('path');
const runner = require('../../../../lib/runners/htmlcs');

describe('lib/runners/htmlcs', () => {
Expand Down Expand Up @@ -60,7 +61,7 @@ describe('lib/runners/htmlcs', () => {
expect(runner.scripts).toHaveLength(1);
expect(
runner.scripts[0].endsWith(
'node_modules/html_codesniffer/build/HTMLCS.js'
path.join('node_modules', 'html_codesniffer', 'build', 'HTMLCS.js')
)
).toEqual(true);
});
Expand Down