Skip to content

Commit 5165ade

Browse files
EhabYEhabY
authored
Fix unit tests on windows (#617)
Closes #604
1 parent a401805 commit 5165ade

File tree

11 files changed

+315
-160
lines changed

11 files changed

+315
-160
lines changed

‎package.json‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@
2424
"lint:fix": "yarn lint --fix",
2525
"package": "webpack --mode production --devtool hidden-source-map",
2626
"package:prerelease": "npx vsce package --pre-release",
27-
"pretest": "tsc -p . --outDir out && yarn run build && yarn run lint",
27+
"pretest": "tsc -p . --outDir out && tsc -p test --outDir out && yarn run build && yarn run lint",
2828
"test": "vitest",
2929
"test:ci": "CI=true yarn test",
3030
"test:integration": "vscode-test",

‎test/fixtures/bin.bash‎ renamed to ‎test/fixtures/scripts/bin.bash‎

File renamed without changes.

‎test/fixtures/bin.old.bash‎ renamed to ‎test/fixtures/scripts/bin.old.bash‎

File renamed without changes.

‎test/unit/core/cliManager.test.ts‎

Lines changed: 19 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ import {
2020
MockProgressReporter,
2121
MockUserInteraction,
2222
} from "../../mocks/testHelpers";
23+
import { expectPathsEqual } from "../../utils/platform";
2324

2425
vi.mock("os");
2526
vi.mock("axios");
@@ -213,7 +214,7 @@ describe("CliManager", () => {
213214
it("accepts valid semver versions", async () => {
214215
withExistingBinary(TEST_VERSION);
215216
const result = await manager.fetchBinary(mockApi, "test");
216-
expect(result).toBe(BINARY_PATH);
217+
expectPathsEqual(result, BINARY_PATH);
217218
});
218219
});
219220

@@ -226,7 +227,7 @@ describe("CliManager", () => {
226227
it("reuses matching binary without downloading", async () => {
227228
withExistingBinary(TEST_VERSION);
228229
const result = await manager.fetchBinary(mockApi, "test");
229-
expect(result).toBe(BINARY_PATH);
230+
expectPathsEqual(result, BINARY_PATH);
230231
expect(mockAxios.get).not.toHaveBeenCalled();
231232
// Verify binary still exists
232233
expect(memfs.existsSync(BINARY_PATH)).toBe(true);
@@ -236,7 +237,7 @@ describe("CliManager", () => {
236237
withExistingBinary("1.0.0");
237238
withSuccessfulDownload();
238239
const result = await manager.fetchBinary(mockApi, "test");
239-
expect(result).toBe(BINARY_PATH);
240+
expectPathsEqual(result, BINARY_PATH);
240241
expect(mockAxios.get).toHaveBeenCalled();
241242
// Verify new binary exists
242243
expect(memfs.existsSync(BINARY_PATH)).toBe(true);
@@ -249,7 +250,7 @@ describe("CliManager", () => {
249250
mockConfig.set("coder.enableDownloads", false);
250251
withExistingBinary("1.0.0");
251252
const result = await manager.fetchBinary(mockApi, "test");
252-
expect(result).toBe(BINARY_PATH);
253+
expectPathsEqual(result, BINARY_PATH);
253254
expect(mockAxios.get).not.toHaveBeenCalled();
254255
// Should still have the old version
255256
expect(memfs.existsSync(BINARY_PATH)).toBe(true);
@@ -262,7 +263,7 @@ describe("CliManager", () => {
262263
withCorruptedBinary();
263264
withSuccessfulDownload();
264265
const result = await manager.fetchBinary(mockApi, "test");
265-
expect(result).toBe(BINARY_PATH);
266+
expectPathsEqual(result, BINARY_PATH);
266267
expect(mockAxios.get).toHaveBeenCalled();
267268
expect(memfs.existsSync(BINARY_PATH)).toBe(true);
268269
expect(memfs.readFileSync(BINARY_PATH).toString()).toBe(
@@ -276,7 +277,7 @@ describe("CliManager", () => {
276277

277278
withSuccessfulDownload();
278279
const result = await manager.fetchBinary(mockApi, "test");
279-
expect(result).toBe(BINARY_PATH);
280+
expectPathsEqual(result, BINARY_PATH);
280281
expect(mockAxios.get).toHaveBeenCalled();
281282

282283
// Verify directory was created and binary exists
@@ -392,7 +393,7 @@ describe("CliManager", () => {
392393
withExistingBinary("1.0.0");
393394
withHttpResponse(304);
394395
const result = await manager.fetchBinary(mockApi, "test");
395-
expect(result).toBe(BINARY_PATH);
396+
expectPathsEqual(result, BINARY_PATH);
396397
// No change
397398
expect(memfs.readFileSync(BINARY_PATH).toString()).toBe(
398399
mockBinaryContent("1.0.0"),
@@ -460,7 +461,7 @@ describe("CliManager", () => {
460461
it("handles missing content-length", async () => {
461462
withSuccessfulDownload({ headers: {} });
462463
const result = await manager.fetchBinary(mockApi, "test");
463-
expect(result).toBe(BINARY_PATH);
464+
expectPathsEqual(result, BINARY_PATH);
464465
expect(memfs.existsSync(BINARY_PATH)).toBe(true);
465466
});
466467
});
@@ -494,7 +495,7 @@ describe("CliManager", () => {
494495
withSuccessfulDownload();
495496
withSignatureResponses([200]);
496497
const result = await manager.fetchBinary(mockApi, "test");
497-
expect(result).toBe(BINARY_PATH);
498+
expectPathsEqual(result, BINARY_PATH);
498499
expect(pgp.verifySignature).toHaveBeenCalled();
499500
const sigFile = expectFileInDir(BINARY_DIR, ".asc");
500501
expect(sigFile).toBeDefined();
@@ -505,7 +506,7 @@ describe("CliManager", () => {
505506
withSignatureResponses([404, 200]);
506507
mockUI.setResponse("Signature not found", "Download signature");
507508
const result = await manager.fetchBinary(mockApi, "test");
508-
expect(result).toBe(BINARY_PATH);
509+
expectPathsEqual(result, BINARY_PATH);
509510
expect(mockAxios.get).toHaveBeenCalledTimes(3);
510511
const sigFile = expectFileInDir(BINARY_DIR, ".asc");
511512
expect(sigFile).toBeDefined();
@@ -519,7 +520,7 @@ describe("CliManager", () => {
519520
);
520521
mockUI.setResponse("Signature does not match", "Run anyway");
521522
const result = await manager.fetchBinary(mockApi, "test");
522-
expect(result).toBe(BINARY_PATH);
523+
expectPathsEqual(result, BINARY_PATH);
523524
expect(memfs.existsSync(BINARY_PATH)).toBe(true);
524525
});
525526

@@ -539,7 +540,7 @@ describe("CliManager", () => {
539540
mockConfig.set("coder.disableSignatureVerification", true);
540541
withSuccessfulDownload();
541542
const result = await manager.fetchBinary(mockApi, "test");
542-
expect(result).toBe(BINARY_PATH);
543+
expectPathsEqual(result, BINARY_PATH);
543544
expect(pgp.verifySignature).not.toHaveBeenCalled();
544545
const files = readdir(BINARY_DIR);
545546
expect(files.find((file) => file.includes(".asc"))).toBeUndefined();
@@ -553,7 +554,7 @@ describe("CliManager", () => {
553554
withHttpResponse(status);
554555
mockUI.setResponse(message, "Run without verification");
555556
const result = await manager.fetchBinary(mockApi, "test");
556-
expect(result).toBe(BINARY_PATH);
557+
expectPathsEqual(result, BINARY_PATH);
557558
expect(pgp.verifySignature).not.toHaveBeenCalled();
558559
});
559560

@@ -615,13 +616,16 @@ describe("CliManager", () => {
615616

616617
withSuccessfulDownload();
617618
const result = await manager.fetchBinary(mockApi, "test label");
618-
expect(result).toBe(`${pathWithSpaces}/test label/bin/${BINARY_NAME}`);
619+
expectPathsEqual(
620+
result,
621+
`${pathWithSpaces}/test label/bin/${BINARY_NAME}`,
622+
);
619623
});
620624

621625
it("handles empty deployment label", async () => {
622626
withExistingBinary(TEST_VERSION, "/path/base/bin");
623627
const result = await manager.fetchBinary(mockApi, "");
624-
expect(result).toBe(path.join(BASE_PATH, "bin", BINARY_NAME));
628+
expectPathsEqual(result, path.join(BASE_PATH, "bin", BINARY_NAME));
625629
});
626630
});
627631

‎test/unit/core/cliUtils.test.ts‎

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import { beforeAll, describe, expect, it } from "vitest";
66
import * as cliUtils from "@/core/cliUtils";
77

88
import { getFixturePath } from "../../utils/fixtures";
9+
import { isWindows } from "../../utils/platform";
910

1011
describe("CliUtils", () => {
1112
const tmp = path.join(os.tmpdir(), "vscode-coder-tests");
@@ -28,12 +29,14 @@ describe("CliUtils", () => {
2829
expect((await cliUtils.stat(binPath))?.size).toBe(4);
2930
});
3031

31-
// TODO: CI only runs on Linux but we should run it on Windows too.
32-
it("version", async () => {
32+
it.skipIf(isWindows())("version", async () => {
3333
const binPath = path.join(tmp, "version");
3434
await expect(cliUtils.version(binPath)).rejects.toThrow("ENOENT");
3535

36-
const binTmpl = await fs.readFile(getFixturePath("bin.bash"), "utf8");
36+
const binTmpl = await fs.readFile(
37+
getFixturePath("scripts", "bin.bash"),
38+
"utf8",
39+
);
3740
await fs.writeFile(binPath, binTmpl.replace("$ECHO", "hello"));
3841
await expect(cliUtils.version(binPath)).rejects.toThrow("EACCES");
3942

@@ -56,7 +59,10 @@ describe("CliUtils", () => {
5659
);
5760
expect(await cliUtils.version(binPath)).toBe("v0.0.0");
5861

59-
const oldTmpl = await fs.readFile(getFixturePath("bin.old.bash"), "utf8");
62+
const oldTmpl = await fs.readFile(
63+
getFixturePath("scripts", "bin.old.bash"),
64+
"utf8",
65+
);
6066
const old = (stderr: string, stdout: string): string => {
6167
return oldTmpl.replace("$STDERR", stderr).replace("$STDOUT", stdout);
6268
};

‎test/unit/core/pathResolver.test.ts‎

Lines changed: 18 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,10 @@
11
import * as path from "path";
2-
import { beforeEach, describe, expect, it, vi } from "vitest";
2+
import { beforeEach, describe, it, vi } from "vitest";
33

44
import { PathResolver } from "@/core/pathResolver";
55

66
import { MockConfigurationProvider } from "../../mocks/testHelpers";
7+
import { expectPathsEqual } from "../../utils/platform";
78

89
describe("PathResolver", () => {
910
const basePath =
@@ -19,32 +20,36 @@ describe("PathResolver", () => {
1920
});
2021

2122
it("should use base path for empty labels", () => {
22-
expect(pathResolver.getGlobalConfigDir("")).toBe(basePath);
23-
expect(pathResolver.getSessionTokenPath("")).toBe(
23+
expectPathsEqual(pathResolver.getGlobalConfigDir(""), basePath);
24+
expectPathsEqual(
25+
pathResolver.getSessionTokenPath(""),
2426
path.join(basePath, "session"),
2527
);
26-
expect(pathResolver.getUrlPath("")).toBe(path.join(basePath, "url"));
28+
expectPathsEqual(pathResolver.getUrlPath(""), path.join(basePath, "url"));
2729
});
2830

2931
describe("getBinaryCachePath", () => {
3032
it("should use custom binary destination when configured", () => {
3133
mockConfig.set("coder.binaryDestination", "/custom/binary/path");
32-
expect(pathResolver.getBinaryCachePath("deployment")).toBe(
34+
expectPathsEqual(
35+
pathResolver.getBinaryCachePath("deployment"),
3336
"/custom/binary/path",
3437
);
3538
});
3639

3740
it("should use default path when custom destination is empty or whitespace", () => {
3841
vi.stubEnv("CODER_BINARY_DESTINATION", " ");
3942
mockConfig.set("coder.binaryDestination", " ");
40-
expect(pathResolver.getBinaryCachePath("deployment")).toBe(
43+
expectPathsEqual(
44+
pathResolver.getBinaryCachePath("deployment"),
4145
path.join(basePath, "deployment", "bin"),
4246
);
4347
});
4448

4549
it("should normalize custom paths", () => {
4650
mockConfig.set("coder.binaryDestination", "/custom/../binary/./path");
47-
expect(pathResolver.getBinaryCachePath("deployment")).toBe(
51+
expectPathsEqual(
52+
pathResolver.getBinaryCachePath("deployment"),
4853
"/binary/path",
4954
);
5055
});
@@ -53,19 +58,22 @@ describe("PathResolver", () => {
5358
// Use the global storage when the environment variable and setting are unset/blank
5459
vi.stubEnv("CODER_BINARY_DESTINATION", "");
5560
mockConfig.set("coder.binaryDestination", "");
56-
expect(pathResolver.getBinaryCachePath("deployment")).toBe(
61+
expectPathsEqual(
62+
pathResolver.getBinaryCachePath("deployment"),
5763
path.join(basePath, "deployment", "bin"),
5864
);
5965

6066
// Test environment variable takes precedence over global storage
6167
vi.stubEnv("CODER_BINARY_DESTINATION", " /env/binary/path ");
62-
expect(pathResolver.getBinaryCachePath("deployment")).toBe(
68+
expectPathsEqual(
69+
pathResolver.getBinaryCachePath("deployment"),
6370
"/env/binary/path",
6471
);
6572

6673
// Test setting takes precedence over environment variable
6774
mockConfig.set("coder.binaryDestination", " /setting/path ");
68-
expect(pathResolver.getBinaryCachePath("deployment")).toBe(
75+
expectPathsEqual(
76+
pathResolver.getBinaryCachePath("deployment"),
6977
"/setting/path",
7078
);
7179
});

‎test/unit/globalFlags.test.ts‎

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,8 @@ import { type WorkspaceConfiguration } from "vscode";
33

44
import { getGlobalFlags } from "@/globalFlags";
55

6+
import { isWindows } from "../utils/platform";
7+
68
describe("Global flags suite", () => {
79
it("should return global-config and header args when no global flags configured", () => {
810
const config = {
@@ -53,10 +55,11 @@ describe("Global flags suite", () => {
5355
});
5456

5557
it("should not filter header-command flags, header args appended at end", () => {
58+
const headerCommand = "echo test";
5659
const config = {
5760
get: (key: string) => {
5861
if (key === "coder.headerCommand") {
59-
return "echo test";
62+
return headerCommand;
6063
}
6164
if (key === "coder.globalFlags") {
6265
return ["-v", "--header-command custom", "--no-feature-warning"];
@@ -73,7 +76,13 @@ describe("Global flags suite", () => {
7376
"--global-config",
7477
'"/config/dir"',
7578
"--header-command",
76-
"'echo test'",
79+
quoteCommand(headerCommand),
7780
]);
7881
});
7982
});
83+
84+
function quoteCommand(value: string): string {
85+
// Used to escape environment variables in commands. See `getHeaderArgs` in src/headers.ts
86+
const quote = isWindows() ? '"' : "'";
87+
return `${quote}${value}${quote}`;
88+
}

0 commit comments

Comments
 (0)