From fe803cc585f1f7c70281c9e4979a22acf398a40b Mon Sep 17 00:00:00 2001 From: manaswinidas Date: Tue, 7 May 2024 16:27:54 +0530 Subject: [PATCH] Tests for model versions table --- frontend/src/__mocks__/mockModelVersion.ts | 14 +- .../src/__mocks__/mockModelVersionList.ts | 10 +- .../e2e/modelRegistry/ModelRegistry.cy.ts | 24 ++- .../e2e/modelRegistry/ModelVersions.cy.ts | 178 ++++++++++++++++++ .../cypress/cypress/pages/modelRegistry.ts | 52 ++++- .../cypress/cypress/support/commands/odh.ts | 11 +- .../screens/EmptyModelRegistryState.tsx | 4 +- .../modelRegistry/screens/ModelRegistry.tsx | 3 +- .../screens/ModelVersionListView.tsx | 5 +- .../modelRegistry/screens/ModelVersions.tsx | 4 +- .../screens/ModelVersionsHeaderActions.tsx | 28 +-- .../screens/ModelVersionsTableRow.tsx | 2 +- 12 files changed, 289 insertions(+), 46 deletions(-) create mode 100644 frontend/src/__tests__/cypress/cypress/e2e/modelRegistry/ModelVersions.cy.ts diff --git a/frontend/src/__mocks__/mockModelVersion.ts b/frontend/src/__mocks__/mockModelVersion.ts index afe27ff872..c1c2667784 100644 --- a/frontend/src/__mocks__/mockModelVersion.ts +++ b/frontend/src/__mocks__/mockModelVersion.ts @@ -1,20 +1,26 @@ -import { ModelVersion, ModelVersionState } from '~/concepts/modelRegistry/types'; +import { ModelRegistryBase, ModelVersion, ModelVersionState } from '~/concepts/modelRegistry/types'; type MockModelVersionType = { author?: string; + id?: string; registeredModelId?: string; + name?: string; + customProperties?: ModelRegistryBase['customProperties']; }; export const mockModelVersion = ({ author = 'Test author', registeredModelId = '1', + name = 'new model version', + customProperties = {}, + id = '', }: MockModelVersionType): ModelVersion => ({ author, createTimeSinceEpoch: '1712234877179', - customProperties: {}, - id: '26', + customProperties, + id, lastUpdateTimeSinceEpoch: '1712234877179', - name: 'fraud detection model version 1', + name, state: ModelVersionState.ARCHIVED, registeredModelId, }); diff --git a/frontend/src/__mocks__/mockModelVersionList.ts b/frontend/src/__mocks__/mockModelVersionList.ts index c9e845c93f..4affcb06a0 100644 --- a/frontend/src/__mocks__/mockModelVersionList.ts +++ b/frontend/src/__mocks__/mockModelVersionList.ts @@ -1,9 +1,11 @@ +/* eslint-disable camelcase */ import { ModelVersionList } from '~/concepts/modelRegistry/types'; -import { mockModelVersion } from './mockModelVersion'; -export const mockModelVersionList = (): ModelVersionList => ({ - items: [mockModelVersion({ author: 'Author 1', registeredModelId: '1' })], +export const mockModelVersionList = ({ + items = [], +}: Partial): ModelVersionList => ({ + items, nextPageToken: '', pageSize: 0, - size: 1, + size: items.length, }); diff --git a/frontend/src/__tests__/cypress/cypress/e2e/modelRegistry/ModelRegistry.cy.ts b/frontend/src/__tests__/cypress/cypress/e2e/modelRegistry/ModelRegistry.cy.ts index f09391c755..92059cc5f6 100644 --- a/frontend/src/__tests__/cypress/cypress/e2e/modelRegistry/ModelRegistry.cy.ts +++ b/frontend/src/__tests__/cypress/cypress/e2e/modelRegistry/ModelRegistry.cy.ts @@ -1,20 +1,31 @@ +/* eslint-disable camelcase */ import { mockK8sResourceList, mockRouteK8sResourceModelRegistry } from '~/__mocks__'; import { mockComponents } from '~/__mocks__/mockComponents'; import { mockDashboardConfig } from '~/__mocks__/mockDashboardConfig'; import { MODEL_REGISTRY_API_VERSION } from '~/concepts/modelRegistry/const'; import { mockModelRegistry } from '~/__mocks__/mockModelRegistry'; -import { mockModelVersionList } from '~/__mocks__/mockModelVersionList'; import { mockRegisteredModelList } from '~/__mocks__/mockRegisteredModelsList'; import { labelModal, modelRegistry } from '~/__tests__/cypress/cypress/pages/modelRegistry'; import { be } from '~/__tests__/cypress/cypress/utils/should'; import { ModelRegistryModel, RouteModel } from '~/__tests__/cypress/cypress/utils/models'; +import { mockModelVersionList } from '~/__mocks__/mockModelVersionList'; +import { mockModelVersion } from '~/__mocks__/mockModelVersion'; +import { ModelVersion } from '~/concepts/modelRegistry/types'; type HandlersProps = { disableModelRegistryFeature?: boolean; - size?: number; + registeredModelsSize?: number; + modelVersions?: ModelVersion[]; }; -const initIntercepts = ({ disableModelRegistryFeature = false, size = 4 }: HandlersProps) => { +const initIntercepts = ({ + disableModelRegistryFeature = false, + registeredModelsSize = 4, + modelVersions = [ + mockModelVersion({ author: 'Author 1' }), + mockModelVersion({ name: 'model version' }), + ], +}: HandlersProps) => { cy.interceptOdh( 'GET /api/config', mockDashboardConfig({ @@ -37,14 +48,15 @@ const initIntercepts = ({ disableModelRegistryFeature = false, size = 4 }: Handl namespace: 'odh-model-registries', }), ); + cy.interceptOdh( `GET /api/service/modelregistry/modelregistry-sample/api/model_registry/${MODEL_REGISTRY_API_VERSION}/registered_models`, - mockRegisteredModelList({ size }), + mockRegisteredModelList({ size: registeredModelsSize }), ); cy.interceptOdh( `GET /api/service/modelregistry/modelregistry-sample/api/model_registry/${MODEL_REGISTRY_API_VERSION}/registered_models/1/versions`, - mockModelVersionList(), + mockModelVersionList({ items: modelVersions }), ); }; @@ -71,7 +83,7 @@ describe('Model Registry', () => { it('No registered models in the selected Model Registry', () => { initIntercepts({ disableModelRegistryFeature: false, - size: 0, + registeredModelsSize: 0, }); modelRegistry.visit(); diff --git a/frontend/src/__tests__/cypress/cypress/e2e/modelRegistry/ModelVersions.cy.ts b/frontend/src/__tests__/cypress/cypress/e2e/modelRegistry/ModelVersions.cy.ts new file mode 100644 index 0000000000..5e3573747a --- /dev/null +++ b/frontend/src/__tests__/cypress/cypress/e2e/modelRegistry/ModelVersions.cy.ts @@ -0,0 +1,178 @@ +/* eslint-disable camelcase */ +import { mockK8sResourceList } from '~/__mocks__'; +import { mockDashboardConfig } from '~/__mocks__/mockDashboardConfig'; +import { MODEL_REGISTRY_API_VERSION } from '~/concepts/modelRegistry/const'; +import { mockModelRegistry } from '~/__mocks__/mockModelRegistry'; +import { mockModelVersionList } from '~/__mocks__/mockModelVersionList'; +import { mockRegisteredModelList } from '~/__mocks__/mockRegisteredModelsList'; +import { labelModal, modelRegistry } from '~/__tests__/cypress/cypress/pages/modelRegistry'; +import { be } from '~/__tests__/cypress/cypress/utils/should'; +import { ModelRegistryModel } from '~/__tests__/cypress/cypress/utils/models'; +import { verifyRelativeURL } from '~/__tests__/cypress/cypress/utils/url'; +import { mockRegisteredModel } from '~/__mocks__/mockRegisteredModel'; +import { ModelVersion } from '~/concepts/modelRegistry/types'; +import { mockModelVersion } from '~/__mocks__/mockModelVersion'; + +type HandlersProps = { + disableModelRegistryFeature?: boolean; + registeredModelsSize?: number; + modelVersions?: ModelVersion[]; +}; + +const initIntercepts = ({ + disableModelRegistryFeature = false, + registeredModelsSize = 4, + modelVersions = [ + mockModelVersion({ + author: 'Author 1', + id: '1', + customProperties: { + Financial: { + metadataType: 'MetadataStringValue', + string_value: 'non-empty', + }, + 'Financial data': { + metadataType: 'MetadataStringValue', + string_value: '', + }, + 'Fraud detection': { + metadataType: 'MetadataStringValue', + string_value: '', + }, + 'Test label': { + metadataType: 'MetadataStringValue', + string_value: '', + }, + 'Machine learning': { + metadataType: 'MetadataStringValue', + string_value: '', + }, + 'Next data to be overflow': { + metadataType: 'MetadataStringValue', + string_value: '', + }, + 'Test label x': { + metadataType: 'MetadataStringValue', + string_value: '', + }, + 'Test label y': { + metadataType: 'MetadataStringValue', + string_value: '', + }, + 'Test label z': { + metadataType: 'MetadataStringValue', + string_value: '', + }, + }, + }), + mockModelVersion({ id: '2', name: 'model version' }), + ], +}: HandlersProps) => { + cy.interceptOdh( + 'GET /api/config', + mockDashboardConfig({ + disableModelRegistry: disableModelRegistryFeature, + }), + ); + + cy.interceptK8sList( + ModelRegistryModel, + mockK8sResourceList([mockModelRegistry({}), mockModelRegistry({ name: 'test-registry' })]), + ); + + cy.interceptK8s(ModelRegistryModel, mockModelRegistry({})); + + cy.interceptOdh( + `GET /api/service/modelregistry/modelregistry-sample/api/model_registry/${MODEL_REGISTRY_API_VERSION}/registered_models`, + mockRegisteredModelList({ size: registeredModelsSize }), + ); + + cy.interceptOdh( + `GET /api/service/modelregistry/modelregistry-sample/api/model_registry/${MODEL_REGISTRY_API_VERSION}/registered_models/1/versions`, + mockModelVersionList({ items: modelVersions }), + ); + + cy.interceptOdh( + `GET /api/service/modelregistry/modelregistry-sample/api/model_registry/${MODEL_REGISTRY_API_VERSION}/registered_models/1`, + mockRegisteredModel({}), + ); +}; +describe('Model Versions', () => { + it('No model versions in the selected registered model', () => { + initIntercepts({ + disableModelRegistryFeature: false, + modelVersions: [], + }); + + modelRegistry.visit(); + const registeredModelRow = modelRegistry.getRow('Fraud detection model'); + registeredModelRow.findName().contains('Fraud detection model').click(); + verifyRelativeURL(`/modelRegistry/modelregistry-sample/registeredModels/1/versions`); + modelRegistry.shouldmodelVersionsEmpty(); + }); + + it('Model versions table', () => { + initIntercepts({ + disableModelRegistryFeature: false, + }); + + modelRegistry.visit(); + const registeredModelRow = modelRegistry.getRow('Fraud detection model'); + registeredModelRow.findName().contains('Fraud detection model').click(); + verifyRelativeURL(`/modelRegistry/modelregistry-sample/registeredModels/1/versions`); + modelRegistry.findModelBreadcrumbItem().contains('test'); + modelRegistry + .findModelVersionsTableKebab() + .findDropdownItem('View archived versions') + .should('be.disabled'); + modelRegistry + .findModelVersionsHeaderAction() + .findDropdownItem('Archive model') + .should('be.disabled'); + modelRegistry.findModelVersionsTable().should('be.visible'); + modelRegistry.findModelVersionsTableRows().should('have.length', 2); + + // Label modal + const modelVersionRow = modelRegistry.getModelVersionRow('new model version'); + + modelVersionRow.findLabelModalText().contains('5 more'); + modelVersionRow.findLabelModalText().click(); + labelModal.shouldContainsModalLabels([ + 'Financial', + 'Financial data', + 'Fraud detection', + 'Test label', + 'Machine learning', + 'Next data to be overflow', + 'Test label x', + 'Test label y', + 'Test label y', + ]); + labelModal.findModalSearchInput().type('Financial'); + labelModal.shouldContainsModalLabels(['Financial', 'Financial data']); + labelModal.findCloseModal().click(); + + // sort by model version name + modelRegistry.findModelVersionsTableHeaderButton('Version name').should(be.sortAscending); + modelRegistry.findModelVersionsTableHeaderButton('Version name').click(); + modelRegistry.findModelVersionsTableHeaderButton('Version name').should(be.sortDescending); + + // sort by model version owner + modelRegistry.findModelVersionsTableHeaderButton('Owner').click(); + modelRegistry.findModelVersionsTableHeaderButton('Owner').should(be.sortAscending); + modelRegistry.findModelVersionsTableHeaderButton('Owner').click(); + modelRegistry.findModelVersionsTableHeaderButton('Owner').should(be.sortDescending); + + // filtering by keyword + modelRegistry.findModelVersionsTableSearch().type('new model version'); + modelRegistry.findModelVersionsTableRows().should('have.length', 1); + modelRegistry.findModelVersionsTableRows().contains('new model version'); + modelRegistry.findModelVersionsTableSearch().focused().clear(); + + // filtering by owner + modelRegistry.findModelVersionsTableFilter().findDropdownItem('Owner').click(); + modelRegistry.findModelVersionsTableSearch().type('Test author'); + modelRegistry.findModelVersionsTableRows().should('have.length', 1); + modelRegistry.findModelVersionsTableRows().contains('Test author'); + }); +}); diff --git a/frontend/src/__tests__/cypress/cypress/pages/modelRegistry.ts b/frontend/src/__tests__/cypress/cypress/pages/modelRegistry.ts index f0ace5cf88..7f3c1530b9 100644 --- a/frontend/src/__tests__/cypress/cypress/pages/modelRegistry.ts +++ b/frontend/src/__tests__/cypress/cypress/pages/modelRegistry.ts @@ -21,7 +21,7 @@ class LabelModal extends Modal { } } -class RegisteredModelTableRow extends TableRow { +class ModelRegistryTableRow extends TableRow { findName() { return this.find().findByTestId('model-name'); } @@ -66,6 +66,7 @@ class ModelRegistry { private wait() { cy.findByTestId('app-page-title').should('exist'); + cy.findByTestId('app-page-title').contains('Registered models'); cy.testA11y(); } @@ -79,7 +80,11 @@ class ModelRegistry { } shouldregisteredModelsEmpty() { - cy.findByTestId('empty-model-registry').should('exist'); + cy.findByTestId('empty-registered-models').should('exist'); + } + + shouldmodelVersionsEmpty() { + cy.findByTestId('empty-model-versions').should('exist'); } shouldModelRegistrySelectorExist() { @@ -104,23 +109,64 @@ class ModelRegistry { return cy.findByTestId('registered-model-table'); } + findModelVersionsTable() { + return cy.findByTestId('model-versions-table'); + } + findTableRows() { return this.findTable().find('tbody tr'); } + findModelVersionsTableRows() { + return this.findModelVersionsTable().find('tbody tr'); + } + getRow(name: string) { - return new RegisteredModelTableRow(() => + return new ModelRegistryTableRow(() => this.findTable().find(`[data-label="Model name"]`).contains(name).parents('tr'), ); } + getModelVersionRow(name: string) { + return new ModelRegistryTableRow(() => + this.findModelVersionsTable() + .find(`[data-label="Version name"]`) + .contains(name) + .parents('tr'), + ); + } + findRegisteredModelTableHeaderButton(name: string) { return this.findTable().find('thead').findByRole('button', { name }); } + findModelVersionsTableHeaderButton(name: string) { + return this.findModelVersionsTable().find('thead').findByRole('button', { name }); + } + findTableSearch() { return cy.findByTestId('registered-model-table-search'); } + + findModelVersionsTableSearch() { + return cy.findByTestId('model-versions-table-search'); + } + + findModelBreadcrumbItem() { + return cy.findByTestId('breadcrumb-model'); + } + + findModelVersionsTableKebab() { + return cy.findByTestId('model-versions-table-kebab-action'); + } + + findModelVersionsHeaderAction() { + return cy.findByTestId('model-version-action-toggle'); + } + + findModelVersionsTableFilter() { + return cy.findByTestId('model-versions-table-filter'); + } } export const modelRegistry = new ModelRegistry(); diff --git a/frontend/src/__tests__/cypress/cypress/support/commands/odh.ts b/frontend/src/__tests__/cypress/cypress/support/commands/odh.ts index 8b3e30b8bc..324ec54e45 100644 --- a/frontend/src/__tests__/cypress/cypress/support/commands/odh.ts +++ b/frontend/src/__tests__/cypress/cypress/support/commands/odh.ts @@ -1,7 +1,11 @@ import { K8sResourceListResult, K8sStatus } from '@openshift/dynamic-plugin-sdk-utils'; import type { GenericStaticResponse, RouteHandlerController } from 'cypress/types/net-stubbing'; import { BaseMetricCreationResponse, BaseMetricListResponse } from '~/api'; -import { ModelVersionList, RegisteredModelList } from '~/concepts/modelRegistry/types'; +import { + ModelVersionList, + RegisteredModel, + RegisteredModelList, +} from '~/concepts/modelRegistry/types'; import type { DashboardConfigKind, DataScienceClusterInitializationKindStatus, @@ -338,6 +342,11 @@ declare global { }, response: OdhResponse<{ notebook: NotebookKind; isRunning: boolean }>, ): Cypress.Chainable; + + interceptOdh( + type: 'GET /api/service/modelregistry/modelregistry-sample/api/model_registry/v1alpha3/registered_models/1', + response: OdhResponse, + ): Cypress.Chainable; } } } diff --git a/frontend/src/pages/modelRegistry/screens/EmptyModelRegistryState.tsx b/frontend/src/pages/modelRegistry/screens/EmptyModelRegistryState.tsx index 3b62e2312e..8445ef4d4f 100644 --- a/frontend/src/pages/modelRegistry/screens/EmptyModelRegistryState.tsx +++ b/frontend/src/pages/modelRegistry/screens/EmptyModelRegistryState.tsx @@ -13,6 +13,7 @@ import { PlusCircleIcon } from '@patternfly/react-icons'; import * as React from 'react'; type EmptyModelRegistryStateType = { + testid?: string; title: string; description: string; primaryActionText: string; @@ -22,6 +23,7 @@ type EmptyModelRegistryStateType = { }; const EmptyModelRegistryState: React.FC = ({ + testid, title, description, primaryActionText, @@ -29,7 +31,7 @@ const EmptyModelRegistryState: React.FC = ({ primaryActionOnClick, secondaryActionOnClick, }) => ( - + } /> {description} diff --git a/frontend/src/pages/modelRegistry/screens/ModelRegistry.tsx b/frontend/src/pages/modelRegistry/screens/ModelRegistry.tsx index f62695e1c9..14a48f962f 100644 --- a/frontend/src/pages/modelRegistry/screens/ModelRegistry.tsx +++ b/frontend/src/pages/modelRegistry/screens/ModelRegistry.tsx @@ -17,6 +17,7 @@ const ModelRegistry: React.FC = () => { empty={registeredModels.size === 0} emptyStatePage={ { /> } title={ - + } description="View and manage your registered models." headerContent={ diff --git a/frontend/src/pages/modelRegistry/screens/ModelVersionListView.tsx b/frontend/src/pages/modelRegistry/screens/ModelVersionListView.tsx index 67dfd68105..29c8cd2a10 100644 --- a/frontend/src/pages/modelRegistry/screens/ModelVersionListView.tsx +++ b/frontend/src/pages/modelRegistry/screens/ModelVersionListView.tsx @@ -43,6 +43,7 @@ const ModelVersionListView: React.FC = ({ if (unfilteredmodelVersions.length === 0) { return ( = ({ chips={search === '' ? [] : [search]} deleteChip={() => setSearch('')} deleteChipGroup={() => setSearch('')} - categoryName="Keyword" + categoryName={searchType} > ({ key, label: key, @@ -107,6 +109,7 @@ const ModelVersionListView: React.FC = ({ onOpenChange={(isOpen: boolean) => setIsArchivedModelVersionKebabOpen(isOpen)} toggle={(tr: React.Ref) => ( diff --git a/frontend/src/pages/modelRegistry/screens/ModelVersions.tsx b/frontend/src/pages/modelRegistry/screens/ModelVersions.tsx index a2b1b5f9df..0d613ce255 100644 --- a/frontend/src/pages/modelRegistry/screens/ModelVersions.tsx +++ b/frontend/src/pages/modelRegistry/screens/ModelVersions.tsx @@ -37,7 +37,9 @@ const ModelVersions: React.FC = ({ tab, ...pageProps }) => { )} /> - {rm?.name} + + {rm?.name} + } title={rm?.name} diff --git a/frontend/src/pages/modelRegistry/screens/ModelVersionsHeaderActions.tsx b/frontend/src/pages/modelRegistry/screens/ModelVersionsHeaderActions.tsx index 64ccf37f36..453fc956df 100644 --- a/frontend/src/pages/modelRegistry/screens/ModelVersionsHeaderActions.tsx +++ b/frontend/src/pages/modelRegistry/screens/ModelVersionsHeaderActions.tsx @@ -1,11 +1,5 @@ import * as React from 'react'; -import { - Dropdown, - DropdownList, - MenuToggle, - MenuToggleAction, - DropdownItem, -} from '@patternfly/react-core'; +import { Dropdown, DropdownList, MenuToggle, DropdownItem } from '@patternfly/react-core'; const ModelVersionsHeaderActions: React.FC = () => { const [isOpen, setOpen] = React.useState(false); @@ -23,23 +17,11 @@ const ModelVersionsHeaderActions: React.FC = () => { ref={toggleRef} onClick={() => setOpen(!isOpen)} isExpanded={isOpen} - splitButtonOptions={{ - variant: 'action', - items: [ - undefined} - > - Actions - , - ], - }} aria-label="Model version action toggle" - data-testid="model-version-split-button" - /> + data-testid="model-version-action-toggle" + > + Actions + )} > diff --git a/frontend/src/pages/modelRegistry/screens/ModelVersionsTableRow.tsx b/frontend/src/pages/modelRegistry/screens/ModelVersionsTableRow.tsx index 8a35711176..3e3b699206 100644 --- a/frontend/src/pages/modelRegistry/screens/ModelVersionsTableRow.tsx +++ b/frontend/src/pages/modelRegistry/screens/ModelVersionsTableRow.tsx @@ -17,7 +17,7 @@ const ModelVersionsTableRow: React.FC = ({ modelVers return ( - +