Skip to content

Commit

Permalink
Added accelerator column and field
Browse files Browse the repository at this point in the history 
added tests

tooltip to popover

naming fix, and new ux

added new disbaled state
  • Loading branch information
@Gkrumbach07
Gkrumbach07 committed Nov 10, 2023
1 parent 0b7492e commit 9573e4b
Show file tree
Hide file tree
Showing 12 changed files with 364 additions and 49 deletions.
20 changes: 20 additions & 0 deletions backend/src/routes/api/images/imageUtils.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -242,6 +242,9 @@ const mapImageStreamToBYONImage = (is: ImageStream): BYONImage => ({
imported_time: is.metadata.creationTimestamp,
url: is.metadata.annotations['opendatahub.io/notebook-image-url'],
provider: is.metadata.annotations['opendatahub.io/notebook-image-creator'],
recommendedAcceleratorIdentifiers: jsonParseRecommendedAcceleratorIdentifiers(
is.metadata.annotations['opendatahub.io/recommended-accelerators'],
),
});

export const postImage = async (
Expand Down Expand Up @@ -282,6 +285,9 @@ export const postImage = async (
'opendatahub.io/notebook-image-name': body.display_name,
'opendatahub.io/notebook-image-url': fullURL,
'opendatahub.io/notebook-image-creator': body.provider,
'opendatahub.io/recommended-accelerators': JSON.stringify(
body.recommendedAcceleratorIdentifiers ?? [],
),
},
name: `custom-${translateDisplayNameForK8s(body.display_name)}`,
namespace: namespace,
Expand Down Expand Up @@ -419,6 +425,12 @@ export const updateImage = async (
imageStream.metadata.annotations['opendatahub.io/notebook-image-desc'] = body.description;
}

if (body.recommendedAcceleratorIdentifiers !== undefined) {
imageStream.metadata.annotations['opendatahub.io/recommended-accelerators'] = JSON.stringify(
body.recommendedAcceleratorIdentifiers,
);
}

await customObjectsApi
.patchNamespacedCustomObject(
'image.openshift.io',
Expand Down Expand Up @@ -452,3 +464,11 @@ const jsonParsePackage = (unparsedPackage: string): BYONImagePackage[] => {
return [];
}
};

const jsonParseRecommendedAcceleratorIdentifiers = (unparsedRecommendations: string): string[] => {
try {
return JSON.parse(unparsedRecommendations) || [];
} catch {
return [];
}
};
1 change: 1 addition & 0 deletions backend/src/types.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -485,6 +485,7 @@ export type BYONImage = {
visible: boolean;
software: BYONImagePackage[];
packages: BYONImagePackage[];
recommendedAcceleratorIdentifiers: string[];
};

export type ImageTag = {
Expand Down
1 change: 1 addition & 0 deletions frontend/src/__mocks__/mockByon.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ export const mockByon = (opts?: RecursivePartial<BYONImage[]>): BYONImage[] =>
name: 'byon-123',
display_name: 'Testing Custom Image',
description: 'A custom notebook image',
recommendedAcceleratorIdentifiers: [],
visible: true,
packages: [
{
Expand Down
32 changes: 31 additions & 1 deletion frontend/src/__tests__/integration/pages/notebookImageSettings/NotebookImageSettings.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,12 @@ test('Table filtering, sorting, searching', async ({ page }) => {
await page.getByRole('button', { name: 'Provider' }).click();
expect(page.getByText('image-0'));

// by accelerator
await page.getByRole('button', { name: 'Recommended accelerators' }).click();
expect(page.getByText('test-accelerator')).toHaveCount(0);
await page.getByRole('button', { name: 'Recommended accelerators' }).click();
expect(page.getByText('test-accelerator'));

// by enabled
await page.getByRole('button', { name: 'Enable', exact: true }).click();
expect(page.getByText('image-14'));
Expand Down Expand Up @@ -114,6 +120,30 @@ test('Import form fields', async ({ page }) => {
await page.getByLabel('Name *').fill('image');
await expect(page.getByRole('button', { name: 'Import' })).toBeEnabled();

// test accelerator select field
// select accelerator from api call
await page.getByPlaceholder('Example, nvidia.com/gpu').click();
await page.getByRole('option', { name: 'nvidia.com/gpu' }).click();

// create new and select
await page.getByPlaceholder('Example, nvidia.com/gpu').click();
await page.getByPlaceholder('Example, nvidia.com/gpu').fill('test.com/gpu');
await page.getByRole('option', { name: 'Create "test.com/gpu"' }).click();
await page.getByRole('button', { name: 'Options menu' }).click();
expect(page.getByText('test.com/gpu'));

// remove custom
await page.getByRole('button', { name: 'Remove test.com/gpu' }).click();
await page.getByRole('button', { name: 'Options menu' }).click();
await expect(page.getByText('test.com/gpu')).toHaveCount(0);

// reselect custom
await page
.getByRole('dialog', { name: 'Import notebook image' })
.getByRole('button', { name: 'Options menu' })
.click();
await page.getByRole('option', { name: 'test.com/gpu' }).click();

// test form is disabled after entering software add form
await page.getByTestId('add-software-button').click();
await expect(page.getByRole('button', { name: 'Import' })).toBeDisabled();
Expand Down Expand Up @@ -204,7 +234,7 @@ test('Edit form fields match', async ({ page }) => {
expect(await page.getByLabel('Image Location *').inputValue()).toBe('test-image:latest');
expect(await page.getByLabel('Name *').inputValue()).toBe('Testing Custom Image');
expect(await page.getByLabel('Description').inputValue()).toBe('A custom notebook image');

expect(page.getByText('nvidia.com/gpu'));
// test software and packages have correct values
expect(page.getByRole('gridcell', { name: 'test-software' }));
expect(page.getByRole('gridcell', { name: '2.0' }));
Expand Down
127 changes: 80 additions & 47 deletions ...d/src/__tests__/integration/pages/notebookImageSettings/NotebookImageSettings.stories.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,24 +5,48 @@ import { rest } from 'msw';
import { userEvent, within } from '@storybook/testing-library';
import BYONImages from '~/pages/BYONImages/BYONImages';
import { mockByon } from '~/__mocks__/mockByon';
import { mockAcceleratorProfile } from '~/__mocks__/mockAcceleratorProfile';
import { mockK8sResourceList } from '~/__mocks__/mockK8sResourceList';
import { mockProjectK8sResource } from '~/__mocks__/mockProjectK8sResource';
import { mockStatus } from '~/__mocks__/mockStatus';
import useDetectUser from '~/utilities/useDetectUser';

export default {
component: BYONImages,
} as Meta<typeof BYONImages>;

const Template: StoryFn<typeof BYONImages> = (args) => <BYONImages {...args} />;

export const Default: StoryObj = {
render: Template,
parameters: {
msw: {
handlers: [
rest.get('/api/images/byon', (req, res, ctx) =>
res(ctx.json(mockByon([{ url: 'test-image:latest' }]))),
handlers: {
status: [
rest.get('/api/k8s/apis/project.openshift.io/v1/projects', (req, res, ctx) =>
res(ctx.json(mockK8sResourceList([mockProjectK8sResource({})]))),
),
rest.get('/api/status', (req, res, ctx) => res(ctx.json(mockStatus()))),
],
accelerators: rest.get(
'/api/k8s/apis/dashboard.opendatahub.io/v1/namespaces/opendatahub/acceleratorprofiles',
(req, res, ctx) => res(ctx.json(mockK8sResourceList([mockAcceleratorProfile()]))),
),
],
images: rest.get('/api/images/byon', (req, res, ctx) =>
res(
ctx.json(
mockByon([
{ url: 'test-image:latest', recommendedAcceleratorIdentifiers: ['nvidia.com/gpu'] },
]),
),
),
),
},
},
},
} as Meta<typeof BYONImages>;

const Template: StoryFn<typeof BYONImages> = (args) => {
useDetectUser();
return <BYONImages {...args} />;
};

export const Default: StoryObj = {
render: Template,
play: async ({ canvasElement }) => {
// load page and wait until settled
const canvas = within(canvasElement);
Expand All @@ -34,7 +58,7 @@ export const Empty: StoryObj = {
render: Template,
parameters: {
msw: {
handlers: [rest.get('/api/images/byon', (req, res, ctx) => res(ctx.json([])))],
handlers: { images: rest.get('/api/images/byon', (req, res, ctx) => res(ctx.json([]))) },
},
},
play: async ({ canvasElement }) => {
Expand All @@ -48,7 +72,7 @@ export const LoadingError: StoryObj = {
render: Template,
parameters: {
msw: {
handlers: [rest.get('/api/images/byon', (req, res, ctx) => res(ctx.status(404)))],
handlers: { images: rest.get('/api/images/byon', (req, res, ctx) => res(ctx.status(404))) },
},
},
play: async ({ canvasElement }) => {
Expand All @@ -62,8 +86,8 @@ export const LargeList: StoryObj = {
render: Template,
parameters: {
msw: {
handlers: [
rest.get('/api/images/byon', (req, res, ctx) =>
handlers: {
images: rest.get('/api/images/byon', (req, res, ctx) =>
res(
ctx.json(
Array.from(
Expand All @@ -77,13 +101,14 @@ export const LargeList: StoryObj = {
description: `description-${i}`,
provider: `provider-${i}`,
visible: i % 3 === 0,
recommendedAcceleratorIdentifiers: i % 3 ? ['nvidia.com/gpu'] : [],
},
])[0],
),
),
),
),
],
},
},
},
play: async ({ canvasElement }) => {
Expand All @@ -97,39 +122,41 @@ export const ImageError: StoryObj = {
render: Template,
parameters: {
msw: {
handlers: [
rest.post('/api/images', (req, res, ctx) =>
res(
ctx.json({
success: false,
error: 'Testing create error message',
}),
handlers: {
images: [
rest.post('/api/images', (req, res, ctx) =>
res(
ctx.json({
success: false,
error: 'Testing create error message',
}),
),
),
),
rest.put('/api/images/byon-1', (req, res, ctx) =>
res(
ctx.json({
success: false,
error: 'Testing edit error message',
}),
rest.put('/api/images/byon-1', (req, res, ctx) =>
res(
ctx.json({
success: false,
error: 'Testing edit error message',
}),
),
),
),
rest.delete('/api/images/byon-1', (req, res, ctx) =>
res(ctx.status(404, 'Testing delete error message')),
),
rest.get('/api/images/byon', (req, res, ctx) =>
res(
ctx.json(
mockByon([
{
name: 'byon-1',
error: 'Testing error message',
},
]),
rest.delete('/api/images/byon-1', (req, res, ctx) =>
res(ctx.status(404, 'Testing delete error message')),
),
rest.get('/api/images/byon', (req, res, ctx) =>
res(
ctx.json(
mockByon([
{
name: 'byon-1',
error: 'Testing error message',
},
]),
),
),
),
),
],
],
},
},
},
play: async ({ canvasElement }) => {
Expand All @@ -148,7 +175,9 @@ export const EditModal: StoryObj = {
element: '.pf-c-backdrop',
},
msw: {
handlers: [rest.get('/api/images/byon', (req, res, ctx) => res(ctx.json(mockByon())))],
handlers: {
images: rest.get('/api/images/byon', (req, res, ctx) => res(ctx.json(mockByon()))),
},
},
},
play: async ({ canvasElement }) => {
Expand All @@ -169,7 +198,9 @@ export const DeleteModal: StoryObj = {
element: '.pf-c-backdrop',
},
msw: {
handlers: [rest.get('/api/images/byon', (req, res, ctx) => res(ctx.json(mockByon())))],
handlers: {
images: rest.get('/api/images/byon', (req, res, ctx) => res(ctx.json(mockByon()))),
},
},
},
play: async ({ canvasElement }) => {
Expand All @@ -190,7 +221,9 @@ export const ImportModal: StoryObj = {
element: '.pf-c-backdrop',
},
msw: {
handlers: [rest.get('/api/images/byon', (req, res, ctx) => res(ctx.json(mockByon())))],
handlers: {
images: rest.get('/api/images/byon', (req, res, ctx) => res(ctx.json(mockByon()))),
},
},
},
play: async ({ canvasElement }) => {
Expand Down
Loading

0 comments on commit 9573e4b

Please sign in to comment.