Skip to content
Open
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 10 additions & 1 deletion src/install/installationManager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -398,19 +398,28 @@ export class InstallationManager implements HasTelemetry {
if (process.platform !== 'win32') return;
if (installation.virtualEnvironment.selectedDevice !== 'nvidia') return;

const config = useDesktopConfig();
if (config.get('suppressNvidiaDriverWarningFor') === NVIDIA_DRIVER_MIN_VERSION) return;

const driverVersion =
(await this.getNvidiaDriverVersionFromSmi()) ?? (await this.getNvidiaDriverVersionFromSmiFallback());
if (!driverVersion) return;

if (!isNvidiaDriverBelowMinimum(driverVersion)) return;

await this.appWindow.showMessageBox({
const { checkboxChecked } = await this.appWindow.showMessageBox({
type: 'warning',
title: 'Update NVIDIA Driver',
message: 'Your NVIDIA driver may be too old for PyTorch 2.9.1 + cu130.',
detail: `Detected driver version: ${driverVersion}\nRecommended minimum: ${NVIDIA_DRIVER_MIN_VERSION}\n\nPlease consider updating your NVIDIA drivers and retrying if you run into issues.`,
buttons: ['OK'],
checkboxLabel: "Don't show again",
checkboxChecked: false,
});

if (checkboxChecked) {
config.set('suppressNvidiaDriverWarningFor', NVIDIA_DRIVER_MIN_VERSION);
}
}

/**
Expand Down
2 changes: 2 additions & 0 deletions src/store/desktopSettings.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,4 +35,6 @@ export type DesktopSettings = {
versionConsentedMetrics?: string;
/** Whether the user has generated an image successfully. */
hasGeneratedSuccessfully?: boolean;
/** The minimum NVIDIA driver version for which the warning was dismissed. */
suppressNvidiaDriverWarningFor?: string;
};
82 changes: 78 additions & 4 deletions tests/unit/install/installationManager.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,13 +33,15 @@ vi.mock('node:fs/promises', () => ({
}));

const config = {
get: vi.fn((key: string) => {
get: vi.fn((key: string): string | undefined => {
if (key === 'installState') return 'installed';
if (key === 'basePath') return 'valid/base';
return undefined;
}),
set: vi.fn((key: string, value: string) => {
if (key !== 'basePath') throw new Error(`Unexpected key: ${key}`);
if (!value) throw new Error(`Unexpected value: [${value}]`);
set: vi.fn((key: string, value: unknown) => {
const allowedKeys = new Set(['basePath', 'suppressNvidiaDriverWarningFor']);
if (!allowedKeys.has(key)) throw new Error(`Unexpected key: ${key}`);
if (key === 'basePath' && !value) throw new Error(`Unexpected value: [${value}]`);
}),
};
vi.mock('@/store/desktopConfig', () => ({
Expand Down Expand Up @@ -110,6 +112,7 @@ const createMockAppWindow = () => {
send: vi.fn(),
loadPage: vi.fn(() => Promise.resolve(null)),
showOpenDialog: vi.fn(),
showMessageBox: vi.fn(() => Promise.resolve({ response: 0, checkboxChecked: false })),
maximize: vi.fn(),
};
return mock as unknown as AppWindow;
Expand Down Expand Up @@ -251,6 +254,77 @@ describe('InstallationManager', () => {
cleanup?.();
});
});

describe('warnIfNvidiaDriverTooOld', () => {
const createInstallation = (device: string) =>
({ virtualEnvironment: { selectedDevice: device } }) as ComfyInstallation;

const getManagerMethods = () =>
manager as unknown as {
warnIfNvidiaDriverTooOld: (installation: ComfyInstallation) => Promise<void>;
getNvidiaDriverVersionFromSmi: () => Promise<string | undefined>;
getNvidiaDriverVersionFromSmiFallback: () => Promise<string | undefined>;
};

beforeEach(() => {
vi.mocked(config.get).mockImplementation((key: string) => {
if (key === 'installState') return 'installed';
if (key === 'basePath') return 'valid/base';
return undefined;
});
vi.mocked(config.set).mockClear();
vi.mocked(mockAppWindow.showMessageBox).mockClear();
});

it('skips dialog when warning is suppressed for the current minimum version', async () => {
const platformSpy = vi.spyOn(process, 'platform', 'get').mockReturnValue('win32');
const managerMethods = getManagerMethods();
const smiSpy = vi.spyOn(managerMethods, 'getNvidiaDriverVersionFromSmi').mockResolvedValue('570.0');
const smiFallbackSpy = vi
.spyOn(managerMethods, 'getNvidiaDriverVersionFromSmiFallback')
.mockResolvedValue(undefined);

try {
vi.mocked(config.get).mockImplementation((key: string) => {
if (key === 'installState') return 'installed';
if (key === 'basePath') return 'valid/base';
if (key === 'suppressNvidiaDriverWarningFor') return '580';
return undefined;
});

await managerMethods.warnIfNvidiaDriverTooOld(createInstallation('nvidia'));

expect(smiSpy).not.toHaveBeenCalled();
expect(smiFallbackSpy).not.toHaveBeenCalled();
expect(mockAppWindow.showMessageBox).not.toHaveBeenCalled();
} finally {
smiSpy.mockRestore();
smiFallbackSpy.mockRestore();
platformSpy.mockRestore();
}
});

it('stores the current minimum version when dismissed', async () => {
const platformSpy = vi.spyOn(process, 'platform', 'get').mockReturnValue('win32');
const managerMethods = getManagerMethods();
const smiSpy = vi.spyOn(managerMethods, 'getNvidiaDriverVersionFromSmi').mockResolvedValue('570.0');
const smiFallbackSpy = vi
.spyOn(managerMethods, 'getNvidiaDriverVersionFromSmiFallback')
.mockResolvedValue(undefined);

try {
vi.mocked(mockAppWindow.showMessageBox).mockResolvedValue({ response: 0, checkboxChecked: true });

await managerMethods.warnIfNvidiaDriverTooOld(createInstallation('nvidia'));

expect(config.set).toHaveBeenCalledWith('suppressNvidiaDriverWarningFor', '580');
} finally {
smiSpy.mockRestore();
smiFallbackSpy.mockRestore();
platformSpy.mockRestore();
}
});
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick | 🔵 Trivial

Use test instead of it per coding guidelines.

The tests are well-structured with proper mock setup and cleanup. However, per coding guidelines for tests/unit/**/*.test.{js,ts}: "Use test instead of it for defining tests."

Suggested change
-    it('skips dialog when warning is suppressed for the current minimum version', async () => {
+    test('skips dialog when warning is suppressed for the current minimum version', async () => {
-    it('stores the current minimum version when dismissed', async () => {
+    test('stores the current minimum version when dismissed', async () => {
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
it('skips dialog when warning is suppressed for the current minimum version', async () => {
const platformSpy = vi.spyOn(process, 'platform', 'get').mockReturnValue('win32');
const managerMethods = getManagerMethods();
const smiSpy = vi.spyOn(managerMethods, 'getNvidiaDriverVersionFromSmi').mockResolvedValue('570.0');
const smiFallbackSpy = vi
.spyOn(managerMethods, 'getNvidiaDriverVersionFromSmiFallback')
.mockResolvedValue(undefined);
try {
vi.mocked(config.get).mockImplementation((key: string) => {
if (key === 'installState') return 'installed';
if (key === 'basePath') return 'valid/base';
if (key === 'suppressNvidiaDriverWarningFor') return '580';
return undefined;
});
await managerMethods.warnIfNvidiaDriverTooOld(createInstallation('nvidia'));
expect(smiSpy).not.toHaveBeenCalled();
expect(smiFallbackSpy).not.toHaveBeenCalled();
expect(mockAppWindow.showMessageBox).not.toHaveBeenCalled();
} finally {
smiSpy.mockRestore();
smiFallbackSpy.mockRestore();
platformSpy.mockRestore();
}
});
it('stores the current minimum version when dismissed', async () => {
const platformSpy = vi.spyOn(process, 'platform', 'get').mockReturnValue('win32');
const managerMethods = getManagerMethods();
const smiSpy = vi.spyOn(managerMethods, 'getNvidiaDriverVersionFromSmi').mockResolvedValue('570.0');
const smiFallbackSpy = vi
.spyOn(managerMethods, 'getNvidiaDriverVersionFromSmiFallback')
.mockResolvedValue(undefined);
try {
vi.mocked(mockAppWindow.showMessageBox).mockResolvedValue({ response: 0, checkboxChecked: true });
await managerMethods.warnIfNvidiaDriverTooOld(createInstallation('nvidia'));
expect(config.set).toHaveBeenCalledWith('suppressNvidiaDriverWarningFor', '580');
} finally {
smiSpy.mockRestore();
smiFallbackSpy.mockRestore();
platformSpy.mockRestore();
}
});
test('skips dialog when warning is suppressed for the current minimum version', async () => {
const platformSpy = vi.spyOn(process, 'platform', 'get').mockReturnValue('win32');
const managerMethods = getManagerMethods();
const smiSpy = vi.spyOn(managerMethods, 'getNvidiaDriverVersionFromSmi').mockResolvedValue('570.0');
const smiFallbackSpy = vi
.spyOn(managerMethods, 'getNvidiaDriverVersionFromSmiFallback')
.mockResolvedValue(undefined);
try {
vi.mocked(config.get).mockImplementation((key: string) => {
if (key === 'installState') return 'installed';
if (key === 'basePath') return 'valid/base';
if (key === 'suppressNvidiaDriverWarningFor') return '580';
return undefined;
});
await managerMethods.warnIfNvidiaDriverTooOld(createInstallation('nvidia'));
expect(smiSpy).not.toHaveBeenCalled();
expect(smiFallbackSpy).not.toHaveBeenCalled();
expect(mockAppWindow.showMessageBox).not.toHaveBeenCalled();
} finally {
smiSpy.mockRestore();
smiFallbackSpy.mockRestore();
platformSpy.mockRestore();
}
});
test('stores the current minimum version when dismissed', async () => {
const platformSpy = vi.spyOn(process, 'platform', 'get').mockReturnValue('win32');
const managerMethods = getManagerMethods();
const smiSpy = vi.spyOn(managerMethods, 'getNvidiaDriverVersionFromSmi').mockResolvedValue('570.0');
const smiFallbackSpy = vi
.spyOn(managerMethods, 'getNvidiaDriverVersionFromSmiFallback')
.mockResolvedValue(undefined);
try {
vi.mocked(mockAppWindow.showMessageBox).mockResolvedValue({ response: 0, checkboxChecked: true });
await managerMethods.warnIfNvidiaDriverTooOld(createInstallation('nvidia'));
expect(config.set).toHaveBeenCalledWith('suppressNvidiaDriverWarningFor', '580');
} finally {
smiSpy.mockRestore();
smiFallbackSpy.mockRestore();
platformSpy.mockRestore();
}
});
🤖 Prompt for AI Agents
In `@tests/unit/install/installationManager.test.ts` around lines 279 - 326,
Replace the two test declarations that use "it(" with "test(" to follow the
repository guideline; specifically change the blocks that call
warnIfNvidiaDriverTooOld (the test titled "skips dialog when warning is
suppressed for the current minimum version" and the one titled "stores the
current minimum version when dismissed") so they use test(...) instead of
it(...), leaving all mocks and assertions involving
getNvidiaDriverVersionFromSmi, getNvidiaDriverVersionFromSmiFallback,
mockAppWindow.showMessageBox, and config.set unchanged.

});
Copy link

Copilot AI Jan 15, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The test coverage is missing a test case for when the user dismisses the dialog WITHOUT checking the "Don't show again" checkbox. This would verify that the config is not updated when checkboxChecked is false, ensuring the warning appears again on the next trigger. Consider adding a test case that mocks showMessageBox to return checkboxChecked: false and verifies that config.set is not called with 'suppressNvidiaDriverWarningFor'.

Copilot uses AI. Check for mistakes.
});

describe('parseNvidiaDriverVersionFromSmiOutput', () => {
Expand Down
Loading