From 987063fedefb92e27be5998a2237d4aee239e1f3 Mon Sep 17 00:00:00 2001 From: Vanalite Date: Mon, 29 Sep 2025 17:59:15 +0700 Subject: [PATCH] feat: Add tests for the model displayName modification --- ...DropdownModelProvider.displayName.test.tsx | 267 ++++++++++++++++++ .../containers/__tests__/EditModel.test.tsx | 184 ++++++++++++ .../hooks/__tests__/useModelProvider.test.ts | 182 ++++++++++++ web-app/src/lib/__tests__/utils.test.ts | 50 ++++ web-app/src/services/__tests__/models.test.ts | 9 +- 5 files changed, 688 insertions(+), 4 deletions(-) create mode 100644 web-app/src/containers/__tests__/DropdownModelProvider.displayName.test.tsx create mode 100644 web-app/src/containers/__tests__/EditModel.test.tsx create mode 100644 web-app/src/hooks/__tests__/useModelProvider.test.ts diff --git a/web-app/src/containers/__tests__/DropdownModelProvider.displayName.test.tsx b/web-app/src/containers/__tests__/DropdownModelProvider.displayName.test.tsx new file mode 100644 index 000000000..0921359bc --- /dev/null +++ b/web-app/src/containers/__tests__/DropdownModelProvider.displayName.test.tsx @@ -0,0 +1,267 @@ +import { describe, it, expect, beforeEach, vi } from 'vitest' +import { render, screen } from '@testing-library/react' +import '@testing-library/jest-dom' +import DropdownModelProvider from '../DropdownModelProvider' +import { getModelDisplayName } from '@/lib/utils' +import { useModelProvider } from '@/hooks/useModelProvider' + +// Define basic types to avoid missing declarations +type ModelProvider = { + provider: string + active: boolean + models: Array<{ + id: string + displayName?: string + capabilities: string[] + }> + settings: any[] +} + +type Model = { + id: string + displayName?: string + capabilities?: string[] +} + +// Mock the dependencies +vi.mock('@/hooks/useModelProvider', () => ({ + useModelProvider: vi.fn(), +})) + +vi.mock('@/hooks/useThreads', () => ({ + useThreads: vi.fn(() => ({ + updateCurrentThreadModel: vi.fn(), + })), +})) + +vi.mock('@/hooks/useServiceHub', () => ({ + useServiceHub: vi.fn(() => ({ + models: () => ({ + checkMmprojExists: vi.fn(() => Promise.resolve(false)), + checkMmprojExistsAndUpdateOffloadMMprojSetting: vi.fn(() => Promise.resolve()), + }), + })), +})) + +vi.mock('@/i18n/react-i18next-compat', () => ({ + useTranslation: vi.fn(() => ({ + t: (key: string) => key, + })), +})) + +vi.mock('@tanstack/react-router', () => ({ + useNavigate: vi.fn(() => vi.fn()), +})) + +vi.mock('@/hooks/useFavoriteModel', () => ({ + useFavoriteModel: vi.fn(() => ({ + favoriteModels: [], + })), +})) + +vi.mock('@/lib/platform/const', () => ({ + PlatformFeatures: { + WEB_AUTO_MODEL_SELECTION: false, + MODEL_PROVIDER_SETTINGS: true, + }, +})) + +// Mock UI components +vi.mock('@/components/ui/popover', () => ({ + Popover: ({ children }: { children: React.ReactNode }) =>
{children}
, + PopoverTrigger: ({ children }: { children: React.ReactNode }) => ( +
{children}
+ ), + PopoverContent: ({ children }: { children: React.ReactNode }) => ( +
{children}
+ ), +})) + +vi.mock('../ProvidersAvatar', () => ({ + default: ({ provider }: { provider: any }) => ( +
+ ), +})) + +vi.mock('../Capabilities', () => ({ + default: ({ capabilities }: { capabilities: string[] }) => ( +
{capabilities.join(',')}
+ ), +})) + +vi.mock('../ModelSetting', () => ({ + ModelSetting: () =>
, +})) + +vi.mock('../ModelSupportStatus', () => ({ + ModelSupportStatus: () =>
, +})) + +describe('DropdownModelProvider - Display Name Integration', () => { + const mockProviders: ModelProvider[] = [ + { + provider: 'llamacpp', + active: true, + models: [ + { + id: 'model1.gguf', + displayName: 'Custom Model 1', + capabilities: ['completion'], + }, + { + id: 'model2-very-long-filename.gguf', + displayName: 'Short Name', + capabilities: ['completion'], + }, + { + id: 'model3.gguf', + // No displayName - should fall back to ID + capabilities: ['completion'], + }, + ], + settings: [], + }, + ] + + const mockSelectedModel = { + id: 'model1.gguf', + displayName: 'Custom Model 1', + capabilities: ['completion'], + } + + beforeEach(() => { + vi.clearAllMocks() + + // Reset the mock for each test + vi.mocked(useModelProvider).mockReturnValue({ + providers: mockProviders, + selectedProvider: 'llamacpp', + selectedModel: mockSelectedModel, + getProviderByName: vi.fn((name: string) => + mockProviders.find((p: ModelProvider) => p.provider === name) + ), + selectModelProvider: vi.fn(), + getModelBy: vi.fn((id: string) => + mockProviders[0].models.find((m: any) => m.id === id) + ), + updateProvider: vi.fn(), + } as any) + }) + + it('should display custom model name in the trigger button', () => { + render() + + // Should show the display name in both trigger and dropdown + expect(screen.getAllByText('Custom Model 1')).toHaveLength(2) // One in trigger, one in dropdown + // Model ID should not be visible as text (it's only in title attributes) + expect(screen.queryByDisplayValue('model1.gguf')).not.toBeInTheDocument() + }) + + it('should fall back to model ID when no displayName is set', () => { + vi.mocked(useModelProvider).mockReturnValue({ + providers: mockProviders, + selectedProvider: 'llamacpp', + selectedModel: mockProviders[0].models[2], // model3 without displayName + getProviderByName: vi.fn((name: string) => + mockProviders.find((p: ModelProvider) => p.provider === name) + ), + selectModelProvider: vi.fn(), + getModelBy: vi.fn((id: string) => + mockProviders[0].models.find((m: any) => m.id === id) + ), + updateProvider: vi.fn(), + } as any) + + render() + + expect(screen.getAllByText('model3.gguf')).toHaveLength(2) // Trigger and dropdown + }) + + it('should show display names in the model list items', () => { + render() + + // Check if the display names are shown in the options + expect(screen.getAllByText('Custom Model 1')).toHaveLength(2) // Selected: Trigger + dropdown + expect(screen.getByText('Short Name')).toBeInTheDocument() // Only in dropdown + expect(screen.getByText('model3.gguf')).toBeInTheDocument() // Only in dropdown + }) + + it('should use getModelDisplayName utility correctly', () => { + // Test the utility function directly with different model scenarios + const modelWithDisplayName = { + id: 'long-model-name.gguf', + displayName: 'Short Name', + } as Model + + const modelWithoutDisplayName = { + id: 'model-without-display-name.gguf', + } as Model + + const modelWithEmptyDisplayName = { + id: 'model-with-empty.gguf', + displayName: '', + } as Model + + expect(getModelDisplayName(modelWithDisplayName)).toBe('Short Name') + expect(getModelDisplayName(modelWithoutDisplayName)).toBe('model-without-display-name.gguf') + expect(getModelDisplayName(modelWithEmptyDisplayName)).toBe('model-with-empty.gguf') + }) + + it('should maintain model ID for internal operations while showing display name', () => { + const mockSelectModelProvider = vi.fn() + + vi.mocked(useModelProvider).mockReturnValue({ + providers: mockProviders, + selectedProvider: 'llamacpp', + selectedModel: mockSelectedModel, + getProviderByName: vi.fn((name: string) => + mockProviders.find((p: ModelProvider) => p.provider === name) + ), + selectModelProvider: mockSelectModelProvider, + getModelBy: vi.fn((id: string) => + mockProviders[0].models.find((m: any) => m.id === id) + ), + updateProvider: vi.fn(), + } as any) + + render() + + // Verify that display name is shown in UI + expect(screen.getAllByText('Custom Model 1')).toHaveLength(2) // Trigger + dropdown + + // The actual model ID should still be preserved for backend operations + // This would be tested in the click handlers, but that requires more complex mocking + expect(mockSelectedModel.id).toBe('model1.gguf') + }) + + it('should handle updating display model when selection changes', () => { + // Test that when a new model is selected, the trigger updates correctly + // First render with model1 selected + const { rerender } = render() + + // Check trigger shows Custom Model 1 + const triggerButton = screen.getByRole('button') + expect(triggerButton).toHaveTextContent('Custom Model 1') + + // Update to select model2 + vi.mocked(useModelProvider).mockReturnValue({ + providers: mockProviders, + selectedProvider: 'llamacpp', + selectedModel: mockProviders[0].models[1], // model2 + getProviderByName: vi.fn((name: string) => + mockProviders.find((p: ModelProvider) => p.provider === name) + ), + selectModelProvider: vi.fn(), + getModelBy: vi.fn((id: string) => + mockProviders[0].models.find((m: any) => m.id === id) + ), + updateProvider: vi.fn(), + } as any) + + rerender() + // Check trigger now shows Short Name + expect(triggerButton).toHaveTextContent('Short Name') + // Both models are still visible in the dropdown, so we can't test for absence + expect(screen.getAllByText('Short Name')).toHaveLength(2) // trigger + dropdown + }) +}) \ No newline at end of file diff --git a/web-app/src/containers/__tests__/EditModel.test.tsx b/web-app/src/containers/__tests__/EditModel.test.tsx new file mode 100644 index 000000000..a02e72476 --- /dev/null +++ b/web-app/src/containers/__tests__/EditModel.test.tsx @@ -0,0 +1,184 @@ +import { describe, it, expect, beforeEach, vi } from 'vitest' +import { render, screen, fireEvent, waitFor } from '@testing-library/react' +import userEvent from '@testing-library/user-event' +import { DialogEditModel } from '../dialogs/EditModel' +import { useModelProvider } from '@/hooks/useModelProvider' +import '@testing-library/jest-dom' + +// Mock the dependencies +vi.mock('@/hooks/useModelProvider', () => ({ + useModelProvider: vi.fn(() => ({ + updateProvider: vi.fn(), + setProviders: vi.fn(), + })), +})) + +vi.mock('@/hooks/useServiceHub', () => ({ + useServiceHub: vi.fn(() => ({ + providers: () => ({ + getProviders: vi.fn(() => Promise.resolve([])), + }), + })), +})) + +vi.mock('@/i18n/react-i18next-compat', () => ({ + useTranslation: vi.fn(() => ({ + t: (key: string) => key, + })), +})) + +vi.mock('sonner', () => ({ + toast: { + success: vi.fn(), + error: vi.fn(), + }, +})) + +// Mock Dialog components +vi.mock('@/components/ui/dialog', () => ({ + Dialog: ({ children, open }: { children: React.ReactNode; open: boolean }) => + open ?
{children}
: null, + DialogContent: ({ children }: { children: React.ReactNode }) => ( +
{children}
+ ), + DialogHeader: ({ children }: { children: React.ReactNode }) => ( +
{children}
+ ), + DialogTitle: ({ children }: { children: React.ReactNode }) => ( +

{children}

+ ), + DialogDescription: ({ children }: { children: React.ReactNode }) => ( +

{children}

+ ), + DialogTrigger: ({ children }: { children: React.ReactNode }) => ( +
{children}
+ ), +})) + +vi.mock('@/components/ui/input', () => ({ + Input: ({ value, onChange, ...props }: any) => ( + + ), +})) + +vi.mock('@/components/ui/button', () => ({ + Button: ({ children, onClick, ...props }: any) => ( + + ), +})) + +// Mock other UI components +vi.mock('@tabler/icons-react', () => ({ + IconPencil: () =>
, + IconCheck: () =>
, + IconX: () =>
, + IconAlertTriangle: () =>
, + IconEye: () =>
, + IconTool: () =>
, + IconLoader2: () =>
, +})) + +describe('DialogEditModel - Basic Component Tests', () => { + const mockProvider = { + provider: 'llamacpp', + active: true, + models: [ + { + id: 'test-model.gguf', + displayName: 'My Custom Model', + capabilities: ['completion'], + }, + ], + settings: [], + } as any + + const mockUpdateProvider = vi.fn() + const mockSetProviders = vi.fn() + + beforeEach(() => { + vi.clearAllMocks() + + vi.mocked(useModelProvider).mockReturnValue({ + updateProvider: mockUpdateProvider, + setProviders: mockSetProviders, + } as any) + }) + + it('should render without errors', () => { + const { container } = render( + + ) + + // Component should render without throwing errors + expect(container).toBeInTheDocument() + }) + + it('should handle provider without models', () => { + const emptyProvider = { + ...mockProvider, + models: [], + } as any + + const { container } = render( + + ) + + // Component should handle empty models gracefully + expect(container).toBeInTheDocument() + }) + + it('should accept provider and modelId props', () => { + const { container } = render( + + ) + + expect(container).toBeInTheDocument() + }) + + it('should not crash with minimal props', () => { + const minimalProvider = { + provider: 'test', + active: false, + models: [], + settings: [], + } as any + + expect(() => { + render( + + ) + }).not.toThrow() + }) + + it('should have mocked dependencies available', () => { + render( + + ) + + // Verify our mocks are in place + expect(mockUpdateProvider).toBeDefined() + expect(mockSetProviders).toBeDefined() + }) +}) \ No newline at end of file diff --git a/web-app/src/hooks/__tests__/useModelProvider.test.ts b/web-app/src/hooks/__tests__/useModelProvider.test.ts new file mode 100644 index 000000000..88272cf57 --- /dev/null +++ b/web-app/src/hooks/__tests__/useModelProvider.test.ts @@ -0,0 +1,182 @@ +import { describe, it, expect, beforeEach, vi } from 'vitest' +import { act, renderHook } from '@testing-library/react' +import { useModelProvider } from '../useModelProvider' + +// Mock getServiceHub +vi.mock('@/hooks/useServiceHub', () => ({ + getServiceHub: vi.fn(() => ({ + path: () => ({ + sep: () => '/', + }), + })), +})) + +// Mock the localStorage key constants +vi.mock('@/constants/localStorage', () => ({ + localStorageKey: { + modelProvider: 'jan-model-provider', + }, +})) + +// Mock localStorage +const localStorageMock = { + getItem: vi.fn(), + setItem: vi.fn(), + removeItem: vi.fn(), + clear: vi.fn(), +} +Object.defineProperty(window, 'localStorage', { + value: localStorageMock, + writable: true, +}) + +describe('useModelProvider - displayName functionality', () => { + beforeEach(() => { + vi.clearAllMocks() + localStorageMock.getItem.mockReturnValue(null) + + // Reset Zustand store to default state + act(() => { + useModelProvider.setState({ + providers: [], + selectedProvider: 'llamacpp', + selectedModel: null, + deletedModels: [], + }) + }) + }) + + it('should handle models without displayName property', () => { + const { result } = renderHook(() => useModelProvider()) + + const provider = { + provider: 'llamacpp', + active: true, + models: [ + { + id: 'test-model.gguf', + capabilities: ['completion'], + }, + ], + settings: [], + } as any + + // First add the provider, then update it (since updateProvider only updates existing providers) + act(() => { + result.current.addProvider(provider) + }) + + const updatedProvider = result.current.getProviderByName('llamacpp') + expect(updatedProvider?.models[0].displayName).toBeUndefined() + expect(updatedProvider?.models[0].id).toBe('test-model.gguf') + }) + + it('should preserve displayName when merging providers in setProviders', () => { + const { result } = renderHook(() => useModelProvider()) + + // First, set up initial state with displayName via direct state manipulation + // This simulates the scenario where a user has already customized a display name + act(() => { + useModelProvider.setState({ + providers: [ + { + provider: 'llamacpp', + active: true, + models: [ + { + id: 'test-model.gguf', + displayName: 'My Custom Model', + capabilities: ['completion'], + }, + ], + settings: [], + }, + ] as any, + selectedProvider: 'llamacpp', + selectedModel: null, + deletedModels: [], + }) + }) + + // Now simulate setProviders with fresh data (like from server refresh) + const freshProviders = [ + { + provider: 'llamacpp', + active: true, + persist: true, + models: [ + { + id: 'test-model.gguf', + capabilities: ['completion'], + // Note: no displayName in fresh data + }, + ], + settings: [], + }, + ] as any + + act(() => { + result.current.setProviders(freshProviders) + }) + + // The displayName should be preserved from existing state + const provider = result.current.getProviderByName('llamacpp') + expect(provider?.models[0].displayName).toBe('My Custom Model') + }) + + it('should provide basic functionality without breaking existing behavior', () => { + const { result } = renderHook(() => useModelProvider()) + + // Test that basic provider operations work + expect(result.current.providers).toEqual([]) + expect(result.current.selectedProvider).toBe('llamacpp') + expect(result.current.selectedModel).toBeNull() + + // Test addProvider functionality + const provider = { + provider: 'openai', + active: true, + models: [], + settings: [], + } as any + + act(() => { + result.current.addProvider(provider) + }) + + expect(result.current.providers).toHaveLength(1) + expect(result.current.getProviderByName('openai')).toBeDefined() + }) + + it('should handle provider operations with models that have displayName', () => { + const { result } = renderHook(() => useModelProvider()) + + // Test that we can at least get and set providers with displayName models + const providerWithDisplayName = { + provider: 'llamacpp', + active: true, + models: [ + { + id: 'test-model.gguf', + displayName: 'Custom Model Name', + capabilities: ['completion'], + }, + ], + settings: [], + } as any + + // Set the state directly (simulating what would happen in real usage) + act(() => { + useModelProvider.setState({ + providers: [providerWithDisplayName], + selectedProvider: 'llamacpp', + selectedModel: null, + deletedModels: [], + }) + }) + + const provider = result.current.getProviderByName('llamacpp') + expect(provider?.models[0].displayName).toBe('Custom Model Name') + expect(provider?.models[0].id).toBe('test-model.gguf') + }) +}) \ No newline at end of file diff --git a/web-app/src/lib/__tests__/utils.test.ts b/web-app/src/lib/__tests__/utils.test.ts index 25bc91334..33c51447e 100644 --- a/web-app/src/lib/__tests__/utils.test.ts +++ b/web-app/src/lib/__tests__/utils.test.ts @@ -6,6 +6,7 @@ import { toGigabytes, formatMegaBytes, formatDuration, + getModelDisplayName, } from '../utils' describe('getProviderLogo', () => { @@ -200,3 +201,52 @@ describe('formatDuration', () => { expect(formatDuration(start, 86400000)).toBe('1d 0h 0m 0s') // exactly 1 day }) }) + +describe('getModelDisplayName', () => { + it('returns displayName when it exists', () => { + const model = { + id: 'llama-3.2-1b-instruct-q4_k_m.gguf', + displayName: 'My Custom Model', + } as Model + expect(getModelDisplayName(model)).toBe('My Custom Model') + }) + + it('returns model.id when displayName is undefined', () => { + const model = { + id: 'llama-3.2-1b-instruct-q4_k_m.gguf', + } as Model + expect(getModelDisplayName(model)).toBe('llama-3.2-1b-instruct-q4_k_m.gguf') + }) + + it('returns model.id when displayName is empty string', () => { + const model = { + id: 'llama-3.2-1b-instruct-q4_k_m.gguf', + displayName: '', + } as Model + expect(getModelDisplayName(model)).toBe('llama-3.2-1b-instruct-q4_k_m.gguf') + }) + + it('returns model.id when displayName is null', () => { + const model = { + id: 'llama-3.2-1b-instruct-q4_k_m.gguf', + displayName: null as any, + } as Model + expect(getModelDisplayName(model)).toBe('llama-3.2-1b-instruct-q4_k_m.gguf') + }) + + it('handles models with complex display names', () => { + const model = { + id: 'very-long-model-file-name-with-lots-of-details.gguf', + displayName: 'Short Name 🤖', + } as Model + expect(getModelDisplayName(model)).toBe('Short Name 🤖') + }) + + it('handles models with special characters in displayName', () => { + const model = { + id: 'model.gguf', + displayName: 'Model (Version 2.0) - Fine-tuned', + } as Model + expect(getModelDisplayName(model)).toBe('Model (Version 2.0) - Fine-tuned') + }) +}) diff --git a/web-app/src/services/__tests__/models.test.ts b/web-app/src/services/__tests__/models.test.ts index 4322cfe40..d0c9daa2d 100644 --- a/web-app/src/services/__tests__/models.test.ts +++ b/web-app/src/services/__tests__/models.test.ts @@ -1,7 +1,7 @@ import { describe, it, expect, vi, beforeEach } from 'vitest' import { DefaultModelsService } from '../models/default' import type { HuggingFaceRepo, CatalogModel } from '../models/types' -import { EngineManager, Model } from '@janhq/core' +import { EngineManager } from '@janhq/core' // Mock EngineManager vi.mock('@janhq/core', () => ({ @@ -131,18 +131,19 @@ describe('DefaultModelsService', () => { expect(mockEngine.update).not.toHaveBeenCalled() }) - it('should update model when modelId differs from model.id', async () => { + it('should handle model when modelId differs from model.id', async () => { const modelId = 'old-model-id' const model = { id: 'new-model-id', settings: [{ key: 'temperature', value: 0.7 }], } - mockEngine.update.mockResolvedValue(undefined) await modelsService.updateModel(modelId, model as any) expect(mockEngine.updateSettings).toHaveBeenCalledWith(model.settings) - expect(mockEngine.update).toHaveBeenCalledWith(modelId, model) + // Note: Model ID updates are now handled at the provider level in the frontend + // The engine no longer has an update method for model metadata + expect(mockEngine.update).not.toHaveBeenCalled() }) })