From 012acb04de00cdcf4dcbbb86a8c50fc27a112dcf Mon Sep 17 00:00:00 2001 From: Moshe Atlow Date: Tue, 2 Aug 2022 08:52:41 +0300 Subject: [PATCH 01/24] feat: verbous error when entire test tree is canceled PR-URL: https://github.com/nodejs/node/pull/44060 Reviewed-By: Antoine du Hamel Reviewed-By: Benjamin Gruenbaum (cherry picked from commit 8cf33850bea691d8c53b2d4175c959c8549aa76c) --- lib/internal/test_runner/harness.js | 8 +++++--- lib/internal/test_runner/test.js | 17 ++++------------- test/message/test_runner_no_refs.out | 4 ++-- test/message/test_runner_unresolved_promise.out | 4 ++-- 4 files changed, 13 insertions(+), 20 deletions(-) diff --git a/lib/internal/test_runner/harness.js b/lib/internal/test_runner/harness.js index 4d6e9c4..987de95 100644 --- a/lib/internal/test_runner/harness.js +++ b/lib/internal/test_runner/harness.js @@ -1,4 +1,4 @@ -// https://github.com/nodejs/node/blob/659dc126932f986fc33c7f1c878cb2b57a1e2fac/lib/internal/test_runner/harness.js +// https://github.com/nodejs/node/blob/8cf33850bea691d8c53b2d4175c959c8549aa76c/lib/internal/test_runner/harness.js 'use strict' const { ArrayPrototypeForEach, @@ -15,7 +15,7 @@ const { } } = require('#internal/errors') const { getOptionValue } = require('#internal/options') -const { Test, ItTest, Suite } = require('#internal/test_runner/test') +const { kCancelledByParent, Test, ItTest, Suite } = require('#internal/test_runner/test') const isTestRunner = getOptionValue('--test') const testResources = new SafeMap() @@ -80,7 +80,9 @@ function setup (root) { createProcessEventHandler('unhandledRejection', root) const exitHandler = () => { - root.postRun() + root.postRun(new ERR_TEST_FAILURE( + 'Promise resolution is still pending but the event loop has already resolved', + kCancelledByParent)) let passCount = 0 let failCount = 0 diff --git a/lib/internal/test_runner/test.js b/lib/internal/test_runner/test.js index 89b2f35..b4fac63 100644 --- a/lib/internal/test_runner/test.js +++ b/lib/internal/test_runner/test.js @@ -513,7 +513,7 @@ class Test extends AsyncResource { this.postRun() } - postRun () { + postRun (pendingSubtestsError) { let failedSubtests = 0 // If the test was failed before it even started, then the end time will @@ -530,8 +530,8 @@ class Test extends AsyncResource { const subtest = this.subtests[i] if (!subtest.finished) { - subtest.cancel() - subtest.postRun() + subtest.cancel(pendingSubtestsError) + subtest.postRun(pendingSubtestsError) } if (!subtest.passed) { @@ -694,13 +694,4 @@ class Suite extends Test { } } -module.exports = { - ItTest, - kCancelledByParent, - kDefaultIndent, - kHookFailure, - kSubtestsFailed, - kTestCodeFailure, - Suite, - Test -} +module.exports = { kCancelledByParent, kDefaultIndent, kSubtestsFailed, kTestCodeFailure, Test, Suite, ItTest } diff --git a/test/message/test_runner_no_refs.out b/test/message/test_runner_no_refs.out index 63b79cd..e8560c5 100644 --- a/test/message/test_runner_no_refs.out +++ b/test/message/test_runner_no_refs.out @@ -5,7 +5,7 @@ TAP version 13 --- duration_ms: * failureType: 'cancelledByParent' - error: 'test did not finish before its parent and was cancelled' + error: 'Promise resolution is still pending but the event loop has already resolved' code: 'ERR_TEST_FAILURE' stack: |- * @@ -15,7 +15,7 @@ not ok 1 - does not keep event loop alive --- duration_ms: * failureType: 'cancelledByParent' - error: 'test did not finish before its parent and was cancelled' + error: 'Promise resolution is still pending but the event loop has already resolved' code: 'ERR_TEST_FAILURE' stack: |- * diff --git a/test/message/test_runner_unresolved_promise.out b/test/message/test_runner_unresolved_promise.out index 2bb543c..b4d6cba 100644 --- a/test/message/test_runner_unresolved_promise.out +++ b/test/message/test_runner_unresolved_promise.out @@ -9,7 +9,7 @@ not ok 2 - never resolving promise --- duration_ms: * failureType: 'cancelledByParent' - error: 'test did not finish before its parent and was cancelled' + error: 'Promise resolution is still pending but the event loop has already resolved' code: 'ERR_TEST_FAILURE' stack: |- * @@ -19,7 +19,7 @@ not ok 3 - fail --- duration_ms: 0 failureType: 'cancelledByParent' - error: 'test did not finish before its parent and was cancelled' + error: 'Promise resolution is still pending but the event loop has already resolved' code: 'ERR_TEST_FAILURE' stack: |- * From d885ee2860aff8098fff41c184081f3577f80b34 Mon Sep 17 00:00:00 2001 From: Moshe Atlow Date: Mon, 15 Aug 2022 12:12:11 +0300 Subject: [PATCH 02/24] feat: support programmatically running `--test` PR-URL: https://github.com/nodejs/node/pull/44241 Fixes: https://github.com/nodejs/node/issues/44023 Fixes: https://github.com/nodejs/node/issues/43675 Reviewed-By: Benjamin Gruenbaum Reviewed-By: Antoine du Hamel (cherry picked from commit 59527de13d39327eb3dfa8dedc92241eb40066d5) --- README.md | 74 ++++++++ lib/internal/main/test_runner.js | 149 +--------------- lib/internal/per_context/primordials.js | 4 + .../{bootstrap => process}/pre_execution.js | 0 lib/internal/test_runner/harness.js | 93 ++++------ lib/internal/test_runner/runner.js | 165 ++++++++++++++++++ lib/internal/test_runner/tap_stream.js | 36 ++-- lib/internal/test_runner/test.js | 61 +++++-- lib/internal/validators.js | 12 ++ lib/test.js | 20 ++- test/common/index.js | 27 ++- test/message/test_runner_no_tests.out | 2 +- test/parallel.mjs | 2 +- test/parallel/test-runner-run.mjs | 75 ++++++++ 14 files changed, 480 insertions(+), 240 deletions(-) rename lib/internal/{bootstrap => process}/pre_execution.js (100%) create mode 100644 lib/internal/test_runner/runner.js create mode 100644 test/parallel/test-runner-run.mjs diff --git a/README.md b/README.md index 66bc7f5..aa93361 100644 --- a/README.md +++ b/README.md @@ -324,6 +324,35 @@ Otherwise, the test is considered to be a failure. Test files must be executable by Node.js, but are not required to use the `node:test` module internally. +## `run([options])` + + + +* `options` {Object} Configuration options for running tests. The following + properties are supported: + * `concurrency` {number|boolean} If a number is provided, + then that many files would run in parallel. + If truthy, it would run (number of cpu cores - 1) + files in parallel. + If falsy, it would only run one file at a time. + If unspecified, subtests inherit this value from their parent. + **Default:** `true`. + * `files`: {Array} An array containing the list of files to run. + **Default** matching files from [test runner execution model][]. + * `signal` {AbortSignal} Allows aborting an in-progress test execution. + * `timeout` {number} A number of milliseconds the test execution will + fail after. + If unspecified, subtests inherit this value from their parent. + **Default:** `Infinity`. +* Returns: {TapStream} + +```js +run({ files: [path.resolve('./tests/test.js')] }) + .pipe(process.stdout); +``` + ## `test([name][, options][, fn])` - `name` {string} The name of the test, which is displayed when reporting test @@ -541,6 +570,47 @@ describe('tests', async () => { }); ``` +## Class: `TapStream` + + + +* Extends {ReadableStream} + +A successful call to [`run()`][] method will return a new {TapStream} +object, streaming a [TAP][] output +`TapStream` will emit events, in the order of the tests definition + +### Event: `'test:diagnostic'` + +* `message` {string} The diagnostic message. + +Emitted when [`context.diagnostic`][] is called. + +### Event: `'test:fail'` + +* `data` {Object} + * `duration` {number} The test duration. + * `error` {Error} The failure casing test to fail. + * `name` {string} The test name. + * `testNumber` {number} The ordinal number of the test. + * `todo` {string|undefined} Present if [`context.todo`][] is called + * `skip` {string|undefined} Present if [`context.skip`][] is called + +Emitted when a test fails. + +### Event: `'test:pass'` + +* `data` {Object} + * `duration` {number} The test duration. + * `name` {string} The test name. + * `testNumber` {number} The ordinal number of the test. + * `todo` {string|undefined} Present if [`context.todo`][] is called + * `skip` {string|undefined} Present if [`context.skip`][] is called + +Emitted when a test passes. + ## Class: `TestContext` An instance of `TestContext` is passed to each test function in order to @@ -712,6 +782,10 @@ The name of the suite. [TAP]: https://testanything.org/ [`SuiteContext`]: #class-suitecontext [`TestContext`]: #class-testcontext +[`context.diagnostic`]: #contextdiagnosticmessage +[`context.skip`]: #contextskipmessage +[`context.todo`]: #contexttodomessage +[`run()`]: #runoptions [`test()`]: #testname-options-fn [describe options]: #describename-options-fn [it options]: #testname-options-fn diff --git a/lib/internal/main/test_runner.js b/lib/internal/main/test_runner.js index db41695..ce20a1e 100644 --- a/lib/internal/main/test_runner.js +++ b/lib/internal/main/test_runner.js @@ -1,148 +1,15 @@ -// https://github.com/nodejs/node/blob/2fd4c013c221653da2a7921d08fe1aa96aaba504/lib/internal/main/test_runner.js +// https://github.com/nodejs/node/blob/59527de13d39327eb3dfa8dedc92241eb40066d5/lib/internal/main/test_runner.js 'use strict' -const { - ArrayFrom, - ArrayPrototypeFilter, - ArrayPrototypeIncludes, - ArrayPrototypeJoin, - ArrayPrototypePush, - ArrayPrototypeSlice, - ArrayPrototypeSort, - SafePromiseAll, - SafeSet -} = require('#internal/per_context/primordials') const { prepareMainThreadExecution -} = require('#internal/bootstrap/pre_execution') -const { spawn } = require('child_process') -const { readdirSync, statSync } = require('fs') -const { - codes: { - ERR_TEST_FAILURE - } -} = require('#internal/errors') -const { toArray } = require('#internal/streams/operators').promiseReturningOperators -const { test } = require('#internal/test_runner/harness') -const { kSubtestsFailed } = require('#internal/test_runner/test') -const { - isSupportedFileType, - doesPathMatchFilter -} = require('#internal/test_runner/utils') -const { basename, join, resolve } = require('path') -const { once } = require('events') -const kFilterArgs = ['--test'] +} = require('#internal/process/pre_execution') +const { run } = require('#internal/test_runner/runner') prepareMainThreadExecution(false) // markBootstrapComplete(); -// TODO(cjihrig): Replace this with recursive readdir once it lands. -function processPath (path, testFiles, options) { - const stats = statSync(path) - - if (stats.isFile()) { - if (options.userSupplied || - (options.underTestDir && isSupportedFileType(path)) || - doesPathMatchFilter(path)) { - testFiles.add(path) - } - } else if (stats.isDirectory()) { - const name = basename(path) - - if (!options.userSupplied && name === 'node_modules') { - return - } - - // 'test' directories get special treatment. Recursively add all .js, - // .cjs, and .mjs files in the 'test' directory. - const isTestDir = name === 'test' - const { underTestDir } = options - const entries = readdirSync(path) - - if (isTestDir) { - options.underTestDir = true - } - - options.userSupplied = false - - for (let i = 0; i < entries.length; i++) { - processPath(join(path, entries[i]), testFiles, options) - } - - options.underTestDir = underTestDir - } -} - -function createTestFileList () { - const cwd = process.cwd() - const hasUserSuppliedPaths = process.argv.length > 1 - const testPaths = hasUserSuppliedPaths - ? ArrayPrototypeSlice(process.argv, 1) - : [cwd] - const testFiles = new SafeSet() - - try { - for (let i = 0; i < testPaths.length; i++) { - const absolutePath = resolve(testPaths[i]) - - processPath(absolutePath, testFiles, { userSupplied: true }) - } - } catch (err) { - if (err?.code === 'ENOENT') { - console.error(`Could not find '${err.path}'`) - process.exit(1) - } - - throw err - } - - return ArrayPrototypeSort(ArrayFrom(testFiles)) -} - -function filterExecArgv (arg) { - return !ArrayPrototypeIncludes(kFilterArgs, arg) -} - -function runTestFile (path) { - return test(path, async (t) => { - const args = ArrayPrototypeFilter(process.execArgv, filterExecArgv) - ArrayPrototypePush(args, path) - - const child = spawn(process.execPath, args, { signal: t.signal, encoding: 'utf8' }) - // TODO(cjihrig): Implement a TAP parser to read the child's stdout - // instead of just displaying it all if the child fails. - let err - - child.on('error', (error) => { - err = error - }) - - const { 0: { 0: code, 1: signal }, 1: stdout, 2: stderr } = await SafePromiseAll([ - once(child, 'exit', { signal: t.signal }), - toArray.call(child.stdout, { signal: t.signal }), - toArray.call(child.stderr, { signal: t.signal }) - ]) - - if (code !== 0 || signal !== null) { - if (!err) { - err = new ERR_TEST_FAILURE('test failed', kSubtestsFailed) - err.exitCode = code - err.signal = signal - err.stdout = ArrayPrototypeJoin(stdout, '') - err.stderr = ArrayPrototypeJoin(stderr, '') - // The stack will not be useful since the failures came from tests - // in a child process. - err.stack = undefined - } - - throw err - } - }) -} - -;(async function main () { - const testFiles = createTestFileList() - - for (let i = 0; i < testFiles.length; i++) { - runTestFile(testFiles[i]) - } -})() +const tapStream = run() +tapStream.pipe(process.stdout) +tapStream.once('test:fail', () => { + process.exitCode = 1 +}) diff --git a/lib/internal/per_context/primordials.js b/lib/internal/per_context/primordials.js index e599da9..70f1abf 100644 --- a/lib/internal/per_context/primordials.js +++ b/lib/internal/per_context/primordials.js @@ -3,6 +3,8 @@ const replaceAll = require('string.prototype.replaceall') exports.ArrayFrom = (it, mapFn) => Array.from(it, mapFn) +exports.ArrayIsArray = Array.isArray +exports.ArrayPrototypeConcat = (arr, ...el) => arr.concat(...el) exports.ArrayPrototypeFilter = (arr, fn) => arr.filter(fn) exports.ArrayPrototypeForEach = (arr, fn, thisArg) => arr.forEach(fn, thisArg) exports.ArrayPrototypeIncludes = (arr, el, fromIndex) => arr.includes(el, fromIndex) @@ -20,6 +22,7 @@ exports.FunctionPrototype = Function.prototype exports.FunctionPrototypeBind = (fn, obj, ...args) => fn.bind(obj, ...args) exports.MathMax = (...args) => Math.max(...args) exports.Number = Number +exports.ObjectAssign = (target, ...sources) => Object.assign(target, ...sources) exports.ObjectCreate = obj => Object.create(obj) exports.ObjectDefineProperties = (obj, props) => Object.defineProperties(obj, props) exports.ObjectDefineProperty = (obj, key, descr) => Object.defineProperty(obj, key, descr) @@ -41,6 +44,7 @@ exports.SafePromiseAll = (array, mapFn) => Promise.all(mapFn ? array.map(mapFn) exports.SafePromiseRace = (array, mapFn) => Promise.race(mapFn ? array.map(mapFn) : array) exports.SafeSet = Set exports.SafeWeakMap = WeakMap +exports.SafeWeakSet = WeakSet exports.StringPrototypeIncludes = (str, needle) => str.includes(needle) exports.StringPrototypeMatch = (str, reg) => str.match(reg) exports.StringPrototypeReplace = (str, search, replacement) => diff --git a/lib/internal/bootstrap/pre_execution.js b/lib/internal/process/pre_execution.js similarity index 100% rename from lib/internal/bootstrap/pre_execution.js rename to lib/internal/process/pre_execution.js diff --git a/lib/internal/test_runner/harness.js b/lib/internal/test_runner/harness.js index 987de95..bddfedc 100644 --- a/lib/internal/test_runner/harness.js +++ b/lib/internal/test_runner/harness.js @@ -1,9 +1,9 @@ -// https://github.com/nodejs/node/blob/8cf33850bea691d8c53b2d4175c959c8549aa76c/lib/internal/test_runner/harness.js +// https://github.com/nodejs/node/blob/59527de13d39327eb3dfa8dedc92241eb40066d5/lib/internal/test_runner/harness.js 'use strict' const { ArrayPrototypeForEach, - FunctionPrototypeBind, - SafeMap + SafeMap, + SafeWeakSet } = require('#internal/per_context/primordials') const { createHook, @@ -14,13 +14,18 @@ const { ERR_TEST_FAILURE } } = require('#internal/errors') +const { kEmptyObject } = require('#internal/util') const { getOptionValue } = require('#internal/options') const { kCancelledByParent, Test, ItTest, Suite } = require('#internal/test_runner/test') +const { bigint: hrtime } = process.hrtime -const isTestRunner = getOptionValue('--test') +const isTestRunnerCli = getOptionValue('--test') const testResources = new SafeMap() -const root = new Test({ __proto__: null, name: '' }) -let wasRootSetup = false +const wasRootSetup = new SafeWeakSet() + +function createTestTree (options = kEmptyObject) { + return setup(new Test({ __proto__: null, ...options, name: '' })) +} function createProcessEventHandler (eventName, rootTest) { return (err) => { @@ -51,7 +56,7 @@ function createProcessEventHandler (eventName, rootTest) { } function setup (root) { - if (wasRootSetup) { + if (wasRootSetup.has(root)) { return root } const hook = createHook({ @@ -84,52 +89,9 @@ function setup (root) { 'Promise resolution is still pending but the event loop has already resolved', kCancelledByParent)) - let passCount = 0 - let failCount = 0 - let skipCount = 0 - let todoCount = 0 - let cancelledCount = 0 - - for (let i = 0; i < root.subtests.length; i++) { - const test = root.subtests[i] - - // Check SKIP and TODO tests first, as those should not be counted as - // failures. - if (test.skipped) { - skipCount++ - } else if (test.isTodo) { - todoCount++ - } else if (test.cancelled) { - cancelledCount++ - } else if (!test.passed) { - failCount++ - } else { - passCount++ - } - } - - root.reporter.plan(root.indent, root.subtests.length) - - for (let i = 0; i < root.diagnostics.length; i++) { - root.reporter.diagnostic(root.indent, root.diagnostics[i]) - } - - root.reporter.diagnostic(root.indent, `tests ${root.subtests.length}`) - root.reporter.diagnostic(root.indent, `pass ${passCount}`) - root.reporter.diagnostic(root.indent, `fail ${failCount}`) - root.reporter.diagnostic(root.indent, `cancelled ${cancelledCount}`) - root.reporter.diagnostic(root.indent, `skipped ${skipCount}`) - root.reporter.diagnostic(root.indent, `todo ${todoCount}`) - root.reporter.diagnostic(root.indent, `duration_ms ${process.uptime()}`) - - root.reporter.push(null) hook.disable() process.removeListener('unhandledRejection', rejectionHandler) process.removeListener('uncaughtException', exceptionHandler) - - if (failCount > 0 || cancelledCount > 0) { - process.exitCode = 1 - } } const terminationHandler = () => { @@ -140,29 +102,41 @@ function setup (root) { process.on('uncaughtException', exceptionHandler) process.on('unhandledRejection', rejectionHandler) process.on('beforeExit', exitHandler) - // TODO(MoLow): Make it configurable to hook when isTestRunner === false. - if (isTestRunner) { + // TODO(MoLow): Make it configurable to hook when isTestRunnerCli === false. + if (isTestRunnerCli) { process.on('SIGINT', terminationHandler) process.on('SIGTERM', terminationHandler) } - root.reporter.pipe(process.stdout) + root.startTime = hrtime() root.reporter.version() - wasRootSetup = true + wasRootSetup.add(root) return root } +let globalRoot +function getGlobalRoot () { + if (!globalRoot) { + globalRoot = createTestTree() + globalRoot.reporter.pipe(process.stdout) + globalRoot.reporter.once('test:fail', () => { + process.exitCode = 1 + }) + } + return globalRoot +} + function test (name, options, fn) { - const subtest = setup(root).createSubtest(Test, name, options, fn) + const subtest = getGlobalRoot().createSubtest(Test, name, options, fn) return subtest.start() } function runInParentContext (Factory) { function run (name, options, fn, overrides) { - const parent = testResources.get(executionAsyncId()) || setup(root) + const parent = testResources.get(executionAsyncId()) || getGlobalRoot() const subtest = parent.createSubtest(Factory, name, options, fn, overrides) - if (parent === root) { + if (parent === getGlobalRoot()) { subtest.start() } } @@ -181,13 +155,14 @@ function runInParentContext (Factory) { function hook (hook) { return (fn, options) => { - const parent = testResources.get(executionAsyncId()) || setup(root) + const parent = testResources.get(executionAsyncId()) || getGlobalRoot() parent.createHook(hook, fn, options) } } module.exports = { - test: FunctionPrototypeBind(test, root), + createTestTree, + test, describe: runInParentContext(Suite), it: runInParentContext(ItTest), before: hook('before'), diff --git a/lib/internal/test_runner/runner.js b/lib/internal/test_runner/runner.js new file mode 100644 index 0000000..8266087 --- /dev/null +++ b/lib/internal/test_runner/runner.js @@ -0,0 +1,165 @@ +// https://github.com/nodejs/node/blob/59527de13d39327eb3dfa8dedc92241eb40066d5/lib/internal/test_runner/runner.js +'use strict' +const { + ArrayFrom, + ArrayPrototypeConcat, + ArrayPrototypeFilter, + ArrayPrototypeIncludes, + ArrayPrototypeJoin, + ArrayPrototypeSlice, + ArrayPrototypeSort, + ObjectAssign, + PromisePrototypeThen, + SafePromiseAll, + SafeSet +} = require('#internal/per_context/primordials') + +const { spawn } = require('child_process') +const { readdirSync, statSync } = require('fs') +const { + codes: { + ERR_TEST_FAILURE + } +} = require('#internal/errors') +const { toArray } = require('#internal/streams/operators').promiseReturningOperators +const { validateArray } = require('#internal/validators') +const { kEmptyObject } = require('#internal/util') +const { createTestTree } = require('#internal/test_runner/harness') +const { kSubtestsFailed, Test } = require('#internal/test_runner/test') +const { + isSupportedFileType, + doesPathMatchFilter +} = require('#internal/test_runner/utils') +const { basename, join, resolve } = require('path') +const { once } = require('events') + +const kFilterArgs = ['--test'] + +// TODO(cjihrig): Replace this with recursive readdir once it lands. +function processPath (path, testFiles, options) { + const stats = statSync(path) + + if (stats.isFile()) { + if (options.userSupplied || + (options.underTestDir && isSupportedFileType(path)) || + doesPathMatchFilter(path)) { + testFiles.add(path) + } + } else if (stats.isDirectory()) { + const name = basename(path) + + if (!options.userSupplied && name === 'node_modules') { + return + } + + // 'test' directories get special treatment. Recursively add all .js, + // .cjs, and .mjs files in the 'test' directory. + const isTestDir = name === 'test' + const { underTestDir } = options + const entries = readdirSync(path) + + if (isTestDir) { + options.underTestDir = true + } + + options.userSupplied = false + + for (let i = 0; i < entries.length; i++) { + processPath(join(path, entries[i]), testFiles, options) + } + + options.underTestDir = underTestDir + } +} + +function createTestFileList () { + const cwd = process.cwd() + const hasUserSuppliedPaths = process.argv.length > 1 + const testPaths = hasUserSuppliedPaths + ? ArrayPrototypeSlice(process.argv, 1) + : [cwd] + const testFiles = new SafeSet() + + try { + for (let i = 0; i < testPaths.length; i++) { + const absolutePath = resolve(testPaths[i]) + + processPath(absolutePath, testFiles, { userSupplied: true }) + } + } catch (err) { + if (err?.code === 'ENOENT') { + console.error(`Could not find '${err.path}'`) + process.exit(1) + } + + throw err + } + + return ArrayPrototypeSort(ArrayFrom(testFiles)) +} + +function filterExecArgv (arg) { + return !ArrayPrototypeIncludes(kFilterArgs, arg) +} + +function runTestFile (path, root) { + const subtest = root.createSubtest(Test, path, async (t) => { + const args = ArrayPrototypeConcat( + ArrayPrototypeFilter(process.execArgv, filterExecArgv), + path) + + const child = spawn(process.execPath, args, { signal: t.signal, encoding: 'utf8' }) + // TODO(cjihrig): Implement a TAP parser to read the child's stdout + // instead of just displaying it all if the child fails. + let err + + child.on('error', (error) => { + err = error + }) + + const { 0: { 0: code, 1: signal }, 1: stdout, 2: stderr } = await SafePromiseAll([ + once(child, 'exit', { signal: t.signal }), + toArray.call(child.stdout, { signal: t.signal }), + toArray.call(child.stderr, { signal: t.signal }) + ]) + + if (code !== 0 || signal !== null) { + if (!err) { + err = ObjectAssign(new ERR_TEST_FAILURE('test failed', kSubtestsFailed), { + __proto__: null, + exitCode: code, + signal, + stdout: ArrayPrototypeJoin(stdout, ''), + stderr: ArrayPrototypeJoin(stderr, ''), + // The stack will not be useful since the failures came from tests + // in a child process. + stack: undefined + }) + } + + throw err + } + }) + return subtest.start() +} + +function run (options) { + if (options === null || typeof options !== 'object') { + options = kEmptyObject + } + const { concurrency, timeout, signal, files } = options + + if (files != null) { + validateArray(files, 'options.files') + } + + const root = createTestTree({ concurrency, timeout, signal }) + const testFiles = files ?? createTestFileList() + + PromisePrototypeThen(SafePromiseAll(testFiles, (path) => runTestFile(path, root)), + () => root.postRun()) + + return root.reporter +} + +module.exports = { run } diff --git a/lib/internal/test_runner/tap_stream.js b/lib/internal/test_runner/tap_stream.js index e240598..769d275 100644 --- a/lib/internal/test_runner/tap_stream.js +++ b/lib/internal/test_runner/tap_stream.js @@ -1,10 +1,11 @@ -// https://github.com/nodejs/node/blob/0d46cf6af8977d1e2e4c4886bf0f7e0dbe76d21c/lib/internal/test_runner/tap_stream.js +// https://github.com/nodejs/node/blob/59527de13d39327eb3dfa8dedc92241eb40066d5/lib/internal/test_runner/tap_stream.js 'use strict' const { ArrayPrototypeForEach, ArrayPrototypeJoin, + ArrayPrototypeMap, ArrayPrototypePush, ArrayPrototypeShift, ObjectEntries, @@ -14,7 +15,7 @@ const { } = require('#internal/per_context/primordials') const { inspectWithNoCustomRetry } = require('#internal/errors') const Readable = require('#internal/streams/readable') -const { isError } = require('#internal/util') +const { isError, kEmptyObject } = require('#internal/util') const kFrameStartRegExp = /^ {4}at / const kLineBreakRegExp = /\n|\r\n/ const inspectOptions = { colors: false, breakLength: Infinity } @@ -53,12 +54,16 @@ class TapStream extends Readable { this.#tryPush(`Bail out!${message ? ` ${tapEscape(message)}` : ''}\n`) } - fail (indent, testNumber, description, directive) { - this.#test(indent, testNumber, 'not ok', description, directive) + fail (indent, testNumber, name, duration, error, directive) { + this.emit('test:fail', { __proto__: null, name, testNumber, duration, ...directive, error }) + this.#test(indent, testNumber, 'not ok', name, directive) + this.#details(indent, duration, error) } - ok (indent, testNumber, description, directive) { - this.#test(indent, testNumber, 'ok', description, directive) + ok (indent, testNumber, name, duration, directive) { + this.emit('test:pass', { __proto__: null, name, testNumber, duration, ...directive }) + this.#test(indent, testNumber, 'ok', name, directive) + this.#details(indent, duration, null) } plan (indent, count, explanation) { @@ -68,18 +73,18 @@ class TapStream extends Readable { } getSkip (reason) { - return `SKIP${reason ? ` ${tapEscape(reason)}` : ''}` + return { __proto__: null, skip: reason } } getTodo (reason) { - return `TODO${reason ? ` ${tapEscape(reason)}` : ''}` + return { __proto__: null, todo: reason } } subtest (indent, name) { this.#tryPush(`${indent}# Subtest: ${tapEscape(name)}\n`) } - details (indent, duration, error) { + #details (indent, duration, error) { let details = `${indent} ---\n` details += jsToYaml(indent, 'duration_ms', duration) @@ -89,6 +94,7 @@ class TapStream extends Readable { } diagnostic (indent, message) { + this.emit('test:diagnostic', message) this.#tryPush(`${indent}# ${tapEscape(message)}\n`) } @@ -96,16 +102,16 @@ class TapStream extends Readable { this.#tryPush('TAP version 13\n') } - #test (indent, testNumber, status, description, directive) { + #test (indent, testNumber, status, name, directive = kEmptyObject) { let line = `${indent}${status} ${testNumber}` - if (description) { - line += ` ${tapEscape(description)}` + if (name) { + line += ` ${tapEscape(`- ${name}`)}` } - if (directive) { - line += ` # ${directive}` - } + line += ArrayPrototypeJoin(ArrayPrototypeMap(ObjectEntries(directive), ({ 0: key, 1: value }) => ( + ` # ${key.toUpperCase()}${value ? ` ${tapEscape(value)}` : ''}` + )), '') line += '\n' this.#tryPush(line) diff --git a/lib/internal/test_runner/test.js b/lib/internal/test_runner/test.js index b4fac63..20c1d98 100644 --- a/lib/internal/test_runner/test.js +++ b/lib/internal/test_runner/test.js @@ -1,4 +1,4 @@ -// https://github.com/nodejs/node/blob/0d46cf6af8977d1e2e4c4886bf0f7e0dbe76d21c/lib/internal/test_runner/test.js +// https://github.com/nodejs/node/blob/59527de13d39327eb3dfa8dedc92241eb40066d5/lib/internal/test_runner/test.js 'use strict' @@ -179,7 +179,7 @@ class Test extends AsyncResource { case 'boolean': if (concurrency) { - this.concurrency = isTestRunner ? MathMax(cpus().length - 1, 1) : Infinity + this.concurrency = parent === null ? MathMax(cpus().length - 1, 1) : Infinity } else { this.concurrency = 1 } @@ -514,7 +514,7 @@ class Test extends AsyncResource { } postRun (pendingSubtestsError) { - let failedSubtests = 0 + const counters = { __proto__: null, failed: 0, passed: 0, cancelled: 0, skipped: 0, todo: 0, totalFailed: 0 } // If the test was failed before it even started, then the end time will // be earlier than the start time. Correct that here. @@ -534,14 +534,28 @@ class Test extends AsyncResource { subtest.postRun(pendingSubtestsError) } + // Check SKIP and TODO tests first, as those should not be counted as + // failures. + if (subtest.skipped) { + counters.skipped++ + } else if (subtest.isTodo) { + counters.todo++ + } else if (subtest.cancelled) { + counters.cancelled++ + } else if (!subtest.passed) { + counters.failed++ + } else { + counters.passed++ + } + if (!subtest.passed) { - failedSubtests++ + counters.totalFailed++ } } - if (this.passed && failedSubtests > 0) { - const subtestString = `subtest${failedSubtests > 1 ? 's' : ''}` - const msg = `${failedSubtests} ${subtestString} failed` + if ((this.passed || this.parent === null) && counters.totalFailed > 0) { + const subtestString = `subtest${counters.totalFailed > 1 ? 's' : ''}` + const msg = `${counters.totalFailed} ${subtestString} failed` this.fail(new ERR_TEST_FAILURE(msg, kSubtestsFailed)) } @@ -553,6 +567,22 @@ class Test extends AsyncResource { this.parent.addReadySubtest(this) this.parent.processReadySubtestRange(false) this.parent.processPendingSubtests() + } else if (!this.reported) { + this.reported = true + this.reporter.plan(this.indent, this.subtests.length) + + for (let i = 0; i < this.diagnostics.length; i++) { + this.reporter.diagnostic(this.indent, this.diagnostics[i]) + } + + this.reporter.diagnostic(this.indent, `tests ${this.subtests.length}`) + this.reporter.diagnostic(this.indent, `pass ${counters.passed}`) + this.reporter.diagnostic(this.indent, `fail ${counters.failed}`) + this.reporter.diagnostic(this.indent, `cancelled ${counters.cancelled}`) + this.reporter.diagnostic(this.indent, `skipped ${counters.skipped}`) + this.reporter.diagnostic(this.indent, `todo ${counters.todo}`) + this.reporter.diagnostic(this.indent, `duration_ms ${this.#duration()}`) + this.reporter.push(null) } } @@ -586,10 +616,12 @@ class Test extends AsyncResource { this.finished = true } - report () { + #duration () { // Duration is recorded in BigInt nanoseconds. Convert to seconds. - const duration = Number(this.endTime - this.startTime) / 1_000_000_000 - const message = `- ${this.name}` + return Number(this.endTime - this.startTime) / 1_000_000_000 + } + + report () { let directive if (this.skipped) { @@ -599,13 +631,11 @@ class Test extends AsyncResource { } if (this.passed) { - this.reporter.ok(this.indent, this.testNumber, message, directive) + this.reporter.ok(this.indent, this.testNumber, this.name, this.#duration(), directive) } else { - this.reporter.fail(this.indent, this.testNumber, message, directive) + this.reporter.fail(this.indent, this.testNumber, this.name, this.#duration(), this.error, directive) } - this.reporter.details(this.indent, duration, this.error) - for (let i = 0; i < this.diagnostics.length; i++) { this.reporter.diagnostic(this.indent, this.diagnostics[i]) } @@ -630,6 +660,9 @@ class TestHook extends Test { getRunArgs () { return this.#args } + + postRun () { + } } class ItTest extends Test { diff --git a/lib/internal/validators.js b/lib/internal/validators.js index 511e61b..06e5746 100644 --- a/lib/internal/validators.js +++ b/lib/internal/validators.js @@ -1,5 +1,6 @@ // https://github.com/nodejs/node/blob/60da0a1b364efdd84870269d23b39faa12fb46d8/lib/internal/validators.js const { + ArrayIsArray, ArrayPrototypeIncludes, ArrayPrototypeJoin, ArrayPrototypeMap @@ -64,9 +65,20 @@ const validateOneOf = (value, name, oneOf) => { } } +const validateArray = (value, name, minLength = 0) => { + if (!ArrayIsArray(value)) { + throw new ERR_INVALID_ARG_TYPE(name, 'Array', value) + } + if (value.length < minLength) { + const reason = `must be longer than ${minLength}` + throw new ERR_INVALID_ARG_VALUE(name, value, reason) + } +} + module.exports = { isUint32, validateAbortSignal, + validateArray, validateNumber, validateOneOf, validateUint32 diff --git a/lib/test.js b/lib/test.js index a65fb3c..1662780 100644 --- a/lib/test.js +++ b/lib/test.js @@ -1,12 +1,16 @@ -// https://github.com/nodejs/node/blob/659dc126932f986fc33c7f1c878cb2b57a1e2fac/lib/test.js 'use strict' +const { ObjectAssign } = require('#internal/per_context/primordials') const { test, describe, it, before, after, beforeEach, afterEach } = require('#internal/test_runner/harness') +const { run } = require('#internal/test_runner/runner') module.exports = test -module.exports.test = test -module.exports.describe = describe -module.exports.it = it -module.exports.before = before -module.exports.after = after -module.exports.beforeEach = beforeEach -module.exports.afterEach = afterEach +ObjectAssign(module.exports, { + after, + afterEach, + before, + beforeEach, + describe, + it, + run, + test +}) diff --git a/test/common/index.js b/test/common/index.js index 91fcd41..a6269ac 100644 --- a/test/common/index.js +++ b/test/common/index.js @@ -37,6 +37,30 @@ function mustCall (fn, exact) { return _mustCallInner(fn, exact, 'exact') } +function getCallSite (top) { + const originalStackFormatter = Error.prepareStackTrace + Error.prepareStackTrace = (err, stack) => // eslint-disable-line n/handle-callback-err + `${stack[0].getFileName()}:${stack[0].getLineNumber()}` + const err = new Error() + Error.captureStackTrace(err, top) + // With the V8 Error API, the stack is not formatted until it is accessed + err.stack // eslint-disable-line no-unused-expressions + Error.prepareStackTrace = originalStackFormatter + return err.stack +} + +function mustNotCall (msg) { + const callSite = getCallSite(mustNotCall) + return function mustNotCall (...args) { + const argsInfo = args.length > 0 + ? `\ncalled with arguments: ${args.map((arg) => util.inspect(arg)).join(', ')}` + : '' + assert.fail( + `${msg || 'function should not have been called'} at ${callSite}` + + argsInfo) + } +} + function _mustCallInner (fn, criteria = 1, field) { if (process._exiting) { throw new Error('Cannot use common.mustCall*() in process exit handler') } if (typeof fn === 'number') { @@ -145,5 +169,6 @@ if (typeof AbortSignal !== 'undefined' && (process.version.startsWith('v14.') || module.exports = { expectsError, isWindow: process.platform === 'win32', - mustCall + mustCall, + mustNotCall } diff --git a/test/message/test_runner_no_tests.out b/test/message/test_runner_no_tests.out index 9f84e58..9daeafb 100644 --- a/test/message/test_runner_no_tests.out +++ b/test/message/test_runner_no_tests.out @@ -1 +1 @@ -bound test +test diff --git a/test/parallel.mjs b/test/parallel.mjs index 48e0f61..e0a5050 100755 --- a/test/parallel.mjs +++ b/test/parallel.mjs @@ -10,7 +10,7 @@ const PARALLEL_DIR = new URL('./parallel/', import.meta.url) const dir = await fs.opendir(PARALLEL_DIR) for await (const { name } of dir) { - if (!name.endsWith('.js')) continue + if (!name.endsWith('.js') && !name.endsWith('.mjs')) continue const cp = spawn( process.execPath, [fileURLToPath(new URL(name, PARALLEL_DIR))], diff --git a/test/parallel/test-runner-run.mjs b/test/parallel/test-runner-run.mjs new file mode 100644 index 0000000..66837ae --- /dev/null +++ b/test/parallel/test-runner-run.mjs @@ -0,0 +1,75 @@ +// https://github.com/nodejs/node/blob/59527de13d39327eb3dfa8dedc92241eb40066d5/test/parallel/test-runner-run.mjs + +import common from '../common/index.js' +import fixtures from '../common/fixtures.js' +import { join } from 'node:path' +import pkg from '#node:test' +import assert from 'node:assert' + +const { describe, it, run } = pkg + +const testFixtures = fixtures.path('test-runner') + +describe('require(\'node:test\').run', { concurrency: true }, () => { + it('should run with no tests', async () => { + const stream = run({ files: [] }) + stream.setEncoding('utf8') + stream.on('test:fail', common.mustNotCall()) + stream.on('test:pass', common.mustNotCall()) + // eslint-disable-next-line no-unused-vars + for await (const _ of stream); // TODO(MoLow): assert.snapshot + }) + + it('should fail with non existing file', async () => { + const stream = run({ files: ['a-random-file-that-does-not-exist.js'] }) + stream.on('test:fail', common.mustCall(1)) + stream.on('test:pass', common.mustNotCall()) + // eslint-disable-next-line no-unused-vars + for await (const _ of stream); // TODO(MoLow): assert.snapshot + }) + + it('should succeed with a file', async () => { + const stream = run({ files: [join(testFixtures, 'test/random.cjs')] }) + stream.on('test:fail', common.mustNotCall()) + stream.on('test:pass', common.mustCall(1)) + // eslint-disable-next-line no-unused-vars + for await (const _ of stream); // TODO(MoLow): assert.snapshot + }) + + it('should run same file twice', async () => { + const stream = run({ files: [join(testFixtures, 'test/random.cjs'), join(testFixtures, 'test/random.cjs')] }) + stream.on('test:fail', common.mustNotCall()) + stream.on('test:pass', common.mustCall(2)) + // eslint-disable-next-line no-unused-vars + for await (const _ of stream); // TODO(MoLow): assert.snapshot + }) + + it('should run a failed test', async () => { + const stream = run({ files: [testFixtures] }) + stream.on('test:fail', common.mustCall(1)) + stream.on('test:pass', common.mustNotCall()) + // eslint-disable-next-line no-unused-vars + for await (const _ of stream); // TODO(MoLow): assert.snapshot + }) + + it('should support timeout', async () => { + const stream = run({ + timeout: 50, + files: [ + fixtures.path('test-runner', 'never_ending_sync.js'), + fixtures.path('test-runner', 'never_ending_async.js') + ] + }) + stream.on('test:fail', common.mustCall(2)) + stream.on('test:pass', common.mustNotCall()) + // eslint-disable-next-line no-unused-vars + for await (const _ of stream); // TODO(MoLow): assert.snapshot + }) + + it('should validate files', async () => { + [Symbol(''), {}, () => {}, 0, 1, 0n, 1n, '', '1', Promise.resolve([]), true, false] + .forEach((files) => assert.throws(() => run({ files }), { + code: 'ERR_INVALID_ARG_TYPE' + })) + }) +}) From 27241c3cd27ab4321ee8177ec1d44a49c261dd42 Mon Sep 17 00:00:00 2001 From: Moshe Atlow Date: Sun, 4 Sep 2022 10:06:24 +0300 Subject: [PATCH 03/24] fix: fix `duration_ms` to be milliseconds PR-URL: https://github.com/nodejs/node/pull/44450 Reviewed-By: Benjamin Gruenbaum Reviewed-By: Antoine du Hamel Reviewed-By: Franziska Hinkelmann Reviewed-By: Rich Trott Reviewed-By: Minwoo Jung (cherry picked from commit 6ee1f3444f8c1cf005153f936ffc74221d55658b) --- lib/internal/test_runner/test.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/internal/test_runner/test.js b/lib/internal/test_runner/test.js index 20c1d98..d213d04 100644 --- a/lib/internal/test_runner/test.js +++ b/lib/internal/test_runner/test.js @@ -1,4 +1,4 @@ -// https://github.com/nodejs/node/blob/59527de13d39327eb3dfa8dedc92241eb40066d5/lib/internal/test_runner/test.js +// https://github.com/nodejs/node/blob/6ee1f3444f8c1cf005153f936ffc74221d55658b/lib/internal/test_runner/test.js 'use strict' @@ -617,8 +617,8 @@ class Test extends AsyncResource { } #duration () { - // Duration is recorded in BigInt nanoseconds. Convert to seconds. - return Number(this.endTime - this.startTime) / 1_000_000_000 + // Duration is recorded in BigInt nanoseconds. Convert to milliseconds. + return Number(this.endTime - this.startTime) / 1_000_000 } report () { From 6755536a5a94653de26be4dca1c0c6003217031f Mon Sep 17 00:00:00 2001 From: Moshe Atlow Date: Sat, 10 Sep 2022 20:01:42 +0300 Subject: [PATCH 04/24] feat: support using `--inspect` with `--test` PR-URL: https://github.com/nodejs/node/pull/44520 Reviewed-By: Benjamin Gruenbaum Reviewed-By: Antoine du Hamel (cherry picked from commit a165193c5c8e4bcfbd12b2c3f6e55a81a251c258) --- README.md | 6 +++ lib/internal/main/test_runner.js | 15 +++++- lib/internal/per_context/primordials.js | 4 ++ lib/internal/test_runner/runner.js | 65 ++++++++++++++++++++----- lib/internal/test_runner/test.js | 6 +-- lib/internal/util/inspector.js | 48 ++++++++++++++++++ test/parallel/test-runner-cli.js | 9 +--- 7 files changed, 126 insertions(+), 27 deletions(-) create mode 100644 lib/internal/util/inspector.js diff --git a/README.md b/README.md index aa93361..9b23018 100644 --- a/README.md +++ b/README.md @@ -346,6 +346,12 @@ added: REPLACEME fail after. If unspecified, subtests inherit this value from their parent. **Default:** `Infinity`. + * `inspectPort` {number|Function} Sets inspector port of test child process. + This can be a number, or a function that takes no arguments and returns a + number. If a nullish value is provided, each process gets its own port, + incremented from the primary's `process.debugPort`. + **Default:** `undefined`. + * Returns: {TapStream} ```js diff --git a/lib/internal/main/test_runner.js b/lib/internal/main/test_runner.js index ce20a1e..ed7c644 100644 --- a/lib/internal/main/test_runner.js +++ b/lib/internal/main/test_runner.js @@ -1,14 +1,25 @@ -// https://github.com/nodejs/node/blob/59527de13d39327eb3dfa8dedc92241eb40066d5/lib/internal/main/test_runner.js +// https://github.com/nodejs/node/blob/a165193c5c8e4bcfbd12b2c3f6e55a81a251c258/lib/internal/main/test_runner.js 'use strict' const { prepareMainThreadExecution } = require('#internal/process/pre_execution') +const { isUsingInspector } = require('#internal/util/inspector') const { run } = require('#internal/test_runner/runner') prepareMainThreadExecution(false) // markBootstrapComplete(); -const tapStream = run() +let concurrency = true +let inspectPort + +if (isUsingInspector()) { + process.emitWarning('Using the inspector with --test forces running at a concurrency of 1. ' + + 'Use the inspectPort option to run with concurrency') + concurrency = 1 + inspectPort = process.debugPort +} + +const tapStream = run({ concurrency, inspectPort }) tapStream.pipe(process.stdout) tapStream.once('test:fail', () => { process.exitCode = 1 diff --git a/lib/internal/per_context/primordials.js b/lib/internal/per_context/primordials.js index 70f1abf..96987ca 100644 --- a/lib/internal/per_context/primordials.js +++ b/lib/internal/per_context/primordials.js @@ -10,10 +10,12 @@ exports.ArrayPrototypeForEach = (arr, fn, thisArg) => arr.forEach(fn, thisArg) exports.ArrayPrototypeIncludes = (arr, el, fromIndex) => arr.includes(el, fromIndex) exports.ArrayPrototypeJoin = (arr, str) => arr.join(str) exports.ArrayPrototypeMap = (arr, mapFn) => arr.map(mapFn) +exports.ArrayPrototypePop = arr => arr.pop() exports.ArrayPrototypePush = (arr, ...el) => arr.push(...el) exports.ArrayPrototypeReduce = (arr, fn, originalVal) => arr.reduce(fn, originalVal) exports.ArrayPrototypeShift = arr => arr.shift() exports.ArrayPrototypeSlice = (arr, offset) => arr.slice(offset) +exports.ArrayPrototypeSome = (arr, fn) => arr.some(fn) exports.ArrayPrototypeSort = (arr, fn) => arr.sort(fn) exports.ArrayPrototypeUnshift = (arr, ...el) => arr.unshift(...el) exports.Error = Error @@ -38,6 +40,7 @@ exports.PromiseAll = iterator => Promise.all(iterator) exports.PromisePrototypeThen = (promise, thenFn, catchFn) => promise.then(thenFn, catchFn) exports.PromiseResolve = val => Promise.resolve(val) exports.PromiseRace = val => Promise.race(val) +exports.RegExpPrototypeSymbolSplit = (reg, str) => reg[Symbol.split](str) exports.SafeArrayIterator = class ArrayIterator {constructor (array) { this.array = array }[Symbol.iterator] () { return this.array.values() }} exports.SafeMap = Map exports.SafePromiseAll = (array, mapFn) => Promise.all(mapFn ? array.map(mapFn) : array) @@ -45,6 +48,7 @@ exports.SafePromiseRace = (array, mapFn) => Promise.race(mapFn ? array.map(mapFn exports.SafeSet = Set exports.SafeWeakMap = WeakMap exports.SafeWeakSet = WeakSet +exports.StringPrototypeEndsWith = (haystack, needle, index) => haystack.endsWith(needle, index) exports.StringPrototypeIncludes = (str, needle) => str.includes(needle) exports.StringPrototypeMatch = (str, reg) => str.match(reg) exports.StringPrototypeReplace = (str, search, replacement) => diff --git a/lib/internal/test_runner/runner.js b/lib/internal/test_runner/runner.js index 8266087..5a40097 100644 --- a/lib/internal/test_runner/runner.js +++ b/lib/internal/test_runner/runner.js @@ -1,19 +1,23 @@ -// https://github.com/nodejs/node/blob/59527de13d39327eb3dfa8dedc92241eb40066d5/lib/internal/test_runner/runner.js +// https://github.com/nodejs/node/blob/a165193c5c8e4bcfbd12b2c3f6e55a81a251c258/lib/internal/test_runner/runner.js 'use strict' const { ArrayFrom, - ArrayPrototypeConcat, ArrayPrototypeFilter, ArrayPrototypeIncludes, ArrayPrototypeJoin, + ArrayPrototypePop, + ArrayPrototypePush, ArrayPrototypeSlice, ArrayPrototypeSort, ObjectAssign, PromisePrototypeThen, + RegExpPrototypeSymbolSplit, SafePromiseAll, - SafeSet + SafeSet, + StringPrototypeEndsWith } = require('#internal/per_context/primordials') +const { Buffer } = require('buffer') const { spawn } = require('child_process') const { readdirSync, statSync } = require('fs') const { @@ -23,6 +27,7 @@ const { } = require('#internal/errors') const { toArray } = require('#internal/streams/operators').promiseReturningOperators const { validateArray } = require('#internal/validators') +const { getInspectPort, isUsingInspector, isInspectorMessage } = require('#internal/util/inspector') const { kEmptyObject } = require('#internal/util') const { createTestTree } = require('#internal/test_runner/harness') const { kSubtestsFailed, Test } = require('#internal/test_runner/test') @@ -102,25 +107,59 @@ function filterExecArgv (arg) { return !ArrayPrototypeIncludes(kFilterArgs, arg) } -function runTestFile (path, root) { +function getRunArgs ({ path, inspectPort }) { + const argv = ArrayPrototypeFilter(process.execArgv, filterExecArgv) + if (isUsingInspector()) { + ArrayPrototypePush(argv, `--inspect-port=${getInspectPort(inspectPort)}`) + } + ArrayPrototypePush(argv, path) + return argv +} + +function makeStderrCallback (callback) { + if (!isUsingInspector()) { + return callback + } + let buffer = Buffer.alloc(0) + return (data) => { + callback(data) + const newData = Buffer.concat([buffer, data]) + const str = newData.toString('utf8') + let lines = str + if (StringPrototypeEndsWith(lines, '\n')) { + buffer = Buffer.alloc(0) + } else { + lines = RegExpPrototypeSymbolSplit(/\r?\n/, str) + buffer = Buffer.from(ArrayPrototypePop(lines), 'utf8') + lines = ArrayPrototypeJoin(lines, '\n') + } + if (isInspectorMessage(lines)) { + process.stderr.write(lines) + } + } +} + +function runTestFile (path, root, inspectPort) { const subtest = root.createSubtest(Test, path, async (t) => { - const args = ArrayPrototypeConcat( - ArrayPrototypeFilter(process.execArgv, filterExecArgv), - path) + const args = getRunArgs({ path, inspectPort }) const child = spawn(process.execPath, args, { signal: t.signal, encoding: 'utf8' }) // TODO(cjihrig): Implement a TAP parser to read the child's stdout // instead of just displaying it all if the child fails. let err + let stderr = '' child.on('error', (error) => { err = error }) - const { 0: { 0: code, 1: signal }, 1: stdout, 2: stderr } = await SafePromiseAll([ + child.stderr.on('data', makeStderrCallback((data) => { + stderr += data + })) + + const { 0: { 0: code, 1: signal }, 1: stdout } = await SafePromiseAll([ once(child, 'exit', { signal: t.signal }), - toArray.call(child.stdout, { signal: t.signal }), - toArray.call(child.stderr, { signal: t.signal }) + toArray.call(child.stdout, { signal: t.signal }) ]) if (code !== 0 || signal !== null) { @@ -130,7 +169,7 @@ function runTestFile (path, root) { exitCode: code, signal, stdout: ArrayPrototypeJoin(stdout, ''), - stderr: ArrayPrototypeJoin(stderr, ''), + stderr, // The stack will not be useful since the failures came from tests // in a child process. stack: undefined @@ -147,7 +186,7 @@ function run (options) { if (options === null || typeof options !== 'object') { options = kEmptyObject } - const { concurrency, timeout, signal, files } = options + const { concurrency, timeout, signal, files, inspectPort } = options if (files != null) { validateArray(files, 'options.files') @@ -156,7 +195,7 @@ function run (options) { const root = createTestTree({ concurrency, timeout, signal }) const testFiles = files ?? createTestFileList() - PromisePrototypeThen(SafePromiseAll(testFiles, (path) => runTestFile(path, root)), + PromisePrototypeThen(SafePromiseAll(testFiles, (path) => runTestFile(path, root, inspectPort)), () => root.postRun()) return root.reporter diff --git a/lib/internal/test_runner/test.js b/lib/internal/test_runner/test.js index d213d04..4049abc 100644 --- a/lib/internal/test_runner/test.js +++ b/lib/internal/test_runner/test.js @@ -1,4 +1,4 @@ -// https://github.com/nodejs/node/blob/6ee1f3444f8c1cf005153f936ffc74221d55658b/lib/internal/test_runner/test.js +// https://github.com/nodejs/node/blob/a165193c5c8e4bcfbd12b2c3f6e55a81a251c258/lib/internal/test_runner/test.js 'use strict' @@ -60,8 +60,6 @@ const kDefaultTimeout = null const noop = FunctionPrototype const isTestRunner = getOptionValue('--test') const testOnlyFlag = !isTestRunner && getOptionValue('--test-only') -// TODO(cjihrig): Use uv_available_parallelism() once it lands. -const rootConcurrency = isTestRunner ? MathMax(cpus().length - 1, 1) : 1 const kShouldAbort = Symbol('kShouldAbort') const kRunHook = Symbol('kRunHook') const kHookNames = ObjectSeal(['before', 'after', 'beforeEach', 'afterEach']) @@ -148,7 +146,7 @@ class Test extends AsyncResource { } if (parent === null) { - this.concurrency = rootConcurrency + this.concurrency = 1 this.indent = '' this.indentString = kDefaultIndent this.only = testOnlyFlag diff --git a/lib/internal/util/inspector.js b/lib/internal/util/inspector.js new file mode 100644 index 0000000..b80129f --- /dev/null +++ b/lib/internal/util/inspector.js @@ -0,0 +1,48 @@ +// https://github.com/nodejs/node/blob/a165193c5c8e4bcfbd12b2c3f6e55a81a251c258/lib/internal/util/inspector.js +const { + ArrayPrototypeSome, + RegExpPrototypeExec +} = require('#internal/per_context/primordials') + +const { validatePort } = require('#internal/validators') + +const kMinPort = 1024 +const kMaxPort = 65535 +const kInspectArgRegex = /--inspect(?:-brk|-port)?|--debug-port/ +const kInspectMsgRegex = /Debugger listening on ws:\/\/\[?(.+?)\]?:(\d+)\/|Debugger attached|Waiting for the debugger to disconnect\.\.\./ + +let _isUsingInspector +function isUsingInspector () { + // Node.js 14.x does not support Logical_nullish_assignment operator + _isUsingInspector = _isUsingInspector ?? + (ArrayPrototypeSome(process.execArgv, (arg) => RegExpPrototypeExec(kInspectArgRegex, arg) !== null) || + RegExpPrototypeExec(kInspectArgRegex, process.env.NODE_OPTIONS) !== null) + return _isUsingInspector +} + +let debugPortOffset = 1 +function getInspectPort (inspectPort) { + if (!isUsingInspector()) { + return null + } + if (typeof inspectPort === 'function') { + inspectPort = inspectPort() + } else if (inspectPort == null) { + inspectPort = process.debugPort + debugPortOffset + if (inspectPort > kMaxPort) { inspectPort = inspectPort - kMaxPort + kMinPort - 1 } + debugPortOffset++ + } + validatePort(inspectPort) + + return inspectPort +} + +function isInspectorMessage (string) { + return isUsingInspector() && RegExpPrototypeExec(kInspectMsgRegex, string) !== null +} + +module.exports = { + isUsingInspector, + getInspectPort, + isInspectorMessage +} diff --git a/test/parallel/test-runner-cli.js b/test/parallel/test-runner-cli.js index d8aad56..63d06e3 100644 --- a/test/parallel/test-runner-cli.js +++ b/test/parallel/test-runner-cli.js @@ -1,4 +1,4 @@ -// https://github.com/nodejs/node/blob/1aab13cad9c800f4121c1d35b554b78c1b17bdbd/test/parallel/test-runner-cli.js +// https://github.com/nodejs/node/blob/a165193c5c8e4bcfbd12b2c3f6e55a81a251c258/test/parallel/test-runner-cli.js 'use strict' require('../common') const assert = require('assert') @@ -106,13 +106,6 @@ const testFixtures = fixtures.path('test-runner') // ['--print', 'console.log("should not print")', '--test'] // ] -// if (process.features.inspector) { -// flags.push( -// // ['--inspect', '--test'], -// // ['--inspect-brk', '--test'] -// ) -// } - // flags.forEach((args) => { // const child = spawnSync(process.execPath, args) From c50f8448b6eb0466fbb8bdde7b1edbe4c2c9a9ee Mon Sep 17 00:00:00 2001 From: Moshe Atlow Date: Wed, 14 Sep 2022 22:28:18 +0300 Subject: [PATCH 05/24] fix: include stack of uncaught exceptions PR-URL: https://github.com/nodejs/node/pull/44614 Fixes: https://github.com/nodejs/node/issues/44611 Reviewed-By: Colin Ihrig Reviewed-By: Benjamin Gruenbaum Reviewed-By: Antoine du Hamel (cherry picked from commit cb7e0c59df10a42cd6930ca7f99d3acee1ce7627) --- lib/internal/test_runner/tap_stream.js | 6 ++--- lib/internal/test_runner/test.js | 17 ++++++++++-- test/message.js | 5 ++++ test/message/test_runner_desctibe_it.out | 1 + test/message/test_runner_output.js | 14 +++++++++- test/message/test_runner_output.out | 33 +++++++++++++++++++++--- 6 files changed, 66 insertions(+), 10 deletions(-) diff --git a/lib/internal/test_runner/tap_stream.js b/lib/internal/test_runner/tap_stream.js index 769d275..003b9f2 100644 --- a/lib/internal/test_runner/tap_stream.js +++ b/lib/internal/test_runner/tap_stream.js @@ -1,4 +1,4 @@ -// https://github.com/nodejs/node/blob/59527de13d39327eb3dfa8dedc92241eb40066d5/lib/internal/test_runner/tap_stream.js +// https://github.com/nodejs/node/blob/cb7e0c59df10a42cd6930ca7f99d3acee1ce7627/lib/internal/test_runner/tap_stream.js 'use strict' @@ -177,7 +177,7 @@ function jsToYaml (indent, name, value) { } if (isErrorObj) { - const { kTestCodeFailure, kHookFailure } = lazyLoadTest() + const { kTestCodeFailure, kUnwrapErrors } = lazyLoadTest() const { cause, code, @@ -191,7 +191,7 @@ function jsToYaml (indent, name, value) { // If the ERR_TEST_FAILURE came from an error provided by user code, // then try to unwrap the original error message and stack. - if (code === 'ERR_TEST_FAILURE' && (failureType === kTestCodeFailure || failureType === kHookFailure)) { + if (code === 'ERR_TEST_FAILURE' && kUnwrapErrors.has(failureType)) { errStack = cause?.stack ?? errStack errCode = cause?.code ?? errCode if (failureType === kTestCodeFailure) { diff --git a/lib/internal/test_runner/test.js b/lib/internal/test_runner/test.js index 4049abc..e8549ca 100644 --- a/lib/internal/test_runner/test.js +++ b/lib/internal/test_runner/test.js @@ -1,4 +1,4 @@ -// https://github.com/nodejs/node/blob/a165193c5c8e4bcfbd12b2c3f6e55a81a251c258/lib/internal/test_runner/test.js +// https://github.com/nodejs/node/blob/cb7e0c59df10a42cd6930ca7f99d3acee1ce7627/lib/internal/test_runner/test.js 'use strict' @@ -16,6 +16,7 @@ const { PromiseResolve, ReflectApply, SafeMap, + SafeSet, SafePromiseAll, SafePromiseRace, Symbol @@ -63,6 +64,9 @@ const testOnlyFlag = !isTestRunner && getOptionValue('--test-only') const kShouldAbort = Symbol('kShouldAbort') const kRunHook = Symbol('kRunHook') const kHookNames = ObjectSeal(['before', 'after', 'beforeEach', 'afterEach']) +const kUnwrapErrors = new SafeSet() + .add(kTestCodeFailure).add(kHookFailure) + .add('uncaughtException').add('unhandledRejection') function stopTest (timeout, signal) { if (timeout === kDefaultTimeout) { @@ -725,4 +729,13 @@ class Suite extends Test { } } -module.exports = { kCancelledByParent, kDefaultIndent, kSubtestsFailed, kTestCodeFailure, Test, Suite, ItTest } +module.exports = { + ItTest, + kCancelledByParent, + kDefaultIndent, + kSubtestsFailed, + kTestCodeFailure, + kUnwrapErrors, + Suite, + Test +} diff --git a/test/message.js b/test/message.js index 4520448..2a35fc4 100755 --- a/test/message.js +++ b/test/message.js @@ -27,6 +27,7 @@ const stackTraceLine = /^\s+\*$/ const stackTraceEndLine = /^\s+\.\.\.$/ const nodejs14NotEmittedWarn = /^# Warning:.*\breject/ +const nodejs14NotEmittedUnhandledRejection = /'unhandledRejection'/ // https://github.com/nodejs/node/blob/1aab13cad9c800f4121c1d35b554b78c1b17bdbd/test/message/testcfg.py#L53 async function IsFailureOutput (self, output) { @@ -38,6 +39,10 @@ async function IsFailureOutput (self, output) { // Node.js 14 doesn't emit some warnings if (process.version.startsWith('v14.') && nodejs14NotEmittedWarn.test(line)) continue + if (process.version.startsWith('v14.') && nodejs14NotEmittedUnhandledRejection.test(line)) { + patterns.push(WAIT_FOR_ELLIPSIS) + continue + } // Sometimes Node.js won't have any stack trace, but we would if (stackTraceEndLine.test(line) && patterns[patterns.length - 1].toString().endsWith("code: 'ERR_TEST_FAILURE'$")) { diff --git a/test/message/test_runner_desctibe_it.out b/test/message/test_runner_desctibe_it.out index 2d58215..812a373 100644 --- a/test/message/test_runner_desctibe_it.out +++ b/test/message/test_runner_desctibe_it.out @@ -421,6 +421,7 @@ not ok 49 - callback async throw code: 'ERR_TEST_FAILURE' stack: |- * + * ... # Subtest: callback async throw after done ok 50 - callback async throw after done diff --git a/test/message/test_runner_output.js b/test/message/test_runner_output.js index e4edf0a..9a1dc66 100644 --- a/test/message/test_runner_output.js +++ b/test/message/test_runner_output.js @@ -1,4 +1,4 @@ -// https://github.com/nodejs/node/blob/a3e110820ff98702e1761831e7beaf0f5f1f75e7/test/message/test_runner_output.js +// https://github.com/nodejs/node/blob/cb7e0c59df10a42cd6930ca7f99d3acee1ce7627/test/message/test_runner_output.js // Flags: --no-warnings 'use strict' require('../common') @@ -371,3 +371,15 @@ test('rejected thenable', () => { } } }) + +test('unfinished test with uncaughtException', async () => { + await new Promise(() => { + setTimeout(() => { throw new Error('foo') }) + }) +}) + +test('unfinished test with unhandledRejection', async () => { + await new Promise(() => { + setTimeout(() => Promise.reject(new Error('bar'))) + }) +}) diff --git a/test/message/test_runner_output.out b/test/message/test_runner_output.out index 49a19fe..fe5698e 100644 --- a/test/message/test_runner_output.out +++ b/test/message/test_runner_output.out @@ -463,6 +463,7 @@ not ok 51 - callback async throw code: 'ERR_TEST_FAILURE' stack: |- * + * ... # Subtest: callback async throw after done ok 52 - callback async throw after done @@ -601,8 +602,32 @@ not ok 62 - rejected thenable error: 'custom error' code: 'ERR_TEST_FAILURE' ... +# Subtest: unfinished test with uncaughtException +not ok 63 - unfinished test with uncaughtException + --- + duration_ms: * + failureType: 'uncaughtException' + error: 'foo' + code: 'ERR_TEST_FAILURE' + stack: |- + * + * + * + ... +# Subtest: unfinished test with unhandledRejection +not ok 64 - unfinished test with unhandledRejection + --- + duration_ms: * + failureType: 'unhandledRejection' + error: 'bar' + code: 'ERR_TEST_FAILURE' + stack: |- + * + * + * + ... # Subtest: invalid subtest fail -not ok 63 - invalid subtest fail +not ok 65 - invalid subtest fail --- duration_ms: * failureType: 'parentAlreadyFinished' @@ -611,16 +636,16 @@ not ok 63 - invalid subtest fail stack: |- * ... -1..63 +1..65 # Warning: Test "unhandled rejection - passes but warns" generated asynchronous activity after the test ended. This activity created the error "Error: rejected from unhandled rejection fail" and would have caused the test to fail, but instead triggered an unhandledRejection event. # Warning: Test "async unhandled rejection - passes but warns" generated asynchronous activity after the test ended. This activity created the error "Error: rejected from async unhandled rejection fail" and would have caused the test to fail, but instead triggered an unhandledRejection event. # Warning: Test "immediate throw - passes but warns" generated asynchronous activity after the test ended. This activity created the error "Error: thrown from immediate throw fail" and would have caused the test to fail, but instead triggered an uncaughtException event. # Warning: Test "immediate reject - passes but warns" generated asynchronous activity after the test ended. This activity created the error "Error: rejected from immediate reject fail" and would have caused the test to fail, but instead triggered an unhandledRejection event. # Warning: Test "callback called twice in different ticks" generated asynchronous activity after the test ended. This activity created the error "Error [ERR_TEST_FAILURE]: callback invoked multiple times" and would have caused the test to fail, but instead triggered an uncaughtException event. # Warning: Test "callback async throw after done" generated asynchronous activity after the test ended. This activity created the error "Error: thrown from callback async throw after done" and would have caused the test to fail, but instead triggered an uncaughtException event. -# tests 63 +# tests 65 # pass 27 -# fail 19 +# fail 21 # cancelled 2 # skipped 10 # todo 5 From 1950b3890190015d26686e18849cdde69c219f19 Mon Sep 17 00:00:00 2001 From: Moshe Atlow Date: Wed, 14 Sep 2022 23:24:20 +0300 Subject: [PATCH 06/24] test: fix test-runner-inspect PR-URL: https://github.com/nodejs/node/pull/44620 Fixes: https://github.com/nodejs/node/issues/44600 Reviewed-By: Benjamin Gruenbaum Reviewed-By: Antoine du Hamel (cherry picked from commit 9825a7e01d35b9d49ebb58efed2c316012c19db6) --- lib/internal/test_runner/runner.js | 46 ++++++++++-------------------- 1 file changed, 15 insertions(+), 31 deletions(-) diff --git a/lib/internal/test_runner/runner.js b/lib/internal/test_runner/runner.js index 5a40097..0091563 100644 --- a/lib/internal/test_runner/runner.js +++ b/lib/internal/test_runner/runner.js @@ -1,25 +1,23 @@ -// https://github.com/nodejs/node/blob/a165193c5c8e4bcfbd12b2c3f6e55a81a251c258/lib/internal/test_runner/runner.js +// https://github.com/nodejs/node/blob/9825a7e01d35b9d49ebb58efed2c316012c19db6/lib/internal/test_runner/runner.js 'use strict' const { ArrayFrom, ArrayPrototypeFilter, ArrayPrototypeIncludes, ArrayPrototypeJoin, - ArrayPrototypePop, ArrayPrototypePush, ArrayPrototypeSlice, ArrayPrototypeSort, ObjectAssign, PromisePrototypeThen, - RegExpPrototypeSymbolSplit, SafePromiseAll, - SafeSet, - StringPrototypeEndsWith + SafeSet } = require('#internal/per_context/primordials') -const { Buffer } = require('buffer') const { spawn } = require('child_process') const { readdirSync, statSync } = require('fs') +// TODO(aduh95): switch to internal/readline/interface when backporting to Node.js 16.x is no longer a concern. +const { createInterface } = require('readline') const { codes: { ERR_TEST_FAILURE @@ -116,29 +114,6 @@ function getRunArgs ({ path, inspectPort }) { return argv } -function makeStderrCallback (callback) { - if (!isUsingInspector()) { - return callback - } - let buffer = Buffer.alloc(0) - return (data) => { - callback(data) - const newData = Buffer.concat([buffer, data]) - const str = newData.toString('utf8') - let lines = str - if (StringPrototypeEndsWith(lines, '\n')) { - buffer = Buffer.alloc(0) - } else { - lines = RegExpPrototypeSymbolSplit(/\r?\n/, str) - buffer = Buffer.from(ArrayPrototypePop(lines), 'utf8') - lines = ArrayPrototypeJoin(lines, '\n') - } - if (isInspectorMessage(lines)) { - process.stderr.write(lines) - } - } -} - function runTestFile (path, root, inspectPort) { const subtest = root.createSubtest(Test, path, async (t) => { const args = getRunArgs({ path, inspectPort }) @@ -153,9 +128,18 @@ function runTestFile (path, root, inspectPort) { err = error }) - child.stderr.on('data', makeStderrCallback((data) => { + child.stderr.on('data', (data) => { stderr += data - })) + }) + + if (isUsingInspector()) { + const rl = createInterface({ input: child.stderr }) + rl.on('line', (line) => { + if (isInspectorMessage(line)) { + process.stderr.write(line + '\n') + } + }) + } const { 0: { 0: code, 1: signal }, 1: stdout } = await SafePromiseAll([ once(child, 'exit', { signal: t.signal }), From c5fd64cc2e2e22b35fd6c94d30dac34f281067ab Mon Sep 17 00:00:00 2001 From: cjihrig Date: Wed, 31 Aug 2022 21:33:39 -0400 Subject: [PATCH 07/24] feat: add --test-name-pattern CLI flag This commit adds support for running tests that match a regular expression. Fixes: https://github.com/nodejs/node/issues/42984 (cherry picked from commit 87170c3f9271da947a7b33d0696ec4cf8aab6eb6) --- README.md | 35 ++++++ bin/node--test-name-pattern.js | 7 ++ bin/node-core-test.js | 5 + lib/internal/test_runner/test.js | 33 +++++- lib/internal/test_runner/utils.js | 23 +++- package-lock.json | 1 + package.json | 1 + test/message.js | 6 +- test/message/test_runner_test_name_pattern.js | 48 ++++++++ .../message/test_runner_test_name_pattern.out | 107 ++++++++++++++++++ ...test_runner_test_name_pattern_with_only.js | 14 +++ ...est_runner_test_name_pattern_with_only.out | 40 +++++++ test/parallel/test-runner-string-to-regexp.js | 21 ++++ 13 files changed, 335 insertions(+), 6 deletions(-) create mode 100644 bin/node--test-name-pattern.js create mode 100644 test/message/test_runner_test_name_pattern.js create mode 100644 test/message/test_runner_test_name_pattern.out create mode 100644 test/message/test_runner_test_name_pattern_with_only.js create mode 100644 test/message/test_runner_test_name_pattern_with_only.out create mode 100644 test/parallel/test-runner-string-to-regexp.js diff --git a/README.md b/README.md index 9b23018..0407239 100644 --- a/README.md +++ b/README.md @@ -228,6 +228,41 @@ test('this test is not run', () => { }) ``` +## Filtering tests by name + +The [`--test-name-pattern`][] command-line option can be used to only run tests +whose name matches the provided pattern. Test name patterns are interpreted as +JavaScript regular expressions. The `--test-name-pattern` option can be +specified multiple times in order to run nested tests. For each test that is +executed, any corresponding test hooks, such as `beforeEach()`, are also +run. + +Given the following test file, starting Node.js with the +`--test-name-pattern="test [1-3]"` option would cause the test runner to execute +`test 1`, `test 2`, and `test 3`. If `test 1` did not match the test name +pattern, then its subtests would not execute, despite matching the pattern. The +same set of tests could also be executed by passing `--test-name-pattern` +multiple times (e.g. `--test-name-pattern="test 1"`, +`--test-name-pattern="test 2"`, etc.). + +```js +test('test 1', async (t) => { + await t.test('test 2'); + await t.test('test 3'); +}); +test('Test 4', async (t) => { + await t.test('Test 5'); + await t.test('test 6'); +}); +``` + +Test name patterns can also be specified using regular expression literals. This +allows regular expression flags to be used. In the previous example, starting +Node.js with `--test-name-pattern="/test [4-5]/i"` would match `Test 4` and +`Test 5` because the pattern is case-insensitive. + +Test name patterns do not change the set of files that the test runner executes. + ## Extraneous asynchronous activity Once a test function finishes executing, the TAP results are output as quickly diff --git a/bin/node--test-name-pattern.js b/bin/node--test-name-pattern.js new file mode 100644 index 0000000..128755a --- /dev/null +++ b/bin/node--test-name-pattern.js @@ -0,0 +1,7 @@ +#!/usr/bin/env node + +const { argv } = require('#internal/options') + +argv['test-name-pattern'] = true + +require('./node-core-test.js') diff --git a/bin/node-core-test.js b/bin/node-core-test.js index 23a1445..4bc6e89 100755 --- a/bin/node-core-test.js +++ b/bin/node-core-test.js @@ -10,9 +10,14 @@ const { argv } = require('#internal/options') Object.assign(argv, minimist(process.argv.slice(2), { boolean: ['test', 'test-only'], + string: ['test-name-pattern'], default: Object.prototype.hasOwnProperty.call(argv, 'test') ? { test: argv.test } : undefined })) +if (typeof argv['test-name-pattern'] === 'string') { + argv['test-name-pattern'] = [argv['test-name-pattern']] +} + process.argv.splice(1, Infinity, ...argv._) if (argv.test) { require('#internal/main/test_runner') diff --git a/lib/internal/test_runner/test.js b/lib/internal/test_runner/test.js index e8549ca..bf09690 100644 --- a/lib/internal/test_runner/test.js +++ b/lib/internal/test_runner/test.js @@ -1,12 +1,14 @@ -// https://github.com/nodejs/node/blob/cb7e0c59df10a42cd6930ca7f99d3acee1ce7627/lib/internal/test_runner/test.js +// https://github.com/nodejs/node/blob/87170c3f9271da947a7b33d0696ec4cf8aab6eb6/lib/internal/test_runner/test.js 'use strict' const { + ArrayPrototypeMap, ArrayPrototypePush, ArrayPrototypeReduce, ArrayPrototypeShift, ArrayPrototypeSlice, + ArrayPrototypeSome, ArrayPrototypeUnshift, FunctionPrototype, MathMax, @@ -15,6 +17,7 @@ const { PromisePrototypeThen, PromiseResolve, ReflectApply, + RegExpPrototypeExec, SafeMap, SafeSet, SafePromiseAll, @@ -33,7 +36,11 @@ const { } = require('#internal/errors') const { getOptionValue } = require('#internal/options') const { TapStream } = require('#internal/test_runner/tap_stream') -const { createDeferredCallback, isTestFailureError } = require('#internal/test_runner/utils') +const { + convertStringToRegExp, + createDeferredCallback, + isTestFailureError +} = require('#internal/test_runner/utils') const { createDeferredPromise, kEmptyObject @@ -61,6 +68,15 @@ const kDefaultTimeout = null const noop = FunctionPrototype const isTestRunner = getOptionValue('--test') const testOnlyFlag = !isTestRunner && getOptionValue('--test-only') +const testNamePatternFlag = isTestRunner + ? null + : getOptionValue('--test-name-pattern') +const testNamePatterns = testNamePatternFlag?.length > 0 + ? ArrayPrototypeMap( + testNamePatternFlag, + (re) => convertStringToRegExp(re, '--test-name-pattern') + ) + : null const kShouldAbort = Symbol('kShouldAbort') const kRunHook = Symbol('kRunHook') const kHookNames = ObjectSeal(['before', 'after', 'beforeEach', 'afterEach']) @@ -196,6 +212,18 @@ class Test extends AsyncResource { this.timeout = timeout } + if (testNamePatterns !== null) { + // eslint-disable-next-line no-use-before-define + const match = this instanceof TestHook || ArrayPrototypeSome( + testNamePatterns, + (re) => RegExpPrototypeExec(re, name) !== null + ) + + if (!match) { + skip = 'test name does not match pattern' + } + } + if (testOnlyFlag && !this.only) { skip = '\'only\' option not set' } @@ -673,6 +701,7 @@ class ItTest extends Test { return { ctx: { signal: this.signal, name: this.name }, args: [] } } } + class Suite extends Test { constructor (options) { super(options) diff --git a/lib/internal/test_runner/utils.js b/lib/internal/test_runner/utils.js index 595431e..c2919d8 100644 --- a/lib/internal/test_runner/utils.js +++ b/lib/internal/test_runner/utils.js @@ -1,16 +1,18 @@ -// https://github.com/nodejs/node/blob/659dc126932f986fc33c7f1c878cb2b57a1e2fac/lib/internal/test_runner/utils.js +// https://github.com/nodejs/node/blob/87170c3f9271da947a7b33d0696ec4cf8aab6eb6/lib/internal/test_runner/utils.js 'use strict' const { RegExpPrototypeExec } = require('#internal/per_context/primordials') const { basename } = require('path') const { createDeferredPromise } = require('#internal/util') const { codes: { + ERR_INVALID_ARG_VALUE, ERR_TEST_FAILURE }, kIsNodeError } = require('#internal/errors') const kMultipleCallbackInvocations = 'multipleCallbackInvocations' +const kRegExpPattern = /^\/(.*)\/([a-z]*)$/ const kSupportedFileExtensions = /\.[cm]?js$/ const kTestFilePattern = /((^test(-.+)?)|(.+[.\-_]test))\.[cm]?js$/ @@ -55,7 +57,26 @@ function isTestFailureError (err) { return err?.code === 'ERR_TEST_FAILURE' && kIsNodeError in err } +function convertStringToRegExp (str, name) { + const match = RegExpPrototypeExec(kRegExpPattern, str) + const pattern = match?.[1] ?? str + const flags = match?.[2] || '' + + try { + return new RegExp(pattern, flags) + } catch (err) { + const msg = err?.message + + throw new ERR_INVALID_ARG_VALUE( + name, + str, + `is an invalid regular expression.${msg ? ` ${msg}` : ''}` + ) + } +} + module.exports = { + convertStringToRegExp, createDeferredCallback, doesPathMatchFilter, isSupportedFileType, diff --git a/package-lock.json b/package-lock.json index 3c3344b..3642736 100644 --- a/package-lock.json +++ b/package-lock.json @@ -14,6 +14,7 @@ }, "bin": { "node--test": "bin/node--test.js", + "node--test-name-pattern": "bin/node--test-name-pattern.js", "node--test-only": "bin/node--test-only.js", "test": "bin/node-core-test.js" }, diff --git a/package.json b/package.json index 8d787e1..b1e00a7 100644 --- a/package.json +++ b/package.json @@ -9,6 +9,7 @@ "bin": { "node--test": "./bin/node--test.js", "node--test-only": "./bin/node--test-only.js", + "node--test-name-pattern": "./bin/node--test-name-pattern.js", "test": "./bin/node-core-test.js" }, "imports": { diff --git a/test/message.js b/test/message.js index 2a35fc4..b2b6234 100755 --- a/test/message.js +++ b/test/message.js @@ -13,7 +13,7 @@ const binPath = resolve(__dirname, '..', bin.test) const MESSAGE_FOLDER = join(__dirname, './message/') const WAIT_FOR_ELLIPSIS = Symbol('wait for ellispis') -const TEST_RUNNER_FLAGS = ['--test', '--test-only'] +const TEST_RUNNER_FLAGS = ['--test', '--test-only', '--test-name-pattern'] function readLines (file) { return createInterface({ @@ -109,8 +109,8 @@ const main = async () => { ) .toString().split(' ') - const nodeFlags = flags.filter(flag => !TEST_RUNNER_FLAGS.includes(flag)).join(' ') - const testRunnerFlags = flags.filter(flag => TEST_RUNNER_FLAGS.includes(flag)).join(' ') + const nodeFlags = flags.filter(flag => !TEST_RUNNER_FLAGS.find(f => flag.startsWith(f))).join(' ') + const testRunnerFlags = flags.filter(flag => TEST_RUNNER_FLAGS.find(f => flag.startsWith(f))).join(' ') const command = testRunnerFlags.length ? `${process.execPath} ${nodeFlags} ${binPath} ${testRunnerFlags} ${filePath}` diff --git a/test/message/test_runner_test_name_pattern.js b/test/message/test_runner_test_name_pattern.js new file mode 100644 index 0000000..f0b44aa --- /dev/null +++ b/test/message/test_runner_test_name_pattern.js @@ -0,0 +1,48 @@ +// https://github.com/nodejs/node/blob/87170c3f9271da947a7b33d0696ec4cf8aab6eb6/test/message/test_runner_test_name_pattern.js +// Flags: --no-warnings --test-name-pattern=enabled --test-name-pattern=/pattern/i +'use strict' +const common = require('../common') +const { + after, + afterEach, + before, + beforeEach, + describe, + it, + test +} = require('#node:test') + +test('top level test disabled', common.mustNotCall()) +test('top level skipped test disabled', { skip: true }, common.mustNotCall()) +test('top level skipped test enabled', { skip: true }, common.mustNotCall()) +it('top level it enabled', common.mustCall()) +it('top level it disabled', common.mustNotCall()) +it.skip('top level skipped it disabled', common.mustNotCall()) +it.skip('top level skipped it enabled', common.mustNotCall()) +describe('top level describe disabled', common.mustNotCall()) +describe.skip('top level skipped describe disabled', common.mustNotCall()) +describe.skip('top level skipped describe enabled', common.mustNotCall()) +test('top level runs because name includes PaTtErN', common.mustCall()) + +test('top level test enabled', common.mustCall(async (t) => { + t.beforeEach(common.mustCall()) + t.afterEach(common.mustCall()) + await t.test( + 'nested test runs because name includes PATTERN', + common.mustCall() + ) +})) + +describe('top level describe enabled', () => { + before(common.mustCall()) + beforeEach(common.mustCall(2)) + afterEach(common.mustCall(2)) + after(common.mustCall()) + + it('nested it disabled', common.mustNotCall()) + it('nested it enabled', common.mustCall()) + describe('nested describe disabled', common.mustNotCall()) + describe('nested describe enabled', common.mustCall(() => { + it('is enabled', common.mustCall()) + })) +}) diff --git a/test/message/test_runner_test_name_pattern.out b/test/message/test_runner_test_name_pattern.out new file mode 100644 index 0000000..be548ad --- /dev/null +++ b/test/message/test_runner_test_name_pattern.out @@ -0,0 +1,107 @@ +TAP version 13 +# Subtest: top level test disabled +ok 1 - top level test disabled # SKIP test name does not match pattern + --- + duration_ms: * + ... +# Subtest: top level skipped test disabled +ok 2 - top level skipped test disabled # SKIP test name does not match pattern + --- + duration_ms: * + ... +# Subtest: top level skipped test enabled +ok 3 - top level skipped test enabled # SKIP + --- + duration_ms: * + ... +# Subtest: top level it enabled +ok 4 - top level it enabled + --- + duration_ms: * + ... +# Subtest: top level it disabled +ok 5 - top level it disabled # SKIP test name does not match pattern + --- + duration_ms: * + ... +# Subtest: top level skipped it disabled +ok 6 - top level skipped it disabled # SKIP test name does not match pattern + --- + duration_ms: * + ... +# Subtest: top level skipped it enabled +ok 7 - top level skipped it enabled # SKIP + --- + duration_ms: * + ... +# Subtest: top level describe disabled +ok 8 - top level describe disabled # SKIP test name does not match pattern + --- + duration_ms: * + ... +# Subtest: top level skipped describe disabled +ok 9 - top level skipped describe disabled # SKIP test name does not match pattern + --- + duration_ms: * + ... +# Subtest: top level skipped describe enabled +ok 10 - top level skipped describe enabled # SKIP + --- + duration_ms: * + ... +# Subtest: top level runs because name includes PaTtErN +ok 11 - top level runs because name includes PaTtErN + --- + duration_ms: * + ... +# Subtest: top level test enabled + # Subtest: nested test runs because name includes PATTERN + ok 1 - nested test runs because name includes PATTERN + --- + duration_ms: * + ... + 1..1 +ok 12 - top level test enabled + --- + duration_ms: * + ... +# Subtest: top level describe enabled + # Subtest: nested it disabled + ok 1 - nested it disabled # SKIP test name does not match pattern + --- + duration_ms: * + ... + # Subtest: nested it enabled + ok 2 - nested it enabled + --- + duration_ms: * + ... + # Subtest: nested describe disabled + ok 3 - nested describe disabled # SKIP test name does not match pattern + --- + duration_ms: * + ... + # Subtest: nested describe enabled + # Subtest: is enabled + ok 1 - is enabled + --- + duration_ms: * + ... + 1..1 + ok 4 - nested describe enabled + --- + duration_ms: * + ... + 1..4 +ok 13 - top level describe enabled + --- + duration_ms: * + ... +1..13 +# tests 13 +# pass 4 +# fail 0 +# cancelled 0 +# skipped 9 +# todo 0 +# duration_ms * diff --git a/test/message/test_runner_test_name_pattern_with_only.js b/test/message/test_runner_test_name_pattern_with_only.js new file mode 100644 index 0000000..4f2d6fb --- /dev/null +++ b/test/message/test_runner_test_name_pattern_with_only.js @@ -0,0 +1,14 @@ +// https://github.com/nodejs/node/blob/87170c3f9271da947a7b33d0696ec4cf8aab6eb6/test/message/test_runner_test_name_pattern_with_only.js +// Flags: --no-warnings --test-only --test-name-pattern=enabled +'use strict' +const common = require('../common') +const { test } = require('#node:test') + +test('enabled and only', { only: true }, common.mustCall(async (t) => { + await t.test('enabled', common.mustCall()) + await t.test('disabled', common.mustNotCall()) +})) + +test('enabled but not only', common.mustNotCall()) +test('only does not match pattern', { only: true }, common.mustNotCall()) +test('not only and does not match pattern', common.mustNotCall()) diff --git a/test/message/test_runner_test_name_pattern_with_only.out b/test/message/test_runner_test_name_pattern_with_only.out new file mode 100644 index 0000000..2e10064 --- /dev/null +++ b/test/message/test_runner_test_name_pattern_with_only.out @@ -0,0 +1,40 @@ +TAP version 13 +# Subtest: enabled and only + # Subtest: enabled + ok 1 - enabled + --- + duration_ms: * + ... + # Subtest: disabled + ok 2 - disabled # SKIP test name does not match pattern + --- + duration_ms: * + ... + 1..2 +ok 1 - enabled and only + --- + duration_ms: * + ... +# Subtest: enabled but not only +ok 2 - enabled but not only # SKIP 'only' option not set + --- + duration_ms: * + ... +# Subtest: only does not match pattern +ok 3 - only does not match pattern # SKIP test name does not match pattern + --- + duration_ms: * + ... +# Subtest: not only and does not match pattern +ok 4 - not only and does not match pattern # SKIP 'only' option not set + --- + duration_ms: * + ... +1..4 +# tests 4 +# pass 1 +# fail 0 +# cancelled 0 +# skipped 3 +# todo 0 +# duration_ms * diff --git a/test/parallel/test-runner-string-to-regexp.js b/test/parallel/test-runner-string-to-regexp.js new file mode 100644 index 0000000..7b6fc0e --- /dev/null +++ b/test/parallel/test-runner-string-to-regexp.js @@ -0,0 +1,21 @@ +// https://github.com/nodejs/node/blob/87170c3f9271da947a7b33d0696ec4cf8aab6eb6/test/parallel/test-runner-string-to-regexp.js + +'use strict' +const common = require('../common') +const { deepStrictEqual, throws } = require('node:assert') +const { convertStringToRegExp } = require('#internal/test_runner/utils') + +deepStrictEqual(convertStringToRegExp('foo', 'x'), /foo/) +deepStrictEqual(convertStringToRegExp('/bar/', 'x'), /bar/) +deepStrictEqual(convertStringToRegExp('/baz/gi', 'x'), /baz/gi) +deepStrictEqual(convertStringToRegExp('/foo/9', 'x'), /\/foo\/9/) + +throws( + () => convertStringToRegExp('/foo/abcdefghijk', 'x'), + common.expectsError({ + code: 'ERR_INVALID_ARG_VALUE', + message: "The argument 'x' is an invalid regular expression. " + + "Invalid flags supplied to RegExp constructor 'abcdefghijk'. " + + 'Received /foo/abcdefghijk' + }) +) From 46dce07d74b47a6d1a06eba2153c850db733b08d Mon Sep 17 00:00:00 2001 From: Bryan English Date: Wed, 26 Oct 2022 00:00:13 -0400 Subject: [PATCH 08/24] feat: add extra fields in AssertionError YAML Many TAP reporters offer special-case handling of YAML objects containing `expected`, `actual`, and `operator` fields, as produced by `AssertionError` and similar userland Error classes. PR-URL: https://github.com/nodejs/node/pull/44952 Reviewed-By: Moshe Atlow Reviewed-By: James M Snell (cherry picked from commit e260b373f13c150eb5bdf4c336d4b6b764b59c8e) --- lib/internal/test_runner/tap_stream.js | 27 +++++++++++++++++++++++- test/message/test_runner_desctibe_it.out | 3 +++ test/message/test_runner_output.out | 3 +++ 3 files changed, 32 insertions(+), 1 deletion(-) diff --git a/lib/internal/test_runner/tap_stream.js b/lib/internal/test_runner/tap_stream.js index 003b9f2..8b53c9b 100644 --- a/lib/internal/test_runner/tap_stream.js +++ b/lib/internal/test_runner/tap_stream.js @@ -1,4 +1,4 @@ -// https://github.com/nodejs/node/blob/cb7e0c59df10a42cd6930ca7f99d3acee1ce7627/lib/internal/test_runner/tap_stream.js +// https://github.com/nodejs/node/blob/e260b373f13c150eb5bdf4c336d4b6b764b59c8e/lib/internal/test_runner/tap_stream.js 'use strict' @@ -183,17 +183,30 @@ function jsToYaml (indent, name, value) { code, failureType, message, + expected, + actual, + operator, stack } = value let errMsg = message ?? '' let errStack = stack let errCode = code + let errExpected = expected + let errActual = actual + let errOperator = operator + let errIsAssertion = isAssertionLike(value) // If the ERR_TEST_FAILURE came from an error provided by user code, // then try to unwrap the original error message and stack. if (code === 'ERR_TEST_FAILURE' && kUnwrapErrors.has(failureType)) { errStack = cause?.stack ?? errStack errCode = cause?.code ?? errCode + if (isAssertionLike(cause)) { + errExpected = cause.expected + errActual = cause.actual + errOperator = cause.operator ?? errOperator + errIsAssertion = true + } if (failureType === kTestCodeFailure) { errMsg = cause?.message ?? errMsg } @@ -205,6 +218,14 @@ function jsToYaml (indent, name, value) { result += jsToYaml(indent, 'code', errCode) } + if (errIsAssertion) { + result += jsToYaml(indent, 'expected', errExpected) + result += jsToYaml(indent, 'actual', errActual) + if (errOperator) { + result += jsToYaml(indent, 'operator', errOperator) + } + } + if (typeof errStack === 'string') { const frames = [] @@ -233,4 +254,8 @@ function jsToYaml (indent, name, value) { return result } +function isAssertionLike (value) { + return value && typeof value === 'object' && 'expected' in value && 'actual' in value +} + module.exports = { TapStream } diff --git a/test/message/test_runner_desctibe_it.out b/test/message/test_runner_desctibe_it.out index 812a373..aaed1a7 100644 --- a/test/message/test_runner_desctibe_it.out +++ b/test/message/test_runner_desctibe_it.out @@ -123,6 +123,9 @@ not ok 13 - async assertion fail true !== false code: 'ERR_ASSERTION' + expected: false + actual: true + operator: 'strictEqual' stack: |- * * diff --git a/test/message/test_runner_output.out b/test/message/test_runner_output.out index fe5698e..938989b 100644 --- a/test/message/test_runner_output.out +++ b/test/message/test_runner_output.out @@ -133,6 +133,9 @@ not ok 13 - async assertion fail true !== false code: 'ERR_ASSERTION' + expected: false + actual: true + operator: 'strictEqual' stack: |- * * From 0bfdb77aeeb913aac43c5cac2c5333d42fde558b Mon Sep 17 00:00:00 2001 From: Colin Ihrig Date: Thu, 27 Oct 2022 04:27:35 -0400 Subject: [PATCH 09/24] fix: call {before,after}Each() on suites Prior to this commit, beforeEach() and afterEach() hooks were not called on test suites (describe()). This commit addresses that. Fixes: https://github.com/nodejs/node/issues/45028 PR-URL: https://github.com/nodejs/node/pull/45161 Reviewed-By: Moshe Atlow Reviewed-By: Yagiz Nizipli Reviewed-By: James M Snell (cherry picked from commit a69a30016cf3395b0bd775c1340ab6ecbac58296) --- lib/internal/test_runner/test.js | 13 ++++++++++++- test/message/test_runner_hooks.js | 4 +++- test/message/test_runner_test_name_pattern.js | 6 +++--- 3 files changed, 18 insertions(+), 5 deletions(-) diff --git a/lib/internal/test_runner/test.js b/lib/internal/test_runner/test.js index bf09690..ce1ae08 100644 --- a/lib/internal/test_runner/test.js +++ b/lib/internal/test_runner/test.js @@ -1,4 +1,4 @@ -// https://github.com/nodejs/node/blob/87170c3f9271da947a7b33d0696ec4cf8aab6eb6/lib/internal/test_runner/test.js +// https://github.com/nodejs/node/blob/a69a30016cf3395b0bd775c1340ab6ecbac58296/lib/internal/test_runner/test.js 'use strict' @@ -738,13 +738,24 @@ class Suite extends Test { } const hookArgs = this.getRunArgs() + + if (this.parent?.hooks.beforeEach.length > 0) { + await this.parent[kRunHook]('beforeEach', hookArgs) + } + await this[kRunHook]('before', hookArgs) + const stopPromise = stopTest(this.timeout, this.signal) const subtests = this.skipped || this.error ? [] : this.subtests const promise = SafePromiseAll(subtests, (subtests) => subtests.start()) await SafePromiseRace([promise, stopPromise]) await this[kRunHook]('after', hookArgs) + + if (this.parent?.hooks.afterEach.length > 0) { + await this.parent[kRunHook]('afterEach', hookArgs) + } + this.pass() } catch (err) { if (isTestFailureError(err)) { diff --git a/test/message/test_runner_hooks.js b/test/message/test_runner_hooks.js index 836ef0e..e74048f 100644 --- a/test/message/test_runner_hooks.js +++ b/test/message/test_runner_hooks.js @@ -1,4 +1,4 @@ -// https://github.com/nodejs/node/blob/659dc126932f986fc33c7f1c878cb2b57a1e2fac/test/message/test_runner_hooks.js +// https://github.com/nodejs/node/blob/a69a30016cf3395b0bd775c1340ab6ecbac58296/test/message/test_runner_hooks.js // Flags: --no-warnings 'use strict' require('../common') @@ -16,10 +16,12 @@ describe('describe hooks', () => { 'before describe hooks', 'beforeEach 1', '1', 'afterEach 1', 'beforeEach 2', '2', 'afterEach 2', + 'beforeEach nested', 'before nested', 'beforeEach nested 1', 'nested 1', 'afterEach nested 1', 'beforeEach nested 2', 'nested 2', 'afterEach nested 2', 'after nested', + 'afterEach nested', 'after describe hooks' ]) }) diff --git a/test/message/test_runner_test_name_pattern.js b/test/message/test_runner_test_name_pattern.js index f0b44aa..5f7c25b 100644 --- a/test/message/test_runner_test_name_pattern.js +++ b/test/message/test_runner_test_name_pattern.js @@ -1,4 +1,4 @@ -// https://github.com/nodejs/node/blob/87170c3f9271da947a7b33d0696ec4cf8aab6eb6/test/message/test_runner_test_name_pattern.js +// https://github.com/nodejs/node/blob/a69a30016cf3395b0bd775c1340ab6ecbac58296/test/message/test_runner_test_name_pattern.js // Flags: --no-warnings --test-name-pattern=enabled --test-name-pattern=/pattern/i 'use strict' const common = require('../common') @@ -35,8 +35,8 @@ test('top level test enabled', common.mustCall(async (t) => { describe('top level describe enabled', () => { before(common.mustCall()) - beforeEach(common.mustCall(2)) - afterEach(common.mustCall(2)) + beforeEach(common.mustCall(4)) + afterEach(common.mustCall(4)) after(common.mustCall()) it('nested it disabled', common.mustNotCall()) From 08269c5d1d213ceb8ef9c0ab0c56172835c76d8f Mon Sep 17 00:00:00 2001 From: Moshe Atlow Date: Fri, 28 Oct 2022 10:28:21 +0300 Subject: [PATCH 10/24] fix: report tap subtest in order PR-URL: https://github.com/nodejs/node/pull/45220 Reviewed-By: Colin Ihrig Reviewed-By: Benjamin Gruenbaum (cherry picked from commit 3e57891ee2fde0971e18fc383c25acf8f90def05) --- lib/internal/test_runner/test.js | 25 +++++++++++++------ test/message/test_runner_describe_nested.js | 11 +++++++++ test/message/test_runner_describe_nested.out | 26 ++++++++++++++++++++ 3 files changed, 54 insertions(+), 8 deletions(-) create mode 100644 test/message/test_runner_describe_nested.js create mode 100644 test/message/test_runner_describe_nested.out diff --git a/lib/internal/test_runner/test.js b/lib/internal/test_runner/test.js index ce1ae08..166053f 100644 --- a/lib/internal/test_runner/test.js +++ b/lib/internal/test_runner/test.js @@ -1,4 +1,4 @@ -// https://github.com/nodejs/node/blob/a69a30016cf3395b0bd775c1340ab6ecbac58296/lib/internal/test_runner/test.js +// https://github.com/nodejs/node/blob/3e57891ee2fde0971e18fc383c25acf8f90def05/lib/internal/test_runner/test.js 'use strict' @@ -146,6 +146,7 @@ class TestContext { class Test extends AsyncResource { #abortController #outerSignal + #reportedSubtest constructor (options) { super('Test') @@ -312,7 +313,7 @@ class Test extends AsyncResource { } if (i === 1 && this.parent !== null) { - this.reporter.subtest(this.indent, this.name) + this.reportSubtest() } // Report the subtest's results and remove it from the ready map. @@ -635,12 +636,6 @@ class Test extends AsyncResource { this.processReadySubtestRange(true) // Output this test's results and update the parent's waiting counter. - if (this.subtests.length > 0) { - this.reporter.plan(this.subtests[0].indent, this.subtests.length) - } else { - this.reporter.subtest(this.indent, this.name) - } - this.report() this.parent.waitingOn++ this.finished = true @@ -652,6 +647,11 @@ class Test extends AsyncResource { } report () { + if (this.subtests.length > 0) { + this.reporter.plan(this.subtests[0].indent, this.subtests.length) + } else { + this.reportSubtest() + } let directive if (this.skipped) { @@ -670,6 +670,15 @@ class Test extends AsyncResource { this.reporter.diagnostic(this.indent, this.diagnostics[i]) } } + + reportSubtest () { + if (this.#reportedSubtest || this.parent === null) { + return + } + this.#reportedSubtest = true + this.parent.reportSubtest() + this.reporter.subtest(this.indent, this.name) + } } class TestHook extends Test { diff --git a/test/message/test_runner_describe_nested.js b/test/message/test_runner_describe_nested.js new file mode 100644 index 0000000..e60ebf6 --- /dev/null +++ b/test/message/test_runner_describe_nested.js @@ -0,0 +1,11 @@ +// https://github.com/nodejs/node/blob/3e57891ee2fde0971e18fc383c25acf8f90def05/test/message/test_runner_describe_nested.js +// Flags: --no-warnings +'use strict' +require('../common') +const { describe, it } = require('#node:test') + +describe('nested - no tests', () => { + describe('nested', () => { + it('nested', () => {}) + }) +}) diff --git a/test/message/test_runner_describe_nested.out b/test/message/test_runner_describe_nested.out new file mode 100644 index 0000000..1d3fe31 --- /dev/null +++ b/test/message/test_runner_describe_nested.out @@ -0,0 +1,26 @@ +TAP version 13 +# Subtest: nested - no tests + # Subtest: nested + # Subtest: nested + ok 1 - nested + --- + duration_ms: * + ... + 1..1 + ok 1 - nested + --- + duration_ms: * + ... + 1..1 +ok 1 - nested - no tests + --- + duration_ms: * + ... +1..1 +# tests 1 +# pass 1 +# fail 0 +# cancelled 0 +# skipped 0 +# todo 0 +# duration_ms * From f2815afc2ff26902d027c0dba85eed773040c1a2 Mon Sep 17 00:00:00 2001 From: Jithil P Ponnan Date: Mon, 7 Nov 2022 16:58:39 +1100 Subject: [PATCH 11/24] fix: fix afterEach not running on test failures PR-URL: https://github.com/nodejs/node/pull/45204 Fixes: https://github.com/nodejs/node/issues/45192 Reviewed-By: Benjamin Gruenbaum Reviewed-By: Moshe Atlow (cherry picked from commit 3759935ee29d8042d917d3ceaa768521c14413ff) --- lib/internal/per_context/primordials.js | 1 + lib/internal/test_runner/test.js | 33 +++-- lib/internal/util.js | 17 ++- test/message/test_runner_desctibe_it.out | 2 - test/message/test_runner_hooks.js | 28 +++- test/message/test_runner_hooks.out | 172 ++++++++++++++++++++++- test/message/test_runner_output.out | 2 - 7 files changed, 227 insertions(+), 28 deletions(-) diff --git a/lib/internal/per_context/primordials.js b/lib/internal/per_context/primordials.js index 96987ca..596454a 100644 --- a/lib/internal/per_context/primordials.js +++ b/lib/internal/per_context/primordials.js @@ -59,6 +59,7 @@ exports.StringPrototypeSlice = (str, ...args) => str.slice(...args) exports.StringPrototypeSplit = (str, search, limit) => str.split(search, limit) exports.Symbol = Symbol exports.SymbolFor = repr => Symbol.for(repr) +exports.ReflectApply = (target, self, args) => Reflect.apply(target, self, args) exports.RegExpPrototypeExec = (reg, str) => reg.exec(str) exports.RegExpPrototypeSymbolReplace = (regexp, str, replacement) => regexp[Symbol.replace](str, replacement) diff --git a/lib/internal/test_runner/test.js b/lib/internal/test_runner/test.js index 166053f..c3adaf3 100644 --- a/lib/internal/test_runner/test.js +++ b/lib/internal/test_runner/test.js @@ -1,4 +1,4 @@ -// https://github.com/nodejs/node/blob/3e57891ee2fde0971e18fc383c25acf8f90def05/lib/internal/test_runner/test.js +// https://github.com/nodejs/node/blob/3759935ee29d8042d917d3ceaa768521c14413ff/lib/internal/test_runner/test.js 'use strict' @@ -43,7 +43,8 @@ const { } = require('#internal/test_runner/utils') const { createDeferredPromise, - kEmptyObject + kEmptyObject, + once: runOnce } = require('#internal/util') const { isPromise } = require('#internal/util/types') const { @@ -486,8 +487,14 @@ class Test extends AsyncResource { return } + const { args, ctx } = this.getRunArgs() + const afterEach = runOnce(async () => { + if (this.parent?.hooks.afterEach.length > 0) { + await this.parent[kRunHook]('afterEach', { args, ctx }) + } + }) + try { - const { args, ctx } = this.getRunArgs() if (this.parent?.hooks.beforeEach.length > 0) { await this.parent[kRunHook]('beforeEach', { args, ctx }) } @@ -522,12 +529,10 @@ class Test extends AsyncResource { return } - if (this.parent?.hooks.afterEach.length > 0) { - await this.parent[kRunHook]('afterEach', { args, ctx }) - } - + await afterEach() this.pass() } catch (err) { + try { await afterEach() } catch { /* test is already failing, let's the error */ } if (isTestFailureError(err)) { if (err.failureType === kTestTimeoutFailure) { this.cancel(err) @@ -735,6 +740,12 @@ class Suite extends Test { } async run () { + const hookArgs = this.getRunArgs() + const afterEach = runOnce(async () => { + if (this.parent?.hooks.afterEach.length > 0) { + await this.parent[kRunHook]('afterEach', hookArgs) + } + }) try { this.parent.activeSubtests++ await this.buildSuite @@ -746,8 +757,6 @@ class Suite extends Test { return } - const hookArgs = this.getRunArgs() - if (this.parent?.hooks.beforeEach.length > 0) { await this.parent[kRunHook]('beforeEach', hookArgs) } @@ -760,13 +769,11 @@ class Suite extends Test { await SafePromiseRace([promise, stopPromise]) await this[kRunHook]('after', hookArgs) - - if (this.parent?.hooks.afterEach.length > 0) { - await this.parent[kRunHook]('afterEach', hookArgs) - } + await afterEach() this.pass() } catch (err) { + try { await afterEach() } catch { /* test is already failing, let's the error */ } if (isTestFailureError(err)) { this.fail(err) } else { diff --git a/lib/internal/util.js b/lib/internal/util.js index 413a655..9cbb706 100644 --- a/lib/internal/util.js +++ b/lib/internal/util.js @@ -1,9 +1,10 @@ -// https://github.com/nodejs/node/blob/a9b1fd3987fae5ad5340859a6088b86179b576c5/lib/internal/util.js +// https://github.com/nodejs/node/blob/3759935ee29d8042d917d3ceaa768521c14413ff/lib/internal/util.js 'use strict' const { ObjectCreate, - ObjectFreeze + ObjectFreeze, + ReflectApply } = require('#internal/per_context/primordials') const { types: { isNativeError } @@ -27,10 +28,20 @@ function isError (e) { return isNativeError(e) || e instanceof Error } +function once (callback) { + let called = false + return function (...args) { + if (called) return + called = true + return ReflectApply(callback, this, args) + } +} + const kEmptyObject = ObjectFreeze(ObjectCreate(null)) module.exports = { createDeferredPromise, isError, - kEmptyObject + kEmptyObject, + once } diff --git a/test/message/test_runner_desctibe_it.out b/test/message/test_runner_desctibe_it.out index aaed1a7..ec9337e 100644 --- a/test/message/test_runner_desctibe_it.out +++ b/test/message/test_runner_desctibe_it.out @@ -42,8 +42,6 @@ not ok 4 - sync fail todo with message # TODO this is a failing todo * * * - * - * ... # Subtest: sync skip pass ok 5 - sync skip pass # SKIP diff --git a/test/message/test_runner_hooks.js b/test/message/test_runner_hooks.js index e74048f..80af89a 100644 --- a/test/message/test_runner_hooks.js +++ b/test/message/test_runner_hooks.js @@ -1,7 +1,7 @@ -// https://github.com/nodejs/node/blob/a69a30016cf3395b0bd775c1340ab6ecbac58296/test/message/test_runner_hooks.js +// https://github.com/nodejs/node/blob/3759935ee29d8042d917d3ceaa768521c14413ff/test/message/test_runner_hooks.js // Flags: --no-warnings 'use strict' -require('../common') +const common = require('../common') const assert = require('assert') const { test, describe, it, before, after, beforeEach, afterEach } = require('#node:test') @@ -77,6 +77,18 @@ describe('afterEach throws', () => { it('2', () => {}) }) +describe('afterEach when test fails', () => { + afterEach(common.mustCall(2)) + it('1', () => { throw new Error('test') }) + it('2', () => {}) +}) + +describe('afterEach throws and test fails', () => { + afterEach(() => { throw new Error('afterEach') }) + it('1', () => { throw new Error('test') }) + it('2', () => {}) +}) + test('test hooks', async (t) => { const testArr = [] t.beforeEach((t) => testArr.push('beforeEach ' + t.name)) @@ -112,3 +124,15 @@ test('t.afterEach throws', async (t) => { await t.test('1', () => {}) await t.test('2', () => {}) }) + +test('afterEach when test fails', async (t) => { + t.afterEach(common.mustCall(2)) + await t.test('1', () => { throw new Error('test') }) + await t.test('2', () => {}) +}) + +test('afterEach throws and test fails', async (t) => { + afterEach(() => { throw new Error('afterEach') }) + await t.test('1', () => { throw new Error('test') }) + await t.test('2', () => {}) +}) diff --git a/test/message/test_runner_hooks.out b/test/message/test_runner_hooks.out index 57008a4..e0b3e91 100644 --- a/test/message/test_runner_hooks.out +++ b/test/message/test_runner_hooks.out @@ -189,6 +189,86 @@ not ok 5 - afterEach throws error: '2 subtests failed' code: 'ERR_TEST_FAILURE' ... +# Subtest: afterEach when test fails + # Subtest: 1 + not ok 1 - 1 + --- + duration_ms: * + failureType: 'testCodeFailure' + error: 'test' + code: 'ERR_TEST_FAILURE' + stack: |- + * + * + * + * + * + * + * + * + * + * + ... + # Subtest: 2 + ok 2 - 2 + --- + duration_ms: * + ... + 1..2 +not ok 6 - afterEach when test fails + --- + duration_ms: * + failureType: 'subtestsFailed' + error: '1 subtest failed' + code: 'ERR_TEST_FAILURE' + ... +# Subtest: afterEach throws and test fails + # Subtest: 1 + not ok 1 - 1 + --- + duration_ms: * + failureType: 'testCodeFailure' + error: 'test' + code: 'ERR_TEST_FAILURE' + stack: |- + * + * + * + * + * + * + * + * + * + * + ... + # Subtest: 2 + not ok 2 - 2 + --- + duration_ms: * + failureType: 'hookFailed' + error: 'failed running afterEach hook' + code: 'ERR_TEST_FAILURE' + stack: |- + * + * + * + * + * + * + * + * + * + * + ... + 1..2 +not ok 7 - afterEach throws and test fails + --- + duration_ms: * + failureType: 'subtestsFailed' + error: '2 subtests failed' + code: 'ERR_TEST_FAILURE' + ... # Subtest: test hooks # Subtest: 1 ok 1 - 1 @@ -217,7 +297,7 @@ not ok 5 - afterEach throws duration_ms: * ... 1..3 -ok 6 - test hooks +ok 8 - test hooks --- duration_ms: * ... @@ -261,7 +341,7 @@ ok 6 - test hooks * ... 1..2 -not ok 7 - t.beforeEach throws +not ok 9 - t.beforeEach throws --- duration_ms: * failureType: 'subtestsFailed' @@ -308,17 +388,97 @@ not ok 7 - t.beforeEach throws * ... 1..2 -not ok 8 - t.afterEach throws +not ok 10 - t.afterEach throws + --- + duration_ms: * + failureType: 'subtestsFailed' + error: '2 subtests failed' + code: 'ERR_TEST_FAILURE' + ... +# Subtest: afterEach when test fails + # Subtest: 1 + not ok 1 - 1 + --- + duration_ms: * + failureType: 'testCodeFailure' + error: 'test' + code: 'ERR_TEST_FAILURE' + stack: |- + * + * + * + * + * + * + * + * + * + * + ... + # Subtest: 2 + ok 2 - 2 + --- + duration_ms: * + ... + 1..2 +not ok 11 - afterEach when test fails + --- + duration_ms: * + failureType: 'subtestsFailed' + error: '1 subtest failed' + code: 'ERR_TEST_FAILURE' + ... +# Subtest: afterEach throws and test fails + # Subtest: 1 + not ok 1 - 1 + --- + duration_ms: * + failureType: 'testCodeFailure' + error: 'test' + code: 'ERR_TEST_FAILURE' + stack: |- + * + * + * + * + * + * + * + * + * + * + ... + # Subtest: 2 + not ok 2 - 2 + --- + duration_ms: * + failureType: 'hookFailed' + error: 'failed running afterEach hook' + code: 'ERR_TEST_FAILURE' + stack: |- + * + * + * + * + * + * + * + * + * + * + ... + 1..2 +not ok 12 - afterEach throws and test fails --- duration_ms: * failureType: 'subtestsFailed' error: '2 subtests failed' code: 'ERR_TEST_FAILURE' ... -1..8 -# tests 8 +1..12 +# tests 12 # pass 2 -# fail 6 +# fail 10 # cancelled 0 # skipped 0 # todo 0 diff --git a/test/message/test_runner_output.out b/test/message/test_runner_output.out index 938989b..96d977b 100644 --- a/test/message/test_runner_output.out +++ b/test/message/test_runner_output.out @@ -42,8 +42,6 @@ not ok 4 - sync fail todo with message # TODO this is a failing todo * * * - * - * ... # Subtest: sync skip pass ok 5 - sync skip pass # SKIP From cff397a46139411528314d5a9399ae12e1b48660 Mon Sep 17 00:00:00 2001 From: MURAKAMI Masahiko Date: Mon, 7 Nov 2022 21:02:56 +0900 Subject: [PATCH 12/24] fix: avoid swallowing of asynchronously thrown errors Fixes: https://github.com/nodejs/node/issues/44612 PR-URL: https://github.com/nodejs/node/pull/45264 Reviewed-By: Moshe Atlow Reviewed-By: Benjamin Gruenbaum (cherry picked from commit 06603c44a5b0e92b1a3591ace467ce9770bf9658) --- README.md | 4 +-- lib/internal/test_runner/harness.js | 3 +- .../extraneous_set_immediate_async.mjs | 6 ++++ .../extraneous_set_timeout_async.mjs | 6 ++++ .../test-runner-extraneous-async-activity.js | 32 +++++++++++++++++++ 5 files changed, 48 insertions(+), 3 deletions(-) create mode 100644 test/fixtures/test-runner/extraneous_set_immediate_async.mjs create mode 100644 test/fixtures/test-runner/extraneous_set_timeout_async.mjs create mode 100644 test/parallel/test-runner-extraneous-async-activity.js diff --git a/README.md b/README.md index 0407239..563e158 100644 --- a/README.md +++ b/README.md @@ -279,8 +279,8 @@ top level of the file's TAP output. The second `setImmediate()` creates an `uncaughtException` event. `uncaughtException` and `unhandledRejection` events originating from a completed -test are handled by the `test` module and reported as diagnostic warnings in -the top level of the file's TAP output. +test are marked as failed by the `test` module and reported as diagnostic +warnings in the top level of the file's TAP output. ```js test('a test that creates asynchronous activity', t => { diff --git a/lib/internal/test_runner/harness.js b/lib/internal/test_runner/harness.js index bddfedc..26d35ba 100644 --- a/lib/internal/test_runner/harness.js +++ b/lib/internal/test_runner/harness.js @@ -1,4 +1,4 @@ -// https://github.com/nodejs/node/blob/59527de13d39327eb3dfa8dedc92241eb40066d5/lib/internal/test_runner/harness.js +// https://github.com/nodejs/node/blob/06603c44a5b0e92b1a3591ace467ce9770bf9658/lib/internal/test_runner/harness.js 'use strict' const { ArrayPrototypeForEach, @@ -47,6 +47,7 @@ function createProcessEventHandler (eventName, rootTest) { `triggered an ${eventName} event.` rootTest.diagnostic(msg) + process.exitCode = 1 return } diff --git a/test/fixtures/test-runner/extraneous_set_immediate_async.mjs b/test/fixtures/test-runner/extraneous_set_immediate_async.mjs new file mode 100644 index 0000000..241e262 --- /dev/null +++ b/test/fixtures/test-runner/extraneous_set_immediate_async.mjs @@ -0,0 +1,6 @@ +// https://github.com/nodejs/node/blob/06603c44a5b0e92b1a3591ace467ce9770bf9658/test/fixtures/test-runner/extraneous_set_immediate_async.mjs +import test from '#node:test' + +test('extraneous async activity test', () => { + setImmediate(() => { throw new Error() }) +}) diff --git a/test/fixtures/test-runner/extraneous_set_timeout_async.mjs b/test/fixtures/test-runner/extraneous_set_timeout_async.mjs new file mode 100644 index 0000000..2b5ee72 --- /dev/null +++ b/test/fixtures/test-runner/extraneous_set_timeout_async.mjs @@ -0,0 +1,6 @@ +// https://github.com/nodejs/node/blob/06603c44a5b0e92b1a3591ace467ce9770bf9658/test/fixtures/test-runner/extraneous_set_timeout_async.mjs +import test from '#node:test' + +test('extraneous async activity test', () => { + setTimeout(() => { throw new Error() }, 100) +}) diff --git a/test/parallel/test-runner-extraneous-async-activity.js b/test/parallel/test-runner-extraneous-async-activity.js new file mode 100644 index 0000000..da826c4 --- /dev/null +++ b/test/parallel/test-runner-extraneous-async-activity.js @@ -0,0 +1,32 @@ +// https://github.com/nodejs/node/blob/06603c44a5b0e92b1a3591ace467ce9770bf9658/test/parallel/test-runner-extraneous-async-activity.js +'use strict' +require('../common') +const fixtures = require('../common/fixtures') +const assert = require('assert') +const { spawnSync } = require('child_process') + +{ + const child = spawnSync(process.execPath, [ + '--test', + fixtures.path('test-runner', 'extraneous_set_immediate_async.mjs') + ]) + const stdout = child.stdout.toString() + assert.match(stdout, /^# pass 0$/m) + assert.match(stdout, /^# fail 1$/m) + assert.match(stdout, /^# cancelled 0$/m) + assert.strictEqual(child.status, 1) + assert.strictEqual(child.signal, null) +} + +{ + const child = spawnSync(process.execPath, [ + '--test', + fixtures.path('test-runner', 'extraneous_set_timeout_async.mjs') + ]) + const stdout = child.stdout.toString() + assert.match(stdout, /^# pass 0$/m) + assert.match(stdout, /^# fail 1$/m) + assert.match(stdout, /^# cancelled 0$/m) + assert.strictEqual(child.status, 1) + assert.strictEqual(child.signal, null) +} From 2e499ee084d2f5c10b5b8405904c397ad1eb517f Mon Sep 17 00:00:00 2001 From: cjihrig Date: Mon, 4 Apr 2022 18:36:40 -0400 Subject: [PATCH 13/24] feat: support function mocking This commit allows tests in the test runner to mock functions and methods. PR-URL: https://github.com/nodejs/node/pull/45326 Reviewed-By: Moshe Atlow Reviewed-By: Matteo Collina (cherry picked from commit 7c6682957b3c5f86d0616cebc0ad09cc2a1fd50d) --- README.md | 334 ++++++++++ lib/internal/per_context/primordials.js | 7 + lib/internal/test_runner/mock.js | 297 +++++++++ lib/internal/test_runner/test.js | 10 +- lib/internal/validators.js | 44 +- lib/test.js | 20 +- test/parallel/test-runner-mocking.js | 802 ++++++++++++++++++++++++ 7 files changed, 1511 insertions(+), 3 deletions(-) create mode 100644 lib/internal/test_runner/mock.js create mode 100644 test/parallel/test-runner-mocking.js diff --git a/README.md b/README.md index 563e158..802477f 100644 --- a/README.md +++ b/README.md @@ -359,6 +359,78 @@ Otherwise, the test is considered to be a failure. Test files must be executable by Node.js, but are not required to use the `node:test` module internally. +## Mocking + +The `node:test` module supports mocking during testing via a top-level `mock` +object. The following example creates a spy on a function that adds two numbers +together. The spy is then used to assert that the function was called as +expected. + +```mjs +import assert from 'node:assert'; +import { mock, test } from 'node:test'; +test('spies on a function', () => { + const sum = mock.fn((a, b) => { + return a + b; + }); + assert.strictEqual(sum.mock.calls.length, 0); + assert.strictEqual(sum(3, 4), 7); + assert.strictEqual(sum.mock.calls.length, 1); + const call = sum.mock.calls[0]; + assert.deepStrictEqual(call.arguments, [3, 4]); + assert.strictEqual(call.result, 7); + assert.strictEqual(call.error, undefined); + // Reset the globally tracked mocks. + mock.reset(); +}); +``` + +```cjs +'use strict'; +const assert = require('node:assert'); +const { mock, test } = require('node:test'); +test('spies on a function', () => { + const sum = mock.fn((a, b) => { + return a + b; + }); + assert.strictEqual(sum.mock.calls.length, 0); + assert.strictEqual(sum(3, 4), 7); + assert.strictEqual(sum.mock.calls.length, 1); + const call = sum.mock.calls[0]; + assert.deepStrictEqual(call.arguments, [3, 4]); + assert.strictEqual(call.result, 7); + assert.strictEqual(call.error, undefined); + // Reset the globally tracked mocks. + mock.reset(); +}); +``` + +The same mocking functionality is also exposed on the [`TestContext`][] object +of each test. The following example creates a spy on an object method using the +API exposed on the `TestContext`. The benefit of mocking via the test context is +that the test runner will automatically restore all mocked functionality once +the test finishes. + +```js +test('spies on an object method', (t) => { + const number = { + value: 5, + add(a) { + return this.value + a; + }, + }; + t.mock.method(number, 'add'); + assert.strictEqual(number.add.mock.calls.length, 0); + assert.strictEqual(number.add(3), 8); + assert.strictEqual(number.add.mock.calls.length, 1); + const call = number.add.mock.calls[0]; + assert.deepStrictEqual(call.arguments, [3]); + assert.strictEqual(call.result, 8); + assert.strictEqual(call.target, undefined); + assert.strictEqual(call.this, number); +}); +``` + ## `run([options])` + +The `MockFunctionContext` class is used to inspect or manipulate the behavior of +mocks created via the [`MockTracker`][] APIs. + +### `ctx.calls` + + + +* {Array} + +A getter that returns a copy of the internal array used to track calls to the +mock. Each entry in the array is an object with the following properties. + +* `arguments` {Array} An array of the arguments passed to the mock function. +* `error` {any} If the mocked function threw then this property contains the + thrown value. **Default:** `undefined`. +* `result` {any} The value returned by the mocked function. +* `stack` {Error} An `Error` object whose stack can be used to determine the + callsite of the mocked function invocation. +* `target` {Function|undefined} If the mocked function is a constructor, this + field contains the class being constructed. Otherwise this will be + `undefined`. +* `this` {any} The mocked function's `this` value. + +### `ctx.callCount()` + + + +* Returns: {integer} The number of times that this mock has been invoked. + +This function returns the number of times that this mock has been invoked. This +function is more efficient than checking `ctx.calls.length` because `ctx.calls` +is a getter that creates a copy of the internal call tracking array. + +### `ctx.mockImplementation(implementation)` + + + +* `implementation` {Function|AsyncFunction} The function to be used as the + mock's new implementation. + +This function is used to change the behavior of an existing mock. + +The following example creates a mock function using `t.mock.fn()`, calls the +mock function, and then changes the mock implementation to a different function. + +```js +test('changes a mock behavior', (t) => { + let cnt = 0; + function addOne() { + cnt++; + return cnt; + } + function addTwo() { + cnt += 2; + return cnt; + } + const fn = t.mock.fn(addOne); + assert.strictEqual(fn(), 1); + fn.mock.mockImplementation(addTwo); + assert.strictEqual(fn(), 3); + assert.strictEqual(fn(), 5); +}); +``` + +### `ctx.mockImplementationOnce(implementation[, onCall])` + + + +* `implementation` {Function|AsyncFunction} The function to be used as the + mock's implementation for the invocation number specified by `onCall`. +* `onCall` {integer} The invocation number that will use `implementation`. If + the specified invocation has already occurred then an exception is thrown. + **Default:** The number of the next invocation. + +This function is used to change the behavior of an existing mock for a single +invocation. Once invocation `onCall` has occurred, the mock will revert to +whatever behavior it would have used had `mockImplementationOnce()` not been +called. + +The following example creates a mock function using `t.mock.fn()`, calls the +mock function, changes the mock implementation to a different function for the +next invocation, and then resumes its previous behavior. + +```js +test('changes a mock behavior once', (t) => { + let cnt = 0; + function addOne() { + cnt++; + return cnt; + } + function addTwo() { + cnt += 2; + return cnt; + } + const fn = t.mock.fn(addOne); + assert.strictEqual(fn(), 1); + fn.mock.mockImplementationOnce(addTwo); + assert.strictEqual(fn(), 3); + assert.strictEqual(fn(), 4); +}); +``` + +### `ctx.restore()` + + + +Resets the implementation of the mock function to its original behavior. The +mock can still be used after calling this function. + +## Class: `MockTracker` + + + +The `MockTracker` class is used to manage mocking functionality. The test runner +module provides a top level `mock` export which is a `MockTracker` instance. +Each test also provides its own `MockTracker` instance via the test context's +`mock` property. + +### `mock.fn([original[, implementation]][, options])` + + + +* `original` {Function|AsyncFunction} An optional function to create a mock on. + **Default:** A no-op function. +* `implementation` {Function|AsyncFunction} An optional function used as the + mock implementation for `original`. This is useful for creating mocks that + exhibit one behavior for a specified number of calls and then restore the + behavior of `original`. **Default:** The function specified by `original`. +* `options` {Object} Optional configuration options for the mock function. The + following properties are supported: + * `times` {integer} The number of times that the mock will use the behavior of + `implementation`. Once the mock function has been called `times` times, it + will automatically restore the behavior of `original`. This value must be an + integer greater than zero. **Default:** `Infinity`. +* Returns: {Proxy} The mocked function. The mocked function contains a special + `mock` property, which is an instance of [`MockFunctionContext`][], and can + be used for inspecting and changing the behavior of the mocked function. + +This function is used to create a mock function. + +The following example creates a mock function that increments a counter by one +on each invocation. The `times` option is used to modify the mock behavior such +that the first two invocations add two to the counter instead of one. + +```js +test('mocks a counting function', (t) => { + let cnt = 0; + function addOne() { + cnt++; + return cnt; + } + function addTwo() { + cnt += 2; + return cnt; + } + const fn = t.mock.fn(addOne, addTwo, { times: 2 }); + assert.strictEqual(fn(), 2); + assert.strictEqual(fn(), 4); + assert.strictEqual(fn(), 5); + assert.strictEqual(fn(), 6); +}); +``` + +### `mock.method(object, methodName[, implementation][, options])` + + + +* `object` {Object} The object whose method is being mocked. +* `methodName` {string|symbol} The identifier of the method on `object` to mock. + If `object[methodName]` is not a function, an error is thrown. +* `implementation` {Function|AsyncFunction} An optional function used as the + mock implementation for `object[methodName]`. **Default:** The original method + specified by `object[methodName]`. +* `options` {Object} Optional configuration options for the mock method. The + following properties are supported: + * `getter` {boolean} If `true`, `object[methodName]` is treated as a getter. + This option cannot be used with the `setter` option. **Default:** false. + * `setter` {boolean} If `true`, `object[methodName]` is treated as a setter. + This option cannot be used with the `getter` option. **Default:** false. + * `times` {integer} The number of times that the mock will use the behavior of + `implementation`. Once the mocked method has been called `times` times, it + will automatically restore the original behavior. This value must be an + integer greater than zero. **Default:** `Infinity`. +* Returns: {Proxy} The mocked method. The mocked method contains a special + `mock` property, which is an instance of [`MockFunctionContext`][], and can + be used for inspecting and changing the behavior of the mocked method. + +This function is used to create a mock on an existing object method. The +following example demonstrates how a mock is created on an existing object +method. + +```js +test('spies on an object method', (t) => { + const number = { + value: 5, + subtract(a) { + return this.value - a; + }, + }; + t.mock.method(number, 'subtract'); + assert.strictEqual(number.subtract.mock.calls.length, 0); + assert.strictEqual(number.subtract(3), 2); + assert.strictEqual(number.subtract.mock.calls.length, 1); + const call = number.subtract.mock.calls[0]; + assert.deepStrictEqual(call.arguments, [3]); + assert.strictEqual(call.result, 2); + assert.strictEqual(call.error, undefined); + assert.strictEqual(call.target, undefined); + assert.strictEqual(call.this, number); +}); +``` + +### `mock.reset()` + + + +This function restores the default behavior of all mocks that were previously +created by this `MockTracker` and disassociates the mocks from the +`MockTracker` instance. Once disassociated, the mocks can still be used, but the +`MockTracker` instance can no longer be used to reset their behavior or +otherwise interact with them. + +After each test completes, this function is called on the test context's +`MockTracker`. If the global `MockTracker` is used extensively, calling this +function manually is recommended. + +### `mock.restoreAll()` + + + +This function restores the default behavior of all mocks that were previously +created by this `MockTracker`. Unlike `mock.reset()`, `mock.restoreAll()` does +not disassociate the mocks from the `MockTracker` instance. + ## Class: `TapStream` + +This function is syntax sugar for [`MockTracker.method`][] with `options.getter` +set to `true`. + ### `mock.method(object, methodName[, implementation][, options])` + +This function is syntax sugar for [`MockTracker.method`][] with `options.setter` +set to `true`. + ## Class: `TapStream` + +* `fn` {Function|AsyncFunction} The hook function. The first argument + to this function is a [`TestContext`][] object. If the hook uses callbacks, + the callback function is passed as the second argument. **Default:** A no-op + function. +* `options` {Object} Configuration options for the hook. The following + properties are supported: + * `signal` {AbortSignal} Allows aborting an in-progress hook. + * `timeout` {number} A number of milliseconds the hook will fail after. + If unspecified, subtests inherit this value from their parent. + **Default:** `Infinity`. + +This function is used to create a hook that runs after the current test +finishes. + +```js +test('top level test', async (t) => { + t.after((t) => t.diagnostic(`finished running ${t.name}`)); + assert.ok('some relevant assertion here'); +}); +``` + ### `context.afterEach([, fn][, options])` * `fn` {Function|AsyncFunction} The hook function. The first argument diff --git a/lib/internal/test_runner/test.js b/lib/internal/test_runner/test.js index 98e1993..6b356fc 100644 --- a/lib/internal/test_runner/test.js +++ b/lib/internal/test_runner/test.js @@ -1,4 +1,4 @@ -// https://github.com/nodejs/node/blob/8302b0add01758713246117d3d0533cd212f160d/lib/internal/test_runner/test.js +// https://github.com/nodejs/node/blob/215c5317d4837287fddb2e3b97872babd53183ac/lib/internal/test_runner/test.js 'use strict' @@ -140,6 +140,10 @@ class TestContext { return subtest.start() } + after (fn, options) { + this.#test.createHook('after', fn, options) + } + beforeEach (fn, options) { this.#test.createHook('beforeEach', fn, options) } @@ -533,6 +537,7 @@ class Test extends AsyncResource { return } + await this.runHook('after', { args, ctx }) await afterEach() this.pass() } catch (err) { diff --git a/test/message/test_runner_hooks.js b/test/message/test_runner_hooks.js index 80af89a..bd38713 100644 --- a/test/message/test_runner_hooks.js +++ b/test/message/test_runner_hooks.js @@ -1,4 +1,4 @@ -// https://github.com/nodejs/node/blob/3759935ee29d8042d917d3ceaa768521c14413ff/test/message/test_runner_hooks.js +// https://github.com/nodejs/node/blob/215c5317d4837287fddb2e3b97872babd53183ac/test/message/test_runner_hooks.js // Flags: --no-warnings 'use strict' const common = require('../common') @@ -91,6 +91,8 @@ describe('afterEach throws and test fails', () => { test('test hooks', async (t) => { const testArr = [] + + t.after(common.mustCall((t) => testArr.push('after ' + t.name))) t.beforeEach((t) => testArr.push('beforeEach ' + t.name)) t.afterEach((t) => testArr.push('afterEach ' + t.name)) await t.test('1', () => testArr.push('1')) @@ -114,25 +116,29 @@ test('test hooks', async (t) => { }) test('t.beforeEach throws', async (t) => { + t.after(common.mustCall()) t.beforeEach(() => { throw new Error('beforeEach') }) await t.test('1', () => {}) await t.test('2', () => {}) }) test('t.afterEach throws', async (t) => { + t.after(common.mustCall()) t.afterEach(() => { throw new Error('afterEach') }) await t.test('1', () => {}) await t.test('2', () => {}) }) test('afterEach when test fails', async (t) => { + t.after(common.mustCall()) t.afterEach(common.mustCall(2)) await t.test('1', () => { throw new Error('test') }) await t.test('2', () => {}) }) test('afterEach throws and test fails', async (t) => { - afterEach(() => { throw new Error('afterEach') }) + t.after(common.mustCall()) + t.afterEach(() => { throw new Error('afterEach') }) await t.test('1', () => { throw new Error('test') }) await t.test('2', () => {}) }) From c0854ac037d0e9a1c54f91b6faf4ff35f0b2a96a Mon Sep 17 00:00:00 2001 From: Pulkit Gupta Date: Mon, 12 Dec 2022 01:32:37 +0530 Subject: [PATCH 19/24] test: fix invalid output TAP if there newline in test name PR-URL: https://github.com/nodejs/node/pull/45742 Fixes: https://github.com/nodejs/node/issues/45396 Reviewed-By: Moshe Atlow Reviewed-By: Colin Ihrig (cherry picked from commit 22dc987fde29734c5bcbb7c33da20d184ff61627) --- lib/internal/test_runner/tap_stream.js | 14 ++++++++++---- test/message/test_runner_output.js | 4 ++-- test/message/test_runner_output.out | 8 ++++---- 3 files changed, 16 insertions(+), 10 deletions(-) diff --git a/lib/internal/test_runner/tap_stream.js b/lib/internal/test_runner/tap_stream.js index 42ebfb9..54b6301 100644 --- a/lib/internal/test_runner/tap_stream.js +++ b/lib/internal/test_runner/tap_stream.js @@ -1,4 +1,4 @@ -// https://github.com/nodejs/node/blob/f8ce9117b19702487eb600493d941f7876e00e01/lib/internal/test_runner/tap_stream.js +// https://github.com/nodejs/node/blob/22dc987fde29734c5bcbb7c33da20d184ff61627/lib/internal/test_runner/tap_stream.js 'use strict' @@ -134,9 +134,15 @@ class TapStream extends Readable { // In certain places, # and \ need to be escaped as \# and \\. function tapEscape (input) { - return StringPrototypeReplaceAll( - StringPrototypeReplaceAll(input, '\\', '\\\\'), '#', '\\#' - ) + let result = StringPrototypeReplaceAll(input, '\\', '\\\\') + result = StringPrototypeReplaceAll(result, '#', '\\#') + result = StringPrototypeReplaceAll(result, '\b', '\\b') + result = StringPrototypeReplaceAll(result, '\f', '\\f') + result = StringPrototypeReplaceAll(result, '\t', '\\t') + result = StringPrototypeReplaceAll(result, '\n', '\\n') + result = StringPrototypeReplaceAll(result, '\r', '\\r') + result = StringPrototypeReplaceAll(result, '\v', '\\v') + return result } function jsToYaml (indent, name, value) { diff --git a/test/message/test_runner_output.js b/test/message/test_runner_output.js index 9a1dc66..a85c16e 100644 --- a/test/message/test_runner_output.js +++ b/test/message/test_runner_output.js @@ -1,4 +1,4 @@ -// https://github.com/nodejs/node/blob/cb7e0c59df10a42cd6930ca7f99d3acee1ce7627/test/message/test_runner_output.js +// https://github.com/nodejs/node/blob/22dc987fde29734c5bcbb7c33da20d184ff61627/test/message/test_runner_output.js // Flags: --no-warnings 'use strict' require('../common') @@ -214,7 +214,7 @@ test('test with a name and options provided', { skip: true }) test({ skip: true }, function functionAndOptions () {}) // A test whose description needs to be escaped. -test('escaped description \\ # \\#\\') +test('escaped description \\ # \\#\\ \n \t \f \v \b \r') // A test whose skip message needs to be escaped. test('escaped skip message', { skip: '#skip' }) diff --git a/test/message/test_runner_output.out b/test/message/test_runner_output.out index 96d977b..14479c7 100644 --- a/test/message/test_runner_output.out +++ b/test/message/test_runner_output.out @@ -127,9 +127,9 @@ not ok 13 - async assertion fail failureType: 'testCodeFailure' error: |- Expected values to be strictly equal: - + true !== false - + code: 'ERR_ASSERTION' expected: false actual: true @@ -353,8 +353,8 @@ ok 36 - functionAndOptions # SKIP --- duration_ms: * ... -# Subtest: escaped description \\ \# \\\#\\ -ok 37 - escaped description \\ \# \\\#\\ +# Subtest: escaped description \\ \# \\\#\\ \n \t \f \v \b \r +ok 37 - escaped description \\ \# \\\#\\ \n \t \f \v \b \r --- duration_ms: * ... From 9b499783f01e8d35a5bba8082d2a3d5eaf7d6a43 Mon Sep 17 00:00:00 2001 From: Antoine du Hamel Date: Sun, 11 Dec 2022 23:34:31 +0100 Subject: [PATCH 20/24] chore: refactor `tap_parser` to use more primordials PR-URL: https://github.com/nodejs/node/pull/45745 Reviewed-By: Moshe Atlow Reviewed-By: Colin Ihrig Reviewed-By: Yagiz Nizipli (cherry picked from commit 7a42a206ac37d95060640b4812aaef32535937e1) --- lib/internal/test_runner/tap_parser.js | 56 ++++++++++++++------------ 1 file changed, 31 insertions(+), 25 deletions(-) diff --git a/lib/internal/test_runner/tap_parser.js b/lib/internal/test_runner/tap_parser.js index 6ba3afd..a309bff 100644 --- a/lib/internal/test_runner/tap_parser.js +++ b/lib/internal/test_runner/tap_parser.js @@ -1,29 +1,31 @@ -// https://github.com/nodejs/node/blob/f8ce9117b19702487eb600493d941f7876e00e01/lib/internal/test_runner/tap_parser.js +// https://github.com/nodejs/node/blob/7a42a206ac37d95060640b4812aaef32535937e1/lib/internal/test_runner/tap_parser.js 'use strict' -const Transform = require('stream').Transform -const { TapLexer, TokenKind } = require('#internal/test_runner/tap_lexer') -const { TapChecker } = require('#internal/test_runner/tap_checker') -const { - codes: { ERR_TAP_VALIDATION_ERROR, ERR_TAP_PARSER_ERROR } -} = require('#internal/errors') -const { kEmptyObject } = require('#internal/util') const { ArrayPrototypeFilter, ArrayPrototypeForEach, + ArrayPrototypeIncludes, ArrayPrototypeJoin, ArrayPrototypeMap, + ArrayPrototypePop, ArrayPrototypePush, - ArrayPrototypeIncludes, - ArrayPrototypeSplice, Boolean, Number, RegExpPrototypeExec, - RegExpPrototypeSymbolReplace, String, - StringPrototypeTrim, - StringPrototypeSplit + StringPrototypeEndsWith, + StringPrototypeReplaceAll, + StringPrototypeSlice, + StringPrototypeSplit, + StringPrototypeTrim } = require('#internal/per_context/primordials') +const Transform = require('stream').Transform +const { TapLexer, TokenKind } = require('#internal/test_runner/tap_lexer') +const { TapChecker } = require('#internal/test_runner/tap_checker') +const { + codes: { ERR_TAP_VALIDATION_ERROR, ERR_TAP_PARSER_ERROR } +} = require('#internal/errors') +const { kEmptyObject } = require('#internal/util') /** * * TAP14 specifications @@ -150,22 +152,26 @@ class TapParser extends Transform { processChunk (chunk) { const str = this.#lastLine + chunk.toString('utf8') const lines = StringPrototypeSplit(str, '\n') - this.#lastLine = ArrayPrototypeSplice(lines, lines.length - 1, 1)[0] + this.#lastLine = ArrayPrototypePop(lines) - let chunkAsString = lines.join('\n') + let chunkAsString = ArrayPrototypeJoin(lines, '\n') // Special case where chunk is emitted by a child process - chunkAsString = RegExpPrototypeSymbolReplace( - /\[out\] /g, + chunkAsString = StringPrototypeReplaceAll( chunkAsString, + '[out] ', '' ) - chunkAsString = RegExpPrototypeSymbolReplace( - /\[err\] /g, + chunkAsString = StringPrototypeReplaceAll( chunkAsString, + '[err] ', '' ) - chunkAsString = RegExpPrototypeSymbolReplace(/\n$/, chunkAsString, '') - chunkAsString = RegExpPrototypeSymbolReplace(/EOF$/, chunkAsString, '') + if (StringPrototypeEndsWith(chunkAsString, '\n')) { + chunkAsString = StringPrototypeSlice(chunkAsString, 0, -1) + } + if (StringPrototypeEndsWith(chunkAsString, 'EOF')) { + chunkAsString = StringPrototypeSlice(chunkAsString, 0, -3) + } return chunkAsString } @@ -372,7 +378,7 @@ class TapParser extends Transform { } #addDiagnosticsToLastTestPoint (currentNode) { - const lastTestPoint = this.#bufferedTestPoints[this.#bufferedTestPoints.length - 1] + const { length, [length - 1]: lastTestPoint } = this.#bufferedTestPoints // Diagnostic nodes are only added to Test points of the same nesting level if (lastTestPoint && lastTestPoint.nesting === currentNode.nesting) { @@ -798,7 +804,7 @@ class TapParser extends Transform { const commentContent = this.#peek() if (commentContent) { - if (/^Subtest:/i.test(commentContent.value)) { + if (RegExpPrototypeExec(/^Subtest:/i, commentContent.value) !== null) { this.#next() // skip subtest keyword const name = StringPrototypeTrim(this.#readNextLiterals()) const node = { @@ -899,7 +905,7 @@ class TapParser extends Transform { // YAMLLine := " " (YAML)* "\n" #YAMLLine () { const yamlLiteral = this.#readNextLiterals() - const { 0: key, 1: value } = StringPrototypeSplit(yamlLiteral, ':') + const { 0: key, 1: value } = StringPrototypeSplit(yamlLiteral, ':', 2) // Note that this.#lastTestPointDetails has been cleared when we encounter a YAML start marker @@ -961,7 +967,7 @@ class TapParser extends Transform { // In some cases, pragma key can be followed by a comma separator, // so we need to remove it - pragmaKey = RegExpPrototypeSymbolReplace(/,/g, pragmaKey, '') + pragmaKey = StringPrototypeReplaceAll(pragmaKey, ',', '') pragmas[pragmaKey] = isEnabled nextToken = this.#peek() From 4e778bc4995cd1b6a250557c4651fd53eabf1aaf Mon Sep 17 00:00:00 2001 From: Antoine du Hamel Date: Mon, 12 Dec 2022 07:30:47 +0100 Subject: [PATCH 21/24] chore: refactor `tap_lexer` to use more primordials PR-URL: https://github.com/nodejs/node/pull/45744 Reviewed-By: Moshe Atlow Reviewed-By: Colin Ihrig (cherry picked from commit 2483da743cbb48f31c6b3f8cb186d89f31d73611) --- lib/internal/test_runner/tap_lexer.js | 38 ++++++++++++++++----------- 1 file changed, 22 insertions(+), 16 deletions(-) diff --git a/lib/internal/test_runner/tap_lexer.js b/lib/internal/test_runner/tap_lexer.js index 9fcf93e..022f75f 100644 --- a/lib/internal/test_runner/tap_lexer.js +++ b/lib/internal/test_runner/tap_lexer.js @@ -1,7 +1,14 @@ -// https://github.com/nodejs/node/blob/f8ce9117b19702487eb600493d941f7876e00e01/lib/internal/test_runner/tap_lexer.js +// https://github.com/nodejs/node/blob/2483da743cbb48f31c6b3f8cb186d89f31d73611/lib/internal/test_runner/tap_lexer.js 'use strict' -const { SafeSet, MathMax, StringPrototypeIncludes } = require('#internal/per_context/primordials') +const { + ArrayPrototypePop, + ArrayPrototypePush, + MathMax, + SafeSet, + StringPrototypeIncludes, + StringPrototypeTrim +} = require('#internal/per_context/primordials') const { codes: { ERR_TAP_LEXER_ERROR } } = require('#internal/errors') @@ -137,21 +144,21 @@ class TapLexer { this.#lastScannedToken = token } + ArrayPrototypePush(chunk, token) if (token.kind === TokenKind.NEWLINE) { // Store the current chunk + NEWLINE token - tokens.push([...chunk, token]) + ArrayPrototypePush(tokens, chunk) chunk = [] - } else { - chunk.push(token) } } if (chunk.length > 0) { - tokens.push([...chunk, this.#scanEOL()]) + ArrayPrototypePush(chunk, this.#scanEOL()) + ArrayPrototypePush(tokens, chunk) } // send EOF as a separate chunk - tokens.push([this.#scanEOF()]) + ArrayPrototypePush(tokens, [this.#scanEOF()]) return tokens } @@ -239,7 +246,7 @@ class TapLexer { this.#hasTheCurrentCharacterBeenEscaped() || this.#source.peek(1) === TokenKind.WHITESPACE ) { - this.#escapeStack.pop() + ArrayPrototypePop(this.#escapeStack) return new Token({ kind: TokenKind.LITERAL, value: char, @@ -250,7 +257,7 @@ class TapLexer { // Otherwise, consume the escape symbol as an escape symbol that should be ignored by the parser // we also need to push the escape symbol to the escape stack // and consume the next character as a literal (done in the next turn) - this.#escapeStack.push(char) + ArrayPrototypePush(this.#escapeStack, char) return new Token({ kind: TokenKind.ESCAPE, value: char, @@ -327,7 +334,7 @@ class TapLexer { const charHasBeenEscaped = this.#hasTheCurrentCharacterBeenEscaped() if (this.#isComment || charHasBeenEscaped) { if (charHasBeenEscaped) { - this.#escapeStack.pop() + ArrayPrototypePop(this.#escapeStack) } return new Token({ @@ -356,7 +363,7 @@ class TapLexer { } } - word = word.trim() + word = StringPrototypeTrim(word) if (TapLexer.Keywords.has(word)) { const token = this.#scanTAPKeyword(word) @@ -381,10 +388,9 @@ class TapLexer { } #scanTAPKeyword (word) { - const isLastScannedTokenEOLorNewLine = StringPrototypeIncludes( - [TokenKind.EOL, TokenKind.NEWLINE], - this.#lastScannedToken.kind - ) + const isLastScannedTokenEOLorNewLine = + TokenKind.EOL === this.#lastScannedToken.kind || + TokenKind.NEWLINE === this.#lastScannedToken.kind if (word === 'TAP' && isLastScannedTokenEOLorNewLine) { return new Token({ @@ -480,7 +486,7 @@ class TapLexer { // We deliberately do not include "# \ + -"" in this list // these are used for comments/reasons explanations, pragma and escape characters // whitespace is not included because it is handled separately - return '!"$%&\'()*,./:;<=>?@[]^_`{|}~'.indexOf(char) > -1 + return StringPrototypeIncludes('!"$%&\'()*,./:;<=>?@[]^_`{|}~', char) } #isWhitespaceSymbol (char) { From d1343a7074dd5ff1d9afaaf496f450b24c7f35d1 Mon Sep 17 00:00:00 2001 From: Moshe Atlow Date: Tue, 13 Dec 2022 16:51:35 +0200 Subject: [PATCH 22/24] feat: parse yaml PR-URL: https://github.com/nodejs/node/pull/45815 Reviewed-By: Colin Ihrig Reviewed-By: Benjamin Gruenbaum Reviewed-By: Antoine du Hamel (cherry picked from commit 232efb06fe8787e9573e298ce7ac293ad23b7684) --- lib/internal/per_context/primordials.js | 4 + lib/internal/test_runner/runner.js | 20 +--- lib/internal/test_runner/tap_stream.js | 7 +- lib/internal/test_runner/test.js | 4 +- lib/internal/test_runner/yaml_parser.js | 121 ++++++++++++++++++++++++ 5 files changed, 134 insertions(+), 22 deletions(-) create mode 100644 lib/internal/test_runner/yaml_parser.js diff --git a/lib/internal/per_context/primordials.js b/lib/internal/per_context/primordials.js index f3c0b24..951bf28 100644 --- a/lib/internal/per_context/primordials.js +++ b/lib/internal/per_context/primordials.js @@ -20,6 +20,7 @@ exports.ArrayPrototypeSome = (arr, fn) => arr.some(fn) exports.ArrayPrototypeSort = (arr, fn) => arr.sort(fn) exports.ArrayPrototypeSplice = (arr, offset, len, ...el) => arr.splice(offset, len, ...el) exports.ArrayPrototypeUnshift = (arr, ...el) => arr.unshift(...el) +exports.Boolean = Boolean exports.Error = Error exports.ErrorCaptureStackTrace = (...args) => Error.captureStackTrace(...args) exports.FunctionPrototype = Function.prototype @@ -28,6 +29,7 @@ exports.FunctionPrototypeCall = (fn, obj, ...args) => fn.call(obj, ...args) exports.MathMax = (...args) => Math.max(...args) exports.Number = Number exports.NumberIsInteger = Number.isInteger +exports.NumberIsNaN = Number.isNaN exports.NumberParseInt = (str, radix) => Number.parseInt(str, radix) exports.NumberMIN_SAFE_INTEGER = Number.MIN_SAFE_INTEGER exports.NumberMAX_SAFE_INTEGER = Number.MAX_SAFE_INTEGER @@ -56,6 +58,7 @@ exports.SafePromiseRace = (array, mapFn) => Promise.race(mapFn ? array.map(mapFn exports.SafeSet = Set exports.SafeWeakMap = WeakMap exports.SafeWeakSet = WeakSet +exports.String = String exports.StringPrototypeEndsWith = (haystack, needle, index) => haystack.endsWith(needle, index) exports.StringPrototypeIncludes = (str, needle) => str.includes(needle) exports.StringPrototypeMatch = (str, reg) => str.match(reg) @@ -66,6 +69,7 @@ exports.StringPrototypeReplaceAll = replaceAll exports.StringPrototypeStartsWith = (haystack, needle, index) => haystack.startsWith(needle, index) exports.StringPrototypeSlice = (str, ...args) => str.slice(...args) exports.StringPrototypeSplit = (str, search, limit) => str.split(search, limit) +exports.StringPrototypeSubstring = (str, ...args) => str.substring(...args) exports.StringPrototypeToUpperCase = str => str.toUpperCase() exports.StringPrototypeTrim = str => str.trim() exports.Symbol = Symbol diff --git a/lib/internal/test_runner/runner.js b/lib/internal/test_runner/runner.js index 33c4fb3..42c83c6 100644 --- a/lib/internal/test_runner/runner.js +++ b/lib/internal/test_runner/runner.js @@ -1,11 +1,10 @@ -// https://github.com/nodejs/node/blob/fec0fbc333c58e6ebbd2322f5120830fda880eb0/lib/internal/test_runner/runner.js +// https://github.com/nodejs/node/blob/232efb06fe8787e9573e298ce7ac293ad23b7684/lib/internal/test_runner/runner.js 'use strict' const { ArrayFrom, ArrayPrototypeFilter, ArrayPrototypeForEach, ArrayPrototypeIncludes, - ArrayPrototypeJoin, ArrayPrototypePush, ArrayPrototypeSlice, ArrayPrototypeSort, @@ -32,6 +31,7 @@ const { kEmptyObject } = require('#internal/util') const { createTestTree } = require('#internal/test_runner/harness') const { kDefaultIndent, kSubtestsFailed, Test } = require('#internal/test_runner/test') const { TapParser } = require('#internal/test_runner/tap_parser') +const { YAMLToJs } = require('#internal/test_runner/yaml_parser') const { TokenKind } = require('#internal/test_runner/tap_lexer') const { isSupportedFileType, @@ -123,18 +123,6 @@ class FileTest extends Test { #handleReportItem ({ kind, node, nesting = 0 }) { const indent = StringPrototypeRepeat(kDefaultIndent, nesting + 1) - const details = (diagnostic) => { - return ( - diagnostic && { - __proto__: null, - yaml: - `${indent} ` + - ArrayPrototypeJoin(diagnostic, `\n${indent} `) + - '\n' - } - ) - } - switch (kind) { case TokenKind.TAP_VERSION: // TODO(manekinekko): handle TAP version coming from the parser. @@ -168,7 +156,7 @@ class FileTest extends Test { indent, node.id, node.description, - details(node.diagnostics), + YAMLToJs(node.diagnostics), directive ) } else { @@ -176,7 +164,7 @@ class FileTest extends Test { indent, node.id, node.description, - details(node.diagnostics), + YAMLToJs(node.diagnostics), directive ) } diff --git a/lib/internal/test_runner/tap_stream.js b/lib/internal/test_runner/tap_stream.js index 54b6301..97a4918 100644 --- a/lib/internal/test_runner/tap_stream.js +++ b/lib/internal/test_runner/tap_stream.js @@ -1,4 +1,4 @@ -// https://github.com/nodejs/node/blob/22dc987fde29734c5bcbb7c33da20d184ff61627/lib/internal/test_runner/tap_stream.js +// https://github.com/nodejs/node/blob/232efb06fe8787e9573e298ce7ac293ad23b7684/lib/internal/test_runner/tap_stream.js 'use strict' @@ -86,11 +86,10 @@ class TapStream extends Readable { } #details (indent, data = kEmptyObject) { - const { error, duration, yaml } = data + const { error, duration_ms } = data // eslint-disable-line camelcase let details = `${indent} ---\n` - details += `${yaml || ''}` - details += jsToYaml(indent, 'duration_ms', duration) + details += jsToYaml(indent, 'duration_ms', duration_ms) details += jsToYaml(indent, null, error) details += `${indent} ...\n` this.#tryPush(details) diff --git a/lib/internal/test_runner/test.js b/lib/internal/test_runner/test.js index 6b356fc..5292da7 100644 --- a/lib/internal/test_runner/test.js +++ b/lib/internal/test_runner/test.js @@ -1,4 +1,4 @@ -// https://github.com/nodejs/node/blob/215c5317d4837287fddb2e3b97872babd53183ac/lib/internal/test_runner/test.js +// https://github.com/nodejs/node/blob/232efb06fe8787e9573e298ce7ac293ad23b7684/lib/internal/test_runner/test.js 'use strict' @@ -668,7 +668,7 @@ class Test extends AsyncResource { this.reportSubtest() } let directive - const details = { __proto__: null, duration: this.#duration() } + const details = { __proto__: null, duration_ms: this.#duration() } if (this.skipped) { directive = this.reporter.getSkip(this.message) diff --git a/lib/internal/test_runner/yaml_parser.js b/lib/internal/test_runner/yaml_parser.js new file mode 100644 index 0000000..2e85db8 --- /dev/null +++ b/lib/internal/test_runner/yaml_parser.js @@ -0,0 +1,121 @@ +// https://github.com/nodejs/node/blob/232efb06fe8787e9573e298ce7ac293ad23b7684/lib/internal/test_runner/yaml_parser.js +'use strict' +const { + codes: { + ERR_TEST_FAILURE + } +} = require('#internal/errors') +const AssertionError = require('assert').AssertionError +const { + ArrayPrototypeJoin, + ArrayPrototypePush, + Error, + Number, + NumberIsNaN, + RegExpPrototypeExec, + StringPrototypeEndsWith, + StringPrototypeRepeat, + StringPrototypeSlice, + StringPrototypeStartsWith, + StringPrototypeSubstring +} = require('#internal/per_context/primordials') + +const kYamlKeyRegex = /^(\s+)?(\w+):(\s)+([>|][-+])?(.*)$/ +const kStackDelimiter = ' at ' + +function reConstructError (parsedYaml) { + if (!('error' in parsedYaml)) { + return parsedYaml + } + const isAssertionError = parsedYaml.code === 'ERR_ASSERTION' || + 'actual' in parsedYaml || 'expected' in parsedYaml || 'operator' in parsedYaml + const isTestFailure = parsedYaml.code === 'ERR_TEST_FAILURE' || 'failureType' in parsedYaml + const stack = parsedYaml.stack ? kStackDelimiter + ArrayPrototypeJoin(parsedYaml.stack, `\n${kStackDelimiter}`) : '' + let error, cause + + if (isAssertionError) { + cause = new AssertionError({ + message: parsedYaml.error, + actual: parsedYaml.actual, + expected: parsedYaml.expected, + operator: parsedYaml.operator + }) + } else { + // eslint-disable-next-line no-restricted-syntax + cause = new Error(parsedYaml.error) + cause.code = parsedYaml.code + } + cause.stack = stack + + if (isTestFailure) { + error = new ERR_TEST_FAILURE(cause, parsedYaml.failureType) + error.stack = stack + } + + parsedYaml.error = error ?? cause + delete parsedYaml.stack + delete parsedYaml.code + delete parsedYaml.failureType + delete parsedYaml.actual + delete parsedYaml.expected + delete parsedYaml.operator + + return parsedYaml +} + +function getYamlValue (value) { + if (StringPrototypeStartsWith(value, "'") && StringPrototypeEndsWith(value, "'")) { + return StringPrototypeSlice(value, 1, -1) + } + if (value === 'true') { + return true + } + if (value === 'false') { + return false + } + if (value !== '') { + const valueAsNumber = Number(value) + return NumberIsNaN(valueAsNumber) ? value : valueAsNumber + } + return value +} + +// This parses the YAML generated by the built-in TAP reporter, +// which is a subset of the full YAML spec. There are some +// YAML features that won't be parsed here. This function should not be exposed publicly. +function YAMLToJs (lines) { + if (lines == null) { + return undefined + } + const result = { __proto__: null } + let isInYamlBlock = false + for (let i = 0; i < lines.length; i++) { + const line = lines[i] + if (isInYamlBlock && !StringPrototypeStartsWith(line, StringPrototypeRepeat(' ', isInYamlBlock.indent))) { + result[isInYamlBlock.key] = isInYamlBlock.key === 'stack' + ? result[isInYamlBlock.key] + : ArrayPrototypeJoin(result[isInYamlBlock.key], '\n') + isInYamlBlock = false + } + if (isInYamlBlock) { + const blockLine = StringPrototypeSubstring(line, isInYamlBlock.indent) + ArrayPrototypePush(result[isInYamlBlock.key], blockLine) + continue + } + const match = RegExpPrototypeExec(kYamlKeyRegex, line) + if (match !== null) { + const { 1: leadingSpaces, 2: key, 4: block, 5: value } = match + if (block) { + isInYamlBlock = { key, indent: (leadingSpaces?.length ?? 0) + 2 } + result[key] = [] + } else { + result[key] = getYamlValue(value) + } + } + } + return reConstructError(result) +} + +module.exports = { + YAMLToJs +} From c80e426984408b2ef131ad05e2beb61715482046 Mon Sep 17 00:00:00 2001 From: Colin Ihrig Date: Sat, 17 Dec 2022 01:36:15 -0500 Subject: [PATCH 23/24] fix: run t.after() if test body throws This commit fixes a bug where t.after() was not called if the test body threw an exception. PR-URL: https://github.com/nodejs/node/pull/45870 Reviewed-By: Moshe Atlow Reviewed-By: Matteo Collina (cherry picked from commit 385d595a4f1d887f6d4221e6071571132498d57c) --- lib/internal/test_runner/test.js | 10 ++++++++-- test/message/test_runner_hooks.js | 9 ++++++++- test/message/test_runner_hooks.out | 24 +++++++++++++++++++++--- 3 files changed, 37 insertions(+), 6 deletions(-) diff --git a/lib/internal/test_runner/test.js b/lib/internal/test_runner/test.js index 5292da7..10cf1fc 100644 --- a/lib/internal/test_runner/test.js +++ b/lib/internal/test_runner/test.js @@ -1,4 +1,4 @@ -// https://github.com/nodejs/node/blob/232efb06fe8787e9573e298ce7ac293ad23b7684/lib/internal/test_runner/test.js +// https://github.com/nodejs/node/blob/385d595a4f1d887f6d4221e6071571132498d57c/lib/internal/test_runner/test.js 'use strict' @@ -496,6 +496,11 @@ class Test extends AsyncResource { } const { args, ctx } = this.getRunArgs() + const after = runOnce(async () => { + if (this.hooks.after.length > 0) { + await this.runHook('after', { args, ctx }) + } + }) const afterEach = runOnce(async () => { if (this.parent?.hooks.afterEach.length > 0) { await this.parent.runHook('afterEach', { args, ctx }) @@ -537,10 +542,11 @@ class Test extends AsyncResource { return } - await this.runHook('after', { args, ctx }) + await after() await afterEach() this.pass() } catch (err) { + try { await after() } catch { /* Ignore error. */ } try { await afterEach() } catch { /* test is already failing, let's the error */ } if (isTestFailureError(err)) { if (err.failureType === kTestTimeoutFailure) { diff --git a/test/message/test_runner_hooks.js b/test/message/test_runner_hooks.js index bd38713..afd6e32 100644 --- a/test/message/test_runner_hooks.js +++ b/test/message/test_runner_hooks.js @@ -1,4 +1,4 @@ -// https://github.com/nodejs/node/blob/215c5317d4837287fddb2e3b97872babd53183ac/test/message/test_runner_hooks.js +// https://github.com/nodejs/node/blob/385d595a4f1d887f6d4221e6071571132498d57c/test/message/test_runner_hooks.js // Flags: --no-warnings 'use strict' const common = require('../common') @@ -142,3 +142,10 @@ test('afterEach throws and test fails', async (t) => { await t.test('1', () => { throw new Error('test') }) await t.test('2', () => {}) }) + +test('t.after() is called if test body throws', (t) => { + t.after(() => { + t.diagnostic('- after() called') + }) + throw new Error('bye') +}) diff --git a/test/message/test_runner_hooks.out b/test/message/test_runner_hooks.out index e0b3e91..6bb1705 100644 --- a/test/message/test_runner_hooks.out +++ b/test/message/test_runner_hooks.out @@ -475,10 +475,28 @@ not ok 12 - afterEach throws and test fails error: '2 subtests failed' code: 'ERR_TEST_FAILURE' ... -1..12 -# tests 12 +# Subtest: t.after() is called if test body throws +not ok 13 - t.after() is called if test body throws + --- + duration_ms: * + failureType: 'testCodeFailure' + error: 'bye' + code: 'ERR_TEST_FAILURE' + stack: |- + * + * + * + * + * + * + * + * + ... +# - after() called +1..13 +# tests 13 # pass 2 -# fail 10 +# fail 11 # cancelled 0 # skipped 0 # todo 0 From 9fa496b0298a9ead260070bc548291f571840c70 Mon Sep 17 00:00:00 2001 From: Erick Wendel Date: Sat, 17 Dec 2022 11:35:09 -0300 Subject: [PATCH 24/24] test: fix mock.method to support class instances It fixes a problem when trying to spy a method from a class instance or static functions on a class instance PR-URL: https://github.com/nodejs/node/pull/45608 Reviewed-By: Colin Ihrig Reviewed-By: Yagiz Nizipli Reviewed-By: Benjamin Gruenbaum Reviewed-By: Antoine du Hamel (cherry picked from commit 929aada39d0f418193ca03cc360ced8c5b4ce553) --- lib/internal/per_context/primordials.js | 1 + lib/internal/test_runner/mock.js | 32 ++++- test/parallel/test-runner-mocking.js | 152 +++++++++++++++++++++++- 3 files changed, 178 insertions(+), 7 deletions(-) diff --git a/lib/internal/per_context/primordials.js b/lib/internal/per_context/primordials.js index 951bf28..2f12a34 100644 --- a/lib/internal/per_context/primordials.js +++ b/lib/internal/per_context/primordials.js @@ -40,6 +40,7 @@ exports.ObjectDefineProperty = (obj, key, descr) => Object.defineProperty(obj, k exports.ObjectEntries = obj => Object.entries(obj) exports.ObjectFreeze = obj => Object.freeze(obj) exports.ObjectGetOwnPropertyDescriptor = (obj, key) => Object.getOwnPropertyDescriptor(obj, key) +exports.ObjectGetPrototypeOf = obj => Object.getPrototypeOf(obj) exports.ObjectIsExtensible = obj => Object.isExtensible(obj) exports.ObjectPrototypeHasOwnProperty = (obj, property) => Object.prototype.hasOwnProperty.call(obj, property) exports.ObjectSeal = (obj) => Object.seal(obj) diff --git a/lib/internal/test_runner/mock.js b/lib/internal/test_runner/mock.js index 9959825..af46e47 100644 --- a/lib/internal/test_runner/mock.js +++ b/lib/internal/test_runner/mock.js @@ -1,4 +1,4 @@ -// https://github.com/nodejs/node/blob/afed1afa55962211b6b56c2068d520b4d8a08888/lib/internal/test_runner/mock.js +// https://github.com/nodejs/node/blob/929aada39d0f418193ca03cc360ced8c5b4ce553/lib/internal/test_runner/mock.js 'use strict' const { ArrayPrototypePush, @@ -7,6 +7,7 @@ const { FunctionPrototypeCall, ObjectDefineProperty, ObjectGetOwnPropertyDescriptor, + ObjectGetPrototypeOf, Proxy, ReflectApply, ReflectConstruct, @@ -127,13 +128,15 @@ class MockTracker { } method ( - object, + objectOrFunction, methodName, implementation = kDefaultFunction, options = kEmptyObject ) { - validateObject(object, 'object') validateStringOrSymbol(methodName, 'methodName') + if (typeof objectOrFunction !== 'function') { + validateObject(objectOrFunction, 'object') + } if (implementation !== null && typeof implementation === 'object') { options = implementation @@ -158,8 +161,8 @@ class MockTracker { 'options.setter', setter, "cannot be used with 'options.getter'" ) } + const descriptor = findMethodOnPrototypeChain(objectOrFunction, methodName) - const descriptor = ObjectGetOwnPropertyDescriptor(object, methodName) let original if (getter) { @@ -176,7 +179,7 @@ class MockTracker { ) } - const restore = { descriptor, object, methodName } + const restore = { descriptor, object: objectOrFunction, methodName } const impl = implementation === kDefaultFunction ? original : implementation @@ -199,7 +202,7 @@ class MockTracker { mockDescriptor.value = mock } - ObjectDefineProperty(object, methodName, mockDescriptor) + ObjectDefineProperty(objectOrFunction, methodName, mockDescriptor) return mock } @@ -348,4 +351,21 @@ function validateTimes (value, name) { validateInteger(value, name, 1) } +function findMethodOnPrototypeChain (instance, methodName) { + let host = instance + let descriptor + + while (host !== null) { + descriptor = ObjectGetOwnPropertyDescriptor(host, methodName) + + if (descriptor) { + break + } + + host = ObjectGetPrototypeOf(host) + } + + return descriptor +} + module.exports = { MockTracker } diff --git a/test/parallel/test-runner-mocking.js b/test/parallel/test-runner-mocking.js index 6bcff47..f956c52 100644 --- a/test/parallel/test-runner-mocking.js +++ b/test/parallel/test-runner-mocking.js @@ -1,4 +1,4 @@ -// https://github.com/nodejs/node/blob/afed1afa55962211b6b56c2068d520b4d8a08888/test/parallel/test-runner-mocking.js +// https://github.com/nodejs/node/blob/929aada39d0f418193ca03cc360ced8c5b4ce553/test/parallel/test-runner-mocking.js 'use strict' const common = require('../common') const assert = require('node:assert') @@ -320,6 +320,156 @@ test('spy functions can be bound', (t) => { assert.strictEqual(sum.bind(0)(2, 11), 13) }) +test('mocks prototype methods on an instance', async (t) => { + class Runner { + async someTask (msg) { + return Promise.resolve(msg) + } + + async method (msg) { + await this.someTask(msg) + return msg + } + } + const msg = 'ok' + const obj = new Runner() + assert.strictEqual(await obj.method(msg), msg) + + t.mock.method(obj, obj.someTask.name) + assert.strictEqual(obj.someTask.mock.calls.length, 0) + + assert.strictEqual(await obj.method(msg), msg) + + const call = obj.someTask.mock.calls[0] + + assert.deepStrictEqual(call.arguments, [msg]) + assert.strictEqual(await call.result, msg) + assert.strictEqual(call.target, undefined) + assert.strictEqual(call.this, obj) + + const obj2 = new Runner() + // Ensure that a brand new instance is not mocked + assert.strictEqual( + obj2.someTask.mock, + undefined + ) + + assert.strictEqual(obj.someTask.mock.restore(), undefined) + assert.strictEqual(await obj.method(msg), msg) + assert.strictEqual(obj.someTask.mock, undefined) + assert.strictEqual(Runner.prototype.someTask.mock, undefined) +}) + +test('spies on async static class methods', async (t) => { + class Runner { + static async someTask (msg) { + return Promise.resolve(msg) + } + + static async method (msg) { + await this.someTask(msg) + return msg + } + } + const msg = 'ok' + assert.strictEqual(await Runner.method(msg), msg) + + t.mock.method(Runner, Runner.someTask.name) + assert.strictEqual(Runner.someTask.mock.calls.length, 0) + + assert.strictEqual(await Runner.method(msg), msg) + + const call = Runner.someTask.mock.calls[0] + + assert.deepStrictEqual(call.arguments, [msg]) + assert.strictEqual(await call.result, msg) + assert.strictEqual(call.target, undefined) + assert.strictEqual(call.this, Runner) + + assert.strictEqual(Runner.someTask.mock.restore(), undefined) + assert.strictEqual(await Runner.method(msg), msg) + assert.strictEqual(Runner.someTask.mock, undefined) + assert.strictEqual(Runner.prototype.someTask, undefined) +}) + +test('given null to a mock.method it throws a invalid argument error', (t) => { + assert.throws(() => t.mock.method(null, {}), { code: 'ERR_INVALID_ARG_TYPE' }) +}) + +test('it should throw given an inexistent property on a object instance', (t) => { + assert.throws(() => t.mock.method({ abc: 0 }, 'non-existent'), { + code: 'ERR_INVALID_ARG_VALUE' + }) +}) + +test('spy functions can be used on classes inheritance', (t) => { + // Makes sure that having a null-prototype doesn't throw our system off + class A extends null { + static someTask (msg) { + return msg + } + + static method (msg) { + return this.someTask(msg) + } + } + class B extends A {} + class C extends B {} + + const msg = 'ok' + assert.strictEqual(C.method(msg), msg) + + t.mock.method(C, C.someTask.name) + assert.strictEqual(C.someTask.mock.calls.length, 0) + + assert.strictEqual(C.method(msg), msg) + + const call = C.someTask.mock.calls[0] + + assert.deepStrictEqual(call.arguments, [msg]) + assert.strictEqual(call.result, msg) + assert.strictEqual(call.target, undefined) + assert.strictEqual(call.this, C) + + assert.strictEqual(C.someTask.mock.restore(), undefined) + assert.strictEqual(C.method(msg), msg) + assert.strictEqual(C.someTask.mock, undefined) +}) + +test('spy functions don\'t affect the prototype chain', (t) => { + class A { + static someTask (msg) { + return msg + } + } + class B extends A {} + class C extends B {} + + const msg = 'ok' + + const ABeforeMockIsUnchanged = Object.getOwnPropertyDescriptor(A, A.someTask.name) + const BBeforeMockIsUnchanged = Object.getOwnPropertyDescriptor(B, B.someTask.name) + const CBeforeMockShouldNotHaveDesc = Object.getOwnPropertyDescriptor(C, C.someTask.name) + + t.mock.method(C, C.someTask.name) + C.someTask(msg) + const BAfterMockIsUnchanged = Object.getOwnPropertyDescriptor(B, B.someTask.name) + + const AAfterMockIsUnchanged = Object.getOwnPropertyDescriptor(A, A.someTask.name) + const CAfterMockHasDescriptor = Object.getOwnPropertyDescriptor(C, C.someTask.name) + + assert.strictEqual(CBeforeMockShouldNotHaveDesc, undefined) + assert.ok(CAfterMockHasDescriptor) + + assert.deepStrictEqual(ABeforeMockIsUnchanged, AAfterMockIsUnchanged) + assert.strictEqual(BBeforeMockIsUnchanged, BAfterMockIsUnchanged) + assert.strictEqual(BBeforeMockIsUnchanged, undefined) + + assert.strictEqual(C.someTask.mock.restore(), undefined) + const CAfterRestoreKeepsDescriptor = Object.getOwnPropertyDescriptor(C, C.someTask.name) + assert.ok(CAfterRestoreKeepsDescriptor) +}) + test('mocked functions report thrown errors', (t) => { const testError = new Error('test error') const fn = t.mock.fn(() => {