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

Add Windows and macOS to the test matrix #647

Merged
merged 12 commits into from
Apr 28, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
32 changes: 17 additions & 15 deletions .github/workflows/tests.yml
Original file line number Diff line number Diff line change
Expand Up @@ -9,32 +9,34 @@ on:
- master
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
name: Node v${{ matrix.node }} on ${{ matrix.os }}
runs-on: ${{ matrix.os }}
strategy:
fail-fast: false
matrix:
os: [ubuntu-latest, windows-latest, macOS-latest]
node: [14, 16, 18]
include:
- node-version: 12.x
- os: ubuntu-latest
node: 14
lint: true # Linter is run only once to shorten the total build time
- node-version: 14.x
- node-version: 16.x

steps:
- name: Set git config
shell: bash
run: |
git config --global core.autocrlf false
if: runner.os == 'Windows'
- name: Checkout code from ${{ github.repository }}
uses: actions/checkout@v2
- name: Setup node
uses: actions/setup-node@v2
uses: actions/checkout@v3
- name: Setup node ${{ matrix.node }}
uses: actions/setup-node@v3
with:
node-version: ${{ matrix.node-version }}
node-version: ${{ matrix.node }}
- name: Install dependencies
run: npm ci
- name: Run linter
Expand Down
14 changes: 9 additions & 5 deletions lib/option.js
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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 @@ -124,7 +125,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 @@ -304,7 +305,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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
@@ -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