refactor: add timeout for race condition in heart test (#5131)

* refactor: add timeout for race condition in heart test

* fixup!: set mtime to 0 and check for update

* fixup!: use utimes directly instead of file open

* fixup!: remove import
This commit is contained in:
Joe Previte 2022-04-26 12:39:37 -05:00 committed by GitHub
parent 18ff99693b
commit 683412cb01
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
1 changed files with 12 additions and 4 deletions

View File

@ -1,5 +1,5 @@
import { logger } from "@coder/logger" import { logger } from "@coder/logger"
import { readFile, writeFile, stat } from "fs/promises" import { readFile, writeFile, stat, utimes } from "fs/promises"
import { Heart, heartbeatTimer } from "../../../src/node/heart" import { Heart, heartbeatTimer } from "../../../src/node/heart"
import { clean, mockLogger, tmpdir } from "../../utils/helpers" import { clean, mockLogger, tmpdir } from "../../utils/helpers"
@ -33,17 +33,26 @@ describe("Heart", () => {
const pathToFile = `${testDir}/file.txt` const pathToFile = `${testDir}/file.txt`
await writeFile(pathToFile, text) await writeFile(pathToFile, text)
const fileContents = await readFile(pathToFile, { encoding: "utf8" }) const fileContents = await readFile(pathToFile, { encoding: "utf8" })
const fileStatusBeforeEdit = await stat(pathToFile) // Explicitly set the modified time to 0 so that we can check
// that the file was indeed modified after calling heart.beat().
// This works around any potential race conditions.
// Docs: https://nodejs.org/api/fs.html#fspromisesutimespath-atime-mtime
await utimes(pathToFile, 0, 0)
expect(fileContents).toBe(text) expect(fileContents).toBe(text)
heart = new Heart(pathToFile, mockIsActive(true)) heart = new Heart(pathToFile, mockIsActive(true))
heart.beat() 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))
// Check that the heart wrote to the heartbeatFilePath and overwrote our text // Check that the heart wrote to the heartbeatFilePath and overwrote our text
const fileContentsAfterBeat = await readFile(pathToFile, { encoding: "utf8" }) const fileContentsAfterBeat = await readFile(pathToFile, { encoding: "utf8" })
expect(fileContentsAfterBeat).not.toBe(text) expect(fileContentsAfterBeat).not.toBe(text)
// Make sure the modified timestamp was updated. // Make sure the modified timestamp was updated.
const fileStatusAfterEdit = await stat(pathToFile) const fileStatusAfterEdit = await stat(pathToFile)
expect(fileStatusAfterEdit.mtimeMs).toBeGreaterThan(fileStatusBeforeEdit.mtimeMs) expect(fileStatusAfterEdit.mtimeMs).toBeGreaterThan(0)
}) })
it("should log a warning when given an invalid file path", async () => { it("should log a warning when given an invalid file path", async () => {
heart = new Heart(`fakeDir/fake.txt`, mockIsActive(false)) heart = new Heart(`fakeDir/fake.txt`, mockIsActive(false))
@ -52,7 +61,6 @@ describe("Heart", () => {
// Therefore, we have to create an artificial wait in order to make sure // Therefore, we have to create an artificial wait in order to make sure
// all async code has completed before asserting // all async code has completed before asserting
await new Promise((r) => setTimeout(r, 100)) await new Promise((r) => setTimeout(r, 100))
// expect(logger.trace).toHaveBeenCalled()
expect(logger.warn).toHaveBeenCalled() expect(logger.warn).toHaveBeenCalled()
}) })
it("should be active after calling beat", () => { it("should be active after calling beat", () => {