From bd69365a717fe0223d503cd256fafdfa5f91cbd6 Mon Sep 17 00:00:00 2001 From: CodingOnStar Date: Tue, 20 Jan 2026 16:20:32 +0800 Subject: [PATCH] test: update unit tests for dataset components to improve clarity and functionality --- .../common/check-rerank-model.spec.ts | 4 +- .../datasets/common/credential-icon.spec.tsx | 23 +- .../index-failed.spec.tsx | 7 +- .../common/image-previewer/index.spec.tsx | 2 +- .../image-uploader/hooks/use-upload.spec.tsx | 354 +++++++++++++----- 5 files changed, 280 insertions(+), 110 deletions(-) diff --git a/web/app/components/datasets/common/check-rerank-model.spec.ts b/web/app/components/datasets/common/check-rerank-model.spec.ts index ac99c6c2da..cba9b27200 100644 --- a/web/app/components/datasets/common/check-rerank-model.spec.ts +++ b/web/app/components/datasets/common/check-rerank-model.spec.ts @@ -64,7 +64,7 @@ const createDefaultRerankModel = (): DefaultModelResponse => ({ describe('check-rerank-model', () => { describe('isReRankModelSelected', () => { - describe('Rendering', () => { + describe('Core Functionality', () => { it('should return true when reranking is disabled', () => { const config = createRetrievalConfig({ reranking_enable: false, @@ -263,7 +263,7 @@ describe('check-rerank-model', () => { }) describe('ensureRerankModelSelected', () => { - describe('Rendering', () => { + describe('Core Functionality', () => { it('should return original config when reranking model already selected', () => { const config = createRetrievalConfig({ reranking_enable: true, diff --git a/web/app/components/datasets/common/credential-icon.spec.tsx b/web/app/components/datasets/common/credential-icon.spec.tsx index dc855f5c8c..b1c3131dfe 100644 --- a/web/app/components/datasets/common/credential-icon.spec.tsx +++ b/web/app/components/datasets/common/credential-icon.spec.tsx @@ -105,20 +105,19 @@ describe('CredentialIcon', () => { }) it('should apply different background colors for different letters', () => { - const { container: container1, unmount: unmount1 } = render() - const wrapper1 = container1.firstChild as HTMLElement - const classes1 = wrapper1.className - - unmount1() - + // 'A' (65) % 4 = 1 → pink, 'B' (66) % 4 = 2 → indigo + const { container: container1 } = render() const { container: container2 } = render() - const wrapper2 = container2.firstChild as HTMLElement - const classes2 = wrapper2.className - // They may or may not be different depending on the modulo calculation - // but both should have a bg class - expect(classes1).toMatch(/bg-components-icon-bg/) - expect(classes2).toMatch(/bg-components-icon-bg/) + const wrapper1 = container1.firstChild as HTMLElement + const wrapper2 = container2.firstChild as HTMLElement + + const bgClass1 = wrapper1.className.match(/bg-components-icon-bg-\S+/)?.[0] + const bgClass2 = wrapper2.className.match(/bg-components-icon-bg-\S+/)?.[0] + + expect(bgClass1).toBeDefined() + expect(bgClass2).toBeDefined() + expect(bgClass1).not.toBe(bgClass2) }) it('should handle empty avatarUrl string', () => { diff --git a/web/app/components/datasets/common/document-status-with-action/index-failed.spec.tsx b/web/app/components/datasets/common/document-status-with-action/index-failed.spec.tsx index 7ac63bbdaf..ac24a2532f 100644 --- a/web/app/components/datasets/common/document-status-with-action/index-failed.spec.tsx +++ b/web/app/components/datasets/common/document-status-with-action/index-failed.spec.tsx @@ -177,10 +177,11 @@ describe('RetryButton (IndexFailed)', () => { const retryButton = screen.getByText(/retry/i) fireEvent.click(retryButton) - // Button should be disabled during retry + // Button should show disabled styling during retry await waitFor(() => { - const button = screen.getByText(/retry/i).closest('div') - expect(button).toBeInTheDocument() + const button = screen.getByText(/retry/i) + expect(button).toHaveClass('cursor-not-allowed') + expect(button).toHaveClass('text-text-disabled') }) }) }) diff --git a/web/app/components/datasets/common/image-previewer/index.spec.tsx b/web/app/components/datasets/common/image-previewer/index.spec.tsx index 7005ae7d2d..01bdb111fb 100644 --- a/web/app/components/datasets/common/image-previewer/index.spec.tsx +++ b/web/app/components/datasets/common/image-previewer/index.spec.tsx @@ -427,7 +427,7 @@ describe('ImagePreviewer', () => { }) describe('Image Cache', () => { - it('should skip fetch for already cached images', async () => { + it('should clean up blob URLs on unmount', async () => { const onClose = vi.fn() const images = createMockImages() diff --git a/web/app/components/datasets/common/image-uploader/hooks/use-upload.spec.tsx b/web/app/components/datasets/common/image-uploader/hooks/use-upload.spec.tsx index 86439416e2..e62deac165 100644 --- a/web/app/components/datasets/common/image-uploader/hooks/use-upload.spec.tsx +++ b/web/app/components/datasets/common/image-uploader/hooks/use-upload.spec.tsx @@ -1,6 +1,7 @@ import type { PropsWithChildren } from 'react' import type { FileEntity } from '../types' -import { act, renderHook, waitFor } from '@testing-library/react' +import { act, fireEvent, render, renderHook, screen, waitFor } from '@testing-library/react' +import * as React from 'react' import { beforeEach, describe, expect, it, vi } from 'vitest' import Toast from '@/app/components/base/toast' import { FileContextProvider } from '../store' @@ -622,131 +623,300 @@ describe('useUpload hook', () => { }) describe('Drag and Drop Functionality', () => { - it('should set dragging to false on dragLeave when target matches dragRef', async () => { - const { result } = renderHook(() => useUpload(), { - wrapper: createWrapper(), - }) + // Test component that renders the hook with actual DOM elements + const TestComponent = ({ onStateChange }: { onStateChange?: (dragging: boolean) => void }) => { + const { dragging, dragRef, dropRef } = useUpload() - // Create a mock div element for the dragRef - const mockDiv = document.createElement('div') + // Report dragging state changes to parent + React.useEffect(() => { + onStateChange?.(dragging) + }, [dragging, onStateChange]) - // Manually set the dragRef - Object.defineProperty(result.current.dragRef, 'current', { - value: mockDiv, - writable: true, - }) + return ( +
+
+ {dragging ? 'dragging' : 'not-dragging'} +
+
+ ) + } - // Initially dragging should be false - expect(result.current.dragging).toBe(false) - }) - - it('should handle drop event with files', async () => { - const onChange = vi.fn() - const wrapper = ({ children }: PropsWithChildren) => ( - - {children} - + it('should set dragging to true on dragEnter when target is not dragRef', async () => { + const onStateChange = vi.fn() + render( + + + , ) - const { result } = renderHook(() => useUpload(), { wrapper }) + const dropZone = screen.getByTestId('drop-zone') - // Create mock drop element - const mockDropDiv = document.createElement('div') - Object.defineProperty(result.current.dropRef, 'current', { - value: mockDropDiv, - writable: true, + // Fire dragenter event on dropZone (not dragRef) + await act(async () => { + fireEvent.dragEnter(dropZone, { + dataTransfer: { items: [] }, + }) }) - // Create mock file - const mockFile = new File(['test'], 'test.png', { type: 'image/png' }) + // Verify dragging state changed to true + expect(screen.getByTestId('dragging-state')).toHaveTextContent('dragging') + }) - // Create mock DataTransfer - const mockDataTransfer = { - items: [ - { - webkitGetAsEntry: () => null, // No entry, will use getAsFile - getAsFile: () => mockFile, + it('should set dragging to false on dragLeave when target matches dragRef', async () => { + render( + + + , + ) + + const dropZone = screen.getByTestId('drop-zone') + const dragBoundary = screen.getByTestId('drag-boundary') + + // First trigger dragenter to set dragging to true + await act(async () => { + fireEvent.dragEnter(dropZone, { + dataTransfer: { items: [] }, + }) + }) + + expect(screen.getByTestId('dragging-state')).toHaveTextContent('dragging') + + // Then trigger dragleave on dragBoundary to set dragging to false + await act(async () => { + fireEvent.dragLeave(dragBoundary, { + dataTransfer: { items: [] }, + }) + }) + + expect(screen.getByTestId('dragging-state')).toHaveTextContent('not-dragging') + }) + + it('should handle drop event with files and reset dragging state', async () => { + const onChange = vi.fn() + + render( + + + , + ) + + const dropZone = screen.getByTestId('drop-zone') + const mockFile = new File(['test content'], 'test.png', { type: 'image/png' }) + + // First trigger dragenter + await act(async () => { + fireEvent.dragEnter(dropZone, { + dataTransfer: { items: [] }, + }) + }) + + expect(screen.getByTestId('dragging-state')).toHaveTextContent('dragging') + + // Then trigger drop with files + await act(async () => { + fireEvent.drop(dropZone, { + dataTransfer: { + items: [{ + webkitGetAsEntry: () => null, + getAsFile: () => mockFile, + }], }, - ], - } + }) + }) - // The drop handler is attached via useEffect - // For now, verify the refs are properly exposed - expect(result.current.dropRef).toBeDefined() - // Verify mock data structures are valid - expect(mockDataTransfer.items).toHaveLength(1) + // Dragging should be reset to false after drop + expect(screen.getByTestId('dragging-state')).toHaveTextContent('not-dragging') }) it('should return early when dataTransfer is null on drop', async () => { - const { result } = renderHook(() => useUpload(), { - wrapper: createWrapper(), - }) - - // The handleDrop function checks for e.dataTransfer and returns early if null - // This is handled internally, we verify the dropRef is available - expect(result.current.dropRef.current).toBeNull() - }) - - it('should handle drop with webkitGetAsEntry for directory traversal', async () => { - const onChange = vi.fn() - const wrapper = ({ children }: PropsWithChildren) => ( - - {children} - + render( + + + , ) - const { result } = renderHook(() => useUpload(), { wrapper }) + const dropZone = screen.getByTestId('drop-zone') - // Create mock file entry (like from drag and drop) - const mockFile = { name: 'test.png', type: 'image/png' } - type FileCallback = (file: typeof mockFile) => void + // Fire dragenter first + await act(async () => { + fireEvent.dragEnter(dropZone) + }) + + // Fire drop without dataTransfer + await act(async () => { + fireEvent.drop(dropZone) + }) + + // Should still reset dragging state + expect(screen.getByTestId('dragging-state')).toHaveTextContent('not-dragging') + }) + + it('should not trigger file upload for invalid file types on drop', async () => { + render( + + + , + ) + + const dropZone = screen.getByTestId('drop-zone') + const invalidFile = new File(['test'], 'test.exe', { type: 'application/x-msdownload' }) + + await act(async () => { + fireEvent.drop(dropZone, { + dataTransfer: { + items: [{ + webkitGetAsEntry: () => null, + getAsFile: () => invalidFile, + }], + }, + }) + }) + + // Should show error toast for invalid file type + await waitFor(() => { + expect(Toast.notify).toHaveBeenCalledWith({ + type: 'error', + message: expect.any(String), + }) + }) + }) + + it('should handle drop with webkitGetAsEntry for file entries', async () => { + const onChange = vi.fn() + const mockFile = new File(['test'], 'test.png', { type: 'image/png' }) + + render( + + + , + ) + + const dropZone = screen.getByTestId('drop-zone') + + // Create a mock file entry that simulates webkitGetAsEntry behavior const mockFileEntry = { isFile: true, isDirectory: false, - file: (callback: FileCallback) => callback(mockFile), + file: (callback: (file: File) => void) => callback(mockFile), } - // Verify dropRef is exposed for attaching event listeners - expect(result.current.dropRef).toBeDefined() - // Verify mock entry is correctly shaped - expect(mockFileEntry.isFile).toBe(true) - }) - - it('should handle item without webkitGetAsEntry or getAsFile', async () => { - const { result } = renderHook(() => useUpload(), { - wrapper: createWrapper(), + await act(async () => { + fireEvent.drop(dropZone, { + dataTransfer: { + items: [{ + webkitGetAsEntry: () => mockFileEntry, + getAsFile: () => mockFile, + }], + }, + }) }) - // The handleDrop will resolve to empty array for such items - // This is internal behavior, we verify the hook doesn't crash - expect(result.current.dropRef).toBeDefined() + // Dragging should be reset + expect(screen.getByTestId('dragging-state')).toHaveTextContent('not-dragging') }) }) describe('Drag Events', () => { - it('should handle dragEnter event', () => { - const { result } = renderHook(() => useUpload(), { - wrapper: createWrapper(), + const TestComponent = () => { + const { dragging, dragRef, dropRef } = useUpload() + return ( +
+
+ {dragging ? 'dragging' : 'not-dragging'} +
+
+ ) + } + + it('should handle dragEnter event and update dragging state', async () => { + render( + + + , + ) + + const dropZone = screen.getByTestId('drop-zone') + + // Initially not dragging + expect(screen.getByTestId('dragging-state')).toHaveTextContent('not-dragging') + + // Fire dragEnter + await act(async () => { + fireEvent.dragEnter(dropZone, { + dataTransfer: { items: [] }, + }) }) - const mockDiv = document.createElement('div') - Object.defineProperty(result.current.dragRef, 'current', { - value: mockDiv, - writable: true, - }) - - // Verify dragRef is set up correctly - expect(result.current.dragRef.current).toBe(mockDiv) + // Should be dragging now + expect(screen.getByTestId('dragging-state')).toHaveTextContent('dragging') }) - it('should handle dragOver event', () => { - const { result } = renderHook(() => useUpload(), { - wrapper: createWrapper(), + it('should handle dragOver event without changing state', async () => { + render( + + + , + ) + + const dropZone = screen.getByTestId('drop-zone') + + // First trigger dragenter to set dragging + await act(async () => { + fireEvent.dragEnter(dropZone) }) - // dragOver just prevents default and stops propagation - // No state changes to verify - expect(result.current.dragging).toBe(false) + expect(screen.getByTestId('dragging-state')).toHaveTextContent('dragging') + + // dragOver should not change the dragging state + await act(async () => { + fireEvent.dragOver(dropZone) + }) + + // Should still be dragging + expect(screen.getByTestId('dragging-state')).toHaveTextContent('dragging') + }) + + it('should not set dragging to true when dragEnter target is dragRef', async () => { + render( + + + , + ) + + const dragBoundary = screen.getByTestId('drag-boundary') + + // Fire dragEnter directly on dragRef + await act(async () => { + fireEvent.dragEnter(dragBoundary) + }) + + // Should not be dragging when target is dragRef itself + expect(screen.getByTestId('dragging-state')).toHaveTextContent('not-dragging') + }) + + it('should not set dragging to false when dragLeave target is not dragRef', async () => { + render( + + + , + ) + + const dropZone = screen.getByTestId('drop-zone') + + // First trigger dragenter on dropZone to set dragging + await act(async () => { + fireEvent.dragEnter(dropZone) + }) + + expect(screen.getByTestId('dragging-state')).toHaveTextContent('dragging') + + // dragLeave on dropZone (not dragRef) should not change dragging state + await act(async () => { + fireEvent.dragLeave(dropZone) + }) + + // Should still be dragging (only dragLeave on dragRef resets) + expect(screen.getByTestId('dragging-state')).toHaveTextContent('dragging') }) }) })