Skip to content

Commit 0c8e88d

Browse files
authored
refactor(test): heavily simplify the context helper (#404)
* refactor(test): heavily simplify the `context` helper - since we're type-casting it anyway, we can heavily simplify this and remove the stubs entirely - they're actually unused in the unit tests, so we don't need them at all - besides the type-checking, which we force with a cast anyway - the `as unknown as` is bad practice, and probably why I didn't use it initially (plus other typing issues), but it's much simpler this way and reflects the intent better -- just making it type-check with the few properties we use - we can also use Jest mocks directly instead of the hacky `contextualLogger` and passing `data` in - `makeContext` now creates the mocks, so we just need to check against `context.error` etc - this is _much_ more familiar as it's what we use in the source and follows idiomatic Jest - rewrite all the checks to test against the mocks instead - I thought this felt too complicated / verbose before, but I had left this as is from brekk's initial test structure - now that I understand all the tests and test intent much better, I could rewrite this to be a good bit simpler - make the `toBeFalsy()` checks more precise by checking that the mock _wasn't_ called - it returns `void` anyway, so `toBeFalsy()` _always_ returns true; it's not much of a test - checking that the low verbosity level didn't trigger the mock to be called actually checks the test's intent * make sure to check funcs in context calls too - `get-options-overrides`'s coverage func % decreased bc funcs passed to `context.debug` weren't being called - took me a bit to notice too since we have no coverage checks - and then another bit to realize _why_ it decreased
1 parent 8886383 commit 0c8e88d

File tree

4 files changed

+54
-94
lines changed

4 files changed

+54
-94
lines changed

__tests__/context.spec.ts

Lines changed: 26 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import { jest, test, expect } from "@jest/globals";
22

3-
import { makeStubbedContext } from "./fixtures/context";
3+
import { makeContext } from "./fixtures/context";
44
import { ConsoleContext, RollupContext } from "../src/context";
55

66
(global as any).console = {
@@ -54,21 +54,20 @@ test("ConsoleContext 0 verbosity", () => {
5454
});
5555

5656
test("RollupContext", () => {
57-
const data = {};
58-
const stubbedContext = makeStubbedContext(data);
59-
const context = new RollupContext(5, false, stubbedContext);
57+
const innerContext = makeContext();
58+
const context = new RollupContext(5, false, innerContext);
6059

6160
context.warn("test");
62-
expect((data as any).warn).toEqual("test");
61+
expect(innerContext.warn).toHaveBeenLastCalledWith("test");
6362

6463
context.warn(() => "test2");
65-
expect((data as any).warn).toEqual("test2");
64+
expect(innerContext.warn).toHaveBeenLastCalledWith("test2");
6665

6766
context.error("test!");
68-
expect((data as any).warn).toEqual("test!");
67+
expect(innerContext.warn).toHaveBeenLastCalledWith("test!");
6968

7069
context.error(() => "test2!");
71-
expect((data as any).warn).toEqual("test2!");
70+
expect(innerContext.warn).toHaveBeenLastCalledWith("test2!");
7271

7372
context.info("test3");
7473
expect(console.log).toHaveBeenLastCalledWith("test3");
@@ -84,29 +83,31 @@ test("RollupContext", () => {
8483
});
8584

8685
test("RollupContext with 0 verbosity", () => {
87-
const data = {};
88-
const stubbedContext = makeStubbedContext(data);
89-
const context = new RollupContext(0, false, stubbedContext);
90-
91-
expect(context.debug("verbosity is too low here")).toBeFalsy();
92-
expect(context.info("verbosity is too low here")).toBeFalsy();
93-
expect(context.warn("verbosity is too low here")).toBeFalsy();
86+
const innerContext = makeContext();
87+
const context = new RollupContext(0, false, innerContext);
88+
89+
context.debug("verbosity is too low here");
90+
expect(innerContext.debug).not.toBeCalled();
91+
context.info("verbosity is too low here");
92+
expect(innerContext.debug).not.toBeCalled();
93+
context.warn("verbosity is too low here")
94+
expect(innerContext.warn).not.toBeCalled();
9495
});
9596

9697
test("RollupContext.error + debug negative verbosity", () => {
97-
const data = {};
98-
const stubbedContext = makeStubbedContext(data);
99-
const context = new RollupContext(-100, true, stubbedContext);
98+
const innerContext = makeContext();
99+
const context = new RollupContext(-100, true, innerContext);
100100

101-
expect(context.error("whatever")).toBeFalsy();
102-
expect(context.debug("whatever")).toBeFalsy();
101+
context.error("verbosity is too low here");
102+
expect(innerContext.error).not.toBeCalled();
103+
context.debug("verbosity is too low here");
104+
expect(innerContext.debug).not.toBeCalled();
103105
});
104106

105107
test("RollupContext.error with bail", () => {
106-
const data = {};
107-
const stubbedContext = makeStubbedContext(data);
108-
const context = new RollupContext(5, true, stubbedContext);
108+
const innerContext = makeContext();
109+
const context = new RollupContext(5, true, innerContext);
109110

110-
expect(context.error("whatever")).toBeFalsy();
111-
expect((data as any).error).toEqual("whatever");
111+
context.error("bail");
112+
expect(innerContext.error).toHaveBeenLastCalledWith("bail");
112113
});

__tests__/fixtures/context.ts

Lines changed: 13 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -1,51 +1,21 @@
1+
import { jest } from "@jest/globals";
12
import { PluginContext } from "rollup";
23

34
import { IContext } from "../../src/context";
45

5-
const stub = (x: any) => x;
6+
// if given a function, make sure to call it (for code coverage etc)
7+
function returnText (message: string | (() => string)) {
8+
if (typeof message === "string")
9+
return message;
610

7-
const contextualLogger = (data: any): IContext => {
8-
return {
9-
warn: (x: any) => {
10-
data.warn = x;
11-
},
12-
error: (x: any) => {
13-
data.error = x;
14-
},
15-
info: (x: any) => {
16-
data.info = x;
17-
},
18-
debug: (x: any) => {
19-
data.debug = x;
20-
},
21-
};
22-
};
11+
return message();
12+
}
2313

24-
export function makeStubbedContext (data: any): PluginContext & IContext {
25-
const { warn, error, info, debug } = contextualLogger(data);
14+
export function makeContext(): PluginContext & IContext {
2615
return {
27-
addWatchFile: stub as any,
28-
getWatchFiles: stub as any,
29-
cache: stub as any,
30-
load: stub as any,
31-
resolve: stub as any,
32-
resolveId: stub as any,
33-
isExternal: stub as any,
34-
meta: stub as any,
35-
emitAsset: stub as any,
36-
emitChunk: stub as any,
37-
emitFile: stub as any,
38-
setAssetSource: stub as any,
39-
getAssetFileName: stub as any,
40-
getChunkFileName: stub as any,
41-
getFileName: stub as any,
42-
parse: stub as any,
43-
warn: warn as any,
44-
error: error as any,
45-
info: info as any,
46-
debug: debug as any,
47-
moduleIds: stub as any,
48-
getModuleIds: stub as any,
49-
getModuleInfo: stub as any
50-
};
16+
error: jest.fn(returnText),
17+
warn: jest.fn(returnText),
18+
info: jest.fn(returnText),
19+
debug: jest.fn(returnText),
20+
} as unknown as PluginContext & IContext;
5121
};

__tests__/get-options-overrides.spec.ts

Lines changed: 8 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,11 @@
1-
import { afterAll, test, expect, jest } from "@jest/globals";
1+
import { afterAll, test, expect } from "@jest/globals";
22
import * as path from "path";
33
import * as ts from "typescript";
44
import { normalizePath as normalize } from "@rollup/pluginutils";
55
import { remove } from "fs-extra";
66

77
import { makeOptions } from "./fixtures/options";
8-
import { makeStubbedContext } from "./fixtures/context";
8+
import { makeContext } from "./fixtures/context";
99
import { getOptionsOverrides, createFilter } from "../src/get-options-overrides";
1010

1111
const local = (x: string) => normalize(path.resolve(__dirname, x));
@@ -100,9 +100,7 @@ test("getOptionsOverrides - with sourceMap", () => {
100100
test("createFilter", () => {
101101
const config = { ...defaultConfig };
102102
const preParsedTsConfig = { ...defaultPreParsedTsConfig };
103-
104-
const stubbedContext = makeStubbedContext({});
105-
const filter = createFilter(stubbedContext, config, preParsedTsConfig);
103+
const filter = createFilter(makeContext(), config, preParsedTsConfig);
106104

107105
expect(filter(filtPath("src/test.ts"))).toBe(true);
108106
expect(filter(filtPath("src/test.js"))).toBe(false);
@@ -112,14 +110,10 @@ test("createFilter", () => {
112110
test("createFilter - context.debug", () => {
113111
const config = { ...defaultConfig };
114112
const preParsedTsConfig = { ...defaultPreParsedTsConfig };
113+
const context = makeContext();
114+
createFilter(context, config, preParsedTsConfig);
115115

116-
// test context.debug() statements
117-
const debug = jest.fn(x => x());
118-
const data = { set debug(x: any) { debug(x) } };
119-
const stubbedContext = makeStubbedContext(data);
120-
121-
createFilter(stubbedContext, config, preParsedTsConfig);
122-
expect(debug.mock.calls.length).toBe(2);
116+
expect(context.debug).toHaveBeenCalledTimes(2);
123117
});
124118

125119
test("createFilter - rootDirs", () => {
@@ -130,9 +124,7 @@ test("createFilter - rootDirs", () => {
130124
rootDirs: ["src", "lib"]
131125
},
132126
};
133-
134-
const stubbedContext = makeStubbedContext({});
135-
const filter = createFilter(stubbedContext, config, preParsedTsConfig);
127+
const filter = createFilter(makeContext(), config, preParsedTsConfig);
136128

137129
expect(filter(filtPath("src/test.ts"))).toBe(true);
138130
expect(filter(filtPath("src/test.js"))).toBe(false);
@@ -155,9 +147,7 @@ test("createFilter - projectReferences", () => {
155147
{ path: "lib" },
156148
],
157149
};
158-
159-
const stubbedContext = makeStubbedContext({});
160-
const filter = createFilter(stubbedContext, config, preParsedTsConfig);
150+
const filter = createFilter(makeContext(), config, preParsedTsConfig);
161151

162152
expect(filter(filtPath("src/test.ts"))).toBe(true);
163153
expect(filter(filtPath("src/test.js"))).toBe(false);

__tests__/parse-tsconfig.spec.ts

Lines changed: 7 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -3,33 +3,32 @@ import * as path from "path";
33
import { normalizePath as normalize } from "@rollup/pluginutils";
44

55
import { makeOptions } from "./fixtures/options";
6-
import { makeStubbedContext } from "./fixtures/context";
6+
import { makeContext } from "./fixtures/context";
77
import { parseTsConfig } from "../src/parse-tsconfig";
88

99
const local = (x: string) => normalize(path.resolve(__dirname, x));
1010

1111
const defaultOpts = makeOptions("", "");
12-
const stubbedContext = makeStubbedContext({});
1312

1413
test("parseTsConfig", () => {
15-
expect(() => parseTsConfig(stubbedContext, defaultOpts)).not.toThrow();
14+
expect(() => parseTsConfig(makeContext(), defaultOpts)).not.toThrow();
1615
});
1716

1817
test("parseTsConfig - tsconfig errors", () => {
19-
const data = { error: "" };
18+
const context = makeContext();
2019

2120
// should not throw when the tsconfig is buggy, but should still print an error (below)
22-
expect(() => parseTsConfig(makeStubbedContext(data), {
21+
expect(() => parseTsConfig(context, {
2322
...defaultOpts,
2423
tsconfigOverride: {
2524
include: "should-be-an-array",
2625
},
2726
})).not.toThrow();
28-
expect(data.error).toMatch("Compiler option 'include' requires a value of type Array");
27+
expect(context.error).toHaveBeenLastCalledWith(expect.stringContaining("Compiler option 'include' requires a value of type Array"));
2928
});
3029

3130
test("parseTsConfig - failed to open", () => {
32-
expect(() => parseTsConfig(stubbedContext, {
31+
expect(() => parseTsConfig(makeContext(), {
3332
...defaultOpts,
3433
tsconfig: "non-existent-tsconfig",
3534
})).toThrow("rpt2: failed to open 'non-existent-tsconfig'");
@@ -38,7 +37,7 @@ test("parseTsConfig - failed to open", () => {
3837
test("parseTsConfig - failed to parse", () => {
3938
const notTsConfigPath = local("fixtures/options.ts"); // a TS file should fail to parse
4039

41-
expect(() => parseTsConfig(stubbedContext, {
40+
expect(() => parseTsConfig(makeContext(), {
4241
...defaultOpts,
4342
tsconfig: notTsConfigPath,
4443
})).toThrow(`rpt2: failed to parse '${notTsConfigPath}'`);

0 commit comments

Comments
 (0)