diff --git a/frontend/src/__tests__/cypress/cypress/e2e/pipelines/CompareRuns.cy.ts b/frontend/src/__tests__/cypress/cypress/e2e/pipelines/CompareRuns.cy.ts index 968e88f614..63104c108b 100644 --- a/frontend/src/__tests__/cypress/cypress/e2e/pipelines/CompareRuns.cy.ts +++ b/frontend/src/__tests__/cypress/cypress/e2e/pipelines/CompareRuns.cy.ts @@ -13,7 +13,11 @@ import { mockStatus, buildMockRunKF, } from '~/__mocks__'; -import { compareRunsGlobal } from '~/__tests__/cypress/cypress/pages/pipelines/compareRuns'; +import { + compareRunsGlobal, + compareRunsListTable, + compareRunParamsTable, +} from '~/__tests__/cypress/cypress/pages/pipelines/compareRuns'; import { verifyRelativeURL } from '~/__tests__/cypress/cypress/utils/url.cy'; const projectName = 'test-project-name'; @@ -34,6 +38,12 @@ const mockRun = buildMockRunKF({ pipeline_version_id: initialMockPipelineVersion.pipeline_version_id, }, experiment_id: mockExperiment.experiment_id, + runtime_config: { + parameters: { + paramOne: true, + paramTwo: false, + }, + }, }); const mockRun2 = buildMockRunKF({ @@ -44,6 +54,13 @@ const mockRun2 = buildMockRunKF({ pipeline_version_id: initialMockPipelineVersion.pipeline_version_id, }, experiment_id: mockExperiment.experiment_id, + runtime_config: { + parameters: { + paramOne: false, + paramTwo: false, + paramThree: 'Threeseretops', + }, + }, }); describe('Compare runs', () => { @@ -64,9 +81,10 @@ describe('Compare runs', () => { cy.wait('@validRun'); compareRunsGlobal.findInvalidRunsError().should('not.exist'); - compareRunsGlobal.findRunListRowByName('Run 1').should('exist'); - compareRunsGlobal.findRunListRowByName('Run 2').should('exist'); + compareRunsListTable.findRowByName('Run 1').should('exist'); + compareRunsListTable.findRowByName('Run 2').should('exist'); }); + it('valid number of runs but it is invalid', () => { cy.intercept( { @@ -123,6 +141,58 @@ describe('Compare runs', () => { `/experiments/${projectName}/${mockExperiment.experiment_id}/compareRuns?runs=invalid_run_id,${mockRun.run_id}`, ); }); + + describe('Parameters', () => { + beforeEach(() => { + cy.visit( + `/experiments/${projectName}/${mockExperiment.experiment_id}/compareRuns?runs=${mockRun.run_id},${mockRun2.run_id}`, + ); + }); + + it('shows empty state when the Runs list has no selections', () => { + compareRunsListTable.findSelectAllCheckbox().click(); + compareRunParamsTable.findEmptyState().should('exist'); + }); + + it('displays table data based on selections from Run list', () => { + compareRunsListTable.findRowByName('Run 1').should('exist'); + compareRunsListTable.findRowByName('Run 2').should('exist'); + + compareRunParamsTable.findColumnByName('Run 1').should('exist'); + compareRunParamsTable.findColumnByName('Run 2').should('exist'); + + compareRunParamsTable.findParamName('paramOne').should('exist'); + compareRunParamsTable.findParamName('paramTwo').should('exist'); + compareRunParamsTable.findParamName('paramThree').should('exist'); + }); + + it('removes run column from params table when run list selection is removed', () => { + compareRunsListTable.findRowByName('Run 1').should('exist'); + compareRunsListTable.findRowByName('Run 2').should('exist'); + + compareRunParamsTable.findColumnByName('Run 1').should('exist'); + compareRunParamsTable.findColumnByName('Run 2').should('exist'); + + compareRunsListTable.getRowByName('Run 2').findCheckbox().click(); + compareRunParamsTable.findColumnByName('Run 1').should('exist'); + compareRunParamsTable.findColumnByName('Run 2').should('not.exist'); + }); + + it('only shows parameters with differences when "Hide parameters with no differences" switch is on', () => { + compareRunsListTable.findRowByName('Run 1').should('exist'); + compareRunsListTable.findRowByName('Run 2').should('exist'); + + compareRunParamsTable.findParamName('paramOne').should('exist'); + compareRunParamsTable.findParamName('paramTwo').should('exist'); + compareRunParamsTable.findParamName('paramThree').should('exist'); + + cy.pfSwitch('hide-same-params-switch').click(); + + compareRunParamsTable.findParamName('paramOne').should('exist'); + compareRunParamsTable.findParamName('paramTwo').should('not.exist'); + compareRunParamsTable.findParamName('paramThree').should('exist'); + }); + }); }); const initIntercepts = () => { diff --git a/frontend/src/__tests__/cypress/cypress/pages/pipelines/compareRuns.ts b/frontend/src/__tests__/cypress/cypress/pages/pipelines/compareRuns.ts index ee16768eb9..1386727ebf 100644 --- a/frontend/src/__tests__/cypress/cypress/pages/pipelines/compareRuns.ts +++ b/frontend/src/__tests__/cypress/cypress/pages/pipelines/compareRuns.ts @@ -1,3 +1,5 @@ +import { TableRow } from '~/__tests__/cypress/cypress/pages/components/table'; + class CompareRunsGlobal { visit(projectName: string, experimentId: string, runIds: string[] = []) { cy.visit(`/experiments/${projectName}/${experimentId}/compareRuns?runs=${runIds.join(',')}`); @@ -6,14 +8,52 @@ class CompareRunsGlobal { findInvalidRunsError() { return cy.findByTestId('compare-runs-invalid-number-runs'); } +} + +class CompareRunsListTableRow extends TableRow { + findCheckbox() { + return this.find().find(`[data-label=Checkbox]`).find('input'); + } +} - findRunList() { +class CompareRunsListTable { + find() { return cy.findByTestId('compare-runs-table'); } - findRunListRowByName(name: string) { - return this.findRunList().findByText(name); + getRowByName(name: string) { + return new CompareRunsListTableRow(() => + this.find().find(`[data-label=Run]`).contains(name).parents('tr'), + ); + } + + findRowByName(name: string) { + return this.getRowByName(name).find(); + } + + findSelectAllCheckbox() { + return this.find().findByLabelText('Select all rows'); + } +} + +class CompareRunParamsTable { + find() { + return cy.findByTestId('compare-runs-params-table'); + } + + findEmptyState() { + return this.find().parent().findByTestId('compare-runs-params-empty-state'); + } + + findColumnByName(name: string) { + return this.find().contains('th', name); + } + + findParamName(name: string) { + return this.find().find(`[data-label="${name}"]`); } } export const compareRunsGlobal = new CompareRunsGlobal(); +export const compareRunsListTable = new CompareRunsListTable(); +export const compareRunParamsTable = new CompareRunParamsTable(); diff --git a/frontend/src/components/table/TableBase.tsx b/frontend/src/components/table/TableBase.tsx index 480c873e0e..b7167db593 100644 --- a/frontend/src/components/table/TableBase.tsx +++ b/frontend/src/components/table/TableBase.tsx @@ -160,6 +160,10 @@ const TableBase = ({ width={col.width} info={col.info} isSubheader={isSubheader} + isStickyColumn={col.isStickyColumn} + hasRightBorder={col.hasRightBorder} + modifier={col.modifier} + className={col.className} > {col.label} @@ -256,16 +260,18 @@ const TableBase = ({ )} - - - {bottomToolbarContent} - {showPagination && ( - - {pagination('bottom')} - - )} - - + {(bottomToolbarContent || showPagination) && ( + + + {bottomToolbarContent} + {showPagination && ( + + {pagination('bottom')} + + )} + + + )} ); }; diff --git a/frontend/src/components/table/types.ts b/frontend/src/components/table/types.ts index a6b9db2e31..b07488647a 100644 --- a/frontend/src/components/table/types.ts +++ b/frontend/src/components/table/types.ts @@ -2,13 +2,14 @@ import { ThProps } from '@patternfly/react-table'; export type GetColumnSort = (columnIndex: number) => ThProps['sort']; -export type SortableData = { +export type SortableData = Pick< + ThProps, + 'hasRightBorder' | 'isStickyColumn' | 'modifier' | 'width' | 'info' | 'className' +> & { label: string; field: string; colSpan?: number; rowSpan?: number; - hasRightBorder?: boolean; - width?: ThProps['width']; /** * Set to false to disable sort. * Set to true to handle string and number fields automatically (everything else is equal). @@ -16,5 +17,4 @@ export type SortableData = { * Assume ASC -- the result will be inverted internally if needed. */ sortable: boolean | ((a: T, b: T, keyField: string) => number); - info?: ThProps['info']; }; diff --git a/frontend/src/concepts/pipelines/content/PipelineJobReferenceName.tsx b/frontend/src/concepts/pipelines/content/PipelineJobReferenceName.tsx index 19afea5c00..c8123f225e 100644 --- a/frontend/src/concepts/pipelines/content/PipelineJobReferenceName.tsx +++ b/frontend/src/concepts/pipelines/content/PipelineJobReferenceName.tsx @@ -20,7 +20,7 @@ const PipelineJobReferenceName: React.FC = ({ {loading ? ( 'loading...' ) : data ? ( - + Run {getPipelineJobExecutionCount(runName)} of {data.display_name} ) : ( diff --git a/frontend/src/concepts/pipelines/content/compareRuns/CompareRunsRunList.tsx b/frontend/src/concepts/pipelines/content/compareRuns/CompareRunsRunList.tsx index 0ab07abe00..f4ed030ba0 100644 --- a/frontend/src/concepts/pipelines/content/compareRuns/CompareRunsRunList.tsx +++ b/frontend/src/concepts/pipelines/content/compareRuns/CompareRunsRunList.tsx @@ -53,6 +53,7 @@ const CompareRunsRunList: React.FC = () => { toggleText="Run list" isExpanded={isExpanded} onToggle={(_, expanded) => setExpanded(expanded)} + isIndented > = ({ run ); } - const details: DetailItem[] = parameters.map(([key, initialValue]) => { - let value = initialValue; - - if (typeof value === 'boolean') { - value = value ? 'True' : 'False'; - } - - if (typeof value === 'object') { - value = JSON.stringify(value); - } - - return { - key, - value, - }; - }); + const details: DetailItem[] = parameters.map(([key, value]) => ({ + key, + value: normalizeInputParamValue(value), + })); return <>{renderDetailItems(details)}; }; diff --git a/frontend/src/concepts/pipelines/content/pipelinesDetails/pipelineRun/utils.tsx b/frontend/src/concepts/pipelines/content/pipelinesDetails/pipelineRun/utils.tsx index 182e91d19b..4a9304f1fb 100644 --- a/frontend/src/concepts/pipelines/content/pipelinesDetails/pipelineRun/utils.tsx +++ b/frontend/src/concepts/pipelines/content/pipelinesDetails/pipelineRun/utils.tsx @@ -11,7 +11,7 @@ import { TimestampFormat, } from '@patternfly/react-core'; import { GlobeAmericasIcon } from '@patternfly/react-icons'; -import { DateTimeKF } from '~/concepts/pipelines/kfTypes'; +import { DateTimeKF, RuntimeConfigParamValue } from '~/concepts/pipelines/kfTypes'; import { PodKind } from '~/k8sTypes'; import { PodContainer } from '~/types'; @@ -80,3 +80,19 @@ export const isEmptyDateKF = (date: DateTimeKF): boolean => { const INVALID_TIMESTAMP = '1970-01-01T00:00:00Z'; return date === INVALID_TIMESTAMP; }; + +export const normalizeInputParamValue = ( + initialValue: RuntimeConfigParamValue, +): string | number => { + let value = initialValue; + + if (typeof value === 'boolean') { + value = value ? 'True' : 'False'; + } + + if (typeof value === 'object') { + value = JSON.stringify(value); + } + + return value; +}; diff --git a/frontend/src/concepts/pipelines/kfTypes.ts b/frontend/src/concepts/pipelines/kfTypes.ts index 73cd7e6987..11e33d3b9d 100644 --- a/frontend/src/concepts/pipelines/kfTypes.ts +++ b/frontend/src/concepts/pipelines/kfTypes.ts @@ -531,7 +531,7 @@ export type PipelineRunKFv2 = PipelineCoreResourceKFv2 & { pipeline_version_reference?: PipelineVersionReferenceKF; // in lue of pipeline_version_reference, the pipeline spec is included pipeline_spec?: PipelineSpecVariable; - runtime_config: PipelineSpecRuntimeConfig; + runtime_config?: PipelineSpecRuntimeConfig; service_account: string; scheduled_at: string; finished_at: string; diff --git a/frontend/src/pages/pipelines/global/experiments/compareRuns/CompareRunParamsSection.tsx b/frontend/src/pages/pipelines/global/experiments/compareRuns/CompareRunParamsSection.tsx new file mode 100644 index 0000000000..9454cb5156 --- /dev/null +++ b/frontend/src/pages/pipelines/global/experiments/compareRuns/CompareRunParamsSection.tsx @@ -0,0 +1,171 @@ +import React from 'react'; + +import { + EmptyState, + EmptyStateBody, + EmptyStateHeader, + EmptyStateVariant, + ExpandableSection, + Flex, + Switch, +} from '@patternfly/react-core'; +import { InnerScrollContainer, TableVariant, Td, Tr } from '@patternfly/react-table'; + +import { SortableData, Table } from '~/components/table'; +import { useCompareRuns } from '~/concepts/pipelines/content/compareRuns/CompareRunsContext'; +import { RuntimeConfigParamValue } from '~/concepts/pipelines/kfTypes'; +import { normalizeInputParamValue } from '~/concepts/pipelines/content/pipelinesDetails/pipelineRun/utils'; + +export const CompareRunParamsSection: React.FunctionComponent = () => { + const { loaded, selectedRuns } = useCompareRuns(); + const [isSectionOpen, setIsSectionOpen] = React.useState(true); + const [isHideSameRowsChecked, setIsHideSameRowsChecked] = React.useState(false); + const hasSelectedRuns = !!selectedRuns.length; + + const runNameColumns: SortableData<[string, Record]>[] = + React.useMemo( + () => + hasSelectedRuns + ? [ + { + label: 'Run name', + field: 'run-name', + isStickyColumn: true, + hasRightBorder: true, + // https://github.com/patternfly/patternfly-react/discussions/10269 + className: 'pf-v5-u-background-color-200', + sortable: false, + }, + ...selectedRuns.map( + (run, index): SortableData<[string, Record]> => ({ + label: run.display_name, + field: run.run_id, + sortable: false, + modifier: 'nowrap', + hasRightBorder: index !== selectedRuns.length - 1, + }), + ), + ] + : [], + [hasSelectedRuns, selectedRuns], + ); + + const runParamsMap = React.useMemo( + () => + selectedRuns.reduce( + (acc: Record>, selectedRun) => { + Object.entries(selectedRun.runtime_config?.parameters || {}).forEach( + ([paramKey, paramValue]) => { + if (!Object.keys(acc).includes(paramKey)) { + acc[paramKey] = { [selectedRun.run_id]: paramValue }; + } else if (!Object.keys(acc[paramKey]).includes(selectedRun.run_id)) { + acc[paramKey] = { ...acc[paramKey], [selectedRun.run_id]: paramValue }; + } + }, + ); + + return acc; + }, + {}, + ), + [selectedRuns], + ); + + const rowRenderer = React.useCallback( + ([paramKey, paramValuesMap]: [string, Record]) => { + const paramValues = Object.values(paramValuesMap); + const hasSameParamValues = paramValues.every( + (value) => normalizeInputParamValue(value) === normalizeInputParamValue(paramValues[0]), + ); + + // Hide all rows that have the same parameter value for every run when switch for diff values only is on + if ( + paramValues.length === selectedRuns.length && + hasSameParamValues && + isHideSameRowsChecked + ) { + return null; + } + + return ( + + + + {selectedRuns.map(({ run_id: runId }, index) => { + const hasRightBorder = index !== selectedRuns.length - 1 && { + hasRightBorder: true, + }; + + if (Object.keys(paramValuesMap).includes(runId)) { + const paramId = `${runId}-value-${index}`; + const paramValue = paramValuesMap[runId]; + + return ( + + ); + } + + return + ); + }, + [isHideSameRowsChecked, selectedRuns], + ); + + return ( + setIsSectionOpen(isOpen)} + isExpanded={isSectionOpen} + isIndented + > + + {hasSelectedRuns && ( + setIsHideSameRowsChecked(checked)} + id="hide-same-params-switch" + data-testid="hide-same-params-switch" + /> + )} + + +
+ {paramKey} + + {normalizeInputParamValue(paramValue)} + ; + })} +
+ + + Select runs from the Run list to compare parameters. + + + } + rowRenderer={rowRenderer} + variant={TableVariant.compact} + id="compare-runs-params-table" + data-testid="compare-runs-params-table" + gridBreakPoint="" + /> + + + + ); +}; diff --git a/frontend/src/pages/pipelines/global/experiments/compareRuns/CompareRunsPage.tsx b/frontend/src/pages/pipelines/global/experiments/compareRuns/CompareRunsPage.tsx index 5014896df5..2942dc4084 100644 --- a/frontend/src/pages/pipelines/global/experiments/compareRuns/CompareRunsPage.tsx +++ b/frontend/src/pages/pipelines/global/experiments/compareRuns/CompareRunsPage.tsx @@ -5,6 +5,7 @@ import { PathProps } from '~/concepts/pipelines/content/types'; import { useCompareRuns } from '~/concepts/pipelines/content/compareRuns/CompareRunsContext'; import { CompareRunsInvalidRunCount } from '~/concepts/pipelines/content/compareRuns/CompareRunInvalidRunCount'; import CompareRunsRunList from '~/concepts/pipelines/content/compareRuns/CompareRunsRunList'; +import { CompareRunParamsSection } from './CompareRunParamsSection'; const CompareRunsPage: React.FC = ({ breadcrumbPath }) => { const { runs, loaded } = useCompareRuns(); @@ -32,6 +33,10 @@ const CompareRunsPage: React.FC = ({ breadcrumbPath }) => { + + + + );