From c9fa8a08aa609eac705fa8d93ce025c3285c5c33 Mon Sep 17 00:00:00 2001 From: Antoine du Hamel Date: Tue, 2 Aug 2022 23:05:22 +0200 Subject: [PATCH] fix: make test runner work even if there is no `AbortSignal` support Fixes a breaking change for Node.js v14.x that was introduced in 3.2.0. Fixes: https://github.com/nodejs/node-core-test/issues/35 --- .github/workflows/ci.yml | 7 +++---- README.md | 17 ++++++++--------- lib/internal/abort_controller.js | 28 +++++++++++++++++++++++++--- test/common/index.js | 4 ++-- test/message.js | 4 ++++ 5 files changed, 42 insertions(+), 18 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 5a2c556..1dd3faa 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -9,9 +9,6 @@ jobs: strategy: matrix: node: ['14', '16', '18'] - include: - - node: '14' - env: --experimental-abortcontroller --no-warnings steps: - uses: actions/checkout@v3 - uses: actions/setup-node@v3 @@ -19,5 +16,7 @@ jobs: node-version: ${{ matrix.node }} - run: npm ci - run: npm test + - name: Run test with experimental flag + run: npm test env: - NODE_OPTIONS: ${{ matrix.env }} + NODE_OPTIONS: --experimental-abortcontroller --no-warnings diff --git a/README.md b/README.md index 854b293..d04c2df 100644 --- a/README.md +++ b/README.md @@ -11,7 +11,6 @@ Minimal dependencies, with full test suite. Differences from the core implementation: - Doesn't hide its own stack frames. -- Requires `--experimental-abortcontroller` CLI flag to work on Node.js v14.x. ## Docs @@ -339,7 +338,7 @@ internally. - `only` {boolean} If truthy, and the test context is configured to run `only` tests, then this test will be run. Otherwise, the test is skipped. **Default:** `false`. - * `signal` {AbortSignal} Allows aborting an in-progress test. + - `signal` {AbortSignal} Allows aborting an in-progress test (except on Node.js v14.x). - `skip` {boolean|string} If truthy, the test is skipped. If a string is provided, that string is displayed in the test results as the reason for skipping the test. **Default:** `false`. @@ -448,7 +447,7 @@ same as [`it([name], { todo: true }[, fn])`][it options]. function. * `options` {Object} Configuration options for the hook. The following properties are supported: - * `signal` {AbortSignal} Allows aborting an in-progress hook. + * `signal` {AbortSignal} Allows aborting an in-progress hook (except on Node.js v14.x). * `timeout` {number} A number of milliseconds the hook will fail after. If unspecified, subtests inherit this value from their parent. **Default:** `Infinity`. @@ -472,7 +471,7 @@ describe('tests', async () => { function. * `options` {Object} Configuration options for the hook. The following properties are supported: - * `signal` {AbortSignal} Allows aborting an in-progress hook. + * `signal` {AbortSignal} Allows aborting an in-progress hook (except on Node.js v14.x). * `timeout` {number} A number of milliseconds the hook will fail after. If unspecified, subtests inherit this value from their parent. **Default:** `Infinity`. @@ -496,7 +495,7 @@ describe('tests', async () => { function. * `options` {Object} Configuration options for the hook. The following properties are supported: - * `signal` {AbortSignal} Allows aborting an in-progress hook. + * `signal` {AbortSignal} Allows aborting an in-progress hook (except on Node.js v14.x). * `timeout` {number} A number of milliseconds the hook will fail after. If unspecified, subtests inherit this value from their parent. **Default:** `Infinity`. @@ -521,7 +520,7 @@ describe('tests', async () => { function. * `options` {Object} Configuration options for the hook. The following properties are supported: - * `signal` {AbortSignal} Allows aborting an in-progress hook. + * `signal` {AbortSignal} Allows aborting an in-progress hook (except on Node.js v14.x). * `timeout` {number} A number of milliseconds the hook will fail after. If unspecified, subtests inherit this value from their parent. **Default:** `Infinity`. @@ -552,7 +551,7 @@ exposed as part of the API. function. * `options` {Object} Configuration options for the hook. The following properties are supported: - * `signal` {AbortSignal} Allows aborting an in-progress hook. + * `signal` {AbortSignal} Allows aborting an in-progress hook (except on Node.js v14.x). * `timeout` {number} A number of milliseconds the hook will fail after. If unspecified, subtests inherit this value from their parent. **Default:** `Infinity`. @@ -580,7 +579,7 @@ test('top level test', async (t) => { function. * `options` {Object} Configuration options for the hook. The following properties are supported: - * `signal` {AbortSignal} Allows aborting an in-progress hook. + * `signal` {AbortSignal} Allows aborting an in-progress hook (except on Node.js v14.x). * `timeout` {number} A number of milliseconds the hook will fail after. If unspecified, subtests inherit this value from their parent. **Default:** `Infinity`. @@ -664,7 +663,7 @@ execution of the test function. This function does not return a value. - `skip` {boolean|string} If truthy, the test is skipped. If a string is provided, that string is displayed in the test results as the reason for skipping the test. **Default:** `false`. - - `signal` {AbortSignal} Allows aborting an in-progress test. + - `signal` {AbortSignal} Allows aborting an in-progress test (except on Node.js v14.x). - `todo` {boolean|string} If truthy, the test marked as `TODO`. If a string is provided, that string is displayed in the test results as the reason why the test is `TODO`. **Default:** `false`. diff --git a/lib/internal/abort_controller.js b/lib/internal/abort_controller.js index 007aa40..232b70d 100644 --- a/lib/internal/abort_controller.js +++ b/lib/internal/abort_controller.js @@ -1,6 +1,28 @@ 'use strict' -module.exports = { - AbortController, - AbortSignal +if (typeof AbortController === 'undefined') { + module.exports = { + AbortController: class AbortController { + #eventListeners = new Set() + signal = { + aborted: false, + addEventListener: (_, listener) => { + this.#eventListeners.add(listener) + }, + removeEventListener: (_, listener) => { + this.#eventListeners.delete(listener) + } + } + + abort () { + this.signal.aborted = true + this.#eventListeners.forEach(listener => listener()) + } + } + } +} else { + module.exports = { + AbortController, + AbortSignal + } } diff --git a/test/common/index.js b/test/common/index.js index ae531e6..91fcd41 100644 --- a/test/common/index.js +++ b/test/common/index.js @@ -102,7 +102,7 @@ function expectsError (validator, exact) { }, exact) } -if (typeof AbortSignal.timeout !== 'function') { +if (typeof AbortSignal !== 'undefined' && typeof AbortSignal.timeout !== 'function') { // `AbortSignal.timeout` is not available on Node.js 14.x, we need to polyfill // it because some tests are using it. End-users don't need to it. @@ -121,7 +121,7 @@ if (typeof AbortSignal.timeout !== 'function') { } } -if (process.version.startsWith('v14.') || process.version.startsWith('v16.')) { +if (typeof AbortSignal !== 'undefined' && (process.version.startsWith('v14.') || process.version.startsWith('v16.'))) { // Implementation of AbortSignal and AbortController differ slightly with the // v18.x one, creating some difference on the TAP output which makes the tests // fail. We are overriding the built-ins to make the test pass, however that's diff --git a/test/message.js b/test/message.js index ea35fb1..4520448 100755 --- a/test/message.js +++ b/test/message.js @@ -84,6 +84,10 @@ const main = async () => { for await (const dirent of dir) { const ext = extname(dirent.name) if (ext === '.js' || ext === '.mjs') { + if (typeof AbortSignal === 'undefined' && dirent.name.startsWith('test_runner_abort')) { + console.log('no AbortSignal support, skipping', dirent.name) + continue + } const filePath = join(MESSAGE_FOLDER, dirent.name) const expected = filePath.replace(/\.m?js$/, '.out') const testFile = await fs.open(filePath)