From 0af29cf1d00a8f479c1bae841fae79b29bd4d5d6 Mon Sep 17 00:00:00 2001 From: Alessandro Menduni Date: Tue, 7 Mar 2023 17:46:01 +0100 Subject: [PATCH 1/4] Remove Source from the wizard steps --- .../src/old-pages/Configure/Configure.tsx | 22 ++++++++----------- .../Configure/useWizardNavigation.ts | 7 +++--- 2 files changed, 13 insertions(+), 16 deletions(-) diff --git a/frontend/src/old-pages/Configure/Configure.tsx b/frontend/src/old-pages/Configure/Configure.tsx index 4b74e609..e489a1a0 100644 --- a/frontend/src/old-pages/Configure/Configure.tsx +++ b/frontend/src/old-pages/Configure/Configure.tsx @@ -21,7 +21,6 @@ import { WizardProps, } from '@cloudscape-design/components' -import {Source, SourceHelpPanel, sourceValidate} from './Source' import {Cluster, clusterValidate} from './Cluster' import { HeadNode, @@ -50,13 +49,16 @@ import Layout from '../Layout' import {useWizardSectionChangeLog} from '../../navigation/useWizardSectionChangeLog' import {NonCancelableEventHandler} from '@cloudscape-design/components/internal/events' import i18next from 'i18next' -import {pages, useWizardNavigation} from './useWizardNavigation' +import { + INITIAL_WIZARD_PAGE, + pages, + useWizardNavigation, +} from './useWizardNavigation' import {ComputeFleetStatus} from '../../types/clusters' import {useClusterPoll} from '../../components/useClusterPoll' import InfoLink from '../../components/InfoLink' const validators: {[key: string]: (...args: any[]) => boolean} = { - source: sourceValidate, cluster: clusterValidate, headNode: headNodeValidate, storage: storageValidate, @@ -73,9 +75,9 @@ function wizardShow(navigate: any) { clearState(['app', 'wizard', 'clusterConfigYaml']) clearState(['app', 'wizard', 'loaded']) setState(['app', 'wizard', 'editing'], false) - setState(['app', 'wizard', 'page'], 'source') + setState(['app', 'wizard', 'page'], INITIAL_WIZARD_PAGE) } - if (!page) setState(['app', 'wizard', 'page'], 'source') + if (!page) setState(['app', 'wizard', 'page'], INITIAL_WIZARD_PAGE) navigate('/configure') } const loadingPath = ['app', 'wizard', 'source', 'loading'] @@ -103,7 +105,7 @@ function Configure() { const open = useState(['app', 'wizard', 'dialog']) const clusterName = useState(['app', 'wizard', 'clusterName']) const editing = useState(['app', 'wizard', 'editing']) - const currentPage = useState(['app', 'wizard', 'page']) || 'source' + const currentPage = useState(['app', 'wizard', 'page']) || INITIAL_WIZARD_PAGE const [refreshing, setRefreshing] = React.useState(false) let navigate = useNavigate() @@ -174,7 +176,7 @@ function Configure() { const showSecondaryActions = () => { return ( - {currentPage !== 'source' && currentPage !== 'create' && ( + {currentPage !== 'create' && ( + - + ) } diff --git a/frontend/src/old-pages/Clusters/__tests__/Clusters.test.tsx b/frontend/src/old-pages/Clusters/__tests__/Clusters.test.tsx index aacdc75d..82ad7de7 100644 --- a/frontend/src/old-pages/Clusters/__tests__/Clusters.test.tsx +++ b/frontend/src/old-pages/Clusters/__tests__/Clusters.test.tsx @@ -86,23 +86,6 @@ describe('given a component to show the clusters list', () => { expect(mockNavigate).toHaveBeenCalledWith('/clusters/test-cluster') }) }) - - describe('when the user clicks on "Create Cluster" button', () => { - it('should redirect to configure', async () => { - const output = await waitFor(() => - render( - - - , - ), - ) - - await userEvent.click( - output.getByRole('button', {name: 'Create cluster'}), - ) - expect(mockNavigate).toHaveBeenCalledWith('/configure') - }) - }) }) describe('when there are no clusters available', () => { From 47a82d575484c57abe5dbe454884e45e5861db91 Mon Sep 17 00:00:00 2001 From: Alessandro Menduni Date: Tue, 7 Mar 2023 17:46:59 +0100 Subject: [PATCH 3/4] Align e2e tests --- e2e/specs/wizard.spec.ts | 6 ++---- e2e/specs/wizard.template.spec.ts | 8 +++----- 2 files changed, 5 insertions(+), 9 deletions(-) diff --git a/e2e/specs/wizard.spec.ts b/e2e/specs/wizard.spec.ts index cba1bc0a..d9a7530e 100644 --- a/e2e/specs/wizard.spec.ts +++ b/e2e/specs/wizard.spec.ts @@ -11,16 +11,14 @@ import { expect, test } from '@playwright/test'; import { visitAndLogin } from '../test-utils/login'; -const CLUSTER_NAME = Math.random().toString(20).substring(8) +const CLUSTER_NAME = 'c' + Math.random().toString(20).substring(8) test.describe('Given an endpoint where AWS ParallelCluster UI is deployed', () => { test('a user should be able to login, navigate till the end of the cluster creation wizard, and perform a dry-run successfully', async ({ page }) => { await visitAndLogin(page) await page.getByRole('button', { name: 'Create cluster' }).first().click(); - - await expect(page.getByRole('heading', { name: 'Source' })).toBeVisible() - await page.getByRole('button', { name: 'Next' }).click(); + await page.getByRole('menuitem', { name: 'Use interface' }).click(); await expect(page.getByRole('heading', { name: 'Cluster', exact: true })).toBeVisible() await page.getByPlaceholder('Enter your cluster name').fill(CLUSTER_NAME); diff --git a/e2e/specs/wizard.template.spec.ts b/e2e/specs/wizard.template.spec.ts index 0a4391e2..0d6d63c0 100644 --- a/e2e/specs/wizard.template.spec.ts +++ b/e2e/specs/wizard.template.spec.ts @@ -12,7 +12,7 @@ import { expect, FileChooser, test } from '@playwright/test'; import { visitAndLogin } from '../test-utils/login'; const TEMPLATE_PATH = './fixtures/wizard.template.yaml' -const CLUSTER_NAME = Math.random().toString(20).substring(8) +const CLUSTER_NAME = 'c' + Math.random().toString(20).substring(8) test.describe('environment: @demo', () => { test.describe('given a cluster configuration template created with single instance type', () => { @@ -21,17 +21,15 @@ test.describe('environment: @demo', () => { await visitAndLogin(page) await page.getByRole('button', { name: 'Create cluster' }).first().click(); - - await expect(page.getByRole('heading', { name: 'Source' })).toBeVisible() page.on("filechooser", (fileChooser: FileChooser) => { fileChooser.setFiles([TEMPLATE_PATH]); }) - await page.getByRole('radio', { name: 'Existing template' }).click(); - await page.getByRole('button', { name: 'Next' }).click(); + await page.getByRole('menuitem', { name: 'Upload a template' }).click(); await expect(page.getByRole('heading', { name: 'Cluster', exact: true })).toBeVisible() await page.getByPlaceholder('Enter your cluster name').fill(CLUSTER_NAME); + await page.getByText(/vpc-.*/).waitFor({state: 'visible'}) await page.getByRole('button', { name: 'Next' }).click(); await expect(page.getByRole('heading', { name: 'Head node' })).toBeVisible() From 78992d1d2d0f20bc24868bc78b5f30a6aa0ed800 Mon Sep 17 00:00:00 2001 From: Alessandro Menduni Date: Wed, 8 Mar 2023 12:16:35 +0100 Subject: [PATCH 4/4] Disable cluster name field when editing a cluster --- frontend/src/old-pages/Configure/Cluster.tsx | 8 +- .../Configure/Cluster/ClusterNameField.tsx | 3 + .../__tests__/ClusterNameField.test.tsx | 100 ++++++++++++++++++ 3 files changed, 108 insertions(+), 3 deletions(-) create mode 100644 frontend/src/old-pages/Configure/Cluster/__tests__/ClusterNameField.test.tsx diff --git a/frontend/src/old-pages/Configure/Cluster.tsx b/frontend/src/old-pages/Configure/Cluster.tsx index fab3721b..774118af 100644 --- a/frontend/src/old-pages/Configure/Cluster.tsx +++ b/frontend/src/old-pages/Configure/Cluster.tsx @@ -100,9 +100,11 @@ function clusterValidate() { clearState([...errorsPath, 'multiUser']) } - const clusterNameValid = validateClusterNameAndSetErrors() - if (!clusterNameValid) { - valid = false + if (!editing) { + const clusterNameValid = validateClusterNameAndSetErrors() + if (!clusterNameValid) { + valid = false + } } const accountingValid = slurmAccountingValidateAndSetErrors() diff --git a/frontend/src/old-pages/Configure/Cluster/ClusterNameField.tsx b/frontend/src/old-pages/Configure/Cluster/ClusterNameField.tsx index e376722f..eb811d99 100644 --- a/frontend/src/old-pages/Configure/Cluster/ClusterNameField.tsx +++ b/frontend/src/old-pages/Configure/Cluster/ClusterNameField.tsx @@ -23,11 +23,13 @@ const clusterNameErrorPath = [ 'source', 'clusterName', ] +const editingPath = ['app', 'wizard', 'editing'] export function ClusterNameField() { const {t} = useTranslation() const clusterName = useState(clusterNamePath) || '' const clusterNameError = useState(clusterNameErrorPath) + const editing = !!useState(editingPath) const onChange: NonCancelableEventHandler = useCallback(({detail}) => { @@ -41,6 +43,7 @@ export function ClusterNameField() { errorText={clusterNameError} > { + const originalModule = jest.requireActual('../../../../store') + + return { + __esModule: true, // Use it when dealing with esModules + ...originalModule, + setState: jest.fn(), + } +}) + +i18n.use(initReactI18next).init({ + resources: {}, + lng: 'en', +}) + +const mockStore = mock() +const MockProviders = (props: any) => ( + + {props.children} + +) + +describe('given a component to set the ClusterName', () => { + let screen: RenderResult + + beforeEach(() => { + ;(mockSetState as jest.Mock).mockClear() + }) + + describe('when user fills in the cluster name', () => { + beforeEach(() => { + screen = render( + + + , + ) + + fireEvent.change( + screen.getByPlaceholderText('wizard.cluster.clusterName.placeholder'), + {target: {value: 'some-name'}}, + ) + }) + + it('should store the ClusterName in the cluster config', () => { + expect(mockSetState).toHaveBeenCalledTimes(1) + expect(mockSetState).toHaveBeenCalledWith( + ['app', 'wizard', 'clusterName'], + 'some-name', + ) + }) + }) + + describe('when user is editing a cluster', () => { + beforeEach(() => { + mockStore.getState.mockReturnValue({ + app: {wizard: {editing: true}}, + }) + }) + + describe('when user tries to fill in the cluster name', () => { + beforeEach(() => { + screen = render( + + + , + ) + + userEvent.type( + screen.getByPlaceholderText('wizard.cluster.clusterName.placeholder'), + 'some-name', + ) + }) + + it('should be disabled', () => { + expect(mockSetState).toHaveBeenCalledTimes(0) + }) + }) + }) +})