diff --git a/src/node/heart.ts b/src/node/heart.ts index 1eada79b3..a03802035 100644 --- a/src/node/heart.ts +++ b/src/node/heart.ts @@ -33,17 +33,7 @@ export class Heart { if (typeof this.heartbeatTimer !== "undefined") { clearTimeout(this.heartbeatTimer) } - this.heartbeatTimer = setTimeout(() => { - this.isActive() - .then((active) => { - if (active) { - this.beat() - } - }) - .catch((error) => { - logger.warn(error.message) - }) - }, this.heartbeatInterval) + this.heartbeatTimer = setTimeout(() => heartbeatTimer(this.isActive, this.beat), this.heartbeatInterval) } /** @@ -55,3 +45,20 @@ export class Heart { } } } + +/** + * Helper function for the heartbeatTimer. + * + * If heartbeat is active, call beat. Otherwise do nothing. + * + * Extracted to make it easier to test. + */ +export async function heartbeatTimer(isActive: Heart["isActive"], beat: Heart["beat"]) { + try { + if (await isActive()) { + beat() + } + } catch (error: unknown) { + logger.warn((error as Error).message) + } +} diff --git a/test/e2e/downloads.test.ts b/test/e2e/downloads.test.ts index 4503e076c..2079c7eba 100644 --- a/test/e2e/downloads.test.ts +++ b/test/e2e/downloads.test.ts @@ -1,5 +1,5 @@ -import * as path from "path" import { promises as fs } from "fs" +import * as path from "path" import { clean } from "../utils/helpers" import { describe, test, expect } from "./baseFixture" diff --git a/test/unit/node/heart.test.ts b/test/unit/node/heart.test.ts new file mode 100644 index 000000000..7996fdab5 --- /dev/null +++ b/test/unit/node/heart.test.ts @@ -0,0 +1,100 @@ +import { logger } from "@coder/logger" +import { readFile, writeFile, stat } from "fs/promises" +import { Heart, heartbeatTimer } from "../../../src/node/heart" +import { clean, mockLogger, tmpdir } from "../../utils/helpers" + +const mockIsActive = (resolveTo: boolean) => jest.fn().mockResolvedValue(resolveTo) + +describe("Heart", () => { + const testName = "heartTests" + let testDir = "" + let heart: Heart + + beforeAll(async () => { + mockLogger() + await clean(testName) + testDir = await tmpdir(testName) + }) + beforeEach(() => { + heart = new Heart(`${testDir}/shutdown.txt`, mockIsActive(true)) + }) + afterAll(() => { + jest.restoreAllMocks() + }) + afterEach(() => { + jest.resetAllMocks() + if (heart) { + heart.dispose() + } + }) + it("should write to a file when given a valid file path", async () => { + // Set up heartbeat file with contents + const text = "test" + const pathToFile = `${testDir}/file.txt` + await writeFile(pathToFile, text) + const fileContents = await readFile(pathToFile, { encoding: "utf8" }) + const fileStatusBeforeEdit = await stat(pathToFile) + expect(fileContents).toBe(text) + + heart = new Heart(pathToFile, mockIsActive(true)) + heart.beat() + // Check that the heart wrote to the heartbeatFilePath and overwrote our text + const fileContentsAfterBeat = await readFile(pathToFile, { encoding: "utf8" }) + expect(fileContentsAfterBeat).not.toBe(text) + // Make sure the modified timestamp was updated. + const fileStatusAfterEdit = await stat(pathToFile) + expect(fileStatusAfterEdit.mtimeMs).toBeGreaterThan(fileStatusBeforeEdit.mtimeMs) + }) + it("should log a warning when given an invalid file path", async () => { + heart = new Heart(`fakeDir/fake.txt`, mockIsActive(false)) + heart.beat() + // HACK@jsjoeio - beat has some async logic but is not an async method + // Therefore, we have to create an artificial wait in order to make sure + // all async code has completed before asserting + await new Promise((r) => setTimeout(r, 100)) + // expect(logger.trace).toHaveBeenCalled() + expect(logger.warn).toHaveBeenCalled() + }) + it("should be active after calling beat", () => { + heart.beat() + + const isAlive = heart.alive() + expect(isAlive).toBe(true) + }) + it("should not be active after dispose is called", () => { + heart.dispose() + + const isAlive = heart.alive() + expect(isAlive).toBe(false) + }) +}) + +describe("heartbeatTimer", () => { + beforeAll(() => { + mockLogger() + }) + afterAll(() => { + jest.restoreAllMocks() + }) + afterEach(() => { + jest.resetAllMocks() + }) + it("should call beat when isActive resolves to true", async () => { + const isActive = true + const mockIsActive = jest.fn().mockResolvedValue(isActive) + const mockBeatFn = jest.fn() + await heartbeatTimer(mockIsActive, mockBeatFn) + expect(mockIsActive).toHaveBeenCalled() + expect(mockBeatFn).toHaveBeenCalled() + }) + it("should log a warning when isActive rejects", async () => { + const errorMsg = "oh no" + const error = new Error(errorMsg) + const mockIsActive = jest.fn().mockRejectedValue(error) + const mockBeatFn = jest.fn() + await heartbeatTimer(mockIsActive, mockBeatFn) + expect(mockIsActive).toHaveBeenCalled() + expect(mockBeatFn).not.toHaveBeenCalled() + expect(logger.warn).toHaveBeenCalledWith(errorMsg) + }) +})