From 3902ddf8981b55c834c1f814dfc2e54e6280ab5c Mon Sep 17 00:00:00 2001 From: Jose Bolos Date: Fri, 8 Apr 2022 15:08:59 +0100 Subject: [PATCH 01/12] Add windows and macos to the test matrix --- .github/workflows/tests.yml | 32 +++++++++++++++++--------------- 1 file changed, 17 insertions(+), 15 deletions(-) diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index 659730f4..72411517 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -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: [12, 14, 16] include: - - node-version: 12.x + - os: ubuntu-latest + node: 12 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 From 2186664c706adbf26d1cb7d0995c7da7d8babfcc Mon Sep 17 00:00:00 2001 From: Aaron Goldenthal Date: Fri, 8 Apr 2022 15:11:06 -0500 Subject: [PATCH 02/12] Set test-specific environment variables within jest setup --- jest.config.json | 1 + package.json | 4 ++-- test/setup.js | 4 ++++ 3 files changed, 7 insertions(+), 2 deletions(-) create mode 100644 test/setup.js diff --git a/jest.config.json b/jest.config.json index 97fe8a9c..1235e049 100644 --- a/jest.config.json +++ b/jest.config.json @@ -13,5 +13,6 @@ }, "globalSetup": "./test/integration/global-setup.js", "globalTeardown": "./test/integration/global-teardown.js", + "setupFiles": ["./test/setup.js"], "setupFilesAfterEnv": ["./test/integration/setup-env.js"] } diff --git a/package.json b/package.json index a0bcb7de..cf4200b3 100644 --- a/package.json +++ b/package.json @@ -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", diff --git a/test/setup.js b/test/setup.js new file mode 100644 index 00000000..57770dd6 --- /dev/null +++ b/test/setup.js @@ -0,0 +1,4 @@ +'use strict'; + +// Set test-specific environment variables +process.env.MOCK_SERVER_PORT = 8081; From e71a65b08e7b2a05c0fae51cb4fb61b347c68b6a Mon Sep 17 00:00:00 2001 From: Aaron Goldenthal Date: Fri, 8 Apr 2022 15:32:01 -0500 Subject: [PATCH 03/12] Fix handling of absolute paths in Windows --- lib/option.js | 10 +++++----- test/unit/lib/option.test.js | 10 ++++++---- 2 files changed, 11 insertions(+), 9 deletions(-) diff --git a/lib/option.js b/lib/option.js index b20f45b5..4ab0b956 100644 --- a/lib/option.js +++ b/lib/option.js @@ -80,17 +80,17 @@ 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, is an + * absolute path, 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://". * @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)) { sanitizedUrl = `file://${path.resolve(process.cwd(), url)}`; } else if (!/^(https?|file):\/\//i.test(url)) { sanitizedUrl = `http://${url}`; diff --git a/test/unit/lib/option.test.js b/test/unit/lib/option.test.js index b7632f27..107857e8 100644 --- a/test/unit/lib/option.test.js +++ b/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 */ }; @@ -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}`); }); }); From ca404d03dcea5b39ce2e767043befd9d7534d628 Mon Sep 17 00:00:00 2001 From: Aaron Goldenthal Date: Fri, 8 Apr 2022 15:42:18 -0500 Subject: [PATCH 04/12] Fix Windows path issues in htmlcs tests --- test/unit/lib/runners/htmlcs.test.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/test/unit/lib/runners/htmlcs.test.js b/test/unit/lib/runners/htmlcs.test.js index 346136e4..dbc993d5 100644 --- a/test/unit/lib/runners/htmlcs.test.js +++ b/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', () => { @@ -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); }); From 5ee0d39dd71789583903ad19fa39064d2aa1a3cb Mon Sep 17 00:00:00 2001 From: Aaron Goldenthal Date: Fri, 8 Apr 2022 15:44:27 -0500 Subject: [PATCH 05/12] Fix issues with absolute paths in Windows --- lib/pa11y.js | 5 +++-- test/unit/lib/pa11y.test.js | 8 +++++--- 2 files changed, 8 insertions(+), 5 deletions(-) diff --git a/lib/pa11y.js b/lib/pa11y.js index 5e2c6387..c67f8074 100644 --- a/lib/pa11y.js +++ b/lib/pa11y.js @@ -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'); @@ -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; @@ -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) { diff --git a/test/unit/lib/pa11y.test.js b/test/unit/lib/pa11y.test.js index fd134d9f..c1620180 100644 --- a/test/unit/lib/pa11y.test.js +++ b/test/unit/lib/pa11y.test.js @@ -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()); }); }); From b67d98fe7b3e01c538fe1731632019d1f70315bf Mon Sep 17 00:00:00 2001 From: Aaron Goldenthal Date: Fri, 8 Apr 2022 16:38:33 -0500 Subject: [PATCH 06/12] Move env var to global since needed there and run before setup --- jest.config.json | 1 - test/integration/global-setup.js | 4 ++++ test/setup.js | 4 ---- 3 files changed, 4 insertions(+), 5 deletions(-) delete mode 100644 test/setup.js diff --git a/jest.config.json b/jest.config.json index 1235e049..97fe8a9c 100644 --- a/jest.config.json +++ b/jest.config.json @@ -13,6 +13,5 @@ }, "globalSetup": "./test/integration/global-setup.js", "globalTeardown": "./test/integration/global-teardown.js", - "setupFiles": ["./test/setup.js"], "setupFilesAfterEnv": ["./test/integration/setup-env.js"] } diff --git a/test/integration/global-setup.js b/test/integration/global-setup.js index 731398cc..cf3e293f 100644 --- a/test/integration/global-setup.js +++ b/test/integration/global-setup.js @@ -2,6 +2,10 @@ const startMockWebsite = require('./mock/website'); +// Set here because this runs before setup-env and is require to start server +// and saved as environment variable for use in setup-env. +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}`; diff --git a/test/setup.js b/test/setup.js deleted file mode 100644 index 57770dd6..00000000 --- a/test/setup.js +++ /dev/null @@ -1,4 +0,0 @@ -'use strict'; - -// Set test-specific environment variables -process.env.MOCK_SERVER_PORT = 8081; From 52f67c1adae2f0d6ca83c3686fd2b8d38551a869 Mon Sep 17 00:00:00 2001 From: Aaron Goldenthal Date: Fri, 8 Apr 2022 16:40:27 -0500 Subject: [PATCH 07/12] Update path resolution to be platform independent --- test/integration/mock/website.js | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/test/integration/mock/website.js b/test/integration/mock/website.js index d0ff8a18..54133fcb 100644 --- a/test/integration/mock/website.js +++ b/test/integration/mock/website.js @@ -2,6 +2,7 @@ const fs = require('fs'); const http = require('http'); +const path = require('path'); const parseUrl = require('url').parse; module.exports = startMockWebsite; @@ -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); From 7d88c97507319abe9b4fb7c069524088bcc8608d Mon Sep 17 00:00:00 2001 From: Aaron Goldenthal Date: Fri, 8 Apr 2022 16:42:30 -0500 Subject: [PATCH 08/12] Update cli runner to call node since Windows ignores the shebang in bin --- test/integration/helper/pa11y-cli.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/integration/helper/pa11y-cli.js b/test/integration/helper/pa11y-cli.js index 54778a5e..fb82a7e9 100644 --- a/test/integration/helper/pa11y-cli.js +++ b/test/integration/helper/pa11y-cli.js @@ -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: '', @@ -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 }); From 4b7aa2490d28690d042fa9140e5ab6d7e633e7d1 Mon Sep 17 00:00:00 2001 From: Aaron Goldenthal Date: Fri, 8 Apr 2022 16:55:20 -0500 Subject: [PATCH 09/12] Clean up comment for clarity --- test/integration/global-setup.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/integration/global-setup.js b/test/integration/global-setup.js index cf3e293f..cd87cf85 100644 --- a/test/integration/global-setup.js +++ b/test/integration/global-setup.js @@ -2,8 +2,8 @@ const startMockWebsite = require('./mock/website'); -// Set here because this runs before setup-env and is require to start server -// and saved as environment variable for use in setup-env. +// 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 () => { From 4208239447a96c0c7d8131c54f4595f39e7c91de Mon Sep 17 00:00:00 2001 From: Aaron Goldenthal Date: Fri, 8 Apr 2022 17:23:01 -0500 Subject: [PATCH 10/12] Update comments to clarify behavior --- lib/option.js | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/lib/option.js b/lib/option.js index 4ab0b956..408abd81 100644 --- a/lib/option.js +++ b/lib/option.js @@ -80,10 +80,11 @@ function defaultOptions(options, defaults) { } /** - * Sanitize a URL, ensuring it has a scheme. If the URL begins with a period, is an - * absolute path, 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. @@ -91,6 +92,9 @@ function defaultOptions(options, defaults) { function sanitizeUrl(url) { let sanitizedUrl = 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}`; From 47b951b5cf44af68d9cbf3e5c7e9db8817dd4689 Mon Sep 17 00:00:00 2001 From: Aaron Goldenthal Date: Fri, 8 Apr 2022 20:10:52 -0500 Subject: [PATCH 11/12] Increase timeout for Mac tests timing out --- test/integration/setup-env.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/integration/setup-env.js b/test/integration/setup-env.js index c2d82282..66db49fa 100644 --- a/test/integration/setup-env.js +++ b/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}`; From 9d7ae8a7de295f46a058bcc68f198f9f64ad9d30 Mon Sep 17 00:00:00 2001 From: Jose Bolos Date: Fri, 22 Apr 2022 14:49:33 +0100 Subject: [PATCH 12/12] Remove node 12 and add node 18 to test matrix Node v18 just came out a few days ago, and v12 is going to be unsupported as of next week. This updates the test matrix accordingly. --- .github/workflows/tests.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index 72411517..bc7cb68a 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -19,10 +19,10 @@ jobs: fail-fast: false matrix: os: [ubuntu-latest, windows-latest, macOS-latest] - node: [12, 14, 16] + node: [14, 16, 18] include: - os: ubuntu-latest - node: 12 + node: 14 lint: true # Linter is run only once to shorten the total build time steps: