Skip to content

Commit 292bc47

Browse files
veelengaveelenga
authored
refactor: improve tests (#84)
* refactor: improve tests * refactor simplify setup * fix: delete XDG_CONFIG_HOME when test env opts out of setting it When setupTestEnv({ setXdgConfig: false }) is called, explicitly delete XDG_CONFIG_HOME so getConfigDir() falls back to $HOME/.config. Without this, a pre-existing XDG_CONFIG_HOME (e.g. from CI) causes diagram-service tests to look in a different directory than where test diagrams are created.
1 parent 5bef9f4 commit 292bc47

File tree

7 files changed

+181
-461
lines changed

7 files changed

+181
-461
lines changed

‎src/handlers.ts‎

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -74,11 +74,9 @@ export async function renderDiagram(options: RenderOptions, liveFilePath: string
7474
await copyFile(outputFile, liveFilePath);
7575
mcpLogger.info(`Diagram rendered successfully: ${previewId}`);
7676
} catch (error) {
77-
const stderr =
78-
error instanceof Error && "stderr" in error && (error as Error & { stderr: string }).stderr
79-
? `\n${(error as Error & { stderr: string }).stderr}`
80-
: "";
8177
const message = error instanceof Error ? error.message : String(error);
78+
const stderrValue = error instanceof Error && "stderr" in error ? (error as any).stderr : "";
79+
const stderr = stderrValue ? `\n${stderrValue}` : "";
8280
mcpLogger.error(`Diagram rendering failed: ${previewId}`, { error: message });
8381
throw new Error(`${message}${stderr}`);
8482
}

‎test/diagram-service.test.ts‎

Lines changed: 5 additions & 52 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
* Tests business logic for diagram data access and management
44
*/
55

6-
import { describe, it, expect, beforeEach, afterEach, vi } from "vitest";
6+
import { describe, it, expect, beforeEach, afterEach } from "vitest";
77
import {
88
listDiagrams,
99
getDiagramInfo,
@@ -12,70 +12,23 @@ import {
1212
getDiagramCount,
1313
deleteDiagram,
1414
} from "../src/diagram-service.js";
15-
import * as fileUtils from "../src/file-utils.js";
16-
import { readdir, stat, mkdir, writeFile, rmdir, unlink, utimes } from "fs/promises";
15+
import { stat, mkdir, writeFile, utimes } from "fs/promises";
1716
import { join } from "path";
1817
import { tmpdir } from "os";
18+
import { setupTestEnv, restoreTestEnv } from "./helpers/env-helpers.js";
1919

2020
describe("Diagram Service", () => {
2121
let testHomeDir: string;
2222
let testLiveDir: string;
23-
let originalHome: string | undefined;
24-
let originalXdgConfigHome: string | undefined;
2523

2624
beforeEach(async () => {
27-
// Save original environment
28-
originalHome = process.env.HOME;
29-
originalXdgConfigHome = process.env.XDG_CONFIG_HOME;
30-
31-
// Clear XDG_CONFIG_HOME to ensure HOME is used
32-
delete process.env.XDG_CONFIG_HOME;
33-
34-
// Create temporary HOME directory to isolate tests
35-
testHomeDir = join(tmpdir(), `diagram-service-test-home-${Date.now()}`);
36-
process.env.HOME = testHomeDir;
37-
38-
// Live dir will be: testHomeDir/.config/claude-mermaid/live
25+
testHomeDir = await setupTestEnv({ setXdgConfig: false });
3926
testLiveDir = join(testHomeDir, ".config", "claude-mermaid", "live");
4027
await mkdir(testLiveDir, { recursive: true });
4128
});
4229

4330
afterEach(async () => {
44-
// Restore original environment
45-
if (originalHome) {
46-
process.env.HOME = originalHome;
47-
} else {
48-
delete process.env.HOME;
49-
}
50-
51-
if (originalXdgConfigHome) {
52-
process.env.XDG_CONFIG_HOME = originalXdgConfigHome;
53-
} else {
54-
delete process.env.XDG_CONFIG_HOME;
55-
}
56-
57-
// Cleanup test directory
58-
try {
59-
const entries = await readdir(testLiveDir, { withFileTypes: true });
60-
for (const entry of entries) {
61-
const fullPath = join(testLiveDir, entry.name);
62-
if (entry.isDirectory()) {
63-
const files = await readdir(fullPath);
64-
for (const file of files) {
65-
await unlink(join(fullPath, file));
66-
}
67-
await rmdir(fullPath);
68-
} else {
69-
await unlink(fullPath);
70-
}
71-
}
72-
await rmdir(testLiveDir);
73-
await rmdir(join(testHomeDir, ".config", "claude-mermaid"));
74-
await rmdir(join(testHomeDir, ".config"));
75-
await rmdir(testHomeDir);
76-
} catch (error) {
77-
// Ignore cleanup errors
78-
}
31+
await restoreTestEnv();
7932
});
8033

8134
describe("listDiagrams", () => {

‎test/file-utils.test.ts‎

Lines changed: 6 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -11,26 +11,18 @@ import {
1111
getConfigDir,
1212
validateSavePath,
1313
} from "../src/file-utils.js";
14-
import { writeFile, unlink, mkdir, rmdir, readdir, mkdtemp } from "fs/promises";
14+
import { unlink, mkdir, rmdir, readdir, mkdtemp, rm } from "fs/promises";
1515
import { tmpdir } from "os";
1616
import { join } from "path";
17+
import { setupTestEnv, restoreTestEnv } from "./helpers/env-helpers.js";
1718

1819
describe("File Utilities", () => {
19-
let originalHome: string | undefined;
20-
21-
// Ensure tests operate in a temporary HOME to avoid touching real config
2220
beforeAll(async () => {
23-
originalHome = process.env.HOME;
24-
const tempHome = await mkdtemp(join(tmpdir(), "claude-mermaid-test-home-"));
25-
process.env.HOME = tempHome;
21+
await setupTestEnv();
2622
});
2723

28-
afterAll(() => {
29-
if (originalHome) {
30-
process.env.HOME = originalHome;
31-
} else {
32-
delete process.env.HOME;
33-
}
24+
afterAll(async () => {
25+
await restoreTestEnv();
3426
});
3527
describe("getLiveDir", () => {
3628
it("should return path containing .config/claude-mermaid/live", () => {
@@ -217,15 +209,7 @@ describe("File Utilities", () => {
217209
});
218210

219211
afterEach(async () => {
220-
try {
221-
const files = await readdir(testDir);
222-
for (const file of files) {
223-
await unlink(join(testDir, file));
224-
}
225-
await rmdir(testDir);
226-
} catch {
227-
// Ignore errors
228-
}
212+
await rm(testDir, { recursive: true, force: true }).catch(() => {});
229213
});
230214

231215
it("should save and load diagram source", async () => {

‎test/handlers.test.ts‎

Lines changed: 33 additions & 113 deletions
Original file line numberDiff line numberDiff line change
@@ -1,37 +1,30 @@
11
import { describe, it, expect, vi, beforeEach, afterEach } from "vitest";
22
import { handleMermaidPreview, handleMermaidSave } from "../src/handlers.js";
33
import { getPreviewDir, getDiagramFilePath } from "../src/file-utils.js";
4-
import { mkdir, readdir, unlink, access, mkdtemp } from "fs/promises";
5-
import { join } from "path";
4+
import { readdir, unlink, access } from "fs/promises";
65
import { execFile } from "child_process";
7-
import { tmpdir } from "os";
6+
import { setupTestEnvWithPreview, restoreTestEnv } from "./helpers/env-helpers.js";
87

98
// Mock execFile to avoid actually running mmdc and create fake output files
109
vi.mock("child_process", () => ({
1110
execFile: vi.fn((_file: string, args: string[], callback: Function) => {
12-
// Find the output file from args array
1311
const outputIndex = args.indexOf("-o");
1412
if (outputIndex !== -1 && outputIndex + 1 < args.length) {
1513
const outputFile = args[outputIndex + 1];
16-
// Create a fake output file synchronously
1714
const fs = require("fs");
1815
const path = require("path");
1916
const dir = path.dirname(outputFile);
2017

21-
// Ensure directory exists
2218
if (!fs.existsSync(dir)) {
2319
fs.mkdirSync(dir, { recursive: true });
2420
}
2521

26-
// Write fake content based on file extension
2722
const ext = path.extname(outputFile);
2823
if (ext === ".svg") {
2924
fs.writeFileSync(outputFile, "<svg>test</svg>", "utf-8");
3025
} else if (ext === ".png") {
31-
// Write minimal PNG header
3226
fs.writeFileSync(outputFile, Buffer.from([137, 80, 78, 71, 13, 10, 26, 10]));
3327
} else if (ext === ".pdf") {
34-
// Write minimal PDF header
3528
fs.writeFileSync(outputFile, "%PDF-1.4\n", "utf-8");
3629
} else {
3730
fs.writeFileSync(outputFile, "test", "utf-8");
@@ -44,7 +37,6 @@ vi.mock("child_process", () => ({
4437
}),
4538
}));
4639

47-
// Mock live server functions
4840
vi.mock("../src/live-server.js", () => ({
4941
ensureLiveServer: vi.fn(async () => 3737),
5042
addLiveDiagram: vi.fn(async () => {}),
@@ -54,53 +46,24 @@ vi.mock("../src/live-server.js", () => ({
5446
describe("handleMermaidPreview", () => {
5547
const testPreviewId = "test-preview";
5648
let testDir: string;
57-
let originalHome: string | undefined;
58-
let originalXdgConfig: string | undefined;
5949

6050
beforeEach(async () => {
61-
// Override config dirs to use a temp HOME/XDG path for isolation
62-
originalHome = process.env.HOME;
63-
originalXdgConfig = process.env.XDG_CONFIG_HOME;
64-
const tempHome = await mkdtemp(join(tmpdir(), "claude-mermaid-test-home-"));
65-
const tempConfigDir = join(tempHome, ".config");
66-
process.env.HOME = tempHome;
67-
process.env.XDG_CONFIG_HOME = tempConfigDir;
68-
69-
await mkdir(tempConfigDir, { recursive: true });
70-
testDir = getPreviewDir(testPreviewId);
71-
await mkdir(testDir, { recursive: true });
51+
testDir = await setupTestEnvWithPreview(testPreviewId);
7252
});
7353

7454
afterEach(async () => {
75-
// Restore original config env vars
76-
if (originalHome) {
77-
process.env.HOME = originalHome;
78-
} else {
79-
delete process.env.HOME;
80-
}
81-
82-
if (originalXdgConfig) {
83-
process.env.XDG_CONFIG_HOME = originalXdgConfig;
84-
} else {
85-
delete process.env.XDG_CONFIG_HOME;
86-
}
55+
await restoreTestEnv();
8756
});
8857

8958
it("should throw error when diagram parameter is missing", async () => {
9059
await expect(
91-
handleMermaidPreview({
92-
diagram: undefined,
93-
preview_id: testPreviewId,
94-
})
60+
handleMermaidPreview({ diagram: undefined, preview_id: testPreviewId })
9561
).rejects.toThrow("diagram parameter is required");
9662
});
9763

9864
it("should throw error when preview_id parameter is missing", async () => {
9965
await expect(
100-
handleMermaidPreview({
101-
diagram: "graph TD; A-->B",
102-
preview_id: undefined,
103-
})
66+
handleMermaidPreview({ diagram: "graph TD; A-->B", preview_id: undefined })
10467
).rejects.toThrow("preview_id parameter is required");
10568
});
10669

@@ -176,69 +139,44 @@ describe("handleMermaidPreview", () => {
176139

177140
it("should include stderr details in error when rendering fails", async () => {
178141
const mockExecFile = vi.mocked(execFile);
179-
const originalImpl = mockExecFile.getMockImplementation()!;
180-
181-
try {
182-
mockExecFile.mockImplementation((_file: string, _args: any, callback: any) => {
183-
const error: any = new Error("Command failed: npx mmdc");
184-
error.stderr = "Parse error on line 3: invalid syntax near 'graph'";
185-
callback(error, { stdout: "", stderr: error.stderr });
186-
});
142+
mockExecFile.mockImplementationOnce((_file: string, _args: any, callback: any) => {
143+
const error: any = new Error("Command failed: npx mmdc");
144+
error.stderr = "Parse error on line 3: invalid syntax near 'graph'";
145+
callback(error, { stdout: "", stderr: error.stderr });
146+
});
187147

188-
const result = await handleMermaidPreview({
189-
diagram: "invalid diagram syntax",
190-
preview_id: testPreviewId,
191-
});
148+
const result = await handleMermaidPreview({
149+
diagram: "invalid diagram syntax",
150+
preview_id: testPreviewId,
151+
});
192152

193-
expect(result.isError).toBe(true);
194-
expect(result.content[0].text).toContain("Parse error on line 3");
195-
expect(result.content[0].text).toContain("Command failed");
196-
} finally {
197-
mockExecFile.mockImplementation(originalImpl);
198-
}
153+
expect(result.isError).toBe(true);
154+
expect(result.content[0].text).toContain("Parse error on line 3");
155+
expect(result.content[0].text).toContain("Command failed");
199156
});
200157

201158
it("should show original error message when stderr is empty", async () => {
202159
const mockExecFile = vi.mocked(execFile);
203-
const originalImpl = mockExecFile.getMockImplementation()!;
204-
205-
try {
206-
mockExecFile.mockImplementation((_file: string, _args: any, callback: any) => {
207-
const error = new Error("Command failed: npx mmdc");
208-
callback(error, { stdout: "", stderr: "" });
209-
});
160+
mockExecFile.mockImplementationOnce((_file: string, _args: any, callback: any) => {
161+
const error = new Error("Command failed: npx mmdc");
162+
callback(error, { stdout: "", stderr: "" });
163+
});
210164

211-
const result = await handleMermaidPreview({
212-
diagram: "invalid diagram syntax",
213-
preview_id: testPreviewId,
214-
});
165+
const result = await handleMermaidPreview({
166+
diagram: "invalid diagram syntax",
167+
preview_id: testPreviewId,
168+
});
215169

216-
expect(result.isError).toBe(true);
217-
expect(result.content[0].text).toContain("Command failed");
218-
} finally {
219-
mockExecFile.mockImplementation(originalImpl);
220-
}
170+
expect(result.isError).toBe(true);
171+
expect(result.content[0].text).toContain("Command failed");
221172
});
222173
});
223174

224175
describe("handleMermaidSave", () => {
225176
const testPreviewId = "test-save";
226-
let testDir: string;
227-
let originalHome: string | undefined;
228-
let originalXdgConfig: string | undefined;
229177

230178
beforeEach(async () => {
231-
// Override config dirs to use a temp HOME/XDG path for isolation
232-
originalHome = process.env.HOME;
233-
originalXdgConfig = process.env.XDG_CONFIG_HOME;
234-
const tempHome = await mkdtemp(join(tmpdir(), "claude-mermaid-test-home-"));
235-
const tempConfigDir = join(tempHome, ".config");
236-
process.env.HOME = tempHome;
237-
process.env.XDG_CONFIG_HOME = tempConfigDir;
238-
239-
await mkdir(tempConfigDir, { recursive: true });
240-
testDir = getPreviewDir(testPreviewId);
241-
await mkdir(testDir, { recursive: true });
179+
await setupTestEnvWithPreview(testPreviewId);
242180

243181
await handleMermaidPreview({
244182
diagram: "graph TD; A-->B",
@@ -248,35 +186,18 @@ describe("handleMermaidSave", () => {
248186
});
249187

250188
afterEach(async () => {
251-
// Restore original config env vars
252-
if (originalHome) {
253-
process.env.HOME = originalHome;
254-
} else {
255-
delete process.env.HOME;
256-
}
257-
258-
if (originalXdgConfig) {
259-
process.env.XDG_CONFIG_HOME = originalXdgConfig;
260-
} else {
261-
delete process.env.XDG_CONFIG_HOME;
262-
}
189+
await restoreTestEnv();
263190
});
264191

265192
it("should throw error when save_path parameter is missing", async () => {
266193
await expect(
267-
handleMermaidSave({
268-
save_path: undefined,
269-
preview_id: testPreviewId,
270-
})
194+
handleMermaidSave({ save_path: undefined, preview_id: testPreviewId })
271195
).rejects.toThrow("save_path parameter is required");
272196
});
273197

274198
it("should throw error when preview_id parameter is missing", async () => {
275199
await expect(
276-
handleMermaidSave({
277-
save_path: "./test.svg",
278-
preview_id: undefined,
279-
})
200+
handleMermaidSave({ save_path: "./test.svg", preview_id: undefined })
280201
).rejects.toThrow("preview_id parameter is required");
281202
});
282203

@@ -320,10 +241,9 @@ describe("handleMermaidSave", () => {
320241
});
321242

322243
it("should handle missing diagram source when saving", async () => {
323-
const nonExistentId = "non-existent-preview";
324244
const result = await handleMermaidSave({
325245
save_path: "/tmp/test-diagram.svg",
326-
preview_id: nonExistentId,
246+
preview_id: "non-existent-preview",
327247
});
328248

329249
expect(result.isError).toBe(true);

0 commit comments

Comments
 (0)