From aca28ab6d6fdf481ce88209565c2f9bc818a8d37 Mon Sep 17 00:00:00 2001 From: Dipanshu Gupta Date: Wed, 8 May 2024 19:25:44 +0530 Subject: [PATCH] Model version details test Refactoring model registry folder structure --- frontend/src/__mocks__/mockModelArtifact.ts | 13 ++ .../src/__mocks__/mockModelArtifactList.ts | 10 ++ frontend/src/__mocks__/mockModelVersion.ts | 1 + .../e2e/modelRegistry/ModelRegistry.cy.ts | 3 +- .../modelRegistry/ModelVersionDetails.cy.ts | 162 ++++++++++++++++++ .../e2e/modelRegistry/ModelVersions.cy.ts | 20 +-- .../modelRegistry/modelVersionDetails.ts | 48 ++++++ .../cypress/cypress/support/commands/odh.ts | 24 ++- .../EditableLabelsDescriptionListGroup.tsx | 13 +- .../EditableTextDescriptionListGroup.tsx | 5 +- .../modelRegistry/ModelRegistryRoutes.tsx | 14 +- .../modelRegistry/screens/ModelRegistry.tsx | 2 +- .../ModelVersionDetails.tsx | 6 +- .../ModelVersionDetailsView.tsx | 21 ++- .../ModelVersionSelector.tsx | 3 +- .../{ => ModelVersions}/ModelDetailsView.tsx | 12 +- .../ModelVersionListView.tsx | 10 +- .../{ => ModelVersions}/ModelVersions.tsx | 10 +- .../ModelVersionsHeaderActions.tsx | 0 .../ModelVersionsTable.tsx | 0 .../ModelVersionsTableColumns.ts | 0 .../ModelVersionsTableRow.tsx | 6 +- .../ModelVersionsTabs.tsx} | 14 +- .../screens/{ => ModelVersions}/const.ts | 2 +- .../RegisteredModelListView.tsx | 0 .../RegisteredModelOwner.tsx | 0 .../RegisteredModelTable.tsx | 0 .../RegisteredModelTableRow.tsx | 10 +- .../RegisteredModelsTableColumns.ts | 0 .../RegisteredModelsTableToolbar.tsx | 0 .../screens/{ => components}/ModelLabels.tsx | 2 +- .../{ => components}/ModelTimestamp.tsx | 0 .../src/pages/modelRegistry/screens/utils.ts | 6 +- 33 files changed, 349 insertions(+), 68 deletions(-) create mode 100644 frontend/src/__mocks__/mockModelArtifact.ts create mode 100644 frontend/src/__mocks__/mockModelArtifactList.ts create mode 100644 frontend/src/__tests__/cypress/cypress/e2e/modelRegistry/ModelVersionDetails.cy.ts create mode 100644 frontend/src/__tests__/cypress/cypress/pages/modelRegistry/modelVersionDetails.ts rename frontend/src/pages/modelRegistry/screens/{ => ModelVersions}/ModelDetailsView.tsx (90%) rename frontend/src/pages/modelRegistry/screens/{ => ModelVersions}/ModelVersionListView.tsx (92%) rename frontend/src/pages/modelRegistry/screens/{ => ModelVersions}/ModelVersions.tsx (91%) rename frontend/src/pages/modelRegistry/screens/{ => ModelVersions}/ModelVersionsHeaderActions.tsx (100%) rename frontend/src/pages/modelRegistry/screens/{ => ModelVersions}/ModelVersionsTable.tsx (100%) rename frontend/src/pages/modelRegistry/screens/{ => ModelVersions}/ModelVersionsTableColumns.ts (100%) rename frontend/src/pages/modelRegistry/screens/{ => ModelVersions}/ModelVersionsTableRow.tsx (89%) rename frontend/src/pages/modelRegistry/screens/{GlobalModelVersionsTabs.tsx => ModelVersions/ModelVersionsTabs.tsx} (83%) rename frontend/src/pages/modelRegistry/screens/{ => ModelVersions}/const.ts (81%) rename frontend/src/pages/modelRegistry/screens/{ => RegisteredModels}/RegisteredModelListView.tsx (100%) rename frontend/src/pages/modelRegistry/screens/{ => RegisteredModels}/RegisteredModelOwner.tsx (100%) rename frontend/src/pages/modelRegistry/screens/{ => RegisteredModels}/RegisteredModelTable.tsx (100%) rename frontend/src/pages/modelRegistry/screens/{ => RegisteredModels}/RegisteredModelTableRow.tsx (86%) rename frontend/src/pages/modelRegistry/screens/{ => RegisteredModels}/RegisteredModelsTableColumns.ts (100%) rename frontend/src/pages/modelRegistry/screens/{ => RegisteredModels}/RegisteredModelsTableToolbar.tsx (100%) rename frontend/src/pages/modelRegistry/screens/{ => components}/ModelLabels.tsx (98%) rename frontend/src/pages/modelRegistry/screens/{ => components}/ModelTimestamp.tsx (100%) diff --git a/frontend/src/__mocks__/mockModelArtifact.ts b/frontend/src/__mocks__/mockModelArtifact.ts new file mode 100644 index 0000000000..3bf055eb33 --- /dev/null +++ b/frontend/src/__mocks__/mockModelArtifact.ts @@ -0,0 +1,13 @@ +import { ModelArtifact } from '~/concepts/modelRegistry/types'; + +export const mockModelArtifact = (): ModelArtifact => ({ + createTimeSinceEpoch: '1712234877179', + id: '1', + lastUpdateTimeSinceEpoch: '1712234877179', + name: 'fraud detection model version 1', + description: 'Description of model version', + artifactType: 'model-artifact', + customProperties: {}, + storagePath: 'test path', + uri: 'https://huggingface.io/mnist.onnx', +}); diff --git a/frontend/src/__mocks__/mockModelArtifactList.ts b/frontend/src/__mocks__/mockModelArtifactList.ts new file mode 100644 index 0000000000..6e4e3edcef --- /dev/null +++ b/frontend/src/__mocks__/mockModelArtifactList.ts @@ -0,0 +1,10 @@ +/* eslint-disable camelcase */ +import { ModelArtifactList } from '~/concepts/modelRegistry/types'; +import { mockModelArtifact } from './mockModelArtifact'; + +export const mockModelArtifactList = (): ModelArtifactList => ({ + items: [mockModelArtifact()], + nextPageToken: '', + pageSize: 0, + size: 1, +}); diff --git a/frontend/src/__mocks__/mockModelVersion.ts b/frontend/src/__mocks__/mockModelVersion.ts index c1c2667784..e4e72faae0 100644 --- a/frontend/src/__mocks__/mockModelVersion.ts +++ b/frontend/src/__mocks__/mockModelVersion.ts @@ -23,4 +23,5 @@ export const mockModelVersion = ({ name, state: ModelVersionState.ARCHIVED, registeredModelId, + description: 'Description of model version', }); 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 92059cc5f6..72afbdc94b 100644 --- a/frontend/src/__tests__/cypress/cypress/e2e/modelRegistry/ModelRegistry.cy.ts +++ b/frontend/src/__tests__/cypress/cypress/e2e/modelRegistry/ModelRegistry.cy.ts @@ -80,6 +80,7 @@ describe('Model Registry', () => { modelRegistry.tabEnabled(); }); + it('No registered models in the selected Model Registry', () => { initIntercepts({ disableModelRegistryFeature: false, @@ -92,7 +93,7 @@ describe('Model Registry', () => { modelRegistry.shouldregisteredModelsEmpty(); }); - it('Model registry table', () => { + it('Registered model table', () => { initIntercepts({ disableModelRegistryFeature: false, }); diff --git a/frontend/src/__tests__/cypress/cypress/e2e/modelRegistry/ModelVersionDetails.cy.ts b/frontend/src/__tests__/cypress/cypress/e2e/modelRegistry/ModelVersionDetails.cy.ts new file mode 100644 index 0000000000..96681bd689 --- /dev/null +++ b/frontend/src/__tests__/cypress/cypress/e2e/modelRegistry/ModelVersionDetails.cy.ts @@ -0,0 +1,162 @@ +/* eslint-disable camelcase */ +import { + mockDashboardConfig, + mockK8sResourceList, + mockRouteK8sResourceModelRegistry, +} from '~/__mocks__'; +import { mockComponents } from '~/__mocks__/mockComponents'; +import { mockModelRegistry } from '~/__mocks__/mockModelRegistry'; +import { ModelRegistryModel, RouteModel } from '~/__tests__/cypress/cypress/utils/models'; +import { MODEL_REGISTRY_API_VERSION } from '~/concepts/modelRegistry/const'; +import { modelVersionDetails } from '~/__tests__/cypress/cypress/pages/modelRegistry/modelVersionDetails'; +import { mockRegisteredModel } from '~/__mocks__/mockRegisteredModel'; +import { mockModelVersion } from '~/__mocks__/mockModelVersion'; +import { mockModelVersionList } from '~/__mocks__/mockModelVersionList'; +import { mockModelArtifactList } from '~/__mocks__/mockModelArtifactList'; +import { verifyRelativeURL } from '~/__tests__/cypress/cypress/utils/url'; +import { ModelRegistryMetadataType } from '~/concepts/modelRegistry/types'; + +const initIntercepts = () => { + cy.interceptOdh( + 'GET /api/config', + mockDashboardConfig({ + disableModelRegistry: false, + }), + ); + cy.interceptOdh('GET /api/components', { query: { installed: 'true' } }, mockComponents()); + + cy.interceptK8sList(ModelRegistryModel, mockK8sResourceList([mockModelRegistry({})])); + + cy.interceptK8s(ModelRegistryModel, mockModelRegistry({})); + + cy.interceptK8s( + RouteModel, + mockRouteK8sResourceModelRegistry({ + name: 'modelregistry-sample-http', + namespace: 'odh-model-registries', + }), + ); + + cy.interceptOdh( + `GET /api/service/modelregistry/modelregistry-sample/api/model_registry/${MODEL_REGISTRY_API_VERSION}/registered_models/1`, + mockRegisteredModel({}), + ); + + cy.interceptOdh( + `GET /api/service/modelregistry/modelregistry-sample/api/model_registry/${MODEL_REGISTRY_API_VERSION}/registered_models/1/versions`, + mockModelVersionList({ + items: [ + mockModelVersion({ name: 'Version 1', author: 'Author 1', registeredModelId: '1' }), + mockModelVersion({ + author: 'Author 2', + registeredModelId: '1', + id: '2', + name: 'Version 2', + }), + ], + }), + ); + + cy.interceptOdh( + `GET /api/service/modelregistry/modelregistry-sample/api/model_registry/${MODEL_REGISTRY_API_VERSION}/model_versions/:id`, + { path: { id: '1' } }, + mockModelVersion({ + id: '1', + name: 'Version 1', + customProperties: { + 'Testing label': { + metadataType: ModelRegistryMetadataType.STRING, + string_value: '', + }, + Financial: { + metadataType: ModelRegistryMetadataType.STRING, + string_value: 'non-empty', + }, + 'Financial data': { + metadataType: ModelRegistryMetadataType.STRING, + string_value: '', + }, + 'Fraud detection': { + metadataType: ModelRegistryMetadataType.STRING, + string_value: '', + }, + 'Long label data to be truncated abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc abc': + { + metadataType: ModelRegistryMetadataType.STRING, + string_value: '', + }, + 'Machine learning': { + metadataType: ModelRegistryMetadataType.STRING, + string_value: '', + }, + 'Next data to be overflow': { + metadataType: ModelRegistryMetadataType.STRING, + string_value: '', + }, + 'Label x': { + metadataType: ModelRegistryMetadataType.STRING, + string_value: '', + }, + 'Label y': { + metadataType: ModelRegistryMetadataType.STRING, + string_value: '', + }, + 'Label z': { + metadataType: ModelRegistryMetadataType.STRING, + string_value: '', + }, + }, + }), + ); + + cy.interceptOdh( + `GET /api/service/modelregistry/modelregistry-sample/api/model_registry/${MODEL_REGISTRY_API_VERSION}/model_versions/:id`, + { path: { id: '2' } }, + mockModelVersion({ id: '2', name: 'Version 2' }), + ); + + cy.interceptOdh( + `GET /api/service/modelregistry/modelregistry-sample/api/model_registry/${MODEL_REGISTRY_API_VERSION}/model_versions/1/artifacts`, + mockModelArtifactList(), + ); +}; + +describe('Model version details', () => { + beforeEach(() => { + initIntercepts(); + modelVersionDetails.visit(); + }); + + it('Model version details page header', () => { + verifyRelativeURL('/modelRegistry/modelregistry-sample/registeredModels/1/versions/1/details'); + cy.findByTestId('app-page-title').should('have.text', 'Version 1'); + cy.findByTestId('breadcrumb-version-name').should('have.text', 'Version 1'); + }); + + it('Model version details tab', () => { + modelVersionDetails.findVersionId().contains('1'); + modelVersionDetails.findDescription().should('have.text', 'Description of model version'); + modelVersionDetails.findMoreLabelsButton().contains('6 more'); + modelVersionDetails.findMoreLabelsButton().click(); + modelVersionDetails.shouldContainsModalLabels([ + 'Testing label', + 'Financial', + 'Financial data', + 'Fraud detection', + 'Machine learning', + 'Next data to be overflow', + 'Label x', + 'Label y', + 'Label z', + ]); + modelVersionDetails.findStorageLocation().contains('https://huggingface.io/mnist.onnx'); + }); + + it('Switching model versions', () => { + modelVersionDetails.findVersionId().contains('1'); + modelVersionDetails.findModelVersionDropdownButton().click(); + modelVersionDetails.findModelVersionDropdownSearch().fill('Version 2'); + modelVersionDetails.findModelVersionDropdownItem('Version 2').click(); + modelVersionDetails.findVersionId().contains('2'); + }); +}); diff --git a/frontend/src/__tests__/cypress/cypress/e2e/modelRegistry/ModelVersions.cy.ts b/frontend/src/__tests__/cypress/cypress/e2e/modelRegistry/ModelVersions.cy.ts index 5e3573747a..2900791bb5 100644 --- a/frontend/src/__tests__/cypress/cypress/e2e/modelRegistry/ModelVersions.cy.ts +++ b/frontend/src/__tests__/cypress/cypress/e2e/modelRegistry/ModelVersions.cy.ts @@ -10,7 +10,7 @@ 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 { ModelRegistryMetadataType, ModelVersion } from '~/concepts/modelRegistry/types'; import { mockModelVersion } from '~/__mocks__/mockModelVersion'; type HandlersProps = { @@ -28,39 +28,39 @@ const initIntercepts = ({ id: '1', customProperties: { Financial: { - metadataType: 'MetadataStringValue', + metadataType: ModelRegistryMetadataType.STRING, string_value: 'non-empty', }, 'Financial data': { - metadataType: 'MetadataStringValue', + metadataType: ModelRegistryMetadataType.STRING, string_value: '', }, 'Fraud detection': { - metadataType: 'MetadataStringValue', + metadataType: ModelRegistryMetadataType.STRING, string_value: '', }, 'Test label': { - metadataType: 'MetadataStringValue', + metadataType: ModelRegistryMetadataType.STRING, string_value: '', }, 'Machine learning': { - metadataType: 'MetadataStringValue', + metadataType: ModelRegistryMetadataType.STRING, string_value: '', }, 'Next data to be overflow': { - metadataType: 'MetadataStringValue', + metadataType: ModelRegistryMetadataType.STRING, string_value: '', }, 'Test label x': { - metadataType: 'MetadataStringValue', + metadataType: ModelRegistryMetadataType.STRING, string_value: '', }, 'Test label y': { - metadataType: 'MetadataStringValue', + metadataType: ModelRegistryMetadataType.STRING, string_value: '', }, 'Test label z': { - metadataType: 'MetadataStringValue', + metadataType: ModelRegistryMetadataType.STRING, string_value: '', }, }, diff --git a/frontend/src/__tests__/cypress/cypress/pages/modelRegistry/modelVersionDetails.ts b/frontend/src/__tests__/cypress/cypress/pages/modelRegistry/modelVersionDetails.ts new file mode 100644 index 0000000000..d28bf590f6 --- /dev/null +++ b/frontend/src/__tests__/cypress/cypress/pages/modelRegistry/modelVersionDetails.ts @@ -0,0 +1,48 @@ +import { modelVersionUrl } from '~/pages/modelRegistry/screens/routeUtils'; + +class ModelVersionDetails { + visit() { + cy.visit(modelVersionUrl('1', '1', 'modelregistry-sample')); + this.wait(); + } + + private wait() { + cy.findByTestId('app-page-title').should('exist'); + cy.testA11y(); + } + + findVersionId() { + return cy.findByTestId('model-version-id'); + } + + findDescription() { + return cy.findByTestId('model-version-description'); + } + + findMoreLabelsButton() { + return cy.findByTestId('label-group').find('button'); + } + + findStorageLocation() { + return cy.findByTestId('storage-location'); + } + + shouldContainsModalLabels(labels: string[]) { + cy.findByTestId('label-group').within(() => labels.map((label) => cy.contains(label))); + return this; + } + + findModelVersionDropdownButton() { + return cy.findByTestId('model-version-toggle-button'); + } + + findModelVersionDropdownSearch() { + return cy.findByTestId('search-input'); + } + + findModelVersionDropdownItem(name: string) { + return cy.findByTestId('model-version-selector-list').find('li').contains(name); + } +} + +export const modelVersionDetails = new ModelVersionDetails(); diff --git a/frontend/src/__tests__/cypress/cypress/support/commands/odh.ts b/frontend/src/__tests__/cypress/cypress/support/commands/odh.ts index ec11d3a070..6d02d1558b 100644 --- a/frontend/src/__tests__/cypress/cypress/support/commands/odh.ts +++ b/frontend/src/__tests__/cypress/cypress/support/commands/odh.ts @@ -2,6 +2,8 @@ import { K8sResourceListResult, K8sStatus } from '@openshift/dynamic-plugin-sdk- import type { GenericStaticResponse, RouteHandlerController } from 'cypress/types/net-stubbing'; import { BaseMetricCreationResponse, BaseMetricListResponse } from '~/api'; import { + ModelArtifactList, + ModelVersion, ModelVersionList, RegisteredModel, RegisteredModelList, @@ -321,9 +323,19 @@ declare global { ): Cypress.Chainable; interceptOdh( - type: 'DELETE /api/service/pipelines/:projectId/dspa/apis/v2beta1/recurringruns/:pipeline_id', - options: { path: { projectId: string; pipeline_id: string } }, - response: OdhResponse, + type: `GET /api/service/modelregistry/modelregistry-sample/api/model_registry/v1alpha3/registered_models/1`, + response: OdhResponse, + ): Cypress.Chainable; + + interceptOdh( + type: `GET /api/service/modelregistry/modelregistry-sample/api/model_registry/v1alpha3/model_versions/:id`, + options: { path: { id: string } }, + response: OdhResponse, + ): Cypress.Chainable; + + interceptOdh( + type: `GET /api/service/modelregistry/modelregistry-sample/api/model_registry/v1alpha3/model_versions/1/artifacts`, + response: OdhResponse, ): Cypress.Chainable; interceptOdh( @@ -331,6 +343,12 @@ declare global { response: OdhResponse, ): Cypress.Chainable; + interceptOdh( + type: 'DELETE /api/service/pipelines/:projectId/dspa/apis/v2beta1/recurringruns/:pipeline_id', + options: { path: { projectId: string; pipeline_id: string } }, + response: OdhResponse, + ): Cypress.Chainable; + interceptOdh( type: 'GET /api/rolebindings/opendatahub/openshift-ai-notebooks-image-pullers', response: OdhResponse>, diff --git a/frontend/src/components/EditableLabelsDescriptionListGroup.tsx b/frontend/src/components/EditableLabelsDescriptionListGroup.tsx index dca9d45a8a..f25f1b6c9f 100644 --- a/frontend/src/components/EditableLabelsDescriptionListGroup.tsx +++ b/frontend/src/components/EditableLabelsDescriptionListGroup.tsx @@ -103,11 +103,18 @@ const EditableLabelsDescriptionListGroup: React.FC + ) @@ -151,9 +158,9 @@ const EditableLabelsDescriptionListGroup: React.FC - + {labels.map((label) => ( -