From 8349967691299c82816fe18e6faa46bea1b5cd30 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. --- 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 16a994c02a6392..191b4658bc8c8c 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; @@ -162,12 +123,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) => { @@ -184,34 +140,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 0718314239eede..f8ecd1a944a531 100644 --- a/test/fixtures/test-runner/snapshots/unit.js +++ b/test/fixtures/test-runner/snapshots/unit.js @@ -22,3 +22,5 @@ test('`${foo}`', async (t) => { const options = { serializers: [() => { return '***'; }]}; t.assert.snapshot('snapshotted string', options); }); + +require('./imported-tests'); diff --git a/test/parallel/test-runner-snapshot-tests.js b/test/parallel/test-runner-snapshot-tests.js index ae4130190d6ee1..6033abf8749863 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 3/); + t.assert.match(child.stdout, /# tests 4/); t.assert.match(child.stdout, /# pass 0/); - t.assert.match(child.stdout, /# fail 3/); + t.assert.match(child.stdout, /# fail 4/); 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 3/); - t.assert.match(child.stdout, /pass 3/); + t.assert.match(child.stdout, /tests 4/); + t.assert.match(child.stdout, /pass 4/); 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 3/); - t.assert.match(child.stdout, /pass 3/); + t.assert.match(child.stdout, /tests 4/); + t.assert.match(child.stdout, /pass 4/); t.assert.match(child.stdout, /fail 0/); }); });