From bcb8b34e309be547d7875692b69d47fdab26a922 Mon Sep 17 00:00:00 2001 From: Yulia Krimerman Date: Tue, 14 Apr 2026 13:44:14 -0400 Subject: [PATCH 1/2] add unit tests Signed-off-by: Yulia Krimerman --- .../__tests__/useModelVersionById.spec.ts | 139 ++++++++++++++++++ 1 file changed, 139 insertions(+) create mode 100644 clients/ui/frontend/src/app/hooks/__tests__/useModelVersionById.spec.ts diff --git a/clients/ui/frontend/src/app/hooks/__tests__/useModelVersionById.spec.ts b/clients/ui/frontend/src/app/hooks/__tests__/useModelVersionById.spec.ts new file mode 100644 index 0000000000..3076e92b00 --- /dev/null +++ b/clients/ui/frontend/src/app/hooks/__tests__/useModelVersionById.spec.ts @@ -0,0 +1,139 @@ +// eslint-disable-next-line @typescript-eslint/no-unused-vars +import * as React from 'react'; +import { waitFor } from '@testing-library/react'; +import { useFetchState } from 'mod-arch-core'; +import useModelVersionById from '~/app/hooks/useModelVersionById'; +import { useModelRegistryAPI } from '~/app/hooks/useModelRegistryAPI'; +import { ModelRegistryAPIs } from '~/app/types'; +import { mockModelVersion } from '~/__mocks__/mockModelVersion'; +import { testHook } from '~/__tests__/unit/testUtils/hooks'; + +jest.mock('mod-arch-core', () => ({ + useFetchState: jest.fn(), + NotReadyError: class NotReadyError extends Error { + constructor(message: string) { + super(message); + this.name = 'NotReadyError'; + } + }, +})); + +global.fetch = jest.fn(); + +jest.mock('~/app/hooks/useModelRegistryAPI', () => ({ + useModelRegistryAPI: jest.fn(), +})); + +const mockUseModelRegistryAPI = jest.mocked(useModelRegistryAPI); +const mockUseFetchState = jest.mocked(useFetchState); + +const mockModelRegistryAPIs: ModelRegistryAPIs = { + createRegisteredModel: jest.fn(), + createModelVersionForRegisteredModel: jest.fn(), + createModelArtifactForModelVersion: jest.fn(), + getRegisteredModel: jest.fn(), + getModelVersion: jest.fn(), + listModelVersions: jest.fn(), + listRegisteredModels: jest.fn(), + getModelVersionsByRegisteredModel: jest.fn(), + getModelArtifactsByModelVersion: jest.fn(), + patchRegisteredModel: jest.fn(), + patchModelVersion: jest.fn(), + patchModelArtifact: jest.fn(), + listModelTransferJobs: jest.fn(), + getModelTransferJobByName: jest.fn(), + createModelTransferJob: jest.fn(), + updateModelTransferJob: jest.fn(), + deleteModelTransferJob: jest.fn(), + getModelTransferJobEvents: jest.fn(), +}; + +describe('useModelVersionById', () => { + beforeEach(() => { + jest.clearAllMocks(); + }); + + it('should return an error when the API is not available', async () => { + mockUseModelRegistryAPI.mockReturnValue({ + api: mockModelRegistryAPIs, + apiAvailable: false, + refreshAllAPI: jest.fn(), + }); + + const mockError = new Error('API not yet available'); + mockUseFetchState.mockReturnValue([null, false, mockError, jest.fn()]); + + const { result } = testHook(useModelVersionById)('version-1'); + + await waitFor(() => { + const [, , error] = result.current; + expect(error).toBeInstanceOf(Error); + expect(error?.message).toBe('API not yet available'); + }); + }); + + it('should silently fail when modelVersionId is not provided', async () => { + mockUseModelRegistryAPI.mockReturnValue({ + api: mockModelRegistryAPIs, + apiAvailable: true, + refreshAllAPI: jest.fn(), + }); + + mockUseFetchState.mockReturnValue([null, false, undefined, jest.fn()]); + + const { result } = testHook(useModelVersionById)(); + + await waitFor(() => { + const [data, , error] = result.current; + expect(data).toBeNull(); + expect(error).toBeUndefined(); + }); + }); + + it('should fetch the model version when API is available and id is provided', async () => { + const mockedVersion = mockModelVersion({ id: 'version-1', name: 'my-version' }); + + mockUseModelRegistryAPI.mockReturnValue({ + api: { + ...mockModelRegistryAPIs, + getModelVersion: jest.fn().mockResolvedValue(mockedVersion), + }, + apiAvailable: true, + refreshAllAPI: jest.fn(), + }); + + mockUseFetchState.mockReturnValue([mockedVersion, true, undefined, jest.fn()]); + + const { result } = testHook(useModelVersionById)('version-1'); + + await waitFor(() => { + const [data, loaded] = result.current; + expect(loaded).toBe(true); + expect(data).toEqual(mockedVersion); + }); + }); + + it('should pass the correct modelVersionId to api.getModelVersion', async () => { + const getModelVersionMock = jest.fn().mockResolvedValue(mockModelVersion({ id: 'v-42' })); + + mockUseModelRegistryAPI.mockReturnValue({ + api: { ...mockModelRegistryAPIs, getModelVersion: getModelVersionMock }, + apiAvailable: true, + refreshAllAPI: jest.fn(), + }); + + // Capture the callback that the hook passes to useFetchState + mockUseFetchState.mockReturnValue([null, false, undefined, jest.fn()]); + + testHook(useModelVersionById)('v-42'); + + // useFetchState receives the callback as its first argument + const capturedCallback = mockUseFetchState.mock.calls[0][0] as ( + opts: unknown, + ) => Promise; + + await capturedCallback({}); + + expect(getModelVersionMock).toHaveBeenCalledWith({}, 'v-42'); + }); +}); From 8baf47d89387adbcd46a1f41b9b8b28a9154a4e5 Mon Sep 17 00:00:00 2001 From: Yulia Krimerman Date: Thu, 16 Apr 2026 15:22:55 -0400 Subject: [PATCH 2/2] addressed comments Signed-off-by: Yulia Krimerman --- .../__tests__/useModelVersionById.spec.ts | 71 +++++-------------- 1 file changed, 18 insertions(+), 53 deletions(-) diff --git a/clients/ui/frontend/src/app/hooks/__tests__/useModelVersionById.spec.ts b/clients/ui/frontend/src/app/hooks/__tests__/useModelVersionById.spec.ts index 3076e92b00..7154b675a9 100644 --- a/clients/ui/frontend/src/app/hooks/__tests__/useModelVersionById.spec.ts +++ b/clients/ui/frontend/src/app/hooks/__tests__/useModelVersionById.spec.ts @@ -1,6 +1,5 @@ // eslint-disable-next-line @typescript-eslint/no-unused-vars import * as React from 'react'; -import { waitFor } from '@testing-library/react'; import { useFetchState } from 'mod-arch-core'; import useModelVersionById from '~/app/hooks/useModelVersionById'; import { useModelRegistryAPI } from '~/app/hooks/useModelRegistryAPI'; @@ -48,72 +47,44 @@ const mockModelRegistryAPIs: ModelRegistryAPIs = { getModelTransferJobEvents: jest.fn(), }; +const captureCallback = (): ((opts: unknown) => Promise) => { + mockUseFetchState.mockReturnValue([null, false, undefined, jest.fn()]); + return mockUseFetchState.mock.calls[0][0] as (opts: unknown) => Promise; +}; + describe('useModelVersionById', () => { beforeEach(() => { jest.clearAllMocks(); }); - it('should return an error when the API is not available', async () => { + it('should reject with an error when the API is not available', async () => { mockUseModelRegistryAPI.mockReturnValue({ api: mockModelRegistryAPIs, apiAvailable: false, refreshAllAPI: jest.fn(), }); - const mockError = new Error('API not yet available'); - mockUseFetchState.mockReturnValue([null, false, mockError, jest.fn()]); - - const { result } = testHook(useModelVersionById)('version-1'); + testHook(useModelVersionById)('version-1'); + const callback = captureCallback(); - await waitFor(() => { - const [, , error] = result.current; - expect(error).toBeInstanceOf(Error); - expect(error?.message).toBe('API not yet available'); - }); + await expect(callback({})).rejects.toThrow('API not yet available'); }); - it('should silently fail when modelVersionId is not provided', async () => { + it('should reject with NotReadyError when modelVersionId is not provided', async () => { mockUseModelRegistryAPI.mockReturnValue({ api: mockModelRegistryAPIs, apiAvailable: true, refreshAllAPI: jest.fn(), }); - mockUseFetchState.mockReturnValue([null, false, undefined, jest.fn()]); - - const { result } = testHook(useModelVersionById)(); + testHook(useModelVersionById)(); + const callback = captureCallback(); - await waitFor(() => { - const [data, , error] = result.current; - expect(data).toBeNull(); - expect(error).toBeUndefined(); - }); + await expect(callback({})).rejects.toThrow('No model version id'); + await expect(callback({})).rejects.toMatchObject({ name: 'NotReadyError' }); }); - it('should fetch the model version when API is available and id is provided', async () => { - const mockedVersion = mockModelVersion({ id: 'version-1', name: 'my-version' }); - - mockUseModelRegistryAPI.mockReturnValue({ - api: { - ...mockModelRegistryAPIs, - getModelVersion: jest.fn().mockResolvedValue(mockedVersion), - }, - apiAvailable: true, - refreshAllAPI: jest.fn(), - }); - - mockUseFetchState.mockReturnValue([mockedVersion, true, undefined, jest.fn()]); - - const { result } = testHook(useModelVersionById)('version-1'); - - await waitFor(() => { - const [data, loaded] = result.current; - expect(loaded).toBe(true); - expect(data).toEqual(mockedVersion); - }); - }); - - it('should pass the correct modelVersionId to api.getModelVersion', async () => { + it('should call api.getModelVersion with the correct modelVersionId', async () => { const getModelVersionMock = jest.fn().mockResolvedValue(mockModelVersion({ id: 'v-42' })); mockUseModelRegistryAPI.mockReturnValue({ @@ -122,18 +93,12 @@ describe('useModelVersionById', () => { refreshAllAPI: jest.fn(), }); - // Capture the callback that the hook passes to useFetchState - mockUseFetchState.mockReturnValue([null, false, undefined, jest.fn()]); - testHook(useModelVersionById)('v-42'); + const callback = captureCallback(); - // useFetchState receives the callback as its first argument - const capturedCallback = mockUseFetchState.mock.calls[0][0] as ( - opts: unknown, - ) => Promise; - - await capturedCallback({}); + const result = await callback({}); expect(getModelVersionMock).toHaveBeenCalledWith({}, 'v-42'); + expect(result).toEqual(mockModelVersion({ id: 'v-42' })); }); });