test: make explist e2e sort test more reliable

webui/react/src/components/MultiSortMenu.tsx

@@ -2,7 +2,7 @@ import Button from 'hew/Button';
 import { DirectionType, Sort, validSort } from 'hew/DataGrid/DataGrid';
 import Dropdown, { MenuItem } from 'hew/Dropdown';
 import Icon from 'hew/Icon';
-import Select from 'hew/Select';
+import Select, { Option } from 'hew/Select';
 import { Loadable } from 'hew/utils/loadable';
 
 import { V1ColumnType } from 'services/api-ts-sdk';
@@ -130,16 +130,23 @@ export const sortMenuItemsForColumn = (
   });
 };
 
-const DirectionOptions: React.FC<DirectionOptionsProps> = ({ onChange, type, value }) => (
-  <Select
-    data-test="direction"
-    options={optionsByColumnType[type]}
-    placeholder="Select direction"
-    value={value}
-    width="100%"
-    onChange={(val) => onChange?.(val as DirectionType)}
-  />
-);
+const DirectionOptions: React.FC<DirectionOptionsProps> = ({ onChange, type, value }) => {
+  const options = optionsByColumnType[type];
+  return (
+    <Select
+      data-test="direction"
+      placeholder="Select direction"
+      value={value}
+      width="100%"
+      onChange={(val) => onChange?.(val as DirectionType)}>
+      {options.map((o) => (
+        <Option data-select-value={o.value} key={o.value}>
+          {o.label}
+        </Option>
+      ))}
+    </Select>
+  );
+};
 
 const ColumnOptions: React.FC<ColumnOptionsProps> = ({
   onChange,

webui/react/src/e2e/tests/experimentList.spec.ts

@@ -1,3 +1,6 @@
+import dayjs from 'dayjs';
+import utcPlugin from 'dayjs/plugin/utc';
+
 import { expect, test } from 'e2e/fixtures/global-fixtures';
 import { ExperimentRow } from 'e2e/models/components/F_ExperimentList';
 import { ProjectDetails } from 'e2e/models/pages/ProjectDetails';
@@ -6,6 +9,8 @@ import { safeName } from 'e2e/utils/naming';
 import { repeatWithFallback } from 'e2e/utils/polling';
 import { ExperimentBase } from 'types';
 
+dayjs.extend(utcPlugin);
+
 test.describe('Experiment List', () => {
   let projectDetailsPage: ProjectDetails;
   // trial click to wait for the element to be stable won't work here
@@ -442,67 +447,108 @@ test.describe('Experiment List', () => {
     });
   });
   test.describe('Experiment List Multi-sort', () => {
-    const runScenarioAndValidation = (projectDetailsPage: ProjectDetails) => {
-      const multiSortMenu = projectDetailsPage.f_experimentList.tableActionBar.multiSortMenu;
-      const validateByColumn = async (
-        rows: ExperimentRow[],
-        colKey: string,
-        descending: boolean,
-      ) => {
-        const valuesToCompare = await Promise.all(
-          rows.map(async (r) => (await r.getCellByColumnName(colKey)).pwLocator.innerText()),
-        );
-        // eslint-disable-next-line no-console
-        console.log(valuesToCompare[0]);
-
-        const expectedValues = [...valuesToCompare].sort();
-        if (descending) {
-          expectedValues.reverse();
-        }
-        expect(valuesToCompare).toEqual(expectedValues);
-      };
-      const checkTableOrder = async (firstKey: string, secondKey: string, descending = false) => {
-        const rows = await projectDetailsPage.f_experimentList.dataGrid.filterRows(
-          async () => await true,
-        );
-        await validateByColumn(rows, firstKey, descending);
-        await validateByColumn(rows, secondKey, descending);
-      };
-      const sortingScenario = async (
-        firstSortBy: string,
-        firstSortOrder: string,
-        secondSortBy: string,
-        secondSortOrder: string,
-        scenario: () => Promise<void>,
-      ): Promise<void> => {
-        await test.step(`Sort by ${firstSortBy} and ${secondSortBy}`, async () => {
+    type sort = { column: string; direction: 'asc' | 'desc' };
+
+    // best-effort column text parsing -- may need to read the actual column defs to be perfect
+    const parseColumnText = (text: string) => {
+      if (text === '-') {
+        return null;
+      }
+      const num = Number(text);
+      if (!Number.isNaN(num)) {
+        return num;
+      }
+      const date = dayjs.utc(text);
+      if (date.isValid()) {
+        return date;
+      }
+      return text;
+    };
+    const testSorts = (sorts: sort[]) => {
+      const lastSort = sorts[sorts.length - 1];
+      const sortTextList = sorts
+        .slice(0, -1)
+        .map((s) => `${s.column} ${s.direction}`)
+        .join(', ');
+
+      return test(`Sort by ${sortTextList} and ${lastSort.column} ${lastSort.direction}`, async () => {
+        await test.step('Set Up Sorts', async () => {
+          const multiSortMenu = projectDetailsPage.f_experimentList.tableActionBar.multiSortMenu;
           await multiSortMenu.open();
           await multiSortMenu.multiSort.reset.pwLocator.click();
+          // weirdness alert: reset closes the sort menu normally, but doesn't
+          // in playwright locally. in ci the locators become unstable
           await multiSortMenu.close();
           await multiSortMenu.open();
 
-          const firstRow = multiSortMenu.multiSort.rows.nth(0);
-          await firstRow.column.selectMenuOption(firstSortBy);
-          await firstRow.order.selectMenuOption(firstSortOrder);
-
-          await multiSortMenu.multiSort.add.pwLocator.click();
-
-          const secondRow = multiSortMenu.multiSort.rows.nth(1);
-          await secondRow.column.selectMenuOption(secondSortBy);
-          await secondRow.order.selectMenuOption(secondSortOrder);
-
+          for (let i = 0; i < sorts.length; i++) {
+            const sort = sorts[i];
+            const sortRow = multiSortMenu.multiSort.rows.nth(i);
+            await sortRow.column.selectMenuOption(sort.column);
+            // select order menu item by value not label
+            await sortRow.order.openMenu();
+            await sortRow.order._menu.pwLocator
+              .locator(`[data-select-value="${sort.direction}"]`)
+              .click();
+            if (i < sorts.length - 1) {
+              await multiSortMenu.multiSort.add.pwLocator.click();
+            }
+          }
           await multiSortMenu.close();
           await waitTableStable();
-          await scenario();
         });
-      };
 
-      return { checkTableOrder, sortingScenario };
+        await test.step('Verify Order', async () => {
+          const rows = await projectDetailsPage.f_experimentList.dataGrid.filterRows(() =>
+            Promise.resolve(true),
+          );
+          const getValuesForRow = async (r: ExperimentRow) => {
+            const rowColumnValues = await Promise.all(
+              sorts.map(async ({ column }) => ({
+                [column]: await (await r.getCellByColumnName(column)).pwLocator.innerText(),
+              })),
+            );
+            return Object.assign({}, ...rowColumnValues);
+          };
+          const rowValues = await Promise.all(rows.map(getValuesForRow));
+          // sorting all values at the same time to make reported failures easier to decipher
+          // it'd be nice to use _.orderBy, but it's harder to suss out the null placement logic
+          const expectedOrder = [...rowValues].sort((rowA, rowB) => {
+            for (const sort of sorts) {
+              const isAscending = sort.direction === 'asc';
+              const valueA = parseColumnText(rowA[sort.column]);
+              const valueB = parseColumnText(rowB[sort.column]);
+              // nulls are always last
+              if (valueA === null && valueB !== null) {
+                return 1;
+              }
+              if (valueB === null && valueA !== null) {
+                return -1;
+              }
+              if (typeof valueA === 'string' && typeof valueB === 'string' && valueA !== valueB) {
+                const cmpValue = valueA < valueB ? -1 : 1;
+                return isAscending ? cmpValue : cmpValue * -1;
+              }
+              if (typeof valueA === 'number' && typeof valueB === 'number' && valueA !== valueB) {
+                const cmpValue = valueA - valueB;
+                return isAscending ? cmpValue : cmpValue * -1;
+              }
+              if (dayjs.isDayjs(valueA) && dayjs.isDayjs(valueB) && !valueA.isSame(valueB)) {
+                const cmpValue = valueA.isBefore(valueB) ? -1 : 1;
+                return isAscending ? cmpValue : cmpValue * -1;
+              }
+              // if types are different or values are same, skip the column
+            }
+            return 0;
+          });
+          expect(rowValues).toEqual(expectedOrder);
+        });
+      });
     };
 
-    test.beforeAll(async ({ newProject }) => {
+    test.beforeAll(({ newProject }) => {
       // create a new experiment for comparing Searcher Metric and Trial Count
-      await detExecSync(
+      detExecSync(
         `experiment create ${fullPath('examples/tutorials/core_api_pytorch_mnist/checkpoints.yaml')} --paused --project_id ${newProject.response.project.id}`,
       );
     });
@@ -531,32 +577,24 @@ test.describe('Experiment List', () => {
       await projectDetailsPage.f_experimentList.dataGrid.headRow.setColumnDefs();
     });
 
-    test('sort with ID as 0 → 9 and Searcher as A → Z', async () => {
-      const { sortingScenario, checkTableOrder } = runScenarioAndValidation(projectDetailsPage);
-      await sortingScenario('ID', '0 → 9', 'Searcher', 'A → Z', async () => {
-        await checkTableOrder('ID', 'Searcher');
-      });
-    });
-
-    test('sort with ID as 9 → 0 and Searcher as Z → A', async () => {
-      const { sortingScenario, checkTableOrder } = runScenarioAndValidation(projectDetailsPage);
-      await sortingScenario('ID', '9 → 0', 'Searcher', 'Z → A', async () => {
-        await checkTableOrder('ID', 'Searcher', true);
-      });
-    });
-
-    test('sort with Trial count as 0 → 0 and Searcher Metric as A → Z', async () => {
-      const { sortingScenario, checkTableOrder } = runScenarioAndValidation(projectDetailsPage);
-      await sortingScenario('Trial count', '0 → 9', 'Searcher Metric', 'A → Z', async () => {
-        await checkTableOrder('Trial count', 'Searcher Metric');
-      });
-    });
-
-    test('sort with Trial count as 9 → 0 and Searcher Metric as Z → A', async () => {
-      const { sortingScenario, checkTableOrder } = runScenarioAndValidation(projectDetailsPage);
-      await sortingScenario('Trial count', '9 → 0', 'Searcher Metric', 'Z → A', async () => {
-        await checkTableOrder('Trial count', 'Searcher Metric', true);
-      });
-    });
+    testSorts([
+      { column: 'ID', direction: 'asc' },
+      { column: 'Searcher', direction: 'asc' },
+    ]);
+
+    testSorts([
+      { column: 'ID', direction: 'desc' },
+      { column: 'Searcher', direction: 'desc' },
+    ]);
+
+    testSorts([
+      { column: 'Trial count', direction: 'asc' },
+      { column: 'Searcher Metric', direction: 'asc' },
+    ]);
+
+    testSorts([
+      { column: 'Trial count', direction: 'desc' },
+      { column: 'Searcher Metric', direction: 'desc' },
+    ]);
   });
 });