Skip to content

Commit 70fa36d

Browse files
Philip-CarneiroPhilip-Carneiro
committed
truncate error msg from transfer jobs
Signed-off-by: Philip Colares Carneiro <philip.colares@gmail.com>
1 parent 6e18d5a commit 70fa36d

2 files changed

Lines changed: 208 additions & 13 deletions

File tree

‎clients/ui/frontend/src/app/pages/modelRegistry/screens/ModelTransferJobs/ModelTransferJobTableRow.tsx‎

Lines changed: 33 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -130,27 +130,47 @@ const ModelTransferJobTableRow: React.FC<ModelTransferJobTableRowProps> = ({
130130
</Content>
131131
</Td>
132132
<Td dataLabel="Transfer job status">
133-
<Flex alignItems={{ default: 'alignItemsCenter' }} spaceItems={{ default: 'spaceItemsSm' }}>
133+
<Flex direction={{ default: 'column' }} spaceItems={{ default: 'spaceItemsXs' }}>
134134
<FlexItem>
135-
<Label
136-
color={statusInfo.color}
137-
icon={statusInfo.icon}
138-
data-testid="job-status"
139-
isClickable
140-
onClick={() => setIsStatusModalOpen(true)}
135+
<Flex
136+
alignItems={{ default: 'alignItemsCenter' }}
137+
spaceItems={{ default: 'spaceItemsSm' }}
141138
>
142-
{statusInfo.label}
143-
</Label>
139+
<FlexItem>
140+
<Label
141+
color={statusInfo.color}
142+
icon={statusInfo.icon}
143+
data-testid="job-status"
144+
isClickable
145+
onClick={() => setIsStatusModalOpen(true)}
146+
>
147+
{statusInfo.label}
148+
</Label>
149+
</FlexItem>
150+
{job.status === ModelTransferJobStatus.FAILED && onRequestRetry && (
151+
<FlexItem>
152+
<Button
153+
variant="link"
154+
isInline
155+
onClick={() => onRequestRetry(job)}
156+
data-testid="job-retry-button"
157+
>
158+
Retry
159+
</Button>
160+
</FlexItem>
161+
)}
162+
</Flex>
144163
</FlexItem>
145-
{job.status === ModelTransferJobStatus.FAILED && onRequestRetry && (
164+
{job.status === ModelTransferJobStatus.FAILED && job.errorMessage && (
146165
<FlexItem>
147166
<Button
148167
variant="link"
149168
isInline
150-
onClick={() => onRequestRetry(job)}
151-
data-testid="job-retry-button"
169+
onClick={() => setIsStatusModalOpen(true)}
170+
data-testid="job-error-message"
171+
style={{ textDecoration: 'underline dotted', textUnderlineOffset: '3px' }}
152172
>
153-
Retry
173+
<Truncate content={job.errorMessage} />
154174
</Button>
155175
</FlexItem>
156176
)}

‎clients/ui/frontend/src/app/pages/modelRegistry/screens/ModelTransferJobs/__tests__/ModelTransferJobTableRow.spec.tsx‎

Lines changed: 175 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,175 @@
1+
import * as React from 'react';
2+
import { render, screen, fireEvent, within } from '@testing-library/react';
3+
import '@testing-library/jest-dom';
4+
import { MemoryRouter } from 'react-router-dom';
5+
import { Table, Tbody } from '@patternfly/react-table';
6+
import { mockModelTransferJob } from '~/__mocks__/mockModelTransferJob';
7+
import { ModelTransferJobStatus } from '~/app/types';
8+
import { ModelRegistrySelectorContext } from '~/app/context/ModelRegistrySelectorContext';
9+
import ModelTransferJobTableRow from '~/app/pages/modelRegistry/screens/ModelTransferJobs/ModelTransferJobTableRow';
10+
11+
jest.mock('~/app/hooks/useModelRegistryAPI', () => ({
12+
useModelRegistryAPI: jest.fn().mockReturnValue({
13+
api: {
14+
createRegisteredModel: jest.fn(),
15+
createModelVersionForRegisteredModel: jest.fn(),
16+
createModelArtifactForModelVersion: jest.fn(),
17+
getRegisteredModel: jest.fn(),
18+
getModelVersion: jest.fn(),
19+
listModelVersions: jest.fn(),
20+
listRegisteredModels: jest.fn(),
21+
getModelVersionsByRegisteredModel: jest.fn(),
22+
getModelArtifactsByModelVersion: jest.fn(),
23+
patchRegisteredModel: jest.fn(),
24+
patchModelVersion: jest.fn(),
25+
patchModelArtifact: jest.fn(),
26+
listModelTransferJobs: jest.fn(),
27+
getModelTransferJobByName: jest.fn(),
28+
createModelTransferJob: jest.fn(),
29+
updateModelTransferJob: jest.fn(),
30+
deleteModelTransferJob: jest.fn(),
31+
getModelTransferJobEvents: jest.fn(),
32+
},
33+
apiAvailable: true,
34+
refreshAllAPI: jest.fn(),
35+
}),
36+
}));
37+
38+
jest.mock('mod-arch-core', () => {
39+
const actual = jest.requireActual('mod-arch-core');
40+
return {
41+
...actual,
42+
useFetchState: jest.fn().mockReturnValue([[], true, undefined, jest.fn()]),
43+
};
44+
});
45+
46+
const mockContextValue = {
47+
modelRegistriesLoaded: true,
48+
modelRegistriesLoadError: undefined,
49+
modelRegistries: [],
50+
preferredModelRegistry: undefined,
51+
updatePreferredModelRegistry: jest.fn(),
52+
};
53+
54+
const renderRow = (props: React.ComponentProps<typeof ModelTransferJobTableRow>) =>
55+
render(
56+
<MemoryRouter>
57+
<ModelRegistrySelectorContext.Provider value={mockContextValue}>
58+
<Table>
59+
<Tbody>
60+
<ModelTransferJobTableRow {...props} />
61+
</Tbody>
62+
</Table>
63+
</ModelRegistrySelectorContext.Provider>
64+
</MemoryRouter>,
65+
);
66+
67+
describe('ModelTransferJobTableRow', () => {
68+
it('shows truncated error message below Failed label when errorMessage exists', () => {
69+
const job = mockModelTransferJob({
70+
status: ModelTransferJobStatus.FAILED,
71+
errorMessage: 'Connection timeout while uploading to destination bucket',
72+
});
73+
74+
renderRow({ job });
75+
76+
expect(screen.getByTestId('job-status')).toHaveTextContent('Failed');
77+
expect(screen.getByTestId('job-error-message')).toBeInTheDocument();
78+
expect(screen.getByTestId('job-error-message')).toHaveTextContent(
79+
'Connection timeout while uploading to destination bucket',
80+
);
81+
});
82+
83+
it('does not show error message when job is FAILED but errorMessage is undefined', () => {
84+
const job = mockModelTransferJob({
85+
status: ModelTransferJobStatus.FAILED,
86+
errorMessage: undefined,
87+
});
88+
89+
renderRow({ job });
90+
91+
expect(screen.getByTestId('job-status')).toHaveTextContent('Failed');
92+
expect(screen.queryByTestId('job-error-message')).not.toBeInTheDocument();
93+
});
94+
95+
it('does not show error message for non-FAILED statuses', () => {
96+
const job = mockModelTransferJob({
97+
status: ModelTransferJobStatus.COMPLETED,
98+
});
99+
100+
renderRow({ job });
101+
102+
expect(screen.getByTestId('job-status')).toHaveTextContent('Complete');
103+
expect(screen.queryByTestId('job-error-message')).not.toBeInTheDocument();
104+
});
105+
106+
it('opens status modal when clicking the truncated error message', () => {
107+
const job = mockModelTransferJob({
108+
status: ModelTransferJobStatus.FAILED,
109+
errorMessage: 'Upload failed: insufficient storage',
110+
});
111+
112+
renderRow({ job });
113+
114+
expect(screen.queryByTestId('transfer-job-status-modal')).not.toBeInTheDocument();
115+
116+
fireEvent.click(screen.getByTestId('job-error-message'));
117+
118+
expect(screen.getByTestId('transfer-job-status-modal')).toBeInTheDocument();
119+
});
120+
121+
it('opens status modal when clicking the Failed label', () => {
122+
const job = mockModelTransferJob({
123+
status: ModelTransferJobStatus.FAILED,
124+
errorMessage: 'Some error',
125+
});
126+
127+
renderRow({ job });
128+
129+
expect(screen.queryByTestId('transfer-job-status-modal')).not.toBeInTheDocument();
130+
131+
const labelWrapper = screen.getByTestId('job-status');
132+
const labelButton = within(labelWrapper).getByRole('button');
133+
fireEvent.click(labelButton);
134+
135+
expect(screen.getByTestId('transfer-job-status-modal')).toBeInTheDocument();
136+
});
137+
138+
it('shows retry button for failed jobs when onRequestRetry is provided', () => {
139+
const mockRetry = jest.fn();
140+
const job = mockModelTransferJob({
141+
status: ModelTransferJobStatus.FAILED,
142+
errorMessage: 'Connection error',
143+
});
144+
145+
renderRow({ job, onRequestRetry: mockRetry });
146+
147+
expect(screen.getByTestId('job-retry-button')).toBeInTheDocument();
148+
fireEvent.click(screen.getByTestId('job-retry-button'));
149+
expect(mockRetry).toHaveBeenCalledWith(job);
150+
});
151+
152+
it('applies dotted underline style to the error message', () => {
153+
const job = mockModelTransferJob({
154+
status: ModelTransferJobStatus.FAILED,
155+
errorMessage: 'Some error message',
156+
});
157+
158+
renderRow({ job });
159+
160+
const errorButton = screen.getByTestId('job-error-message');
161+
expect(errorButton).toHaveStyle({ textDecoration: 'underline dotted' });
162+
});
163+
164+
it('does not show error message for empty string errorMessage', () => {
165+
const job = mockModelTransferJob({
166+
status: ModelTransferJobStatus.FAILED,
167+
errorMessage: '',
168+
});
169+
170+
renderRow({ job });
171+
172+
expect(screen.getByTestId('job-status')).toHaveTextContent('Failed');
173+
expect(screen.queryByTestId('job-error-message')).not.toBeInTheDocument();
174+
});
175+
});

0 commit comments

Comments
 (0)