test_runner: refactor snapshots to get file from context

cjihrig · targos · commit 6ad6e01bf357 · 2024-07-28T08:53:27.000+02:00
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: #53853 Reviewed-By: Moshe Atlow <moshe@atlow.co.il> Reviewed-By: Chemi Atlow <chemi@atlow.co.il> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
diff --git 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
@@ -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
@@ -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
@@ -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

Original file line number	Diff line number	Diff line change
`@@ -133,7 +133,7 @@ function lazyAssertObject(harness) {`
`133`	`133`	`const { getOptionValue } = require('internal/options');`
`134`	`134`	`if (getOptionValue('--experimental-test-snapshots')) {`
`135`	`135`	`const { SnapshotManager } = require('internal/test_runner/snapshot');`
`136`		`- harness.snapshotManager = new SnapshotManager(kFilename, updateSnapshots);`
	`136`	`+ harness.snapshotManager = new SnapshotManager(updateSnapshots);`
`137`	`137`	`assertObj.set('snapshot', harness.snapshotManager.createAssert());`
`138`	`138`	`}`
`139`	`139`	`}`
`@@ -977,7 +977,7 @@ class Test extends AsyncResource {`
`977`	`977`
`978`	`978`	`// Call this harness.coverage() before collecting diagnostics, since failure to collect coverage is a diagnostic.`
`979`	`979`	`const coverage = harness.coverage();`
`980`		`- harness.snapshotManager?.writeSnapshotFile();`
	`980`	`+ harness.snapshotManager?.writeSnapshotFiles();`
`981`	`981`	`for (let i = 0; i < diagnostics.length; i++) {`
`982`	`982`	`reporter.diagnostic(nesting, loc, diagnostics[i]);`
`983`	`983`	`}`