From de92b195a5425a1cbd52b262b6c3fb9b8218e598 Mon Sep 17 00:00:00 2001 From: Ashton Galloway Date: Wed, 16 Oct 2024 14:26:50 -0400 Subject: [PATCH] test: make explist e2e sort test more reliable this updates the experiment list e2e multi sort test: 1) the test could flake if the table was comparing numeric values lower than 10 with numeric values larger than 10. 2) the test helper now handles both the setup and scenario steps, allowing for more levels of sorting. the multi sort menu is also updated to output the values of the direction selector to the dom to allow selections during the e2e tests that don't need to be aware of the column copy. --- webui/react/src/components/MultiSortMenu.tsx | 29 ++- .../src/e2e/tests/experimentList.spec.ts | 190 +++++++++++------- 2 files changed, 130 insertions(+), 89 deletions(-) diff --git a/webui/react/src/components/MultiSortMenu.tsx b/webui/react/src/components/MultiSortMenu.tsx index 39ff00767c1..68916cb3fd6 100644 --- a/webui/react/src/components/MultiSortMenu.tsx +++ b/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 = ({ onChange, type, value }) => ( - onChange?.(val as DirectionType)}> + {options.map((o) => ( + + ))} + + ); +}; const ColumnOptions: React.FC = ({ onChange, diff --git a/webui/react/src/e2e/tests/experimentList.spec.ts b/webui/react/src/e2e/tests/experimentList.spec.ts index e2bcc21a50b..328a7c6742b 100644 --- a/webui/react/src/e2e/tests/experimentList.spec.ts +++ b/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,104 @@ 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, - ): Promise => { - 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(); - 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 +573,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' }, + ]); }); });