restore -changed for build tests (#315)

observablehq · Dec 5, 2023 · a2b22e8 · a2b22e8
1 parent 2a0fdb9
commit a2b22e8
Show file tree

Hide file tree

Showing 3 changed files with 38 additions and 68 deletions.
diff --git a/src/build.ts b/src/build.ts
@@ -53,7 +53,7 @@ export interface BuildEffects {
 
 export async function build(
   {sourceRoot: root, outputRoot, addPublic = true}: BuildOptions,
-  effects: BuildEffects = new DefaultEffects(outputRoot)
+  effects: BuildEffects = new FileBuildEffects(outputRoot!)
 ): Promise<void> {
   // Make sure all files are readable before starting to write output files.
   for await (const sourceFile of visitMarkdownFiles(root)) {
@@ -140,14 +140,17 @@ export async function build(
   }
 }
 
-class DefaultEffects implements BuildEffects {
+export class FileBuildEffects implements BuildEffects {
   private readonly outputRoot: string;
   readonly logger: Logger;
   readonly output: Writer;
-  constructor(outputRoot?: string) {
+  constructor(
+    outputRoot: string,
+    {logger = console, output = process.stdout}: {logger?: Logger; output?: Writer} = {}
+  ) {
     if (!outputRoot) throw new Error("missing outputRoot");
-    this.logger = console;
-    this.output = process.stdout;
+    this.logger = logger;
+    this.output = output;
     this.outputRoot = outputRoot;
   }
   async copyFile(sourcePath: string, outputPath: string): Promise<void> {

diff --git a/src/dataloader.ts b/src/dataloader.ts
@@ -137,7 +137,6 @@ export abstract class Loader {
    * to the source root; this is within the .observablehq/cache folder within
    * the source root.
    */
-
   async load(effects = defaultEffects): Promise<string> {
     const key = join(this.sourceRoot, this.targetPath);
     let command = runningCommands.get(key);

diff --git a/test/build-test.ts b/test/build-test.ts
@@ -1,12 +1,16 @@
 import assert from "node:assert";
 import {existsSync, readdirSync, statSync} from "node:fs";
-import {open, readFile} from "node:fs/promises";
+import {open, readFile, rm} from "node:fs/promises";
 import {join, normalize, relative} from "node:path";
 import {difference} from "d3-array";
-import type {BuildEffects} from "../src/build.js";
-import {build} from "../src/build.js";
+import {FileBuildEffects, build} from "../src/build.js";
 import {mockJsDelivr} from "./mocks/jsdelivr.js";
 
+const silentEffects = {
+  logger: {log() {}, warn() {}, error() {}},
+  output: {write() {}}
+};
+
 describe("build", async () => {
   mockJsDelivr();
 
@@ -20,48 +24,46 @@ describe("build", async () => {
     const skip = name.startsWith("skip.");
     const outname = only || skip ? name.slice(5) : name;
     (only ? it.only : skip ? it.skip : it)(`${inputRoot}/${name}`, async () => {
+      const actualDir = join(outputRoot, `${outname}-changed`);
       const expectedDir = join(outputRoot, outname);
+      const generate = !existsSync(expectedDir) && process.env.CI !== "true";
+      const outputDir = generate ? expectedDir : actualDir;
       const addPublic = name.endsWith("-public");
 
-      const generate = !existsSync(expectedDir) && process.env.CI !== "true";
-      if (generate) {
-        await generateSnapshots({sourceRoot: path, outputRoot: expectedDir, addPublic});
-        return;
+      await rm(actualDir, {recursive: true, force: true});
+      if (generate) console.warn(`! generating ${expectedDir}`);
+      await build({sourceRoot: path, addPublic}, new FileBuildEffects(outputDir, silentEffects));
+
+      // In the addPublic case, we don’t want to test the contents of the public
+      // files because they change often; replace them with empty files so we
+      // can at least check that the expected files exist.
+      if (addPublic) {
+        const publicDir = join(outputDir, "_observablehq");
+        for (const file of findFiles(publicDir)) {
+          await (await open(join(publicDir, file), "w")).close();
+        }
       }
-      const effects = new TestEffects(addPublic);
-      await build({sourceRoot: path, addPublic}, effects);
 
-      const actualFiles = effects.fileNames;
+      if (generate) return;
+
+      const actualFiles = new Set(findFiles(actualDir));
       const expectedFiles = new Set(findFiles(expectedDir));
       const missingFiles = difference(expectedFiles, actualFiles);
       const unexpectedFiles = difference(actualFiles, expectedFiles);
       if (missingFiles.size > 0) assert.fail(`Missing output files: ${Array.from(missingFiles).join(", ")}`);
       if (unexpectedFiles.size > 0) assert.fail(`Unexpected output files: ${Array.from(unexpectedFiles).join(", ")}`);
 
       for (const path of expectedFiles) {
-        const actual = effects.files[path];
-        const expected = await readFile(join(expectedDir, path));
-        assert.ok(actual.compare(expected) === 0, `${path} must match snapshot`);
+        const actual = await readFile(join(actualDir, path), "utf8");
+        const expected = await readFile(join(expectedDir, path), "utf8");
+        assert.ok(actual === expected, `${path} must match snapshot`);
       }
+
+      await rm(actualDir, {recursive: true, force: true});
     });
   }
 });
 
-async function generateSnapshots({sourceRoot, outputRoot, addPublic}) {
-  console.warn(`! generating ${outputRoot}`);
-  await build({sourceRoot, outputRoot, addPublic});
-
-  // In the addPublic case, we don’t want to test the contents of the public
-  // files because they change often; replace them with empty files so we
-  // can at least check that the expected files exist.
-  if (addPublic) {
-    const publicDir = join(outputRoot, "_observablehq");
-    for (const file of findFiles(publicDir)) {
-      await (await open(join(publicDir, file), "w")).close();
-    }
-  }
-}
-
 function* findFiles(root: string): Iterable<string> {
   const visited = new Set<number>();
   const queue: string[] = [(root = normalize(root))];
@@ -79,37 +81,3 @@ function* findFiles(root: string): Iterable<string> {
     }
   }
 }
-
-function isPublicPath(path: string): boolean {
-  return path.startsWith("_observablehq");
-}
-
-class TestEffects implements BuildEffects {
-  files: Record<string, Buffer> = {};
-  fileNames: Set<string> = new Set();
-  logger = {log() {}, warn() {}, error() {}};
-  output = {write() {}};
-
-  constructor(readonly addPublic: boolean) {}
-
-  _addFile(relativePath: string, contents: Buffer) {
-    if (isPublicPath(relativePath)) {
-      // Public files, if stored, are always blank in tests.
-      if (this.addPublic) {
-        contents = Buffer.from("");
-      } else {
-        return;
-      }
-    }
-    this.fileNames.add(relativePath);
-    this.files[relativePath] = contents;
-  }
-
-  async copyFile(sourcePath: string, relativeOutputPath: string): Promise<void> {
-    this._addFile(relativeOutputPath, await readFile(sourcePath));
-  }
-
-  async writeFile(relativeOutputPath: string, contents: string | Buffer): Promise<void> {
-    this._addFile(relativeOutputPath, Buffer.from(contents));
-  }
-}