test(ui): add unit tests for useModelVersionById hook by YuliaKrimerman · Pull Request #2595 · kubeflow/hub

YuliaKrimerman · 2026-04-14T17:45:20Z

Description

Adds unit tests for the useModelVersionById hook (clients/ui/frontend/src/app/hooks/useModelVersionById.ts), which previously had no test coverage.

The test file covers four scenarios:

API unavailable — verifies the hook returns an error when apiAvailable is false
No modelVersionId — verifies the hook silently fails (returns null, no error) when called without an ID
Successful fetch — verifies the hook returns the fetched ModelVersion and loaded: true
Correct API arguments — captures the callback passed to useFetchState and asserts api.getModelVersion is called with the correct modelVersionId

Follows the same testing patterns established by useModelArtifactsByVersionId.spec.ts and useModelTransferJobs.spec.ts.

How Has This Been Tested?

All 4 unit tests pass locally via npx jest src/app/hooks/__tests__/useModelVersionById.spec.ts
ESLint passes with no errors on the new file

Merge criteria:

All the commits have been signed-off (To pass the DCO check)
The commits have meaningful messages
Automated tests are provided as part of the PR for major new functionalities; testing instructions have been added in the PR body (for PRs involving changes that are not immediately obvious).
The developer has manually tested the changes and verified that the changes work.
Code changes follow the kubeflow contribution guidelines.
For first time contributors: Please reach out to the Reviewers to ensure all tests are being run, ensuring the label ok-to-test has been added to the PR.

Signed-off-by: Yulia Krimerman <juliapiterova@hotmail.com>

manaswinidas

3 of the 4 tests are tautological — they mock useFetchState's return value then assert exactly what was mocked, without ever invoking the hook's callback. Only test 4 (callback capture) genuinely exercises the hook logic, and it only covers the happy path.

Please rewrite tests 1–2 to capture the callback (same pattern as test 4) and verify it rejects with the expected errors. Test 3 can be dropped since test 4 already covers that path. This will give real coverage of all 3 branches.

manaswinidas · 2026-04-15T11:44:08Z

+      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 () => {


Tautological: you mock useFetchState to return [null, false, mockError, …] then assert the error is exactly what you mocked. The hook's !apiAvailable branch is never invoked.

Capture the callback instead (same approach as test 4) and verify it rejects:

const cb = mockUseFetchState.mock.calls[0][0]; await expect(cb({})).rejects.toThrow('API not yet available');

manaswinidas · 2026-04-15T11:44:17Z

+      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();
+    });
+  });


Same issue — useFetchState is mocked to return [null, false, undefined, …], then you assert data is null and error is undefined. This just validates the mock, not the hook.

To genuinely test the !modelVersionId branch, capture the callback and check:

const cb = mockUseFetchState.mock.calls[0][0]; await expect(cb({})).rejects.toThrow('No model version id');

manaswinidas · 2026-04-15T11:49:06Z

+  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);


Also tautological — asserts data and loaded equal exactly what mockUseFetchState was told to return. The getModelVersion mock is set up but never called.

This test can be removed entirely; test 4 already covers the happy path by actually invoking the captured callback and verifying getModelVersion receives the right args.

manaswinidas · 2026-04-15T11:49:21Z

+  });
+
+  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<unknown>;
+
+    await capturedCallback({});
+
+    expect(getModelVersionMock).toHaveBeenCalledWith({}, 'v-42');


This is the only test that genuinely exercises hook logic — nice. Consider reusing the same callback-capture approach for the !apiAvailable and !modelVersionId branches so all three paths are properly covered.

Signed-off-by: Yulia Krimerman <juliapiterova@hotmail.com>

manaswinidas

/lgtm
/approve

All review comments addressed

google-oss-prow · 2026-04-17T05:28:19Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: manaswinidas

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details

Needs approval from an approver in each of these files:

~~clients/ui/OWNERS~~ [manaswinidas]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

* add unit tests Signed-off-by: Yulia Krimerman <juliapiterova@hotmail.com> * addressed comments Signed-off-by: Yulia Krimerman <juliapiterova@hotmail.com> --------- Signed-off-by: Yulia Krimerman <juliapiterova@hotmail.com>

add unit tests

bcb8b34

Signed-off-by: Yulia Krimerman <juliapiterova@hotmail.com>

google-oss-prow Bot requested review from chambridge and fege April 14, 2026 17:45

google-oss-prow Bot added the size/L label Apr 14, 2026

github-actions Bot added the Area/UI label Apr 14, 2026

manaswinidas suggested changes Apr 15, 2026

View reviewed changes

google-oss-prow Bot assigned manaswinidas Apr 15, 2026

addressed comments

8baf47d

Signed-off-by: Yulia Krimerman <juliapiterova@hotmail.com>

manaswinidas approved these changes Apr 17, 2026

View reviewed changes

google-oss-prow Bot added the lgtm label Apr 17, 2026

google-oss-prow Bot added the approved label Apr 17, 2026

google-oss-prow Bot merged commit d9aa591 into kubeflow:main Apr 17, 2026
30 of 31 checks passed

jonburdo mentioned this pull request Apr 17, 2026

[pull] main from kubeflow:main opendatahub-io/model-registry#1707

Merged

This was referenced Apr 17, 2026

Sync from kubeflow/model-registry d941024 manaswinidas/odh-dashboard#12

Closed

Sync from kubeflow/model-registry d941024 opendatahub-io/odh-dashboard#7301

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

test(ui): add unit tests for useModelVersionById hook#2595

test(ui): add unit tests for useModelVersionById hook#2595
google-oss-prow[bot] merged 2 commits intokubeflow:mainfrom
YuliaKrimerman:useModelArtifactsByVersionId-tests

YuliaKrimerman commented Apr 14, 2026

Uh oh!

manaswinidas left a comment

Uh oh!

manaswinidas Apr 15, 2026

Uh oh!

manaswinidas Apr 15, 2026

Uh oh!

manaswinidas Apr 15, 2026

Uh oh!

manaswinidas Apr 15, 2026

Uh oh!

manaswinidas left a comment

Uh oh!

google-oss-prow Bot commented Apr 17, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

YuliaKrimerman commented Apr 14, 2026

Description

How Has This Been Tested?

Merge criteria:

Uh oh!

manaswinidas left a comment

Choose a reason for hiding this comment

Uh oh!

manaswinidas Apr 15, 2026

Choose a reason for hiding this comment

Uh oh!

manaswinidas Apr 15, 2026

Choose a reason for hiding this comment

Uh oh!

manaswinidas Apr 15, 2026

Choose a reason for hiding this comment

Uh oh!

manaswinidas Apr 15, 2026

Choose a reason for hiding this comment

Uh oh!

manaswinidas left a comment

Choose a reason for hiding this comment

Uh oh!

google-oss-prow Bot commented Apr 17, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants