[ui-sb] add test for file upload hooks #4138
Conversation
|
✅ Test files were modified. Ensure that the tests cover all relevant changes. ✅ |
Python Coverage Report •
Pytest Report
|
There was a problem hiding this comment.
Pull Request Overview
This PR adds tests for file upload hooks to improve coverage for both regular and chunked uploads while refining some test naming for clarity. Key changes include:
- Updates to the useSaveData and useLoadData test files to improve error and state handling tests.
- Addition of new test suites for useRegularUpload, useFileUpload, and useChunkUpload.
- A minor refactor in the useChunkUpload hook to correct an import for GET_TASKS_URL.
Reviewed Changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| desktop/core/src/desktop/js/utils/hooks/useSaveData/useSaveData.test.tsx | Updated test naming and moved mockPost initialization |
| desktop/core/src/desktop/js/utils/hooks/useLoadData/useLoadData.test.tsx | Added tests for reload behavior and polling intervals |
| desktop/core/src/desktop/js/utils/hooks/useFileUpload/useRegularUpload.test.tsx | Added tests for regular file upload handling and state updates |
| desktop/core/src/desktop/js/utils/hooks/useFileUpload/useFileUpload.test.tsx | Added tests for file upload type selection and cancellation |
| desktop/core/src/desktop/js/utils/hooks/useFileUpload/useChunkUpload.ts | Refactored GET_TASKS_URL import |
| desktop/core/src/desktop/js/utils/hooks/useFileUpload/useChunkUpload.test.tsx | Added tests to validate chunk upload processing and error handling |
|
|
||
| result.current.addFiles([mockFile]); | ||
|
|
||
| expect(mockSave).toHaveBeenCalledWith(expect.any(FormData), expect.any(Object)); |
There was a problem hiding this comment.
why use any, why not make sure it is called with the actual data?
| expect(mockUpdateFileVariables.mock.calls).toEqual( | ||
| expect.arrayContaining([ | ||
| [mockFile.uuid, { status: FileStatus.Uploading }], | ||
| [mockFile.uuid, { status: FileStatus.Failed, error: expect.any(Error) }] |
There was a problem hiding this comment.
nit: Check that it is the expected error
'Upload server ran out of space. Try again later.'
| it('returns isLoading as true if either upload method is loading', () => { | ||
| mockLoading.mockReturnValue(true); | ||
| const { result } = renderHook(() => | ||
| useFileUpload({ isChunkUpload: false, onComplete: mockOnComplete }) |
There was a problem hiding this comment.
Test state "either upload method" but only non chunked method is tested here
| })); | ||
|
|
||
| describe('useFileUpload', () => { | ||
| const mockOnComplete = jest.fn(); |
There was a problem hiding this comment.
We never test that this mock is actually called. Shouldn't we?
| })) | ||
| })); | ||
|
|
||
| describe('useFileUpload', () => { |
There was a problem hiding this comment.
How about tests for failing uploads?
What changes were proposed in this pull request?
How was this patch tested?
Please review Hue Contributing Guide before opening a pull request.