From c9216765127c85c94f4417e80741a5b3e4f8c68a Mon Sep 17 00:00:00 2001 From: Colin Ihrig Date: Tue, 19 Nov 2024 12:37:35 -0500 Subject: [PATCH] test_runner: mark snapshot testing as stable This commit marks the test runner's snapshot testing API as stable. PR-URL: https://github.com/nodejs/node/pull/55897 Reviewed-By: Moshe Atlow Reviewed-By: Pietro Marchini Reviewed-By: Matteo Collina Reviewed-By: Jacob Smith Reviewed-By: Chemi Atlow --- doc/api/cli.md | 18 ++------- doc/api/test.md | 4 +- doc/node.1 | 3 -- lib/internal/test_runner/test.js | 9 ++--- lib/test.js | 43 ++++++++++----------- src/node_options.cc | 4 +- src/node_options.h | 1 - test/parallel/test-runner-assert.js | 15 +++---- test/parallel/test-runner-snapshot-tests.js | 11 ++---- 9 files changed, 41 insertions(+), 67 deletions(-) diff --git a/doc/api/cli.md b/doc/api/cli.md index d232c342302..609465c253b 100644 --- a/doc/api/cli.md +++ b/doc/api/cli.md @@ -1121,16 +1121,6 @@ added: Enable module mocking in the test runner. -### `--experimental-test-snapshots` - - - -> Stability: 1.0 - Early development - -Enable [snapshot testing][] in the test runner. - ### `--experimental-vm-modules` -> Stability: 1.0 - Early development - Regenerates the snapshot files used by the test runner for [snapshot testing][]. -Node.js must be started with the `--experimental-test-snapshots` flag in order -to use this functionality. ### `--throw-deprecation` diff --git a/doc/api/test.md b/doc/api/test.md index b55eb74e85f..581a350d36e 100644 --- a/doc/api/test.md +++ b/doc/api/test.md @@ -937,8 +937,7 @@ compared against a set of known good values. The known good values are known as snapshots, and are stored in a snapshot file. Snapshot files are managed by the test runner, but are designed to be human readable to aid in debugging. Best practice is for snapshot files to be checked into source control along with your -test files. In order to enable snapshot testing, Node.js must be started with -the [`--experimental-test-snapshots`][] command-line flag. +test files. Snapshot files are generated by starting Node.js with the [`--test-update-snapshots`][] command-line flag. A separate snapshot file is @@ -3593,7 +3592,6 @@ Can be used to abort test subtasks when the test has been aborted. [`--experimental-strip-types`]: cli.md#--experimental-strip-types [`--experimental-test-coverage`]: cli.md#--experimental-test-coverage [`--experimental-test-module-mocks`]: cli.md#--experimental-test-module-mocks -[`--experimental-test-snapshots`]: cli.md#--experimental-test-snapshots [`--import`]: cli.md#--importmodule [`--test-concurrency`]: cli.md#--test-concurrency [`--test-coverage-include`]: cli.md#--test-coverage-include diff --git a/doc/node.1 b/doc/node.1 index 02628f8e238..ac55410f500 100644 --- a/doc/node.1 +++ b/doc/node.1 @@ -191,9 +191,6 @@ Configures the type of test isolation used in the test runner. .It Fl -experimental-test-module-mocks Enable module mocking in the test runner. . -.It Fl -experimental-test-snapshots -Enable snapshot testing in the test runner. -. .It Fl -experimental-strip-types Enable experimental type-stripping for TypeScript files. . diff --git a/lib/internal/test_runner/test.js b/lib/internal/test_runner/test.js index b679410c556..abb8ed276df 100644 --- a/lib/internal/test_runner/test.js +++ b/lib/internal/test_runner/test.js @@ -104,6 +104,7 @@ function lazyAssertObject(harness) { if (assertObj === undefined) { assertObj = new SafeMap(); const assert = require('assert'); + const { SnapshotManager } = require('internal/test_runner/snapshot'); const methodsToCopy = [ 'deepEqual', 'deepStrictEqual', @@ -126,12 +127,8 @@ function lazyAssertObject(harness) { assertObj.set(methodsToCopy[i], assert[methodsToCopy[i]]); } - const { getOptionValue } = require('internal/options'); - if (getOptionValue('--experimental-test-snapshots')) { - const { SnapshotManager } = require('internal/test_runner/snapshot'); - harness.snapshotManager = new SnapshotManager(harness.config.updateSnapshots); - assertObj.set('snapshot', harness.snapshotManager.createAssert()); - } + harness.snapshotManager = new SnapshotManager(harness.config.updateSnapshots); + assertObj.set('snapshot', harness.snapshotManager.createAssert()); } return assertObj; } diff --git a/lib/test.js b/lib/test.js index ba72d6e4ecb..8e7c6295a37 100644 --- a/lib/test.js +++ b/lib/test.js @@ -7,7 +7,6 @@ const { const { test, suite, before, after, beforeEach, afterEach } = require('internal/test_runner/harness'); const { run } = require('internal/test_runner/runner'); -const { getOptionValue } = require('internal/options'); module.exports = test; ObjectAssign(module.exports, { @@ -39,28 +38,26 @@ ObjectDefineProperty(module.exports, 'mock', { }, }); -if (getOptionValue('--experimental-test-snapshots')) { - let lazySnapshot; +let lazySnapshot; - ObjectDefineProperty(module.exports, 'snapshot', { - __proto__: null, - configurable: true, - enumerable: true, - get() { - if (lazySnapshot === undefined) { - const { - setDefaultSnapshotSerializers, - setResolveSnapshotPath, - } = require('internal/test_runner/snapshot'); +ObjectDefineProperty(module.exports, 'snapshot', { + __proto__: null, + configurable: true, + enumerable: true, + get() { + if (lazySnapshot === undefined) { + const { + setDefaultSnapshotSerializers, + setResolveSnapshotPath, + } = require('internal/test_runner/snapshot'); - lazySnapshot = { - __proto__: null, - setDefaultSnapshotSerializers, - setResolveSnapshotPath, - }; - } + lazySnapshot = { + __proto__: null, + setDefaultSnapshotSerializers, + setResolveSnapshotPath, + }; + } - return lazySnapshot; - }, - }); -} + return lazySnapshot; + }, +}); diff --git a/src/node_options.cc b/src/node_options.cc index 5ef5a7b6cf9..344d3e6ed2f 100644 --- a/src/node_options.cc +++ b/src/node_options.cc @@ -692,9 +692,7 @@ EnvironmentOptionsParser::EnvironmentOptionsParser() { AddOption("--experimental-test-module-mocks", "enable module mocking in the test runner", &EnvironmentOptions::test_runner_module_mocks); - AddOption("--experimental-test-snapshots", - "enable snapshot testing in the test runner", - &EnvironmentOptions::test_runner_snapshots); + AddOption("--experimental-test-snapshots", "", NoOp{}); AddOption("--test-name-pattern", "run tests whose name matches this regular expression", &EnvironmentOptions::test_name_pattern, diff --git a/src/node_options.h b/src/node_options.h index aa7a2ce3eba..5446174545a 100644 --- a/src/node_options.h +++ b/src/node_options.h @@ -189,7 +189,6 @@ class EnvironmentOptions : public Options { uint64_t test_coverage_functions = 0; uint64_t test_coverage_lines = 0; bool test_runner_module_mocks = false; - bool test_runner_snapshots = false; bool test_runner_update_snapshots = false; std::vector test_name_pattern; std::vector test_reporter; diff --git a/test/parallel/test-runner-assert.js b/test/parallel/test-runner-assert.js index 8052a0fd84a..73d3b107251 100644 --- a/test/parallel/test-runner-assert.js +++ b/test/parallel/test-runner-assert.js @@ -3,13 +3,14 @@ require('../common'); const assert = require('node:assert'); const test = require('node:test'); -const uncopiedKeys = [ - 'AssertionError', - 'CallTracker', - 'strict', -]; -test('only methods from node:assert are on t.assert', (t) => { - const expectedKeys = Object.keys(assert).filter((key) => !uncopiedKeys.includes(key)).sort(); +test('expected methods are on t.assert', (t) => { + const uncopiedKeys = [ + 'AssertionError', + 'CallTracker', + 'strict', + ]; + const assertKeys = Object.keys(assert).filter((key) => !uncopiedKeys.includes(key)); + const expectedKeys = ['snapshot'].concat(assertKeys).sort(); assert.deepStrictEqual(Object.keys(t.assert).sort(), expectedKeys); }); diff --git a/test/parallel/test-runner-snapshot-tests.js b/test/parallel/test-runner-snapshot-tests.js index bad3645f5fa..eea78ff9aa5 100644 --- a/test/parallel/test-runner-snapshot-tests.js +++ b/test/parallel/test-runner-snapshot-tests.js @@ -1,4 +1,4 @@ -// Flags: --expose-internals --experimental-test-snapshots +// Flags: --expose-internals /* eslint-disable no-template-curly-in-string */ 'use strict'; const common = require('../common'); @@ -299,7 +299,7 @@ test('t.assert.snapshot()', async (t) => { await t.test('fails prior to snapshot generation', async (t) => { const child = await common.spawnPromisified( process.execPath, - ['--experimental-test-snapshots', fixture], + [fixture], { cwd: tmpdir.path }, ); @@ -314,7 +314,7 @@ test('t.assert.snapshot()', async (t) => { await t.test('passes when regenerating snapshots', async (t) => { const child = await common.spawnPromisified( process.execPath, - ['--test-update-snapshots', '--experimental-test-snapshots', fixture], + ['--test-update-snapshots', fixture], { cwd: tmpdir.path }, ); @@ -328,7 +328,7 @@ test('t.assert.snapshot()', async (t) => { await t.test('passes when snapshots exist', async (t) => { const child = await common.spawnPromisified( process.execPath, - ['--experimental-test-snapshots', fixture], + [fixture], { cwd: tmpdir.path }, ); @@ -350,7 +350,6 @@ test('snapshots from multiple files (isolation=none)', async (t) => { const args = [ '--test', '--experimental-test-isolation=none', - '--experimental-test-snapshots', fixture, fixture2, ]; @@ -372,7 +371,6 @@ test('snapshots from multiple files (isolation=none)', async (t) => { const args = [ '--test', '--experimental-test-isolation=none', - '--experimental-test-snapshots', '--test-update-snapshots', fixture, fixture2, @@ -394,7 +392,6 @@ test('snapshots from multiple files (isolation=none)', async (t) => { const args = [ '--test', '--experimental-test-isolation=none', - '--experimental-test-snapshots', fixture, fixture2, ];