Skip to content

Commit dae4143

Browse files
committed
test: add initial watch mode test suite
- 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
1 parent eab48a3 commit dae4143

File tree

5 files changed

+139
-24
lines changed

5 files changed

+139
-24
lines changed

__tests__/integration/errors.spec.ts

+12-12
Original file line numberDiff line numberDiff line change
@@ -11,34 +11,34 @@ import * as helpers from "./helpers";
1111
jest.setTimeout(15000);
1212

1313
const local = (x: string) => path.resolve(__dirname, x);
14-
const cacheRoot = local("__temp/errors/rpt2-cache"); // don't use the one in node_modules
14+
const testDir = local("__temp/errors");
1515

1616
afterAll(async () => {
1717
// 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)
1818
await new Promise(resolve => setTimeout(resolve, 1000));
19-
await fs.remove(cacheRoot);
19+
await fs.remove(testDir);
2020
});
2121

2222
async function genBundle(relInput: string, extraOpts?: RPT2Options, onwarn?: Mock) {
2323
const input = normalize(local(`fixtures/errors/${relInput}`));
2424
return helpers.genBundle({
2525
input,
2626
tsconfig: local("fixtures/errors/tsconfig.json"),
27-
cacheRoot,
27+
testDir,
2828
extraOpts: { include: [input], ...extraOpts }, // only include the input itself, not other error files (to only generate types and type-check the one file)
2929
onwarn,
3030
});
3131
}
3232

3333
test("integration - tsconfig errors", async () => {
3434
// TODO: move to parse-tsconfig unit tests?
35-
expect(genBundle("semantic.ts", {
35+
await expect(genBundle("semantic.ts", {
3636
tsconfig: "non-existent-tsconfig",
3737
})).rejects.toThrow("rpt2: failed to open 'non-existent-tsconfig'");
3838
});
3939

4040
test("integration - semantic error", async () => {
41-
expect(genBundle("semantic.ts")).rejects.toThrow("Type 'string' is not assignable to type 'number'.");
41+
await expect(genBundle("semantic.ts")).rejects.toThrow("Type 'string' is not assignable to type 'number'.");
4242
});
4343

4444
test("integration - semantic error - abortOnError: false / check: false", async () => {
@@ -55,21 +55,21 @@ test("integration - semantic error - abortOnError: false / check: false", async
5555
expect(onwarn).toBeCalledTimes(1);
5656
});
5757

58-
test("integration - syntax error", () => {
59-
expect(genBundle("syntax.ts")).rejects.toThrow("';' expected.");
58+
test("integration - syntax error", async () => {
59+
await expect(genBundle("syntax.ts")).rejects.toThrow("';' expected.");
6060
});
6161

62-
test("integration - syntax error - abortOnError: false / check: false", () => {
62+
test("integration - syntax error - abortOnError: false / check: false", async () => {
6363
const onwarn = jest.fn();
6464
const err = "Unexpected token (Note that you need plugins to import files that are not JavaScript)";
65-
expect(genBundle("syntax.ts", { abortOnError: false }, onwarn)).rejects.toThrow(err);
66-
expect(genBundle("syntax.ts", { check: false }, onwarn)).rejects.toThrow(err);
65+
await expect(genBundle("syntax.ts", { abortOnError: false }, onwarn)).rejects.toThrow(err);
66+
await expect(genBundle("syntax.ts", { check: false }, onwarn)).rejects.toThrow(err);
6767
});
6868

6969
const typeOnlyIncludes = ["**/import-type-error.ts", "**/type-only-import-with-error.ts"];
7070

71-
test("integration - type-only import error", () => {
72-
expect(genBundle("import-type-error.ts", {
71+
test("integration - type-only import error", async () => {
72+
await expect(genBundle("import-type-error.ts", {
7373
include: typeOnlyIncludes,
7474
})).rejects.toThrow("Property 'nonexistent' does not exist on type 'someObj'.");
7575
});

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

+3
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,9 @@ export function sum(a: number, b: number) {
22
return a + b;
33
}
44

5+
import { difference } from "./some-import";
6+
export const diff2 = difference; // add an alias so that this file has to change when the import does (to help test cache invalidation etc)
7+
58
export { difference } from "./some-import"
69
export type { num } from "./type-only-import"
710

__tests__/integration/helpers.ts

+42-9
Original file line numberDiff line numberDiff line change
@@ -1,31 +1,39 @@
1-
import { rollup, RollupOptions, OutputAsset } from "rollup";
1+
import { rollup, watch, RollupOptions, OutputOptions, OutputAsset, RollupWatcher } from "rollup";
2+
import * as path from "path";
23

34
import rpt2, { RPT2Options } from "../../src/index";
45

56
type Params = {
67
input: string,
78
tsconfig: string,
8-
cacheRoot: string,
9+
testDir: string,
910
extraOpts?: RPT2Options,
1011
onwarn?: RollupOptions['onwarn'],
1112
};
1213

13-
export async function genBundle ({ input, tsconfig, cacheRoot, extraOpts, onwarn }: Params) {
14-
const bundle = await rollup({
14+
function createInput ({ input, tsconfig, testDir, extraOpts, onwarn }: Params) {
15+
return {
1516
input,
1617
plugins: [rpt2({
1718
tsconfig,
18-
cacheRoot,
19+
cacheRoot: `${testDir}/rpt2-cache`, // don't use the one in node_modules
1920
...extraOpts,
2021
})],
2122
onwarn,
22-
});
23+
}
24+
}
2325

24-
const esm = await bundle.generate({
25-
file: "./dist/index.js",
26+
function createOutput (testDir: string): OutputOptions {
27+
return {
28+
file: path.resolve(`${testDir}/dist/index.js`), // put outputs in temp test dir
2629
format: "esm",
2730
exports: "named",
28-
});
31+
}
32+
}
33+
34+
export async function genBundle (inputArgs: Params) {
35+
const bundle = await rollup(createInput(inputArgs));
36+
const esm = await bundle.generate(createOutput(inputArgs.testDir));
2937

3038
// Rollup has some deprecated properties like `get isAsset`, so enumerating them with, e.g. `.toEqual`, causes a bunch of warnings to be output
3139
// delete the `isAsset` property for (much) cleaner logs
@@ -39,3 +47,28 @@ export async function genBundle ({ input, tsconfig, cacheRoot, extraOpts, onwarn
3947

4048
return esm;
4149
}
50+
51+
/** wrap Listener interface in a Promise */
52+
export function watchEnd (watcher: RollupWatcher) {
53+
return new Promise<void>((resolve, reject) => {
54+
watcher.on("event", event => {
55+
if ("result" in event)
56+
event.result?.close(); // close all bundles
57+
58+
if (event.code === "END")
59+
resolve();
60+
else if (event.code === "ERROR")
61+
reject(event.error);
62+
});
63+
});
64+
}
65+
66+
export async function watchBundle (inputArgs: Params) {
67+
const watcher = watch({
68+
...createInput(inputArgs),
69+
output: createOutput(inputArgs.testDir),
70+
});
71+
72+
await watchEnd(watcher); // wait for build to end before returning, similar to genBundle
73+
return watcher;
74+
}

__tests__/integration/no-errors.spec.ts

+3-3
Original file line numberDiff line numberDiff line change
@@ -9,15 +9,15 @@ import * as helpers from "./helpers";
99
jest.setTimeout(15000);
1010

1111
const local = (x: string) => path.resolve(__dirname, x);
12-
const cacheRoot = local("__temp/no-errors/rpt2-cache"); // don't use the one in node_modules
12+
const testDir = local("__temp/no-errors");
1313

14-
afterAll(() => fs.remove(cacheRoot));
14+
afterAll(() => fs.remove(testDir));
1515

1616
async function genBundle(relInput: string, extraOpts?: RPT2Options) {
1717
return helpers.genBundle({
1818
input: local(`fixtures/no-errors/${relInput}`),
1919
tsconfig: local("fixtures/no-errors/tsconfig.json"),
20-
cacheRoot,
20+
testDir,
2121
extraOpts,
2222
});
2323
}

__tests__/integration/watch.spec.ts

+79
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,79 @@
1+
import { jest, beforeAll, afterAll, test, expect } from "@jest/globals";
2+
import * as path from "path";
3+
import * as fs from "fs-extra";
4+
5+
import { RPT2Options } from "../../src/index";
6+
import * as helpers from "./helpers";
7+
8+
// increase timeout to 15s for whole file since CI occassionally timed out -- these are integration and cache tests, so longer timeout is warranted
9+
jest.setTimeout(15000);
10+
11+
const local = (x: string) => path.resolve(__dirname, x);
12+
const testDir = local("__temp/watch");
13+
const fixtureDir = `${testDir}/fixtures`;
14+
15+
beforeAll(async () => {
16+
await fs.ensureDir(fixtureDir);
17+
// copy the dir to not interfere with other parallel tests since we need to change files for watch mode
18+
// 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?
19+
await fs.copy(local("fixtures"), fixtureDir);
20+
});
21+
afterAll(() => fs.remove(testDir));
22+
23+
async function watchBundle(input: string, extraOpts?: RPT2Options) {
24+
return helpers.watchBundle({
25+
input,
26+
tsconfig: `${path.dirname(input)}/tsconfig.json`, // use the tsconfig of whatever fixture we're in
27+
testDir,
28+
extraOpts,
29+
});
30+
}
31+
32+
test("integration - watch", async () => {
33+
const srcPath = `${fixtureDir}/no-errors/index.ts`;
34+
const importPath = `${fixtureDir}/no-errors/some-import.ts`;
35+
const distDir = `${testDir}/dist`;
36+
const distPath = `${testDir}/dist/index.js`;
37+
const decPath = `${distDir}/index.d.ts`;
38+
const decMapPath = `${decPath}.map`;
39+
const filesArr = [
40+
"index.js",
41+
"index.d.ts",
42+
"index.d.ts.map",
43+
"some-import.d.ts",
44+
"some-import.d.ts.map",
45+
"type-only-import.d.ts",
46+
"type-only-import.d.ts.map",
47+
];
48+
49+
const watcher = await watchBundle(srcPath);
50+
51+
const files = await fs.readdir(distDir);
52+
expect(files).toEqual(expect.arrayContaining(filesArr));
53+
expect(files.length).toBe(filesArr.length); // no other files
54+
55+
// save content to test against later
56+
const dist = await fs.readFile(distPath, "utf8");
57+
const dec = await fs.readFile(decPath, "utf8");
58+
const decMap = await fs.readFile(decMapPath, "utf8");
59+
60+
// modify an imported file -- this should cause it and index to change
61+
await fs.writeFile(importPath, "export const difference = 2", "utf8");
62+
await helpers.watchEnd(watcher);
63+
64+
// should have same structure, since names haven't changed and dist hasn't been cleaned
65+
const files2 = await fs.readdir(distDir);
66+
expect(files2).toEqual(expect.arrayContaining(filesArr));
67+
expect(files2.length).toBe(filesArr.length); // no other files
68+
69+
// should have different content now though
70+
expect(dist).not.toEqual(await fs.readFile(distPath, "utf8"));
71+
expect(dec).not.toEqual(await fs.readFile(decPath, "utf8"));
72+
expect(decMap).not.toEqual(await fs.readFile(decMapPath, "utf8"));
73+
74+
// modify an imported file to cause a semantic error
75+
await fs.writeFile(importPath, "export const difference = nonexistent", "utf8")
76+
await expect(helpers.watchEnd(watcher)).rejects.toThrow("Cannot find name 'nonexistent'.");
77+
78+
await watcher.close();
79+
});

0 commit comments

Comments
 (0)