Skip to content

Commit 71d942b

Browse files
committed
chore: replace jest mock with dep injection
We can't mock that way with esm dependencies[^1]. There are some workarounds, but nothing worked, so I decided to introduce my own interface and dependency injection. In the `exec` module, apparently, we used to do it this way; there was an old (unused) `ExecDelegate` interface. When implementing `withCache`, I tried to follow that naming and test definition style, balanced against reducing the diff with `main`. [^1]: jestjs/jest#10025
1 parent f82082c commit 71d942b

4 files changed

Lines changed: 81 additions & 36 deletions

File tree

src/stack-cli.test.ts

Lines changed: 16 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,17 @@
1-
import * as exec from "@actions/exec";
1+
import { ExecOptions } from "@actions/exec";
22
import { jest } from "@jest/globals";
33

4-
import { StackCLI } from "./stack-cli.js";
4+
import { ExecDelegate, StackCLI } from "./stack-cli.js";
55

6-
jest.spyOn(exec, "exec");
6+
const exec: ExecDelegate = {
7+
exec: jest.fn((command: string, args: string[], options?: ExecOptions) =>
8+
Promise.resolve(0),
9+
),
10+
};
711

812
describe("StackCLI", () => {
913
test("Respects --resolver given", async () => {
10-
const stackCLI = new StackCLI(["--resolver", "lts"], false);
14+
const stackCLI = new StackCLI(["--resolver", "lts"], false, exec);
1115

1216
await stackCLI.setup([]);
1317

@@ -22,6 +26,7 @@ describe("StackCLI", () => {
2226
const stackCLI = new StackCLI(
2327
["--stack-yaml", "sub/stack-nightly.yaml"],
2428
false,
29+
exec,
2530
);
2631

2732
await stackCLI.setup([]);
@@ -48,6 +53,7 @@ describe("StackCLI", () => {
4853
"nightly-20240201",
4954
],
5055
false,
56+
exec,
5157
);
5258

5359
await stackCLI.setup([]);
@@ -66,7 +72,7 @@ describe("StackCLI", () => {
6672
});
6773

6874
test("installCompilerTools", async () => {
69-
const stackCLI = new StackCLI([], false);
75+
const stackCLI = new StackCLI([], false, exec);
7076
await stackCLI.installCompilerTools(["hlint", "weeder"]);
7177

7278
expect(exec.exec).toHaveBeenCalledWith(
@@ -77,14 +83,14 @@ describe("StackCLI", () => {
7783
});
7884

7985
test("installCompilerTools with empty arguments", async () => {
80-
const stackCLI = new StackCLI([], false);
86+
const stackCLI = new StackCLI([], false, exec);
8187
await stackCLI.installCompilerTools([]);
8288

8389
expect(exec.exec).not.toHaveBeenCalled();
8490
});
8591

8692
test("buildDependencies", async () => {
87-
const stackCLI = new StackCLI([], false);
93+
const stackCLI = new StackCLI([], false, exec);
8894

8995
await stackCLI.buildDependencies(["--coverage"]);
9096

@@ -102,7 +108,7 @@ describe("StackCLI", () => {
102108
});
103109

104110
test("buildNoTest", async () => {
105-
const stackCLI = new StackCLI([], false);
111+
const stackCLI = new StackCLI([], false, exec);
106112

107113
await stackCLI.buildNoTest(["--coverage"]);
108114

@@ -114,7 +120,7 @@ describe("StackCLI", () => {
114120
});
115121

116122
test("buildTest", async () => {
117-
const stackCLI = new StackCLI([], false);
123+
const stackCLI = new StackCLI([], false, exec);
118124

119125
await stackCLI.buildTest(["--coverage"]);
120126

@@ -126,7 +132,7 @@ describe("StackCLI", () => {
126132
});
127133

128134
test("build", async () => {
129-
const stackCLI = new StackCLI([], false);
135+
const stackCLI = new StackCLI([], false, exec);
130136

131137
await stackCLI.build(["--coverage"]);
132138

src/stack-cli.ts

Lines changed: 17 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -2,12 +2,12 @@ import * as fs from "fs";
22
import * as path from "path";
33
import { devNull } from "os";
44
import type { ExecOptions } from "@actions/exec";
5-
import * as exec from "@actions/exec";
5+
import * as realExec from "@actions/exec";
66

7-
import type { StackPath } from "./parse-stack-path.js"
8-
import { parseStackPath } from "./parse-stack-path.js"
9-
import type { StackQuery } from "./parse-stack-query.js"
10-
import { parseStackQuery } from "./parse-stack-query.js"
7+
import type { StackPath } from "./parse-stack-path.js";
8+
import { parseStackPath } from "./parse-stack-path.js";
9+
import type { StackQuery } from "./parse-stack-query.js";
10+
import { parseStackQuery } from "./parse-stack-query.js";
1111

1212
export interface ExecDelegate {
1313
exec: (
@@ -23,10 +23,12 @@ export class StackCLI {
2323

2424
private debug: boolean;
2525
private globalArgs: string[];
26+
private execImpl: ExecDelegate;
2627

27-
constructor(args: string[], debug?: boolean) {
28+
constructor(args: string[], debug?: boolean, exec?: ExecDelegate) {
2829
this.debug = debug ?? false;
2930
this.globalArgs = args;
31+
this.execImpl = exec ?? realExec;
3032

3133
// Capture --stack-yaml if given
3234
const stackYamlIdx = args.indexOf("--stack-yaml");
@@ -49,7 +51,7 @@ export class StackCLI {
4951
}
5052

5153
async installed(): Promise<boolean> {
52-
const ec = await exec.exec("which", ["stack"], {
54+
const ec = await this.execImpl.exec("which", ["stack"], {
5355
silent: true,
5456
ignoreReturnCode: true,
5557
});
@@ -59,14 +61,14 @@ export class StackCLI {
5961
async install(): Promise<void> {
6062
const url = "https://get.haskellstack.org";
6163
const tmp = "install-stack.sh";
62-
await exec.exec("curl", ["-sSL", "-o", tmp, url]);
63-
await exec.exec("sh", [tmp]);
64+
await this.execImpl.exec("curl", ["-sSL", "-o", tmp, url]);
65+
await this.execImpl.exec("sh", [tmp]);
6466
fs.rmSync(tmp);
6567
}
6668

6769
async upgrade(): Promise<number> {
6870
// Avoid this.exec because we don't need/want globalArgs
69-
return await exec.exec("stack", ["upgrade"]);
71+
return await this.execImpl.exec("stack", ["upgrade"]);
7072
}
7173

7274
async setup(args: string[]): Promise<number> {
@@ -139,6 +141,10 @@ export class StackCLI {
139141
}
140142

141143
private async exec(args: string[], options?: ExecOptions): Promise<number> {
142-
return await exec.exec("stack", this.globalArgs.concat(args), options);
144+
return await this.execImpl.exec(
145+
"stack",
146+
this.globalArgs.concat(args),
147+
options,
148+
);
143149
}
144150
}

src/with-cache.test.ts

Lines changed: 33 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,19 @@
11
import * as core from "@actions/core";
2-
import * as cache from "@actions/cache";
32
import { jest } from "@jest/globals";
43

54
import { getCacheKeys } from "./get-cache-keys.js";
6-
import { DEFAULT_CACHE_OPTIONS, withCache } from "./with-cache.js";
5+
import {
6+
CacheDelegate,
7+
DEFAULT_CACHE_OPTIONS,
8+
withCache,
9+
} from "./with-cache.js";
10+
11+
const cache: CacheDelegate = {
12+
restoreCache: jest.fn(() => Promise.resolve("")),
13+
saveCache: jest.fn(() => Promise.resolve(0)),
14+
};
715

816
const restoreCacheMock = jest.spyOn(cache, "restoreCache");
9-
jest.spyOn(cache, "saveCache");
1017
jest.spyOn(core, "info");
1118

1219
async function testFunction(): Promise<number> {
@@ -43,6 +50,7 @@ test("withCache skips on primary-key hit", async () => {
4350
cacheKeys,
4451
testFunction,
4552
DEFAULT_CACHE_OPTIONS,
53+
cache,
4654
);
4755

4856
expect(result).toBeUndefined();
@@ -64,6 +72,7 @@ test("withCache acts and saves if no primary-key hit", async () => {
6472
cacheKeys,
6573
testFunction,
6674
DEFAULT_CACHE_OPTIONS,
75+
cache,
6776
);
6877

6978
expect(result).toEqual(42);
@@ -83,10 +92,16 @@ test("withCache can be configured to act and save anyway", async () => {
8392
const cacheKeys = getCacheKeys(["a-b", "c", "d"]);
8493
restoreCacheMock.mockImplementation(simulateCacheHit);
8594

86-
const result = await withCache(cachePaths, cacheKeys, testFunction, {
87-
...DEFAULT_CACHE_OPTIONS,
88-
skipOnHit: false,
89-
});
95+
const result = await withCache(
96+
cachePaths,
97+
cacheKeys,
98+
testFunction,
99+
{
100+
...DEFAULT_CACHE_OPTIONS,
101+
skipOnHit: false,
102+
},
103+
cache,
104+
);
90105

91106
expect(result).toEqual(42);
92107
expect(cache.restoreCache).toHaveBeenCalledWith(
@@ -110,6 +125,7 @@ test("withCache does not save on error", async () => {
110125
cacheKeys,
111126
testFunctionThrows,
112127
DEFAULT_CACHE_OPTIONS,
128+
cache,
113129
);
114130
}).rejects.toThrow();
115131

@@ -129,10 +145,16 @@ test("withCache can be configured to save on error", async () => {
129145
restoreCacheMock.mockImplementation(simulateCacheMiss);
130146

131147
await expect(async () => {
132-
await withCache(cachePaths, cacheKeys, testFunctionThrows, {
133-
...DEFAULT_CACHE_OPTIONS,
134-
saveOnError: true,
135-
});
148+
await withCache(
149+
cachePaths,
150+
cacheKeys,
151+
testFunctionThrows,
152+
{
153+
...DEFAULT_CACHE_OPTIONS,
154+
saveOnError: true,
155+
},
156+
cache,
157+
);
136158
}).rejects.toThrow();
137159

138160
expect(cache.restoreCache).toHaveBeenCalledWith(

src/with-cache.ts

Lines changed: 15 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import * as core from "@actions/core";
2-
import * as cache from "@actions/cache";
2+
import * as realCache from "@actions/cache";
33
import type { CacheKeys } from "./get-cache-keys.js";
44

55
export type CacheOptions = {
@@ -12,19 +12,30 @@ export const DEFAULT_CACHE_OPTIONS = {
1212
saveOnError: false,
1313
};
1414

15+
export interface CacheDelegate {
16+
restoreCache: (
17+
paths: string[],
18+
primaryKey: string,
19+
restoreKeys?: string[],
20+
) => Promise<string>;
21+
saveCache: (paths: string[], key: string) => Promise<number>;
22+
}
23+
1524
export async function withCache<T>(
1625
paths: string[],
1726
keys: CacheKeys,
1827
fn: () => Promise<T>,
1928
options: CacheOptions = DEFAULT_CACHE_OPTIONS,
29+
cache?: CacheDelegate,
2030
): Promise<T | undefined> {
31+
const cacheImpl = cache ?? realCache;
2132
const { skipOnHit, saveOnError } = options;
2233

2334
core.info(`Cached paths:\n - ${paths.join("\n - ")}`);
2435
core.info(`Cache key: ${keys.primaryKey}`);
2536
core.info(`Cache restore keys:\n - ${keys.restoreKeys.join("\n - ")}`);
2637

27-
const restoredKey = await cache.restoreCache(
38+
const restoredKey = await cacheImpl.restoreCache(
2839
paths,
2940
keys.primaryKey,
3041
keys.restoreKeys,
@@ -49,11 +60,11 @@ export async function withCache<T>(
4960
result = await fn();
5061

5162
if (!primaryKeyHit) {
52-
await cache.saveCache(paths, keys.primaryKey);
63+
await cacheImpl.saveCache(paths, keys.primaryKey);
5364
}
5465
} catch (ex) {
5566
if (saveOnError && !primaryKeyHit) {
56-
await cache.saveCache(paths, keys.primaryKey);
67+
await cacheImpl.saveCache(paths, keys.primaryKey);
5768
}
5869

5970
throw ex;

0 commit comments

Comments
 (0)