Skip to content

Commit

Permalink
Merge branch 'cypress-RHOAIENG-15766' of https://github.com/antowaddl…
Browse files Browse the repository at this point in the history
…e/odh-dashboard into cypress-RHOAIENG-15766
  • Loading branch information
antowaddle committed Dec 10, 2024
2 parents d78472f + 7cc5763 commit 6c096c6
Show file tree
Hide file tree
Showing 17 changed files with 94 additions and 152 deletions.
2 changes: 1 addition & 1 deletion frontend/src/__tests__/cypress/cypress.config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,7 @@ export default defineConfig({
// Apply retries only for tests in the "e2e" folder
return {
...config,
retries: !env.CY_MOCK && !env.CY_RECORD ? 2 : config.retries,
retries: !env.CY_MOCK && !env.CY_RECORD ? { runMode: 2, openMode: 0 } : config.retries,
};
},
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,6 @@ export class DashboardCodeEditor extends Contextual<HTMLElement> {
}

findUpload(): Cypress.Chainable<JQuery<HTMLElement>> {
return this.find().find('.pf-v6-c-code-editor__main input[type="file"]');
return this.find().find('input[type="file"]');
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,7 @@ class ServingRuntimes {
}

findDeleteModel() {
return cy.findByRole('menuitem', { name: 'Delete' });
return cy.contains('button', 'Delete');
}

findDeleteModal() {
Expand Down
2 changes: 1 addition & 1 deletion frontend/src/__tests__/cypress/cypress/pages/workbench.ts
Original file line number Diff line number Diff line change
Expand Up @@ -199,7 +199,7 @@ class AttachExistingStorageModal extends Modal {
cy.findByTestId('persistent-storage-group')
.findByPlaceholderText('Select a persistent storage')
.click();
cy.findByTestId('persistent-storage-group').contains('button.pf-v6-c-menu__item', name).click();
cy.findByTestId('persistent-storage-typeahead').contains(name).click();
}

findStandardPathInput() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ describe('Workbench and PVSs tests', () => {
cy.step(`Create Workbench ${projectName} using storage ${PVCDisplayName}`);
workbenchPage.findCreateButton().click();
createSpawnerPage.getNameInput().fill(workbenchName);
createSpawnerPage.findNotebookImage('s2i-minimal-notebook').click();
createSpawnerPage.findNotebookImage('jupyter-minimal-notebook').click();
createSpawnerPage.findAttachExistingStorageButton().click();
attachExistingStorageModal.selectExistingPersistentStorage(PVCDisplayName);
attachExistingStorageModal.findStandardPathInput().fill(workbenchName);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -68,8 +68,8 @@ describe('Verify Admins Can Import and Delete a Custom Multi-Model Serving Runti
.find()
.within(() => {
servingRuntimes.findEditModel().click();
servingRuntimes.findDeleteModel().click();
});
servingRuntimes.findDeleteModel().click();

servingRuntimes.findDeleteModal().should('be.visible').type(metadataDisplayName);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -73,8 +73,8 @@ describe('Verify Admins Can Import and Delete a Custom Single-Model Serving Runt
.find()
.within(() => {
servingRuntimes.findEditModel().click();
servingRuntimes.findDeleteModel().click();
});
servingRuntimes.findDeleteModel().click();

servingRuntimes.findDeleteModal().should('be.visible').type(metadataSingleDisplayName);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@ describe('Regular Users can make use of the Storage Classes in the Cluster Stora
tearDownClusterStorageSCFeature(dspName);
});

// TODO: This test is failing due to https://issues.redhat.com/browse/RHOAIENG-16609

it('If all SC are disabled except one, the SC dropdown should be disabled', () => {
cy.visitWithLogin('/projects', LDAP_CONTRIBUTOR_USER);
// Open the project
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import * as React from 'react';
import {
ClipboardCopy,
FormGroup,
NumberInput,
Split,
SplitItem,
Stack,
Expand All @@ -13,12 +14,11 @@ import {
RunTypeScheduledData,
ScheduledType,
} from '~/concepts/pipelines/content/createRun/types';
import NumberInputWrapper from '~/components/NumberInputWrapper';
import { replaceNonNumericPartWithString, replaceNumericPartWithString } from '~/utilities/string';
import {
DEFAULT_CRON_STRING,
DEFAULT_PERIODIC_OPTION,
} from '~/concepts/pipelines/content/createRun/const';
import { extractNumberAndTimeUnit } from './utils';

type TriggerTypeFieldProps = {
data: RunTypeScheduledData;
Expand All @@ -27,6 +27,8 @@ type TriggerTypeFieldProps = {

const TriggerTypeField: React.FC<TriggerTypeFieldProps> = ({ data, onChange }) => {
let content: React.ReactNode | null;
const [numberPart, unitPart] = extractNumberAndTimeUnit(data.value);

switch (data.triggerType) {
case ScheduledType.CRON:
content = (
Expand All @@ -50,15 +52,19 @@ const TriggerTypeField: React.FC<TriggerTypeFieldProps> = ({ data, onChange }) =
<FormGroup label="Run every" data-testid="run-every-group">
<Split hasGutter>
<SplitItem>
<NumberInputWrapper
<NumberInput
min={1}
value={parseInt(data.value) || 1}
onChange={(value) =>
value={numberPart}
onChange={(newNumberPart) => {
const updatedValue = `${Number(newNumberPart.currentTarget.value).toLocaleString(
'fullwide',
{ useGrouping: false },
)}${unitPart}`;
onChange({
...data,
value: replaceNumericPartWithString(data.value, value ?? 0),
})
}
value: updatedValue,
});
}}
/>
</SplitItem>
<SplitItem>
Expand All @@ -69,13 +75,14 @@ const TriggerTypeField: React.FC<TriggerTypeFieldProps> = ({ data, onChange }) =
key: v,
label: v,
}))}
value={data.value.replace(/\d+/, '')}
onChange={(value) =>
value={unitPart}
onChange={(newUnitPart) => {
const updatedValue = `${numberPart}${newUnitPart}`;
onChange({
...data,
value: replaceNonNumericPartWithString(data.value, value),
})
}
value: updatedValue,
});
}}
/>
</SplitItem>
</Split>
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
import { extractNumberAndTimeUnit } from '~/concepts/pipelines/content/createRun/contentSections/utils';

describe('extractNumberAndTimeUnit', () => {
test('splits valid numeric and unit parts', () => {
expect(extractNumberAndTimeUnit('1555Days')).toEqual([1555, 'Days']);
expect(extractNumberAndTimeUnit('1.23e+21Week')).toEqual([1.23e21, 'Week']);
expect(extractNumberAndTimeUnit('1.2342342342342342e+32Week')).toEqual([
1.2342342342342342e32,
'Week',
]);
});

test('handles missing numeric part', () => {
expect(extractNumberAndTimeUnit('Day')).toEqual([1, 'Day']);
expect(extractNumberAndTimeUnit('Minute')).toEqual([1, 'Minute']);
});

test('handles edge cases', () => {
expect(extractNumberAndTimeUnit('')).toEqual([1, '']);
expect(extractNumberAndTimeUnit('InfinityYear')).toEqual([1, 'InfinityYear']);
expect(extractNumberAndTimeUnit('-InfinityWeek')).toEqual([1, '-InfinityWeek']);
});

test('trims whitespace', () => {
expect(extractNumberAndTimeUnit(' 123Day ')).toEqual([123, 'Day']);
expect(extractNumberAndTimeUnit(' Day ')).toEqual([1, 'Day']);
});
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
/**
* Splits a string into a numeric part and a unit part
* @param value The input string to be split.
* @returns [numberPart, unitPart]
*/
export const extractNumberAndTimeUnit = (value: string): [number, string] => {
const trimmedValue = value.trim();

const match = trimmedValue.match(/^([+-]?\d+(\.\d+)?([eE][+-]?\d+)?)([a-zA-Z]*)$/);
if (match) {
const numericPart = parseFloat(match[1]);
const unitPart = match[4] || '';
return [numericPart, unitPart];
}

// The required minimum numeric value is set to 1
return [1, trimmedValue];
};
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,9 @@ const createRecurringRun = async (
periodic_schedule:
formData.runType.data.triggerType === ScheduledType.PERIODIC
? {
interval_second: periodicScheduleIntervalTime.toString(),
interval_second: periodicScheduleIntervalTime.toLocaleString('fullwide', {
useGrouping: false,
}),
start_time: startDate,
end_time: endDate,
}
Expand Down
74 changes: 0 additions & 74 deletions frontend/src/utilities/__tests__/string.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,6 @@ import {
containsOnlySlashes,
downloadString,
removeLeadingSlash,
replaceNonNumericPartWithString,
replaceNumericPartWithString,
containsMultipleSlashesPattern,
triggerFileDownload,
joinWithCommaAnd,
Expand Down Expand Up @@ -33,78 +31,6 @@ describe('downloadString', () => {
});
});

describe('replaceNumericPartWithString', () => {
it('should replace the numeric part of a string with a number', () => {
expect(replaceNumericPartWithString('abc123xyz', 456)).toBe('abc456xyz');
});

it('should handle empty input string', () => {
expect(replaceNumericPartWithString('', 789)).toBe('789');
});

it('should handle input string without numeric part', () => {
expect(replaceNumericPartWithString('abcdef', 123)).toBe('123abcdef');
});

it('should handle numeric part at the beginning of the string', () => {
expect(replaceNumericPartWithString('123xyz', 789)).toBe('789xyz');
});

it('should handle numeric part at the end of the string', () => {
expect(replaceNumericPartWithString('abc456', 123)).toBe('abc123');
});

it('should handle Pipeline scheduled time', () => {
expect(replaceNumericPartWithString('123Hour', 43424)).toBe('43424Hour');
});

it('should handle default Pipeline scheduled time', () => {
expect(replaceNumericPartWithString('1Week', 26)).toBe('26Week');
});
});

describe('replaceNonNumericPartWithString', () => {
it('should replace the non-numeric part of a string with another string', () => {
expect(replaceNonNumericPartWithString('abc123xyz', 'XYZ')).toBe('XYZ123xyz');
});

it('should handle empty input string', () => {
expect(replaceNonNumericPartWithString('', 'XYZ')).toBe('XYZ');
});

it('should handle input string with no non-numeric part', () => {
expect(replaceNonNumericPartWithString('123', 'XYZ')).toBe('123XYZ');
});

it('should handle input string with only non-numeric part', () => {
expect(replaceNonNumericPartWithString('abc', 'XYZ')).toBe('XYZ');
});

it('should handle input string with multiple non-numeric parts', () => {
expect(replaceNonNumericPartWithString('abc123def456', 'XYZ')).toBe('XYZ123def456');
});

it('should handle replacement string containing numbers', () => {
expect(replaceNonNumericPartWithString('abc123xyz', '123')).toBe('123123xyz');
});

it('should handle replacement string containing special characters', () => {
expect(replaceNonNumericPartWithString('abc123xyz', '@#$')).toBe('@#$123xyz');
});

it('should handle replacement string containing spaces', () => {
expect(replaceNonNumericPartWithString('abc123xyz', ' ')).toBe(' 123xyz');
});

it('should handle Pipeline scheduled time', () => {
expect(replaceNonNumericPartWithString('123Week', 'Minute')).toBe('123Minute');
});

it('should handle default Pipeline scheduled time', () => {
expect(replaceNonNumericPartWithString('1Week', 'Minute')).toBe('1Minute');
});
});

describe('removeLeadingSlash', () => {
it('removes leading slashes from a string if present', () => {
expect(removeLeadingSlash('/example')).toBe('example');
Expand Down
7 changes: 7 additions & 0 deletions frontend/src/utilities/__tests__/time.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -224,6 +224,13 @@ describe('convertPeriodicTimeToSeconds', () => {
it('should default to 0 seconds for unrecognized units', () => {
expect(convertPeriodicTimeToSeconds('3Weeks')).toBe(0);
});

it('should convert exponential time to seconds', () => {
const timeString = '5.2341124234234124123e+68Hour';
const numericValue = parseFloat('5.2341124234234124123e+68');
const expectedSeconds = numericValue * 60 * 60; // Convert hours to seconds
expect(convertPeriodicTimeToSeconds(timeString)).toBe(expectedSeconds);
});
});

describe('convertSecondsToPeriodicTime', () => {
Expand Down
55 changes: 0 additions & 55 deletions frontend/src/utilities/string.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,61 +24,6 @@ export const triggerFileDownload = (filename: string, href: string): void => {
document.body.removeChild(element);
};

/**
* This function replaces the first occurrence of a numeric part in the input string
* with the specified replacement numeric value.
* @param inputString
* @param replacementString
*/
export const replaceNumericPartWithString = (
inputString: string,
replacementString: number,
): string => {
// If the input string is empty or contains only whitespace, return the replacement as a string.
if (inputString.trim() === '') {
return replacementString.toString();
}

const match = inputString.match(/\d+/); //Find numeric part in string (only first occurance)
let updatedString = inputString;

if (match) {
const matchedNumber = match[0];
updatedString = inputString.replace(matchedNumber, String(replacementString));
} else {
// If no numeric part is found, prepend the replacement numeric value to the input string.
updatedString = replacementString + inputString;
}
return updatedString;
};

/**
* This function replaces the first occurrence of a non-numeric part in the input string
* with the specified replacement string.
* @param inputString
* @param replacementString
*/
export const replaceNonNumericPartWithString = (
inputString: string,
replacementString: string,
): string => {
if (inputString.trim() === '') {
return replacementString;
}

const match = inputString.match(/\D+/); //Find non-numeric part in string (only first occurance)
let updatedString = inputString;

if (match) {
const matchedString = match[0];
updatedString = inputString.replace(matchedString, replacementString);
} else {
// If no non-numeric part is found, append the replacement non-numeric value to the input string.
updatedString = inputString + replacementString;
}
return updatedString;
};

/**
* This function removes the leading slash (/) from string if exists
*/
Expand Down
5 changes: 3 additions & 2 deletions frontend/src/utilities/time.ts
Original file line number Diff line number Diff line change
Expand Up @@ -182,13 +182,14 @@ export const relativeTime = (current: number, previous: number): string => {

/** Function to convert time strings like "2Hour" to seconds */
export const convertPeriodicTimeToSeconds = (timeString: string): number => {
let numericValue = parseInt(timeString, 10);
const numericMatch = timeString.match(/^[\d.eE+-]+/);
let numericValue = numericMatch ? parseFloat(numericMatch[0]) : 1;

if (Number.isNaN(numericValue)) {
numericValue = 1;
}

const unit = timeString.toLowerCase().replace(/\d+/g, '');
const unit = timeString.replace(/^[\d.eE+-]+/, '').toLowerCase();

switch (unit) {
case 'hour':
Expand Down
6 changes: 6 additions & 0 deletions manifests/core-bases/base/cluster-role.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -219,3 +219,9 @@ rules:
- delete
resources:
- accounts
- verbs:
- get
apiGroups:
- ''
resources:
- endpoints

0 comments on commit 6c096c6

Please sign in to comment.