Skip to content

Commit

Permalink
test: make explist e2e sort test more reliable
Browse files Browse the repository at this point in the history 
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.
  • Loading branch information
@ashtonG
ashtonG committed Oct 16, 2024
1 parent 9efd96d commit de92b19
Show file tree
Hide file tree
Showing 2 changed files with 130 additions and 89 deletions.
29 changes: 18 additions & 11 deletions webui/react/src/components/MultiSortMenu.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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,
Expand Down
190 changes: 112 additions & 78 deletions webui/react/src/e2e/tests/experimentList.spec.ts
View file
Original file line number Diff line number Diff line change
@@ -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';
Expand All @@ -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
Expand Down Expand Up @@ -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<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();
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}`,
);
});
Expand Down Expand Up @@ -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' },
]);
});
});

0 comments on commit de92b19

Please sign in to comment.