test: add initial watch mode test suite (#386)

- test starting watch mode, changing a file, and adding a semantic error
  - put this in a separate file as it has its own complexity to deal with (see below)
    - initially put it inside `no-errors`, but it got pretty confusing; this structure is a good bit simpler
    - refactored this a couple times actually

- add two watch mode helpers
  - `watchBundle` is very similar to `genBundle` but with some watch mode nuances (like returning a watcher)
  - `watchEnd` is a bit complicated async wrapper around a listener interface to make imperative testing simpler
  - refactor: abstract out `createInput` and `createOutput` to be used in both `genBundle` and `watchBundle`
    - refactor: make sure `dist` output gets put into a temp test dir
      - the previous config made it output into rpt2's `dist` dir, since `cwd` is project root (when running tests from project root)
      - the Rollup API's `watch` func always writes out; can't just test in-memory like with `bundle.generate`
        - so the `dist` dir becomes relevant as such
      - refactor: pass in a temp `testDir` instead of the `cacheRoot`
        - we can derive the `cacheRoot` and the `dist` output from `testDir`
        - also improve test clean-up by removing `testDir` at the end, not just the `cacheRoot` dir inside it
        - `testDir` is the same var used in the unit tests to define a temp dir for testing

- change the `no-errors` fixture a tiny bit so that changing the import causes it to change too

- this ended up being fairly complex due to having to handle lots of async and parallelism
  - parallel testing means fixtures have to be immutable, but watch mode needs to modify files
    - ended up copying fixtures to a temp dir where I could modify them
  - async events are all over here
    - had to convert a callback listener interface to async too, which was pretty confusing
      - and make sure the listener and bundles got properly closed too so no leaky tests
    - apparently `expect.rejects.toThrow` can return a Promise too, so missed this error for a while
      - refactor: make sure the error spec awaits too (though I think the errors _happen_ to throw synchronously there)
  - and also found a number of bugs while attempting to test cache invalidation within watch mode
    - thought it was a natural place to test since watch mode testing needs to modify files anyway
    - had to trace a bunch to figure out why some code paths weren't being covered; will discuss this separately from this commit
  - testing Rollup watch within Jest watch also causes Jest to re-run a few times
    - I imagine double, overlapping watchers are confusing each other
    - might need to disable these tests when using Jest watch
  - high complexity async + parallel flows makes it pretty confusing to debug, so this code needs to be "handled with care"
    - also this high complexity was even harder to debug when I'm having migraines (which is most of the time, unfortunately)
      - hence why it took me a bit to finally make a PR for this small amount of code; the code was ok, the debugging was not too fun

Author: agilgur5

__tests__/integration/errors.spec.ts

@@ -11,34 +11,34 @@ import * as helpers from "./helpers";
 jest.setTimeout(15000);
 
 const local = (x: string) => normalize(path.resolve(__dirname, x));
-const cacheRoot = local("__temp/errors/rpt2-cache"); // don't use the one in node_modules
+const testDir = local("__temp/errors");
 
 afterAll(async () => {
   // workaround: there seems to be some race condition causing fs.remove to fail, so give it a sec first (c.f. https://github.com/jprichardson/node-fs-extra/issues/532)
   await new Promise(resolve => setTimeout(resolve, 1000));
-  await fs.remove(cacheRoot);
+  await fs.remove(testDir);
 });
 
 async function genBundle(relInput: string, extraOpts?: RPT2Options, onwarn?: Mock) {
   const input = local(`fixtures/errors/${relInput}`);
   return helpers.genBundle({
     input,
     tsconfig: local("fixtures/errors/tsconfig.json"),
-    cacheRoot,
+    testDir,
     extraOpts: { include: [input], ...extraOpts }, // only include the input itself, not other error files (to only generate types and type-check the one file)
     onwarn,
   });
 }
 
 test("integration - tsconfig errors", async () => {
   // TODO: move to parse-tsconfig unit tests?
-  expect(genBundle("semantic.ts", {
+  await expect(genBundle("semantic.ts", {
     tsconfig: "non-existent-tsconfig",
   })).rejects.toThrow("rpt2: failed to open 'non-existent-tsconfig'");
 });
 
 test("integration - semantic error", async () => {
-  expect(genBundle("semantic.ts")).rejects.toThrow("Type 'string' is not assignable to type 'number'.");
+  await expect(genBundle("semantic.ts")).rejects.toThrow("Type 'string' is not assignable to type 'number'.");
 });
 
 test("integration - semantic error - abortOnError: false / check: false", async () => {
@@ -55,21 +55,21 @@ test("integration - semantic error - abortOnError: false / check: false", async
   expect(onwarn).toBeCalledTimes(1);
 });
 
-test("integration - syntax error", () => {
-  expect(genBundle("syntax.ts")).rejects.toThrow("';' expected.");
+test("integration - syntax error", async () => {
+  await expect(genBundle("syntax.ts")).rejects.toThrow("';' expected.");
 });
 
-test("integration - syntax error - abortOnError: false / check: false", () => {
+test("integration - syntax error - abortOnError: false / check: false", async () => {
   const onwarn = jest.fn();
   const err = "Unexpected token (Note that you need plugins to import files that are not JavaScript)";
-  expect(genBundle("syntax.ts", { abortOnError: false }, onwarn)).rejects.toThrow(err);
-  expect(genBundle("syntax.ts", { check: false }, onwarn)).rejects.toThrow(err);
+  await expect(genBundle("syntax.ts", { abortOnError: false }, onwarn)).rejects.toThrow(err);
+  await expect(genBundle("syntax.ts", { check: false }, onwarn)).rejects.toThrow(err);
 });
 
 const typeOnlyIncludes = ["**/import-type-error.ts", "**/type-only-import-with-error.ts"];
 
-test("integration - type-only import error", () => {
-  expect(genBundle("import-type-error.ts", {
+test("integration - type-only import error", async () => {
+  await expect(genBundle("import-type-error.ts", {
     include: typeOnlyIncludes,
   })).rejects.toThrow("Property 'nonexistent' does not exist on type 'someObj'.");
 });

__tests__/integration/fixtures/no-errors/index.ts

@@ -2,6 +2,9 @@ export function sum(a: number, b: number) {
   return a + b;
 }
 
+import { difference } from "./some-import";
+export const diff2 = difference; // add an alias so that this file has to change when the import does (to help test cache invalidation etc)
+
 export { difference } from "./some-import"
 export type { num } from "./type-only-import"
 

__tests__/integration/helpers.ts

@@ -1,31 +1,39 @@
-import { rollup, RollupOptions, OutputAsset } from "rollup";
+import { rollup, watch, RollupOptions, OutputOptions, OutputAsset, RollupWatcher } from "rollup";
+import * as path from "path";
 
 import rpt2, { RPT2Options } from "../../src/index";
 
 type Params = {
   input: string,
   tsconfig: string,
-  cacheRoot: string,
+  testDir: string,
   extraOpts?: RPT2Options,
   onwarn?: RollupOptions['onwarn'],
 };
 
-export async function genBundle ({ input, tsconfig, cacheRoot, extraOpts, onwarn }: Params) {
-  const bundle = await rollup({
+function createInput ({ input, tsconfig, testDir, extraOpts, onwarn }: Params) {
+  return {
     input,
     plugins: [rpt2({
       tsconfig,
-      cacheRoot,
+      cacheRoot: `${testDir}/rpt2-cache`, // don't use the one in node_modules
       ...extraOpts,
     })],
     onwarn,
-  });
+  }
+}
 
-  const esm = await bundle.generate({
-    file: "./dist/index.js",
+function createOutput (testDir: string): OutputOptions {
+  return {
+    file: path.resolve(`${testDir}/dist/index.js`), // put outputs in temp test dir
     format: "esm",
     exports: "named",
-  });
+  }
+}
+
+export async function genBundle (inputArgs: Params) {
+  const bundle = await rollup(createInput(inputArgs));
+  const esm = await bundle.generate(createOutput(inputArgs.testDir));
 
   // Rollup has some deprecated properties like `get isAsset`, so enumerating them with, e.g. `.toEqual`, causes a bunch of warnings to be output
   // delete the `isAsset` property for (much) cleaner logs
@@ -39,3 +47,28 @@ export async function genBundle ({ input, tsconfig, cacheRoot, extraOpts, onwarn
 
   return esm;
 }
+
+/** wrap Listener interface in a Promise */
+export function watchEnd (watcher: RollupWatcher) {
+  return new Promise<void>((resolve, reject) => {
+    watcher.on("event", event => {
+      if ("result" in event)
+        event.result?.close(); // close all bundles
+
+      if (event.code === "END")
+        resolve();
+      else if (event.code === "ERROR")
+        reject(event.error);
+    });
+  });
+}
+
+export async function watchBundle (inputArgs: Params) {
+  const watcher = watch({
+    ...createInput(inputArgs),
+    output: createOutput(inputArgs.testDir),
+  });
+
+  await watchEnd(watcher); // wait for build to end before returning, similar to genBundle
+  return watcher;
+}

__tests__/integration/no-errors.spec.ts

@@ -9,15 +9,15 @@ import * as helpers from "./helpers";
 jest.setTimeout(15000);
 
 const local = (x: string) => path.resolve(__dirname, x);
-const cacheRoot = local("__temp/no-errors/rpt2-cache"); // don't use the one in node_modules
+const testDir = local("__temp/no-errors");
 
-afterAll(() => fs.remove(cacheRoot));
+afterAll(() => fs.remove(testDir));
 
 async function genBundle(relInput: string, extraOpts?: RPT2Options) {
   return helpers.genBundle({
     input: local(`fixtures/no-errors/${relInput}`),
     tsconfig: local("fixtures/no-errors/tsconfig.json"),
-    cacheRoot,
+    testDir,
     extraOpts,
   });
 }

__tests__/integration/watch.spec.ts

@@ -0,0 +1,79 @@
+import { jest, beforeAll, afterAll, test, expect } from "@jest/globals";
+import * as path from "path";
+import * as fs from "fs-extra";
+
+import { RPT2Options } from "../../src/index";
+import * as helpers from "./helpers";
+
+// increase timeout to 15s for whole file since CI occassionally timed out -- these are integration and cache tests, so longer timeout is warranted
+jest.setTimeout(15000);
+
+const local = (x: string) => path.resolve(__dirname, x);
+const testDir = local("__temp/watch");
+const fixtureDir = `${testDir}/fixtures`;
+
+beforeAll(async () => {
+  await fs.ensureDir(fixtureDir);
+  // copy the dir to not interfere with other parallel tests since we need to change files for watch mode
+  // note we're copying the root fixture dir bc we need the _base_ tsconfig too. maybe optimize in the future or use the other fixtures?
+  await fs.copy(local("fixtures"), fixtureDir);
+});
+afterAll(() => fs.remove(testDir));
+
+async function watchBundle(input: string, extraOpts?: RPT2Options) {
+  return helpers.watchBundle({
+    input,
+    tsconfig: `${path.dirname(input)}/tsconfig.json`, // use the tsconfig of whatever fixture we're in
+    testDir,
+    extraOpts,
+  });
+}
+
+test("integration - watch", async () => {
+  const srcPath = `${fixtureDir}/no-errors/index.ts`;
+  const importPath = `${fixtureDir}/no-errors/some-import.ts`;
+  const distDir = `${testDir}/dist`;
+  const distPath = `${testDir}/dist/index.js`;
+  const decPath = `${distDir}/index.d.ts`;
+  const decMapPath = `${decPath}.map`;
+  const filesArr = [
+    "index.js",
+    "index.d.ts",
+    "index.d.ts.map",
+    "some-import.d.ts",
+    "some-import.d.ts.map",
+    "type-only-import.d.ts",
+    "type-only-import.d.ts.map",
+  ];
+
+  const watcher = await watchBundle(srcPath);
+
+  const files = await fs.readdir(distDir);
+  expect(files).toEqual(expect.arrayContaining(filesArr));
+  expect(files.length).toBe(filesArr.length); // no other files
+
+  // save content to test against later
+  const dist = await fs.readFile(distPath, "utf8");
+  const dec = await fs.readFile(decPath, "utf8");
+  const decMap = await fs.readFile(decMapPath, "utf8");
+
+  // modify an imported file -- this should cause it and index to change
+  await fs.writeFile(importPath, "export const difference = 2", "utf8");
+  await helpers.watchEnd(watcher);
+
+  // should have same structure, since names haven't changed and dist hasn't been cleaned
+  const files2 = await fs.readdir(distDir);
+  expect(files2).toEqual(expect.arrayContaining(filesArr));
+  expect(files2.length).toBe(filesArr.length); // no other files
+
+  // should have different content now though
+  expect(dist).not.toEqual(await fs.readFile(distPath, "utf8"));
+  expect(dec).not.toEqual(await fs.readFile(decPath, "utf8"));
+  expect(decMap).not.toEqual(await fs.readFile(decMapPath, "utf8"));
+
+  // modify an imported file to cause a semantic error
+  await fs.writeFile(importPath, "export const difference = nonexistent", "utf8")
+  await expect(helpers.watchEnd(watcher)).rejects.toThrow("Cannot find name 'nonexistent'.");
+
+  await watcher.close();
+});