From 18ff99693bfb4d77ccc94c1a21ba40aa50ac2f60 Mon Sep 17 00:00:00 2001
From: Joe Previte <jjprevite@gmail.com>
Date: Tue, 26 Apr 2022 11:35:40 -0500
Subject: [PATCH] feat: add tests for node/heart.ts (#5122)

* refactor(heart): extract logic into heartbeatTimer fn

To make it easier to test, I extract heartbeatTimer into it's own
function.

* feat(testing): add tests for heart.ts

* fixup

* fixup!: remove unneeded heart call

* Update src/node/heart.ts

Co-authored-by: Asher <ash@coder.com>

* fixup!: use mockResolvedValue everywhere

* fixup!: add stat test for timestamp check

Co-authored-by: Asher <ash@coder.com>
---
 src/node/heart.ts            |  29 ++++++----
 test/e2e/downloads.test.ts   |   2 +-
 test/unit/node/heart.test.ts | 100 +++++++++++++++++++++++++++++++++++
 3 files changed, 119 insertions(+), 12 deletions(-)
 create mode 100644 test/unit/node/heart.test.ts

diff --git a/src/node/heart.ts b/src/node/heart.ts
index 1eada79b..a0380203 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 4503e076..2079c7eb 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 00000000..7996fdab
--- /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)
+  })
+})