Skip to content

Commit 9a8477b

Browse files
committed
Fix test temp dirs not being cleaned up
1 parent f482c2a commit 9a8477b

File tree

7 files changed

+63
-54
lines changed

7 files changed

+63
-54
lines changed

test/e2e/terminal.test.ts

Lines changed: 6 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -2,28 +2,24 @@ import * as cp from "child_process"
22
import * as fs from "fs"
33
import * as path from "path"
44
import util from "util"
5-
import { tmpdir } from "../utils/helpers"
5+
import { clean, tmpdir } from "../utils/helpers"
66
import { describe, expect, test } from "./baseFixture"
77

88
describe("Integrated Terminal", true, () => {
99
// Create a new context with the saved storage state
1010
// so we don't have to logged in
1111
const testFileName = "pipe"
1212
const testString = "new string test from e2e test"
13-
let tmpFolderPath = ""
14-
let tmpFile = ""
1513

14+
const testName = "integrated-terminal"
1615
test.beforeAll(async () => {
17-
tmpFolderPath = await tmpdir("integrated-terminal")
18-
tmpFile = path.join(tmpFolderPath, testFileName)
19-
})
20-
21-
test.afterAll(async () => {
22-
// Ensure directory was removed
23-
await fs.promises.rmdir(tmpFolderPath, { recursive: true })
16+
await clean(testName)
2417
})
2518

2619
test("should echo a string to a file", async ({ codeServerPage }) => {
20+
const tmpFolderPath = await tmpdir(testName)
21+
const tmpFile = path.join(tmpFolderPath, testFileName)
22+
2723
const command = `mkfifo '${tmpFile}' && cat '${tmpFile}'`
2824
const exec = util.promisify(cp.exec)
2925
const output = exec(command, { encoding: "utf8" })

test/unit/helpers.test.ts

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,16 @@
11
import { promises as fs } from "fs"
2-
import { getAvailablePort, tmpdir, useEnv } from "../../test/utils/helpers"
2+
import { clean, getAvailablePort, tmpdir, useEnv } from "../../test/utils/helpers"
33

44
/**
55
* This file is for testing test helpers (not core code).
66
*/
77
describe("test helpers", () => {
8+
const testName = "temp-dir"
9+
beforeAll(async () => {
10+
await clean(testName)
11+
})
12+
813
it("should return a temp directory", async () => {
9-
const testName = "temp-dir"
1014
const pathToTempDir = await tmpdir(testName)
1115
expect(pathToTempDir).toContain(testName)
1216
expect(fs.access(pathToTempDir)).resolves.toStrictEqual(undefined)

test/unit/node/app.test.ts

Lines changed: 5 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,12 @@
11
import { logger } from "@coder/logger"
2-
import { promises, rmdirSync } from "fs"
2+
import { promises } from "fs"
33
import * as http from "http"
44
import * as https from "https"
55
import * as path from "path"
66
import { createApp, ensureAddress, handleArgsSocketCatchError, handleServerError } from "../../../src/node/app"
77
import { OptionalString, setDefaults } from "../../../src/node/cli"
88
import { generateCertificate } from "../../../src/node/util"
9-
import { getAvailablePort, tmpdir } from "../../utils/helpers"
9+
import { clean, getAvailablePort, tmpdir } from "../../utils/helpers"
1010

1111
describe("createApp", () => {
1212
let spy: jest.SpyInstance
@@ -16,7 +16,9 @@ describe("createApp", () => {
1616
let tmpFilePath: string
1717

1818
beforeAll(async () => {
19-
tmpDirPath = await tmpdir("unlink-socket")
19+
const testName = "unlink-socket"
20+
await clean(testName)
21+
tmpDirPath = await tmpdir(testName)
2022
tmpFilePath = path.join(tmpDirPath, "unlink-socket-file")
2123
})
2224

@@ -36,12 +38,6 @@ describe("createApp", () => {
3638
jest.clearAllMocks()
3739
})
3840

39-
afterAll(() => {
40-
jest.restoreAllMocks()
41-
// Ensure directory was removed
42-
rmdirSync(tmpDirPath, { recursive: true })
43-
})
44-
4541
it("should return an Express app, a WebSockets Express app and an http server", async () => {
4642
const defaultArgs = await setDefaults({
4743
port,

test/unit/node/cli.test.ts

Lines changed: 11 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -361,13 +361,11 @@ describe("parser", () => {
361361
})
362362

363363
describe("cli", () => {
364-
let testDir: string
364+
const testName = "cli"
365365
const vscodeIpcPath = path.join(os.tmpdir(), "vscode-ipc")
366366

367367
beforeAll(async () => {
368-
testDir = await tmpdir("cli")
369-
await fs.rmdir(testDir, { recursive: true })
370-
await fs.mkdir(testDir, { recursive: true })
368+
await clean(testName)
371369
})
372370

373371
beforeEach(async () => {
@@ -416,6 +414,7 @@ describe("cli", () => {
416414
args._ = ["./file"]
417415
expect(await shouldOpenInExistingInstance(args)).toStrictEqual(undefined)
418416

417+
const testDir = await tmpdir(testName)
419418
const socketPath = path.join(testDir, "socket")
420419
await fs.writeFile(vscodeIpcPath, socketPath)
421420
expect(await shouldOpenInExistingInstance(args)).toStrictEqual(undefined)
@@ -636,15 +635,13 @@ describe("readSocketPath", () => {
636635
let tmpFilePath: string
637636

638637
beforeEach(async () => {
639-
tmpDirPath = await tmpdir("readSocketPath")
638+
const testName = "readSocketPath"
639+
await clean(testName)
640+
tmpDirPath = await tmpdir(testName)
640641
tmpFilePath = path.join(tmpDirPath, "readSocketPath.txt")
641642
await fs.writeFile(tmpFilePath, fileContents)
642643
})
643644

644-
afterEach(async () => {
645-
await fs.rmdir(tmpDirPath, { recursive: true })
646-
})
647-
648645
it("should throw an error if it can't read the file", async () => {
649646
// TODO@jsjoeio - implement
650647
// Test it on a directory.... ESDIR
@@ -677,9 +674,10 @@ describe("toVsCodeArgs", () => {
677674
version: false,
678675
}
679676

677+
const testName = "vscode-args"
680678
beforeAll(async () => {
681679
// Clean up temporary directories from the previous run.
682-
await clean("vscode-args")
680+
await clean(testName)
683681
})
684682

685683
it("should convert empty args", async () => {
@@ -691,7 +689,7 @@ describe("toVsCodeArgs", () => {
691689
})
692690

693691
it("should convert with workspace", async () => {
694-
const workspace = path.join(await tmpdir("vscode-args"), "test.code-workspace")
692+
const workspace = path.join(await tmpdir(testName), "test.code-workspace")
695693
await fs.writeFile(workspace, "foobar")
696694
expect(await toVsCodeArgs(await setDefaults(parse([workspace])))).toStrictEqual({
697695
...vscodeDefaults,
@@ -702,7 +700,7 @@ describe("toVsCodeArgs", () => {
702700
})
703701

704702
it("should convert with folder", async () => {
705-
const folder = await tmpdir("vscode-args")
703+
const folder = await tmpdir(testName)
706704
expect(await toVsCodeArgs(await setDefaults(parse([folder])))).toStrictEqual({
707705
...vscodeDefaults,
708706
folder,
@@ -712,7 +710,7 @@ describe("toVsCodeArgs", () => {
712710
})
713711

714712
it("should ignore regular file", async () => {
715-
const file = path.join(await tmpdir("vscode-args"), "file")
713+
const file = path.join(await tmpdir(testName), "file")
716714
await fs.writeFile(file, "foobar")
717715
expect(await toVsCodeArgs(await setDefaults(parse([file])))).toStrictEqual({
718716
...vscodeDefaults,

test/unit/node/routes/static.test.ts

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
import { promises as fs } from "fs"
22
import * as path from "path"
33
import { rootPath } from "../../../../src/node/constants"
4-
import { tmpdir } from "../../../utils/helpers"
4+
import { clean, tmpdir } from "../../../utils/helpers"
55
import * as httpserver from "../../../utils/httpserver"
66
import * as integration from "../../../utils/integration"
77

@@ -23,8 +23,10 @@ describe("/_static", () => {
2323
let testFileContent: string | undefined
2424
let nonExistentTestFile: string | undefined
2525

26+
const testName = "_static"
2627
beforeAll(async () => {
27-
const testDir = await tmpdir("_static")
28+
await clean(testName)
29+
const testDir = await tmpdir(testName)
2830
testFile = path.join(testDir, "test")
2931
testFileContent = "static file contents"
3032
nonExistentTestFile = path.join(testDir, "i-am-not-here")

test/unit/node/update.test.ts

Lines changed: 29 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
1-
import { promises as fs } from "fs"
21
import * as http from "http"
32
import * as path from "path"
4-
import { tmpdir } from "../../../src/node/constants"
3+
import { clean, tmpdir } from "../../utils/helpers"
54
import { SettingsProvider, UpdateSettings } from "../../../src/node/settings"
65
import { LatestResponse, UpdateProvider } from "../../../src/node/update"
76

@@ -29,22 +28,29 @@ describe("update", () => {
2928
response.end("not found")
3029
})
3130

32-
const jsonPath = path.join(tmpdir, "tests/updates/update.json")
33-
const settings = new SettingsProvider<UpdateSettings>(jsonPath)
31+
let _settings: SettingsProvider<UpdateSettings> | undefined
32+
const settings = (): SettingsProvider<UpdateSettings> => {
33+
if (!_settings) {
34+
throw new Error("Settings provider has not been created")
35+
}
36+
return _settings
37+
}
3438

3539
let _provider: UpdateProvider | undefined
3640
const provider = (): UpdateProvider => {
3741
if (!_provider) {
38-
const address = server.address()
39-
if (!address || typeof address === "string" || !address.port) {
40-
throw new Error("unexpected address")
41-
}
42-
_provider = new UpdateProvider(`http://${address.address}:${address.port}/latest`, settings)
42+
throw new Error("Update provider has not been created")
4343
}
4444
return _provider
4545
}
4646

4747
beforeAll(async () => {
48+
const testName = "update"
49+
await clean(testName)
50+
const testDir = await tmpdir(testName)
51+
const jsonPath = path.join(testDir, "update.json")
52+
_settings = new SettingsProvider<UpdateSettings>(jsonPath)
53+
4854
await new Promise((resolve, reject) => {
4955
server.on("error", reject)
5056
server.on("listening", resolve)
@@ -53,8 +59,13 @@ describe("update", () => {
5359
host: "localhost",
5460
})
5561
})
56-
await fs.rmdir(path.join(tmpdir, "tests/updates"), { recursive: true })
57-
await fs.mkdir(path.join(tmpdir, "tests/updates"), { recursive: true })
62+
63+
const address = server.address()
64+
if (!address || typeof address === "string" || !address.port) {
65+
throw new Error("unexpected address")
66+
}
67+
68+
_provider = new UpdateProvider(`http://${address.address}:${address.port}/latest`, _settings)
5869
})
5970

6071
afterAll(() => {
@@ -72,7 +83,7 @@ describe("update", () => {
7283
const now = Date.now()
7384
const update = await p.getUpdate()
7485

75-
await expect(settings.read()).resolves.toEqual({ update })
86+
await expect(settings().read()).resolves.toEqual({ update })
7687
expect(isNaN(update.checked)).toEqual(false)
7788
expect(update.checked < Date.now() && update.checked >= now).toEqual(true)
7889
expect(update.version).toStrictEqual("2.1.0")
@@ -86,7 +97,7 @@ describe("update", () => {
8697
const now = Date.now()
8798
const update = await p.getUpdate()
8899

89-
await expect(settings.read()).resolves.toEqual({ update })
100+
await expect(settings().read()).resolves.toEqual({ update })
90101
expect(isNaN(update.checked)).toStrictEqual(false)
91102
expect(update.checked < now).toBe(true)
92103
expect(update.version).toStrictEqual("2.1.0")
@@ -100,7 +111,7 @@ describe("update", () => {
100111
const now = Date.now()
101112
const update = await p.getUpdate(true)
102113

103-
await expect(settings.read()).resolves.toEqual({ update })
114+
await expect(settings().read()).resolves.toEqual({ update })
104115
expect(isNaN(update.checked)).toStrictEqual(false)
105116
expect(update.checked < Date.now() && update.checked >= now).toStrictEqual(true)
106117
expect(update.version).toStrictEqual("4.1.1")
@@ -113,12 +124,12 @@ describe("update", () => {
113124
expect(spy).toEqual([])
114125

115126
let checked = Date.now() - 1000 * 60 * 60 * 23
116-
await settings.write({ update: { checked, version } })
127+
await settings().write({ update: { checked, version } })
117128
await p.getUpdate()
118129
expect(spy).toEqual([])
119130

120131
checked = Date.now() - 1000 * 60 * 60 * 25
121-
await settings.write({ update: { checked, version } })
132+
await settings().write({ update: { checked, version } })
122133

123134
const update = await p.getUpdate()
124135
expect(update.checked).not.toStrictEqual(checked)
@@ -143,14 +154,14 @@ describe("update", () => {
143154
})
144155

145156
it("should not reject if unable to fetch", async () => {
146-
let provider = new UpdateProvider("invalid", settings)
157+
let provider = new UpdateProvider("invalid", settings())
147158
let now = Date.now()
148159
let update = await provider.getUpdate(true)
149160
expect(isNaN(update.checked)).toStrictEqual(false)
150161
expect(update.checked < Date.now() && update.checked >= now).toEqual(true)
151162
expect(update.version).toStrictEqual("unknown")
152163

153-
provider = new UpdateProvider("http://probably.invalid.dev.localhost/latest", settings)
164+
provider = new UpdateProvider("http://probably.invalid.dev.localhost/latest", settings())
154165
now = Date.now()
155166
update = await provider.getUpdate(true)
156167
expect(isNaN(update.checked)).toStrictEqual(false)

test/utils/helpers.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,8 @@ export async function clean(testName: string): Promise<void> {
3131

3232
/**
3333
* Create a uniquely named temporary directory for a test.
34+
*
35+
* `tmpdir` should usually be preceeded by at least one call to `clean`.
3436
*/
3537
export async function tmpdir(testName: string): Promise<string> {
3638
const dir = path.join(os.tmpdir(), `code-server/tests/${testName}`)

0 commit comments

Comments
 (0)