[RHOAIENG-4727] Compare runs - Parameters expandable section

opendatahub-io · Apr 11, 2024 · 8938d5e · 8938d5e
1 parent 65b648d
commit 8938d5e
Show file tree

Hide file tree

Showing 11 changed files with 335 additions and 40 deletions.
diff --git 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
@@ -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
@@ -160,6 +160,10 @@ const TableBase = <T,>({
         width={col.width}
         info={col.info}
         isSubheader={isSubheader}
+        isStickyColumn={col.isStickyColumn}
+        hasRightBorder={col.hasRightBorder}
+        modifier={col.modifier}
+        className={col.className}
       >
         {col.label}
       </Th>
@@ -256,16 +260,18 @@ const TableBase = <T,>({
         </div>
       )}
 
-      <Toolbar inset={{ default: 'insetNone' }} className="pf-v5-u-w-100">
-        <ToolbarContent alignItems="center">
-          {bottomToolbarContent}
-          {showPagination && (
-            <ToolbarItem variant="pagination" align={{ default: 'alignRight' }}>
-              {pagination('bottom')}
-            </ToolbarItem>
-          )}
-        </ToolbarContent>
-      </Toolbar>
+      {(bottomToolbarContent || showPagination) && (
+        <Toolbar inset={{ default: 'insetNone' }} className="pf-v5-u-w-100">
+          <ToolbarContent alignItems="center">
+            {bottomToolbarContent}
+            {showPagination && (
+              <ToolbarItem variant="pagination" align={{ default: 'alignRight' }}>
+                {pagination('bottom')}
+              </ToolbarItem>
+            )}
+          </ToolbarContent>
+        </Toolbar>
+      )}
     </>
   );
 };

diff --git a/frontend/src/components/table/types.ts b/frontend/src/components/table/types.ts
@@ -2,19 +2,19 @@ import { ThProps } from '@patternfly/react-table';
 
 export type GetColumnSort = (columnIndex: number) => ThProps['sort'];
 
-export type SortableData<T> = {
+export type SortableData<T> = 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).
    * Pass a function that will get the two results and what field needs to be matched.
    * 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
@@ -20,7 +20,7 @@ const PipelineJobReferenceName: React.FC<PipelineJobReferenceNameProps> = ({
       {loading ? (
         'loading...'
       ) : data ? (
-        <Text component={TextVariants.p} className="pf-u-pb-sm">
+        <Text component={TextVariants.p} className="pf-v5-u-pb-sm">
           Run {getPipelineJobExecutionCount(runName)} of {data.display_name}
         </Text>
       ) : (

diff --git 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
     >
       <Table
         {...checkboxTableProps}

diff --git a/.../src/concepts/pipelines/content/pipelinesDetails/pipelineRun/PipelineRunTabParameters.tsx b/.../src/concepts/pipelines/content/pipelinesDetails/pipelineRun/PipelineRunTabParameters.tsx
@@ -9,6 +9,7 @@ import {
 import { PipelineRunJobKFv2, PipelineRunKFv2 } from '~/concepts/pipelines/kfTypes';
 import {
   DetailItem,
+  normalizeInputParamValue,
   renderDetailItems,
 } from '~/concepts/pipelines/content/pipelinesDetails/pipelineRun/utils';
 
@@ -39,22 +40,10 @@ const PipelineRunTabParameters: React.FC<PipelineRunTabParametersProps> = ({ 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
@@ -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
@@ -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;