Skip to content

Commit b20f20a

Browse files
webfilteredwebfiltered
authored
[Test] Fix state leakage between unit tests (#978)
- Unit tests are now isolated by performing a full reset of all mocks. - Default mocks are now added using the proper default mock syntax, rather than adding a resettable mock at the start of a file.
1 parent 6aca70f commit b20f20a

10 files changed

+104
-103
lines changed

‎tests/unit/config/comfyServerConfig.test.ts

+10-10
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ import { mkdtemp, readFile, rm, writeFile } from 'node:fs/promises';
55
import fsPromises from 'node:fs/promises';
66
import { tmpdir } from 'node:os';
77
import path from 'node:path';
8-
import { afterAll, afterEach, beforeAll, describe, expect, it, vi } from 'vitest';
8+
import { afterAll, afterEach, beforeAll, beforeEach, describe, expect, it, vi } from 'vitest';
99

1010
import { ComfyServerConfig } from '@/config/comfyServerConfig';
1111

@@ -16,7 +16,7 @@ vi.mock('electron', () => ({
1616
}));
1717

1818
vi.mock('@/install/resourcePaths', () => ({
19-
getAppResourcesPath: vi.fn().mockReturnValue('/mocked/app_resources'),
19+
getAppResourcesPath: vi.fn(() => '/mocked/app_resources'),
2020
}));
2121

2222
async function createTmpDir() {
@@ -30,13 +30,17 @@ async function copyFixture(fixturePath: string, targetPath: string) {
3030
}
3131

3232
describe('ComfyServerConfig', () => {
33+
const mockUserDataPath = '/fake/user/data';
3334
let tempDir = '';
3435

3536
beforeAll(async () => {
3637
tempDir = await createTmpDir();
37-
vi.mocked(app.getPath).mockImplementation((name: string) => {
38-
if (name === 'userData') return '/fake/user/data';
39-
throw new Error(`Unexpected getPath key: ${name}`);
38+
});
39+
40+
beforeEach(() => {
41+
vi.mocked(app.getPath).mockImplementation((key: string) => {
42+
if (key === 'userData') return '/fake/user/data';
43+
throw new Error(`Unexpected getPath key: ${key}`);
4044
});
4145
});
4246

@@ -46,17 +50,13 @@ describe('ComfyServerConfig', () => {
4650

4751
afterEach(() => {
4852
vi.clearAllMocks();
49-
vi.unstubAllGlobals();
5053
});
5154

5255
describe('configPath', () => {
5356
it('should return the correct path', () => {
54-
const mockUserDataPath = '/fake/user/data';
5557
const { getPath } = app;
5658
vi.mocked(getPath).mockImplementation((key: string) => {
57-
if (key === 'userData') {
58-
return mockUserDataPath;
59-
}
59+
if (key === 'userData') return mockUserDataPath;
6060
throw new Error(`Unexpected getPath key: ${key}`);
6161
});
6262

‎tests/unit/desktopApp.test.ts

+16-25
Original file line numberDiff line numberDiff line change
@@ -25,8 +25,8 @@ vi.mock('electron', () => ({
2525
exit: vi.fn(() => {
2626
throw new Error('Test exited via app.exit()');
2727
}),
28-
getPath: vi.fn().mockReturnValue('/mock/app/path'),
29-
getAppPath: vi.fn().mockReturnValue('/mock/app/path'),
28+
getPath: vi.fn(() => '/mock/app/path'),
29+
getAppPath: vi.fn(() => '/mock/app/path'),
3030
},
3131
dialog: {
3232
showErrorBox: vi.fn(),
@@ -41,14 +41,14 @@ vi.mock('electron', () => ({
4141
}));
4242

4343
const mockAppWindow = {
44-
loadPage: vi.fn().mockResolvedValue(undefined),
44+
loadPage: vi.fn(),
4545
send: vi.fn(),
4646
sendServerStartProgress: vi.fn(),
47-
loadComfyUI: vi.fn().mockResolvedValue(undefined),
47+
loadComfyUI: vi.fn(),
4848
};
4949

5050
vi.mock('@/main-process/appWindow', () => ({
51-
AppWindow: vi.fn().mockImplementation(() => mockAppWindow),
51+
AppWindow: vi.fn(() => mockAppWindow),
5252
}));
5353

5454
vi.mock('@/config/comfySettings', () => ({
@@ -59,18 +59,18 @@ vi.mock('@/config/comfySettings', () => ({
5959
saveSettings: vi.fn(),
6060
}),
6161
},
62-
useComfySettings: vi.fn().mockReturnValue({
63-
get: vi.fn().mockReturnValue('true'),
62+
useComfySettings: vi.fn(() => ({
63+
get: vi.fn(),
6464
set: vi.fn(),
6565
saveSettings: vi.fn(),
66-
}),
66+
})),
6767
}));
6868

6969
vi.mock('@/store/desktopConfig', () => ({
70-
useDesktopConfig: vi.fn().mockReturnValue({
71-
get: vi.fn().mockReturnValue('/mock/path'),
70+
useDesktopConfig: vi.fn(() => ({
71+
get: vi.fn(() => '/mock/path'),
7272
set: vi.fn(),
73-
}),
73+
})),
7474
}));
7575

7676
const mockInstallation: Partial<ComfyInstallation> = {
@@ -84,7 +84,7 @@ const mockInstallation: Partial<ComfyInstallation> = {
8484
};
8585

8686
const mockInstallationManager = {
87-
ensureInstalled: vi.fn().mockResolvedValue(mockInstallation),
87+
ensureInstalled: vi.fn(() => Promise.resolve(mockInstallation)),
8888
};
8989
vi.mock('@/install/installationManager', () => ({
9090
InstallationManager: Object.assign(
@@ -94,29 +94,20 @@ vi.mock('@/install/installationManager', () => ({
9494
}));
9595

9696
const mockComfyDesktopApp = {
97-
buildServerArgs: vi.fn().mockResolvedValue({ port: '8188' }),
98-
startComfyServer: vi.fn().mockResolvedValue(undefined),
97+
buildServerArgs: vi.fn(),
98+
startComfyServer: vi.fn(),
9999
};
100100
vi.mock('@/main-process/comfyDesktopApp', () => ({
101-
ComfyDesktopApp: vi.fn().mockImplementation(() => mockComfyDesktopApp),
101+
ComfyDesktopApp: vi.fn(() => mockComfyDesktopApp),
102102
}));
103103

104104
vi.mock('@/services/sentry', () => ({
105105
default: {
106-
setSentryGpuContext: vi.fn().mockResolvedValue(undefined),
106+
setSentryGpuContext: vi.fn(),
107107
getBasePath: vi.fn(),
108108
},
109109
}));
110110

111-
vi.mock('@/services/telemetry', () => ({
112-
getTelemetry: vi.fn().mockReturnValue({
113-
hasConsent: false,
114-
track: vi.fn(),
115-
flush: vi.fn(),
116-
}),
117-
promptMetricsConsent: vi.fn().mockResolvedValue(true),
118-
}));
119-
120111
describe('DesktopApp', () => {
121112
let desktopApp: DesktopApp;
122113
let mockOverrides: Partial<DevOverrides>;

‎tests/unit/handlers/appinfoHandlers.test.ts

+7-7
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ vi.mock('electron', () => ({
1212
app: {
1313
isPackaged: false,
1414
getPath: vi.fn(),
15-
getVersion: vi.fn().mockReturnValue('1.0.0'),
15+
getVersion: vi.fn(() => '1.0.0'),
1616
},
1717
ipcMain: {
1818
on: vi.fn(),
@@ -21,17 +21,17 @@ vi.mock('electron', () => ({
2121
}));
2222

2323
vi.mock('@/store/desktopConfig', () => ({
24-
useDesktopConfig: vi.fn().mockReturnValue({
25-
get: vi.fn().mockImplementation((key) => {
24+
useDesktopConfig: vi.fn(() => ({
25+
get: vi.fn((key: string) => {
2626
if (key === 'basePath') return MOCK_BASE_PATH;
2727
}),
28-
set: vi.fn().mockReturnValue(true),
29-
getAsync: vi.fn().mockImplementation((key) => {
28+
set: vi.fn(),
29+
getAsync: vi.fn((key: string) => {
3030
if (key === 'windowStyle') return Promise.resolve(MOCK_WINDOW_STYLE);
3131
if (key === 'detectedGpu') return Promise.resolve(MOCK_GPU_NAME);
3232
}),
33-
setAsync: vi.fn().mockReturnValue(Promise.resolve(true)),
34-
}),
33+
setAsync: vi.fn(),
34+
})),
3535
}));
3636

3737
vi.mock('@/config/comfyServerConfig', () => ({

‎tests/unit/install/installationManager.test.ts

+36-31
Original file line numberDiff line numberDiff line change
@@ -19,64 +19,67 @@ vi.mock('electron', () => ({
1919
on: vi.fn(),
2020
},
2121
app: {
22-
getPath: vi.fn().mockReturnValue('valid/path'),
22+
getPath: vi.fn(() => 'valid/path'),
2323
},
2424
}));
2525

2626
vi.mock('node:fs/promises', () => ({
2727
default: {
2828
access: vi.fn(),
29-
readFile: vi.fn().mockResolvedValue('{}'),
29+
readFile: vi.fn(() => Promise.resolve('{}')),
3030
},
3131
access: vi.fn(),
32-
readFile: vi.fn().mockResolvedValue('{}'),
32+
readFile: vi.fn(() => Promise.resolve('{}')),
3333
}));
3434

35-
vi.mock('@/store/desktopConfig', () => ({
36-
useDesktopConfig: vi.fn().mockReturnValue({
37-
get: vi.fn().mockImplementation((key: string) => {
38-
if (key === 'installState') return 'installed';
39-
if (key === 'basePath') return 'valid/base';
40-
}),
41-
set: vi.fn().mockImplementation((key: string, value: string) => {
42-
if (key !== 'basePath') throw new Error(`Unexpected key: ${key}`);
43-
if (!value) throw new Error(`Unexpected value: [${value}]`);
44-
}),
35+
const config = {
36+
get: vi.fn((key: string) => {
37+
if (key === 'installState') return 'installed';
38+
if (key === 'basePath') return 'valid/base';
39+
}),
40+
set: vi.fn((key: string, value: string) => {
41+
if (key !== 'basePath') throw new Error(`Unexpected key: ${key}`);
42+
if (!value) throw new Error(`Unexpected value: [${value}]`);
4543
}),
44+
};
45+
vi.mock('@/store/desktopConfig', () => ({
46+
useDesktopConfig: vi.fn(() => config),
4647
}));
4748

4849
vi.mock('@/utils', async () => {
4950
const actual = await vi.importActual<typeof utils>('@/utils');
5051
return {
5152
...actual,
52-
pathAccessible: vi.fn().mockImplementation((path: string) => {
53+
pathAccessible: vi.fn((path: string) => {
5354
const isValid = path.startsWith('valid/') || path.endsWith(`\\System32\\vcruntime140.dll`);
5455
return Promise.resolve(isValid);
5556
}),
56-
canExecute: vi.fn().mockResolvedValue(true),
57-
canExecuteShellCommand: vi.fn().mockResolvedValue(true),
57+
canExecute: vi.fn(() => Promise.resolve(true)),
58+
canExecuteShellCommand: vi.fn(() => Promise.resolve(true)),
5859
};
5960
});
6061

6162
vi.mock('@/config/comfyServerConfig', () => {
6263
return {
6364
ComfyServerConfig: {
6465
configPath: 'valid/extra_models_config.yaml',
65-
exists: vi.fn().mockReturnValue(true),
66-
readBasePathFromConfig: vi.fn().mockResolvedValue({
67-
status: 'success',
68-
path: 'valid/base',
69-
}),
66+
exists: vi.fn(() => Promise.resolve(true)),
67+
readBasePathFromConfig: vi.fn(() =>
68+
Promise.resolve({
69+
status: 'success',
70+
path: 'valid/base',
71+
})
72+
),
7073
},
7174
};
7275
});
7376

7477
// Mock VirtualEnvironment with basic implementation
7578
vi.mock('@/virtualEnvironment', () => {
7679
return {
77-
VirtualEnvironment: vi.fn().mockImplementation(() => ({
78-
exists: vi.fn().mockResolvedValue(true),
79-
hasRequirements: vi.fn().mockResolvedValue(true),
80+
VirtualEnvironment: vi.fn(() => ({
81+
exists: vi.fn(() => Promise.resolve(true)),
82+
hasRequirements: vi.fn(() => Promise.resolve(true)),
8083
pythonInterpreterPath: 'valid/python',
8184
uvPath: 'valid/uv',
8285
venvPath: 'valid/venv',
@@ -88,16 +91,16 @@ vi.mock('@/virtualEnvironment', () => {
8891

8992
// Mock Telemetry
9093
vi.mock('@/services/telemetry', () => ({
91-
getTelemetry: vi.fn().mockReturnValue({
94+
getTelemetry: vi.fn(() => ({
9295
track: vi.fn(),
93-
}),
96+
})),
9497
trackEvent: () => (target: any, propertyKey: string, descriptor: PropertyDescriptor) => descriptor,
9598
}));
9699

97100
const createMockAppWindow = () => {
98101
const mock = {
99102
send: vi.fn(),
100-
loadPage: vi.fn().mockResolvedValue(null),
103+
loadPage: vi.fn(() => Promise.resolve(null)),
101104
showOpenDialog: vi.fn(),
102105
maximize: vi.fn(),
103106
};
@@ -147,10 +150,9 @@ describe('InstallationManager', () => {
147150

148151
describe('ensureInstalled', () => {
149152
beforeEach(() => {
150-
// eslint-disable-next-line @typescript-eslint/require-await
151-
vi.spyOn(ComfyInstallation, 'fromConfig').mockImplementation(async () => {
152-
return new ComfyInstallation('installed', 'valid/base', createMockTelemetry());
153-
});
153+
vi.spyOn(ComfyInstallation, 'fromConfig').mockImplementation(() =>
154+
Promise.resolve(new ComfyInstallation('installed', 'valid/base', createMockTelemetry()))
155+
);
154156
});
155157

156158
it('returns existing valid installation', async () => {
@@ -166,6 +168,9 @@ describe('InstallationManager', () => {
166168
{
167169
scenario: 'detects invalid base path',
168170
mockSetup: () => {
171+
vi.spyOn(ComfyInstallation, 'fromConfig').mockImplementation(() =>
172+
Promise.resolve(new ComfyInstallation('installed', 'invalid/base', createMockTelemetry()))
173+
);
169174
vi.mocked(useDesktopConfig().get).mockImplementation((key: string) => {
170175
if (key === 'installState') return 'installed';
171176
if (key === 'basePath') return 'invalid/base';

‎tests/unit/main-process/appWindow.test.ts

+5-5
Original file line numberDiff line numberDiff line change
@@ -35,19 +35,19 @@ vi.mock('electron', () => ({
3535
}));
3636

3737
vi.mock('electron-store', () => ({
38-
default: vi.fn().mockImplementation(() => ({
38+
default: vi.fn(() => ({
3939
get: vi.fn(),
4040
set: vi.fn(),
4141
})),
4242
}));
4343

4444
vi.mock('@/store/desktopConfig', () => ({
45-
useDesktopConfig: vi.fn().mockReturnValue({
46-
get: vi.fn().mockImplementation((key) => {
45+
useDesktopConfig: vi.fn(() => ({
46+
get: vi.fn((key: string) => {
4747
if (key === 'installState') return 'installed';
4848
}),
49-
set: vi.fn().mockReturnValue(true),
50-
}),
49+
set: vi.fn(),
50+
})),
5151
}));
5252

5353
describe('AppWindow.isOnPage', () => {

‎tests/unit/main-process/comfyDesktopApp.test.ts

+4-4
Original file line numberDiff line numberDiff line change
@@ -15,15 +15,15 @@ import { findAvailablePort, getModelsDirectory } from '@/utils';
1515
// Mock dependencies
1616
vi.mock('@/config/comfySettings', () => {
1717
const mockSettings = {
18-
get: vi.fn().mockReturnValue(true),
18+
get: vi.fn(() => true),
1919
set: vi.fn(),
2020
saveSettings: vi.fn(),
2121
};
2222
return {
2323
ComfySettings: {
24-
load: vi.fn().mockResolvedValue(mockSettings),
24+
load: vi.fn(() => Promise.resolve(mockSettings)),
2525
},
26-
useComfySettings: vi.fn().mockReturnValue(mockSettings),
26+
useComfySettings: vi.fn(() => mockSettings),
2727
};
2828
});
2929

@@ -63,7 +63,7 @@ const mockTerminal = {
6363
restore: vi.fn(),
6464
};
6565
vi.mock('@/shell/terminal', () => ({
66-
Terminal: vi.fn().mockImplementation(() => mockTerminal),
66+
Terminal: vi.fn(() => mockTerminal),
6767
}));
6868

6969
vi.mock('@/main-process/comfyServer', () => ({

0 commit comments

Comments
 (0)