Skip to content

Commit

Permalink
refactor(util): Eliminate side-effects on import
Browse files Browse the repository at this point in the history
Move the util.promisify() call in src/util.ts from the top level to
execBashCommand(). Mock util.promisify() in tests now that it is simpler
to do so before util.promisify() is called. Introduce src/mocks/util.ts
to share the util mock factory between tests.
  • Loading branch information
Kurt-von-Laven committed Mar 28, 2023
1 parent f4d23f7 commit 7f9f984
Show file tree
Hide file tree
Showing 6 changed files with 42 additions and 39 deletions.
2 changes: 1 addition & 1 deletion dist/main/index.js

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion dist/post/index.js

Large diffs are not rendered by default.

34 changes: 17 additions & 17 deletions src/integration.test.ts
Original file line number Diff line number Diff line change
@@ -1,21 +1,22 @@
import { exec } from "node:child_process";
import { testProp } from "@fast-check/jest";
import { jest } from "@jest/globals";
import { string } from "fast-check";

import { consoleOutput } from "./arbitraries/util.js";
import { utilFactory } from "./mocks/util.js";

import type { exec, ExecOptions } from "node:child_process";
import type { ExecOptions } from "node:child_process";
import type { InputOptions } from "@actions/core";
import type { Mock } from "jest-mock";

import type { ConsoleOutput } from "./util.js";

jest.unstable_mockModule("node:child_process", () => ({
exec: jest.fn<typeof exec>(),
}));
jest.unstable_mockModule("node:util", utilFactory);
jest.mock("@actions/cache");
jest.mock("@actions/core");

const child_process = jest.mocked(await import("node:child_process"));
const nodeUtil = jest.mocked(await import("node:util"));
const cache = jest.mocked(await import("@actions/cache"));
const core = jest.mocked(await import("@actions/core"));
const docker = await import("./docker.js");
Expand All @@ -33,6 +34,7 @@ describe("Integration Test", (): void => {
let state: Record<string, string>;
let dockerImages: string[];
let callCount: number;
let execMock: Mock<(command: string) => Promise<ConsoleOutput>>;

beforeEach(async (): Promise<void> => {
loadCommand = `docker load --input ${docker.DOCKER_IMAGES_PATH}`;
Expand Down Expand Up @@ -64,11 +66,7 @@ describe("Integration Test", (): void => {
});

const mockExec = (listStderr: string, otherOutput: ConsoleOutput): void => {
child_process.exec.mockImplementation(<typeof exec>((
command: string,
_options: any,
callback: any
): any => {
execMock = jest.fn((command: string): Promise<ConsoleOutput> => {
let output: ConsoleOutput;
if (command === LIST_COMMAND) {
/* When Docker is running as root, docker image list generates a list
Expand All @@ -85,8 +83,10 @@ describe("Integration Test", (): void => {
} else {
output = otherOutput;
}
callback(null, output);
}));
return Promise.resolve(output);
});

nodeUtil.promisify.mockReturnValue(execMock);
};

const assertExecBashCommand = (
Expand All @@ -96,11 +96,11 @@ describe("Integration Test", (): void => {
output: ConsoleOutput
): void => {
expect(core.info).nthCalledWith<[string]>(infoCallNum, command);
expect(child_process.exec).nthCalledWith<[string, ExecOptions, any]>(
expect(nodeUtil.promisify).nthCalledWith<[typeof exec]>(execCallNum, exec);
expect(execMock).nthCalledWith<[string, ExecOptions]>(
execCallNum,
command,
EXEC_OPTIONS,
expect.anything()
EXEC_OPTIONS
);
expect(core.info).nthCalledWith<[string]>(infoCallNum + 1, output.stdout);
expect(core.error).nthCalledWith<[string]>(execCallNum, output.stderr);
Expand Down Expand Up @@ -128,14 +128,14 @@ describe("Integration Test", (): void => {
const listOutput = joinOutput(dockerImages, listStderr);
assertExecBashCommand(2, 1, LIST_COMMAND, listOutput);
}
expect(child_process.exec).toHaveBeenCalledTimes(1);
expect(execMock).toHaveBeenCalledTimes(1);
};

const assertSaveCacheHit = (key: string): void => {
expect(core.info).lastCalledWith(
`Cache hit occurred on the primary key ${key}, not saving cache.`
);
expect(child_process.exec).not.toHaveBeenCalled();
expect(execMock).not.toHaveBeenCalled();
expect(core.setFailed).not.toHaveBeenCalled();
expect(cache.saveCache).not.toHaveBeenCalled();
};
Expand Down
9 changes: 9 additions & 0 deletions src/mocks/util.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
import { jest } from "@jest/globals";

import type { promisify } from "node:util";

export const utilFactory = (): {
promisify: Omit<typeof promisify, "custom">;
} => ({
promisify: jest.fn<typeof promisify>(),
});
32 changes: 13 additions & 19 deletions src/util.test.ts
Original file line number Diff line number Diff line change
@@ -1,20 +1,17 @@
import { exec } from "node:child_process";
import { testProp } from "@fast-check/jest";
import { jest } from "@jest/globals";
import { string } from "fast-check";

import { consoleOutput, platform } from "./arbitraries/util.js";

import type { exec } from "node:child_process";
import { utilFactory } from "./mocks/util.js";

import type { ConsoleOutput } from "./util.js";

jest.unstable_mockModule("node:child_process", () => ({
exec: jest.fn<typeof exec>(),
}));

jest.unstable_mockModule("node:util", utilFactory);
jest.mock("@actions/core");

const child_process = jest.mocked(await import("node:child_process"));
const nodeUtil = jest.mocked(await import("node:util"));
const core = jest.mocked(await import("@actions/core"));
const util = await import("./util.js");

Expand All @@ -26,25 +23,22 @@ describe("Util", (): void => {
platform: NodeJS.Platform,
output: ConsoleOutput
): Promise<string> => {
child_process.exec.mockImplementationOnce(<typeof exec>((
_command: any,
_options: any,
callback: any
): any => {
callback(error, output);
}));
const execMock = jest.fn(
(): Promise<ConsoleOutput> =>
error ? Promise.reject(error) : Promise.resolve(output)
);

nodeUtil.promisify.mockReturnValueOnce(execMock);

const stdout = await util.execBashCommand(command, platform);

expect(core.info).nthCalledWith<[string]>(1, command);
expect(nodeUtil.promisify).lastCalledWith(exec);
const shell =
platform === "win32"
? "C:\\Program Files\\Git\\bin\\bash.exe"
: "/usr/bin/bash";
expect(child_process.exec).lastCalledWith(
command,
{ shell },
expect.anything()
);
expect(execMock).lastCalledWith(command, { shell });
return stdout;
};

Expand Down
2 changes: 1 addition & 1 deletion src/util.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
import { exec } from "node:child_process";
import { promisify } from "node:util";
const execAsPromised = promisify(exec);

import { error, info, setFailed } from "@actions/core";

Expand All @@ -14,6 +13,7 @@ const execBashCommand = async (
platform: NodeJS.Platform = process.platform
): Promise<string> => {
info(command);
const execAsPromised = promisify(exec);
const shell =
platform === "win32"
? "C:\\Program Files\\Git\\bin\\bash.exe"
Expand Down

0 comments on commit 7f9f984

Please sign in to comment.