From 05ca03569e22f82d5a20428119cfc6b17fcaf36d Mon Sep 17 00:00:00 2001 From: cjihrig Date: Sun, 14 Jul 2024 20:33:13 -0400 Subject: [PATCH] test_runner: refactor snapshots to get file from context This commit refactors the internals of snapshot tests to get the name of the test file from the test context instead of passing it to the SnapshotManager constructor. This is prep work for supporting running test files in the test runner process. PR-URL: https://github.com/nodejs/node/pull/53853 Reviewed-By: Moshe Atlow Reviewed-By: Chemi Atlow Reviewed-By: Benjamin Gruenbaum --- lib/internal/test_runner/snapshot.js | 135 +++++++------- lib/internal/test_runner/test.js | 4 +- .../test-runner/snapshots/imported-tests.js | 8 + test/fixtures/test-runner/snapshots/unit.js | 2 + test/parallel/test-runner-snapshot-tests.js | 165 +++++++++--------- 5 files changed, 171 insertions(+), 143 deletions(-) create mode 100644 test/fixtures/test-runner/snapshots/imported-tests.js diff --git a/lib/internal/test_runner/snapshot.js b/lib/internal/test_runner/snapshot.js index a2ab85239d7299..7e41a0bf76f0cd 100644 --- a/lib/internal/test_runner/snapshot.js +++ b/lib/internal/test_runner/snapshot.js @@ -58,50 +58,12 @@ function setDefaultSnapshotSerializers(serializers) { serializerFns = ArrayPrototypeSlice(serializers); } -class SnapshotManager { - constructor(entryFile, updateSnapshots) { - this.entryFile = entryFile; - this.snapshotFile = undefined; +class SnapshotFile { + constructor(snapshotFile) { + this.snapshotFile = snapshotFile; this.snapshots = { __proto__: null }; this.nameCounts = new SafeMap(); - // A manager instance will only read or write snapshot files based on the - // updateSnapshots argument. - this.loaded = updateSnapshots; - this.updateSnapshots = updateSnapshots; - } - - resolveSnapshotFile() { - if (this.snapshotFile === undefined) { - const resolved = resolveSnapshotPathFn(this.entryFile); - - if (typeof resolved !== 'string') { - const err = new ERR_INVALID_STATE('Invalid snapshot filename.'); - err.filename = resolved; - throw err; - } - - this.snapshotFile = resolved; - } - } - - serialize(input, serializers = serializerFns) { - try { - let value = input; - - for (let i = 0; i < serializers.length; ++i) { - const fn = serializers[i]; - value = fn(value); - } - - return `\n${templateEscape(value)}\n`; - } catch (err) { - const error = new ERR_INVALID_STATE( - 'The provided serializers did not generate a string.', - ); - error.input = input; - error.cause = err; - throw error; - } + this.loaded = false; } getSnapshot(id) { @@ -122,12 +84,11 @@ class SnapshotManager { nextId(name) { const count = this.nameCounts.get(name) ?? 1; - this.nameCounts.set(name, count + 1); return `${name} ${count}`; } - readSnapshotFile() { + readFile() { if (this.loaded) { debug('skipping read of snapshot file'); return; @@ -164,12 +125,7 @@ class SnapshotManager { } } - writeSnapshotFile() { - if (!this.updateSnapshots) { - debug('skipping write of snapshot file'); - return; - } - + writeFile() { try { const keys = ArrayPrototypeSort(ObjectKeys(this.snapshots)); const snapshotStrings = ArrayPrototypeMap(keys, (key) => { @@ -186,34 +142,87 @@ class SnapshotManager { throw error; } } +} + +class SnapshotManager { + constructor(updateSnapshots) { + // A manager instance will only read or write snapshot files based on the + // updateSnapshots argument. + this.updateSnapshots = updateSnapshots; + this.cache = new SafeMap(); + } + + resolveSnapshotFile(entryFile) { + let snapshotFile = this.cache.get(entryFile); + + if (snapshotFile === undefined) { + const resolved = resolveSnapshotPathFn(entryFile); + + if (typeof resolved !== 'string') { + const err = new ERR_INVALID_STATE('Invalid snapshot filename.'); + err.filename = resolved; + throw err; + } + + snapshotFile = new SnapshotFile(resolved); + snapshotFile.loaded = this.updateSnapshots; + this.cache.set(entryFile, snapshotFile); + } + + return snapshotFile; + } + + serialize(input, serializers = serializerFns) { + try { + let value = input; + + for (let i = 0; i < serializers.length; ++i) { + const fn = serializers[i]; + value = fn(value); + } + + return `\n${templateEscape(value)}\n`; + } catch (err) { + const error = new ERR_INVALID_STATE( + 'The provided serializers did not generate a string.', + ); + error.input = input; + error.cause = err; + throw error; + } + } + + writeSnapshotFiles() { + if (!this.updateSnapshots) { + debug('skipping write of snapshot files'); + return; + } + + this.cache.forEach((snapshotFile) => { + snapshotFile.writeFile(); + }); + } createAssert() { const manager = this; return function snapshotAssertion(actual, options = kEmptyObject) { emitExperimentalWarning(kExperimentalWarning); - // Resolve the snapshot file here so that any resolution errors are - // surfaced as early as possible. - manager.resolveSnapshotFile(); - - const { fullName } = this; - const id = manager.nextId(fullName); - validateObject(options, 'options'); - const { serializers = serializerFns, } = options; - validateFunctionArray(serializers, 'options.serializers'); - + const { filePath, fullName } = this; + const snapshotFile = manager.resolveSnapshotFile(filePath); const value = manager.serialize(actual, serializers); + const id = snapshotFile.nextId(fullName); if (manager.updateSnapshots) { - manager.setSnapshot(id, value); + snapshotFile.setSnapshot(id, value); } else { - manager.readSnapshotFile(); - strictEqual(value, manager.getSnapshot(id)); + snapshotFile.readFile(); + strictEqual(value, snapshotFile.getSnapshot(id)); } }; } diff --git a/lib/internal/test_runner/test.js b/lib/internal/test_runner/test.js index 9fbcb0c84afb7f..46490460caf9e9 100644 --- a/lib/internal/test_runner/test.js +++ b/lib/internal/test_runner/test.js @@ -133,7 +133,7 @@ function lazyAssertObject(harness) { const { getOptionValue } = require('internal/options'); if (getOptionValue('--experimental-test-snapshots')) { const { SnapshotManager } = require('internal/test_runner/snapshot'); - harness.snapshotManager = new SnapshotManager(kFilename, updateSnapshots); + harness.snapshotManager = new SnapshotManager(updateSnapshots); assertObj.set('snapshot', harness.snapshotManager.createAssert()); } } @@ -977,7 +977,7 @@ class Test extends AsyncResource { // Call this harness.coverage() before collecting diagnostics, since failure to collect coverage is a diagnostic. const coverage = harness.coverage(); - harness.snapshotManager?.writeSnapshotFile(); + harness.snapshotManager?.writeSnapshotFiles(); for (let i = 0; i < diagnostics.length; i++) { reporter.diagnostic(nesting, loc, diagnostics[i]); } diff --git a/test/fixtures/test-runner/snapshots/imported-tests.js b/test/fixtures/test-runner/snapshots/imported-tests.js new file mode 100644 index 00000000000000..85833bc86ec7c3 --- /dev/null +++ b/test/fixtures/test-runner/snapshots/imported-tests.js @@ -0,0 +1,8 @@ +'use strict'; +const { suite, test } = require('node:test'); + +suite('imported suite', () => { + test('imported test', (t) => { + t.assert.snapshot({ foo: 1, bar: 2 }); + }); +}); diff --git a/test/fixtures/test-runner/snapshots/unit.js b/test/fixtures/test-runner/snapshots/unit.js index 889b47f379a805..0ec7018c261c13 100644 --- a/test/fixtures/test-runner/snapshots/unit.js +++ b/test/fixtures/test-runner/snapshots/unit.js @@ -26,3 +26,5 @@ test('`${foo}`', async (t) => { test('escapes in `\\${foo}`\n', async (t) => { t.assert.snapshot('`\\${foo}`\n'); }); + +require('./imported-tests'); diff --git a/test/parallel/test-runner-snapshot-tests.js b/test/parallel/test-runner-snapshot-tests.js index d3abf356821d56..e00019ef49d4f6 100644 --- a/test/parallel/test-runner-snapshot-tests.js +++ b/test/parallel/test-runner-snapshot-tests.js @@ -20,37 +20,38 @@ tmpdir.refresh(); suite('SnapshotManager', () => { test('uses default snapshot naming scheme', (t) => { - const manager = new SnapshotManager(__filename, false); - manager.resolveSnapshotFile(); - t.assert.strictEqual(manager.snapshotFile, `${__filename}.snapshot`); + const manager = new SnapshotManager(false); + const file = manager.resolveSnapshotFile(__filename); + t.assert.strictEqual(file.snapshotFile, `${__filename}.snapshot`); }); test('generates snapshot IDs based on provided name', (t) => { - const manager = new SnapshotManager(__filename, false); - - t.assert.strictEqual(manager.nextId('foo'), 'foo 1'); - t.assert.strictEqual(manager.nextId('foo'), 'foo 2'); - t.assert.strictEqual(manager.nextId('bar'), 'bar 1'); - t.assert.strictEqual(manager.nextId('baz'), 'baz 1'); - t.assert.strictEqual(manager.nextId('foo'), 'foo 3'); - t.assert.strictEqual(manager.nextId('foo`'), 'foo` 1'); - t.assert.strictEqual(manager.nextId('foo\\'), 'foo\\ 1'); - t.assert.strictEqual(manager.nextId('foo`${x}`'), 'foo`${x}` 1'); + const manager = new SnapshotManager(false); + const file = manager.resolveSnapshotFile(__filename); + + t.assert.strictEqual(file.nextId('foo'), 'foo 1'); + t.assert.strictEqual(file.nextId('foo'), 'foo 2'); + t.assert.strictEqual(file.nextId('bar'), 'bar 1'); + t.assert.strictEqual(file.nextId('baz'), 'baz 1'); + t.assert.strictEqual(file.nextId('foo'), 'foo 3'); + t.assert.strictEqual(file.nextId('foo`'), 'foo` 1'); + t.assert.strictEqual(file.nextId('foo\\'), 'foo\\ 1'); + t.assert.strictEqual(file.nextId('foo`${x}`'), 'foo`${x}` 1'); }); test('throws if snapshot file does not have exports', (t) => { const fixture = fixtures.path( 'test-runner', 'snapshots', 'malformed-exports.js' ); - const manager = new SnapshotManager(fixture, false); + const manager = new SnapshotManager(false); + const file = manager.resolveSnapshotFile(fixture); t.assert.throws(() => { - manager.resolveSnapshotFile(); - manager.readSnapshotFile(); + file.readFile(); }, (err) => { t.assert.strictEqual(err.code, 'ERR_INVALID_STATE'); t.assert.match(err.message, /Cannot read snapshot/); - t.assert.strictEqual(err.filename, manager.snapshotFile); + t.assert.strictEqual(err.filename, file.snapshotFile); t.assert.match(err.cause.message, /Malformed snapshot file/); return true; }); @@ -60,16 +61,16 @@ suite('SnapshotManager', () => { const fixture = fixtures.path( 'test-runner', 'snapshots', 'this-file-should-not-exist.js' ); - const manager = new SnapshotManager(fixture, false); + const manager = new SnapshotManager(false); + const file = manager.resolveSnapshotFile(fixture); t.assert.throws(() => { - manager.resolveSnapshotFile(); - manager.readSnapshotFile(); + file.readFile(); }, /Missing snapshots can be generated by rerunning the command/); }); test('throws if serialization cannot generate a string', (t) => { - const manager = new SnapshotManager(__filename, false); + const manager = new SnapshotManager(false); const cause = new Error('boom'); const input = { foo: 1, @@ -90,7 +91,7 @@ suite('SnapshotManager', () => { }); test('serializes values using provided functions', (t) => { - const manager = new SnapshotManager(__filename, false); + const manager = new SnapshotManager(false); const output = manager.serialize({ foo: 1 }, [ (value) => { return JSON.stringify(value); }, (value) => { return value + '424242'; }, @@ -100,14 +101,14 @@ suite('SnapshotManager', () => { }); test('serialized values get cast to string', (t) => { - const manager = new SnapshotManager(__filename, false); + const manager = new SnapshotManager(false); const output = manager.serialize(5, []); t.assert.strictEqual(output, '\n5\n'); }); test('serialized values get escaped', (t) => { - const manager = new SnapshotManager(__filename, false); + const manager = new SnapshotManager(false); const output = manager.serialize('fo\\o`${x}`', []); t.assert.strictEqual(output, '\nfo\\\\o\\`\\${x}\\`\n'); @@ -115,53 +116,56 @@ suite('SnapshotManager', () => { test('reads individual snapshots from snapshot file', (t) => { const fixture = fixtures.path('test-runner', 'snapshots', 'simple.js'); - const manager = new SnapshotManager(fixture, false); - manager.resolveSnapshotFile(); - manager.readSnapshotFile(); - const snapshot = manager.getSnapshot('foo 1'); + const manager = new SnapshotManager(false); + const file = manager.resolveSnapshotFile(fixture); + file.readFile(); + const snapshot = file.getSnapshot('foo 1'); t.assert.strictEqual(snapshot, '\n{\n "bar": 1,\n "baz": 2\n}\n'); }); test('snapshot file is not read in update mode', (t) => { const fixture = fixtures.path('test-runner', 'snapshots', 'simple.js'); - const manager = new SnapshotManager(fixture, true); - manager.readSnapshotFile(); + const manager = new SnapshotManager(true); + const file = manager.resolveSnapshotFile(fixture); + file.readFile(); t.assert.throws(() => { - manager.getSnapshot('foo 1'); + file.getSnapshot('foo 1'); }, /Snapshot 'foo 1' not found/); }); test('throws if requested snapshot does not exist in file', (t) => { const fixture = fixtures.path('test-runner', 'snapshots', 'simple.js'); - const manager = new SnapshotManager(fixture, false); + const manager = new SnapshotManager(false); + const file = manager.resolveSnapshotFile(fixture); t.assert.throws(() => { - manager.getSnapshot('does not exist 1'); + file.getSnapshot('does not exist 1'); }, (err) => { t.assert.strictEqual(err.code, 'ERR_INVALID_STATE'); t.assert.match(err.message, /Snapshot 'does not exist 1' not found/); t.assert.strictEqual(err.snapshot, 'does not exist 1'); - t.assert.strictEqual(err.filename, manager.snapshotFile); + t.assert.strictEqual(err.filename, file.snapshotFile); return true; }); }); test('snapshot IDs are escaped when stored', (t) => { const fixture = fixtures.path('test-runner', 'snapshots', 'simple.js'); - const manager = new SnapshotManager(fixture, false); + const manager = new SnapshotManager(false); + const file = manager.resolveSnapshotFile(fixture); - manager.setSnapshot('foo`${x}` 1', 'test'); - t.assert.strictEqual(manager.getSnapshot('foo\\`\\${x}\\` 1'), 'test'); + file.setSnapshot('foo`${x}` 1', 'test'); + t.assert.strictEqual(file.getSnapshot('foo\\`\\${x}\\` 1'), 'test'); }); test('throws if snapshot file cannot be resolved', (t) => { - const manager = new SnapshotManager(null, false); + const manager = new SnapshotManager(false); const assertion = manager.createAssert(); t.assert.throws(() => { - assertion('foo'); + Reflect.apply(assertion, { filePath: null }, ['foo']); }, (err) => { t.assert.strictEqual(err.code, 'ERR_INVALID_STATE'); t.assert.match(err.message, /Invalid snapshot filename/); @@ -170,54 +174,59 @@ suite('SnapshotManager', () => { }); }); - test('writes the specified snapshot file', (t) => { - const testFile = tmpdir.resolve('test1.js'); - const manager = new SnapshotManager(testFile, true); - manager.resolveSnapshotFile(); - manager.setSnapshot('foo 1', 'foo value'); - t.assert.strictEqual(fs.existsSync(manager.snapshotFile), false); - manager.writeSnapshotFile(); - t.assert.strictEqual(fs.existsSync(manager.snapshotFile), true); + test('writes the specified snapshot files', (t) => { + const testFile1 = tmpdir.resolve('test1.js'); + const testFile2 = tmpdir.resolve('test2.js'); + const manager = new SnapshotManager(true); + const file1 = manager.resolveSnapshotFile(testFile1); + const file2 = manager.resolveSnapshotFile(testFile2); + file1.setSnapshot('foo 1', 'foo 1 value'); + file2.setSnapshot('foo 2', 'foo 2 value'); + t.assert.strictEqual(fs.existsSync(file1.snapshotFile), false); + t.assert.strictEqual(fs.existsSync(file2.snapshotFile), false); + manager.writeSnapshotFiles(); + t.assert.strictEqual(fs.existsSync(file1.snapshotFile), true); + t.assert.strictEqual(fs.existsSync(file2.snapshotFile), true); }); test('creates snapshot directory if it does not exist', (t) => { const testFile = tmpdir.resolve('foo/bar/baz/test2.js'); - const manager = new SnapshotManager(testFile, true); - manager.resolveSnapshotFile(); - manager.setSnapshot('foo 1', 'foo value'); - t.assert.strictEqual(fs.existsSync(manager.snapshotFile), false); - manager.writeSnapshotFile(); - t.assert.strictEqual(fs.existsSync(manager.snapshotFile), true); + const manager = new SnapshotManager(true); + const file = manager.resolveSnapshotFile(testFile); + file.setSnapshot('foo 1', 'foo value'); + t.assert.strictEqual(fs.existsSync(file.snapshotFile), false); + manager.writeSnapshotFiles(); + t.assert.strictEqual(fs.existsSync(file.snapshotFile), true); }); - test('does not write snapshot file in read mode', (t) => { + test('does not write snapshot files in read mode', (t) => { const testFile = tmpdir.resolve('test3.js'); - const manager = new SnapshotManager(testFile, false); - manager.resolveSnapshotFile(); - manager.setSnapshot('foo 1', 'foo value'); - t.assert.strictEqual(fs.existsSync(manager.snapshotFile), false); - manager.writeSnapshotFile(); - t.assert.strictEqual(fs.existsSync(manager.snapshotFile), false); + const manager = new SnapshotManager(false); + const file = manager.resolveSnapshotFile(testFile); + file.setSnapshot('foo 1', 'foo value'); + t.assert.strictEqual(fs.existsSync(file.snapshotFile), false); + manager.writeSnapshotFiles(); + t.assert.strictEqual(fs.existsSync(file.snapshotFile), false); }); - test('throws if snapshot file cannot be written', (t) => { + test('throws if snapshot files cannot be written', (t) => { const testFile = tmpdir.resolve('test4.js'); const error = new Error('boom'); - const manager = new SnapshotManager(testFile, true); - manager.resolveSnapshotFile(); - manager.snapshots['foo 1'] = { toString() { throw error; } }; - t.assert.strictEqual(fs.existsSync(manager.snapshotFile), false); + const manager = new SnapshotManager(true); + const file = manager.resolveSnapshotFile(testFile); + file.snapshots['foo 1'] = { toString() { throw error; } }; + t.assert.strictEqual(fs.existsSync(file.snapshotFile), false); t.assert.throws(() => { - manager.writeSnapshotFile(); + manager.writeSnapshotFiles(); }, (err) => { t.assert.strictEqual(err.code, 'ERR_INVALID_STATE'); t.assert.match(err.message, /Cannot write snapshot file/); - t.assert.strictEqual(err.filename, manager.snapshotFile); + t.assert.strictEqual(err.filename, file.snapshotFile); t.assert.strictEqual(err.cause, error); return true; }); - t.assert.strictEqual(fs.existsSync(manager.snapshotFile), false); + t.assert.strictEqual(fs.existsSync(file.snapshotFile), false); }); }); @@ -254,9 +263,9 @@ suite('setResolveSnapshotPath()', () => { }); snapshot.setResolveSnapshotPath(() => { return 'foobarbaz'; }); - const manager = new SnapshotManager(__filename, false); - manager.resolveSnapshotFile(); - t.assert.strictEqual(manager.snapshotFile, 'foobarbaz'); + const manager = new SnapshotManager(false); + const file = manager.resolveSnapshotFile(__filename); + t.assert.strictEqual(file.snapshotFile, 'foobarbaz'); }); }); @@ -276,7 +285,7 @@ suite('setDefaultSnapshotSerializers()', () => { }); snapshot.setDefaultSnapshotSerializers([() => { return 'foobarbaz'; }]); - const manager = new SnapshotManager(__filename, false); + const manager = new SnapshotManager(false); const output = manager.serialize({ foo: 1 }); t.assert.strictEqual(output, '\nfoobarbaz\n'); }); @@ -296,9 +305,9 @@ test('t.assert.snapshot()', async (t) => { t.assert.strictEqual(child.code, 1); t.assert.strictEqual(child.signal, null); - t.assert.match(child.stdout, /# tests 4/); + t.assert.match(child.stdout, /# tests 5/); t.assert.match(child.stdout, /# pass 0/); - t.assert.match(child.stdout, /# fail 4/); + t.assert.match(child.stdout, /# fail 5/); t.assert.match(child.stdout, /Missing snapshots/); }); @@ -311,8 +320,8 @@ test('t.assert.snapshot()', async (t) => { t.assert.strictEqual(child.code, 0); t.assert.strictEqual(child.signal, null); - t.assert.match(child.stdout, /tests 4/); - t.assert.match(child.stdout, /pass 4/); + t.assert.match(child.stdout, /tests 5/); + t.assert.match(child.stdout, /pass 5/); t.assert.match(child.stdout, /fail 0/); }); @@ -325,8 +334,8 @@ test('t.assert.snapshot()', async (t) => { t.assert.strictEqual(child.code, 0); t.assert.strictEqual(child.signal, null); - t.assert.match(child.stdout, /tests 4/); - t.assert.match(child.stdout, /pass 4/); + t.assert.match(child.stdout, /tests 5/); + t.assert.match(child.stdout, /pass 5/); t.assert.match(child.stdout, /fail 0/); }); });